From 424206e026b33480365dbf252381369901e3e0e5 Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Fri, 11 Mar 2022 20:07:30 -0800 Subject: [PATCH 1/7] Throw HelmTemplateError exception when Helm returns errors Signed-off-by: Gerard Hickey Changelog: other --- doc/development/rspec.md | 4 ++++ spec/helm_template_helper.rb | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/doc/development/rspec.md b/doc/development/rspec.md index e27abcb395..f68c388aef 100644 --- a/doc/development/rspec.md +++ b/doc/development/rspec.md @@ -52,6 +52,10 @@ obj.dig('ConfigMap/test-gitaly', 'data', 'config.toml.erb') This will return the contents of the `config.toml.erb` file contained in the `test-gitaly` ConfigMap. +If Helm failed to generate YAML, typically because of template errors, then +a `HelmTemplateError` exception is thrown with the stderr as the exception +message. + NOTE: Using the `HelmTemplate` class will always use the release name of "test" when executing the `helm template` command. diff --git a/spec/helm_template_helper.rb b/spec/helm_template_helper.rb index 56a0aa10b8..4cbc4f5e30 100644 --- a/spec/helm_template_helper.rb +++ b/spec/helm_template_helper.rb @@ -1,6 +1,8 @@ require 'yaml' require 'open3' +class HelmTemplateError < RuntimeError; end + class HelmTemplate @_helm_major_version = nil @_helm_minor_version = nil @@ -87,6 +89,12 @@ class HelmTemplate yaml = YAML.load_stream(@stdout) # filter out any empty YAML documents (nil) yaml.select!{ |x| !x.nil? } + + # if we got any stderr then Helm misbehaved somehow + unless @stderr.nil? + raise HelmTemplateError, @stderr + end + # create an indexed Hash keyed on Kind/metdata.name @mapped = yaml.to_h { |doc| [ "#{doc['kind']}/#{doc['metadata']['name']}" , doc ] -- GitLab From d591e596d393683c9d75210453614f4a9e68a8dc Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Fri, 11 Mar 2022 20:24:04 -0800 Subject: [PATCH 2/7] Use .empty? to elimate false positives --- spec/helm_template_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helm_template_helper.rb b/spec/helm_template_helper.rb index 4cbc4f5e30..1453f3e5ad 100644 --- a/spec/helm_template_helper.rb +++ b/spec/helm_template_helper.rb @@ -91,7 +91,7 @@ class HelmTemplate yaml.select!{ |x| !x.nil? } # if we got any stderr then Helm misbehaved somehow - unless @stderr.nil? + unless @stderr.empty? raise HelmTemplateError, @stderr end -- GitLab From 0a028cf97b9ac34199eea9949469f7f8cbaa67e6 Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Fri, 11 Mar 2022 20:50:45 -0800 Subject: [PATCH 3/7] Rubocop fixes --- spec/helm_template_helper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/helm_template_helper.rb b/spec/helm_template_helper.rb index 1453f3e5ad..709b689fb1 100644 --- a/spec/helm_template_helper.rb +++ b/spec/helm_template_helper.rb @@ -1,7 +1,9 @@ require 'yaml' require 'open3' -class HelmTemplateError < RuntimeError; end +# rubocop:disable Cop/CustomErrorClass +class HelmTemplateError < StandardError; end +# rubocop:enable Cop/CustomErrorClass class HelmTemplate @_helm_major_version = nil @@ -91,9 +93,7 @@ class HelmTemplate yaml.select!{ |x| !x.nil? } # if we got any stderr then Helm misbehaved somehow - unless @stderr.empty? - raise HelmTemplateError, @stderr - end + raise HelmTemplateError, @stderr unless @stderr.empty? # create an indexed Hash keyed on Kind/metdata.name @mapped = yaml.to_h { |doc| -- GitLab From 0af9d934bd4c176b28d07584f8a96ca944511d3d Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Tue, 5 Apr 2022 12:10:36 -0700 Subject: [PATCH 4/7] Replaced checking Helm exit code with cheking for exception --- spec/configuration/database_spec.rb | 10 +++++++--- spec/configuration/gitaly_spec.rb | 2 +- spec/configuration/global_spec.rb | 5 +++-- spec/configuration/ingress_spec.rb | 5 +++-- spec/configuration/redis_spec.rb | 5 +++-- spec/configuration/webservice_deployments_spec.rb | 7 +++++-- 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/spec/configuration/database_spec.rb b/spec/configuration/database_spec.rb index 127cd9bce9..c09d5b9bf2 100644 --- a/spec/configuration/database_spec.rb +++ b/spec/configuration/database_spec.rb @@ -114,9 +114,13 @@ describe 'Database configuration' do end it 'fail due to checkConfig' do - t = HelmTemplate.new(values) - expect(t.exit_code).not_to eq(0) - expect(t.stderr).to include("PostgreSQL is set to install, but database load balancing is also enabled.") + expect { + t = HelmTemplate.new(values) + }.to raise_error(HelmTemplateError) + + expect { + t = HelmTemplate.new(values) + }.to raise_error.with_message(/PostgreSQL is set to install, but database load balancing is also enabled./) end end diff --git a/spec/configuration/gitaly_spec.rb b/spec/configuration/gitaly_spec.rb index 5d8f4fea02..08be887751 100644 --- a/spec/configuration/gitaly_spec.rb +++ b/spec/configuration/gitaly_spec.rb @@ -137,7 +137,7 @@ describe 'Gitaly configuration' do let(:gitaly_stateful_set) { 'StatefulSet/test-gitaly' } - it 'should render securityContext correctly' do + it 'should render securityContext correctly', :focus => true do t = HelmTemplate.new(values) gitaly_set = t.resources_by_kind('StatefulSet').select { |key| key == gitaly_stateful_set } security_context = gitaly_set[gitaly_stateful_set]['spec']['template']['spec']['securityContext'] diff --git a/spec/configuration/global_spec.rb b/spec/configuration/global_spec.rb index b0e6a8807b..f7344d6fa4 100644 --- a/spec/configuration/global_spec.rb +++ b/spec/configuration/global_spec.rb @@ -21,8 +21,9 @@ describe 'global configuration' do context 'default settings' do it 'fails to create a helm release' do - t = HelmTemplate.new({}) - expect(t.exit_code).to eq(256), "Unexpected error code #{t.exit_code} -- #{t.stderr}" + expect { + t = HelmTemplate.new({}) + }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/ingress_spec.rb b/spec/configuration/ingress_spec.rb index 51cee62093..d86542eaad 100644 --- a/spec/configuration/ingress_spec.rb +++ b/spec/configuration/ingress_spec.rb @@ -118,8 +118,9 @@ describe 'GitLab Ingress configuration(s)' do end it 'fails due to gitlab.webservice.ingress.requireBasePath' do - template = HelmTemplate.new(bogus) - expect(template.exit_code).not_to eq(0) + expect { + template = HelmTemplate.new(bogus) + }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/redis_spec.rb b/spec/configuration/redis_spec.rb index e30d816900..1e9f6e885a 100644 --- a/spec/configuration/redis_spec.rb +++ b/spec/configuration/redis_spec.rb @@ -61,8 +61,9 @@ describe 'Redis configuration' do end it 'fails to template (checkConfig)' do - t = HelmTemplate.new(values) - expect(t.exit_code).not_to eq(0) + expect { + t = HelmTemplate.new(values) + }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/webservice_deployments_spec.rb b/spec/configuration/webservice_deployments_spec.rb index 3dd8dc20f4..7e5d5bba76 100644 --- a/spec/configuration/webservice_deployments_spec.rb +++ b/spec/configuration/webservice_deployments_spec.rb @@ -272,8 +272,11 @@ describe 'Webservice Deployments configuration' do )).deep_merge(default_values) end - it 'template fails' do - expect(datamodel.exit_code).not_to eq(0) + it 'template fails', :focus => true do + expect { + datamodel + }.to raise_error(HelmTemplateError) + # expect(datamodel.exit_code).not_to eq(0) end end -- GitLab From 87238efc2cbe1456b9ee313dbf1abd89289d6e69 Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Tue, 5 Apr 2022 12:31:54 -0700 Subject: [PATCH 5/7] Rubocop fixes --- spec/configuration/database_spec.rb | 9 +++------ spec/configuration/gitaly_spec.rb | 2 +- spec/configuration/global_spec.rb | 4 +--- spec/configuration/ingress_spec.rb | 4 +--- spec/configuration/redis_spec.rb | 4 +--- spec/configuration/webservice_deployments_spec.rb | 7 ++----- 6 files changed, 9 insertions(+), 21 deletions(-) diff --git a/spec/configuration/database_spec.rb b/spec/configuration/database_spec.rb index c09d5b9bf2..72627a10c9 100644 --- a/spec/configuration/database_spec.rb +++ b/spec/configuration/database_spec.rb @@ -114,13 +114,10 @@ describe 'Database configuration' do end it 'fail due to checkConfig' do - expect { - t = HelmTemplate.new(values) - }.to raise_error(HelmTemplateError) + expect { HelmTemplate.new(values) }.to raise_error(HelmTemplateError) - expect { - t = HelmTemplate.new(values) - }.to raise_error.with_message(/PostgreSQL is set to install, but database load balancing is also enabled./) + expect { HelmTemplate.new(values) }.to raise_error.with_message( + /PostgreSQL is set to install, but database load balancing is also enabled./) end end diff --git a/spec/configuration/gitaly_spec.rb b/spec/configuration/gitaly_spec.rb index 08be887751..5d8f4fea02 100644 --- a/spec/configuration/gitaly_spec.rb +++ b/spec/configuration/gitaly_spec.rb @@ -137,7 +137,7 @@ describe 'Gitaly configuration' do let(:gitaly_stateful_set) { 'StatefulSet/test-gitaly' } - it 'should render securityContext correctly', :focus => true do + it 'should render securityContext correctly' do t = HelmTemplate.new(values) gitaly_set = t.resources_by_kind('StatefulSet').select { |key| key == gitaly_stateful_set } security_context = gitaly_set[gitaly_stateful_set]['spec']['template']['spec']['securityContext'] diff --git a/spec/configuration/global_spec.rb b/spec/configuration/global_spec.rb index f7344d6fa4..bb9bf212a4 100644 --- a/spec/configuration/global_spec.rb +++ b/spec/configuration/global_spec.rb @@ -21,9 +21,7 @@ describe 'global configuration' do context 'default settings' do it 'fails to create a helm release' do - expect { - t = HelmTemplate.new({}) - }.to raise_error(HelmTemplateError) + expect { HelmTemplate.new({}) }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/ingress_spec.rb b/spec/configuration/ingress_spec.rb index d86542eaad..8a2fa6e8ed 100644 --- a/spec/configuration/ingress_spec.rb +++ b/spec/configuration/ingress_spec.rb @@ -118,9 +118,7 @@ describe 'GitLab Ingress configuration(s)' do end it 'fails due to gitlab.webservice.ingress.requireBasePath' do - expect { - template = HelmTemplate.new(bogus) - }.to raise_error(HelmTemplateError) + expect { HelmTemplate.new(bogus) }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/redis_spec.rb b/spec/configuration/redis_spec.rb index 1e9f6e885a..62b2177cf9 100644 --- a/spec/configuration/redis_spec.rb +++ b/spec/configuration/redis_spec.rb @@ -61,9 +61,7 @@ describe 'Redis configuration' do end it 'fails to template (checkConfig)' do - expect { - t = HelmTemplate.new(values) - }.to raise_error(HelmTemplateError) + expect { HelmTemplate.new(values) }.to raise_error(HelmTemplateError) end end diff --git a/spec/configuration/webservice_deployments_spec.rb b/spec/configuration/webservice_deployments_spec.rb index 7e5d5bba76..803b40720c 100644 --- a/spec/configuration/webservice_deployments_spec.rb +++ b/spec/configuration/webservice_deployments_spec.rb @@ -272,11 +272,8 @@ describe 'Webservice Deployments configuration' do )).deep_merge(default_values) end - it 'template fails', :focus => true do - expect { - datamodel - }.to raise_error(HelmTemplateError) - # expect(datamodel.exit_code).not_to eq(0) + it 'template fails' do + expect { datamodel }.to raise_error(HelmTemplateError) end end -- GitLab From a14b27d2acd8542a7a4209e94b16124654e799bd Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Fri, 8 Apr 2022 13:57:03 -0700 Subject: [PATCH 6/7] Ignore warnings when looking to raise HelmTemplateError --- spec/helm_template_helper.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/helm_template_helper.rb b/spec/helm_template_helper.rb index 709b689fb1..53f12032d6 100644 --- a/spec/helm_template_helper.rb +++ b/spec/helm_template_helper.rb @@ -93,7 +93,9 @@ class HelmTemplate yaml.select!{ |x| !x.nil? } # if we got any stderr then Helm misbehaved somehow - raise HelmTemplateError, @stderr unless @stderr.empty? + # but ignore any warnings that are produced + errors = @stderr.split("\n").filter { |line| !line.include? 'warning' } + raise HelmTemplateError, @stderr unless errors # create an indexed Hash keyed on Kind/metdata.name @mapped = yaml.to_h { |doc| -- GitLab From 3dbfb46064dbf984fd25bbdfef3855ea1ab33bc1 Mon Sep 17 00:00:00 2001 From: Gerard Hickey Date: Fri, 8 Apr 2022 14:36:14 -0700 Subject: [PATCH 7/7] Added missing test for empty list --- spec/helm_template_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helm_template_helper.rb b/spec/helm_template_helper.rb index 53f12032d6..b8dcf10534 100644 --- a/spec/helm_template_helper.rb +++ b/spec/helm_template_helper.rb @@ -95,7 +95,7 @@ class HelmTemplate # if we got any stderr then Helm misbehaved somehow # but ignore any warnings that are produced errors = @stderr.split("\n").filter { |line| !line.include? 'warning' } - raise HelmTemplateError, @stderr unless errors + raise HelmTemplateError, @stderr unless errors.empty? # create an indexed Hash keyed on Kind/metdata.name @mapped = yaml.to_h { |doc| -- GitLab