Commit 6dcd3b15 authored by Emily Ring's avatar Emily Ring Committed by Nick Thomas

Support multiple terraform plan reports

Updated terraform/tfplan.rb to send through multiple plans
Adds a new value job_name to terraform/tfplan.rb
Update associated views and tests
parent b8df9151
...@@ -39,7 +39,8 @@ export default { ...@@ -39,7 +39,8 @@ export default {
return this.plan.job_path; return this.plan.job_path;
}, },
plan() { plan() {
return this.plans['tfplan.json'] || {}; const firstPlanKey = Object.keys(this.plans)[0];
return this.plans[firstPlanKey] ?? {};
}, },
validPlanValues() { validPlanValues() {
return this.addNum + this.changeNum + this.deleteNum >= 0; return this.addNum + this.changeNum + this.deleteNum >= 0;
......
---
title: Allow Tf Plan to genrate multiple reports
merge_request: 33867
author:
type: changed
...@@ -8,15 +8,11 @@ module Gitlab ...@@ -8,15 +8,11 @@ module Gitlab
TfplanParserError = Class.new(Gitlab::Ci::Parsers::ParserError) TfplanParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
def parse!(json_data, terraform_reports, artifact:) def parse!(json_data, terraform_reports, artifact:)
tfplan = Gitlab::Json.parse(json_data).tap do |parsed_data| plan_data = Gitlab::Json.parse(json_data)
parsed_data['job_path'] = Gitlab::Routing.url_helpers.project_job_path(
artifact.job.project, artifact.job
)
end
raise TfplanParserError, 'Tfplan missing required key' unless valid_supported_keys?(tfplan) raise TfplanParserError, 'Tfplan missing required key' unless has_required_keys?(plan_data)
terraform_reports.add_plan(artifact.filename, tfplan) terraform_reports.add_plan(artifact.job.id.to_s, tfplan(plan_data, artifact.job))
rescue JSON::ParserError rescue JSON::ParserError
raise TfplanParserError, 'JSON parsing failed' raise TfplanParserError, 'JSON parsing failed'
rescue rescue
...@@ -25,8 +21,18 @@ module Gitlab ...@@ -25,8 +21,18 @@ module Gitlab
private private
def valid_supported_keys?(tfplan) def has_required_keys?(plan_data)
tfplan.keys == %w[create update delete job_path] (%w[create update delete] - plan_data.keys).empty?
end
def tfplan(plan_data, artifact_job)
{
'create' => plan_data['create'].to_i,
'delete' => plan_data['delete'].to_i,
'job_name' => artifact_job.options.dig(:artifacts, :name).to_s,
'job_path' => Gitlab::Routing.url_helpers.project_job_path(artifact_job.project, artifact_job),
'update' => plan_data['update'].to_i
}
end end
end end
end end
......
...@@ -10,14 +10,6 @@ module Gitlab ...@@ -10,14 +10,6 @@ module Gitlab
@plans = {} @plans = {}
end end
def pick(keys)
terraform_plans = plans.select do |key|
keys.include?(key)
end
{ plans: terraform_plans }
end
def add_plan(name, plan) def add_plan(name, plan)
plans[name] = plan plans[name] = plan
end end
......
...@@ -1183,15 +1183,19 @@ RSpec.describe Projects::MergeRequestsController do ...@@ -1183,15 +1183,19 @@ RSpec.describe Projects::MergeRequestsController do
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match(
a_hash_including( pipeline.builds.each do |build|
'tfplan.json' => hash_including( expect(json_response).to match(
'create' => 0, a_hash_including(
'delete' => 0, build.id.to_s => hash_including(
'update' => 1 'create' => 0,
'delete' => 0,
'update' => 1,
'job_name' => build.options.dig(:artifacts, :name).to_s
)
) )
) )
) end
end end
end end
......
...@@ -110,6 +110,7 @@ FactoryBot.define do ...@@ -110,6 +110,7 @@ FactoryBot.define do
after(:build) do |pipeline, evaluator| after(:build) do |pipeline, evaluator|
pipeline.builds << build(:ci_build, :terraform_reports, pipeline: pipeline, project: pipeline.project) pipeline.builds << build(:ci_build, :terraform_reports, pipeline: pipeline, project: pipeline.project)
pipeline.builds << build(:ci_build, :terraform_reports, pipeline: pipeline, project: pipeline.project)
end end
end end
......
...@@ -38,7 +38,7 @@ describe('MrWidgetTerraformPlan', () => { ...@@ -38,7 +38,7 @@ describe('MrWidgetTerraformPlan', () => {
describe('loading poll', () => { describe('loading poll', () => {
beforeEach(() => { beforeEach(() => {
mockPollingApi(200, { 'tfplan.json': plan }, {}); mockPollingApi(200, { '123': plan }, {});
return mountWrapper().then(() => { return mountWrapper().then(() => {
wrapper.setData({ loading: true }); wrapper.setData({ loading: true });
...@@ -65,7 +65,7 @@ describe('MrWidgetTerraformPlan', () => { ...@@ -65,7 +65,7 @@ describe('MrWidgetTerraformPlan', () => {
pollRequest = jest.spyOn(Poll.prototype, 'makeRequest'); pollRequest = jest.spyOn(Poll.prototype, 'makeRequest');
pollStop = jest.spyOn(Poll.prototype, 'stop'); pollStop = jest.spyOn(Poll.prototype, 'stop');
mockPollingApi(200, { 'tfplan.json': plan }, {}); mockPollingApi(200, { '123': plan }, {});
return mountWrapper(); return mountWrapper();
}); });
......
...@@ -8,7 +8,7 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do ...@@ -8,7 +8,7 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do
let(:reports) { Gitlab::Ci::Reports::TerraformReports.new } let(:reports) { Gitlab::Ci::Reports::TerraformReports.new }
context 'when data is tfplan.json' do context 'when data is invalid' do
context 'when there is no data' do context 'when there is no data' do
it 'raises an error' do it 'raises an error' do
plan = '{}' plan = '{}'
...@@ -19,31 +19,67 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do ...@@ -19,31 +19,67 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do
end end
end end
context 'when there is data' do context 'when data is not a JSON file' do
it 'parses JSON and returns a report' do it 'raises an error' do
plan = '{ "create": 0, "update": 1, "delete": 0 }' plan = { 'create' => 0, 'update' => 1, 'delete' => 0 }.to_s
expect { subject.parse!(plan, reports, artifact: artifact) }.not_to raise_error expect { subject.parse!(plan, reports, artifact: artifact) }.to raise_error(
described_class::TfplanParserError
)
end
end
expect(reports.plans).to match( context 'when JSON is missing a required key' do
a_hash_including( it 'raises an error' do
'tfplan.json' => a_hash_including( plan = '{ "wrong_key": 1 }'
'create' => 0,
'update' => 1, expect { subject.parse!(plan, reports, artifact: artifact) }.to raise_error(
'delete' => 0 described_class::TfplanParserError
)
)
) )
end end
end end
end end
context 'when data is not tfplan.json' do context 'when data is valid' do
it 'raises an error' do it 'parses JSON and returns a report' do
plan = { 'create' => 0, 'update' => 1, 'delete' => 0 }.to_s plan = '{ "create": 0, "update": 1, "delete": 0 }'
expect { subject.parse!(plan, reports, artifact: artifact) }.not_to raise_error
expect { subject.parse!(plan, reports, artifact: artifact) }.to raise_error( reports.plans.each do |key, hash_value|
described_class::TfplanParserError expect(hash_value.keys).to match_array(%w[create delete job_name job_path update])
end
expect(reports.plans).to match(
a_hash_including(
artifact.job.id.to_s => a_hash_including(
'create' => 0,
'update' => 1,
'delete' => 0,
'job_name' => artifact.job.options.dig(:artifacts, :name).to_s
)
)
)
end
it 'parses JSON when extra keys are present' do
plan = '{ "create": 0, "update": 1, "delete": 0, "extra_key": 4 }'
expect { subject.parse!(plan, reports, artifact: artifact) }.not_to raise_error
reports.plans.each do |key, hash_value|
expect(hash_value.keys).to match_array(%w[create delete job_name job_path update])
end
expect(reports.plans).to match(
a_hash_including(
artifact.job.id.to_s => a_hash_including(
'create' => 0,
'update' => 1,
'delete' => 0,
'job_name' => artifact.job.options.dig(:artifacts, :name).to_s
)
)
) )
end end
end end
......
...@@ -10,23 +10,23 @@ describe Gitlab::Ci::Reports::TerraformReports do ...@@ -10,23 +10,23 @@ describe Gitlab::Ci::Reports::TerraformReports do
describe '#add_plan' do describe '#add_plan' do
context 'when providing two unique plans' do context 'when providing two unique plans' do
it 'returns two plans' do it 'returns two plans' do
subject.add_plan('a/tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 }) subject.add_plan('123', { 'create' => 1, 'update' => 2, 'delete' => 3 })
subject.add_plan('b/tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 }) subject.add_plan('456', { 'create' => 4, 'update' => 5, 'delete' => 6 })
expect(subject.plans).to eq({ expect(subject.plans).to eq({
'a/tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 }, '123' => { 'create' => 1, 'update' => 2, 'delete' => 3 },
'b/tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 } '456' => { 'create' => 4, 'update' => 5, 'delete' => 6 }
}) })
end end
end end
context 'when providing the same plan twice' do context 'when providing the same plan twice' do
it 'returns the last added plan' do it 'returns the last added plan' do
subject.add_plan('tfplan.json', { 'create' => 0, 'update' => 0, 'delete' => 0 }) subject.add_plan('123', { 'create' => 0, 'update' => 0, 'delete' => 0 })
subject.add_plan('tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 }) subject.add_plan('123', { 'create' => 1, 'update' => 2, 'delete' => 3 })
expect(subject.plans).to eq({ expect(subject.plans).to eq({
'tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 } '123' => { 'create' => 1, 'update' => 2, 'delete' => 3 }
}) })
end end
end end
......
...@@ -68,7 +68,7 @@ describe Gitlab::UsageData, :aggregate_failures do ...@@ -68,7 +68,7 @@ describe Gitlab::UsageData, :aggregate_failures do
expect(count_data[:projects_with_prometheus_alerts]).to eq(2) expect(count_data[:projects_with_prometheus_alerts]).to eq(2)
expect(count_data[:projects_with_terraform_reports]).to eq(2) expect(count_data[:projects_with_terraform_reports]).to eq(2)
expect(count_data[:projects_with_terraform_states]).to eq(2) expect(count_data[:projects_with_terraform_states]).to eq(2)
expect(count_data[:terraform_reports]).to eq(3) expect(count_data[:terraform_reports]).to eq(6)
expect(count_data[:terraform_states]).to eq(3) expect(count_data[:terraform_states]).to eq(3)
expect(count_data[:issues_created_from_gitlab_error_tracking_ui]).to eq(1) expect(count_data[:issues_created_from_gitlab_error_tracking_ui]).to eq(1)
expect(count_data[:issues_with_associated_zoom_link]).to eq(2) expect(count_data[:issues_with_associated_zoom_link]).to eq(2)
......
...@@ -4068,10 +4068,11 @@ describe Ci::Build do ...@@ -4068,10 +4068,11 @@ describe Ci::Build do
expect(terraform_reports.plans).to match( expect(terraform_reports.plans).to match(
a_hash_including( a_hash_including(
'tfplan.json' => a_hash_including( build.id.to_s => a_hash_including(
'create' => 0, 'create' => 0,
'update' => 1, 'update' => 1,
'delete' => 0 'delete' => 0,
'job_name' => build.options.dig(:artifacts, :name).to_s
) )
) )
) )
......
...@@ -12,15 +12,23 @@ describe Ci::GenerateTerraformReportsService do ...@@ -12,15 +12,23 @@ describe Ci::GenerateTerraformReportsService do
context 'when head pipeline has terraform reports' do context 'when head pipeline has terraform reports' do
it 'returns status and data' do it 'returns status and data' do
result = subject.execute(nil, merge_request.head_pipeline) pipeline = merge_request.head_pipeline
result = subject.execute(nil, pipeline)
expect(result).to match(
status: :parsed, pipeline.builds.each do |build|
data: match( expect(result).to match(
a_hash_including('tfplan.json' => a_hash_including('create' => 0, 'update' => 1, 'delete' => 0)) status: :parsed,
), data: match(
key: an_instance_of(Array) a_hash_including(build.id.to_s => hash_including(
) 'create' => 0,
'delete' => 0,
'update' => 1,
'job_name' => build.options.dig(:artifacts, :name).to_s
))
),
key: an_instance_of(Array)
)
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment