Merge branch 'emilyring-tf-widget-multiple' into 'master'

Terraform Plan Widget, Multiple Plans

See merge request gitlab-org/gitlab!33867

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_terraform_plan.vue

@@ -39,7 +39,8 @@ export default {
       return this.plan.job_path;
     },
     plan() {
-      return this.plans['tfplan.json'] || {};
+      const firstPlanKey = Object.keys(this.plans)[0];
+      return this.plans[firstPlanKey] ?? {};
     },
     validPlanValues() {
       return this.addNum + this.changeNum + this.deleteNum >= 0;

changelogs/unreleased/emilyring-tf-widget-multiple.yml

+---
+title: Allow Tf Plan to genrate multiple reports
+merge_request: 33867
+author:
+type: changed

lib/gitlab/ci/parsers/terraform/tfplan.rb

@@ -8,15 +8,11 @@ module Gitlab
           TfplanParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
 
           def parse!(json_data, terraform_reports, artifact:)
-            tfplan = Gitlab::Json.parse(json_data).tap do |parsed_data|
-              parsed_data['job_path'] = Gitlab::Routing.url_helpers.project_job_path(
-                artifact.job.project, artifact.job
-              )
-            end
+            plan_data = Gitlab::Json.parse(json_data)
 
-            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
             raise TfplanParserError, 'JSON parsing failed'
           rescue
@@ -25,8 +21,18 @@ module Gitlab
 
           private
 
-          def valid_supported_keys?(tfplan)
-            tfplan.keys == %w[create update delete job_path]
+          def has_required_keys?(plan_data)
+            (%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

lib/gitlab/ci/reports/terraform_reports.rb

@@ -10,14 +10,6 @@ module Gitlab
           @plans = {}
         end
 
-        def pick(keys)
-          terraform_plans = plans.select do |key|
-            keys.include?(key)
-          end
-
-          { plans: terraform_plans }
-        end
-
         def add_plan(name, plan)
           plans[name] = plan
         end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -1183,17 +1183,21 @@ RSpec.describe Projects::MergeRequestsController do
           subject
 
           expect(response).to have_gitlab_http_status(:ok)
+
+          pipeline.builds.each do |build|
             expect(json_response).to match(
               a_hash_including(
-              'tfplan.json' => hash_including(
+                build.id.to_s => hash_including(
                   'create' => 0,
                   'delete' => 0,
-                'update' => 1
+                  'update' => 1,
+                  'job_name' => build.options.dig(:artifacts, :name).to_s
                 )
               )
             )
           end
         end
+      end
 
       context 'when user created corrupted terraform reports' do
         let(:report) { { status: :error, status_reason: 'Failed to parse terraform reports' } }

spec/factories/ci/pipelines.rb

@@ -110,6 +110,7 @@ FactoryBot.define do
 
         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)
         end
       end
 

spec/frontend/vue_mr_widget/components/mr_widget_terraform_plan_spec.js

@@ -38,7 +38,7 @@ describe('MrWidgetTerraformPlan', () => {
 
   describe('loading poll', () => {
     beforeEach(() => {
-      mockPollingApi(200, { 'tfplan.json': plan }, {});
+      mockPollingApi(200, { '123': plan }, {});
 
       return mountWrapper().then(() => {
         wrapper.setData({ loading: true });
@@ -65,7 +65,7 @@ describe('MrWidgetTerraformPlan', () => {
       pollRequest = jest.spyOn(Poll.prototype, 'makeRequest');
       pollStop = jest.spyOn(Poll.prototype, 'stop');
 
-      mockPollingApi(200, { 'tfplan.json': plan }, {});
+      mockPollingApi(200, { '123': plan }, {});
 
       return mountWrapper();
     });

spec/lib/gitlab/ci/parsers/terraform/tfplan_spec.rb

@@ -8,7 +8,7 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do
 
     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
         it 'raises an error' do
           plan = '{}'
@@ -19,31 +19,67 @@ describe Gitlab::Ci::Parsers::Terraform::Tfplan do
         end
       end
 
-      context 'when there is data' do
+      context 'when data is not a JSON file' do
+        it 'raises an error' do
+          plan = { 'create' => 0, 'update' => 1, 'delete' => 0 }.to_s
+
+          expect { subject.parse!(plan, reports, artifact: artifact) }.to raise_error(
+            described_class::TfplanParserError
+          )
+        end
+      end
+
+      context 'when JSON is missing a required key' do
+        it 'raises an error' do
+          plan = '{ "wrong_key": 1 }'
+
+          expect { subject.parse!(plan, reports, artifact: artifact) }.to raise_error(
+            described_class::TfplanParserError
+          )
+        end
+      end
+    end
+
+    context 'when data is valid' do
       it 'parses JSON and returns a report' do
         plan = '{ "create": 0, "update": 1, "delete": 0 }'
 
         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(
-              'tfplan.json' => a_hash_including(
+            artifact.job.id.to_s => a_hash_including(
               'create' => 0,
               'update' => 1,
-                'delete' => 0
+              'delete' => 0,
+              'job_name' => artifact.job.options.dig(:artifacts, :name).to_s
             )
           )
         )
       end
-      end
-    end
 
-    context 'when data is not tfplan.json' do
-      it 'raises an error' do
-        plan = { 'create' => 0, 'update' => 1, 'delete' => 0 }.to_s
+      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) }.to raise_error(
-          described_class::TfplanParserError
+        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

spec/lib/gitlab/ci/reports/terraform_reports_spec.rb

@@ -10,23 +10,23 @@ describe Gitlab::Ci::Reports::TerraformReports do
   describe '#add_plan' do
     context 'when providing two unique plans' do
       it 'returns two plans' do
-        subject.add_plan('a/tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 })
-        subject.add_plan('b/tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 })
+        subject.add_plan('123', { 'create' => 1, 'update' => 2, 'delete' => 3 })
+        subject.add_plan('456', { 'create' => 4, 'update' => 5, 'delete' => 6 })
 
         expect(subject.plans).to eq({
-          'a/tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 },
-          'b/tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 }
+          '123' => { 'create' => 1, 'update' => 2, 'delete' => 3 },
+          '456' => { 'create' => 4, 'update' => 5, 'delete' => 6 }
         })
       end
     end
 
     context 'when providing the same plan twice' do
       it 'returns the last added plan' do
-        subject.add_plan('tfplan.json', { 'create' => 0, 'update' => 0, 'delete' => 0 })
-        subject.add_plan('tfplan.json', { 'create' => 0, 'update' => 1, 'delete' => 0 })
+        subject.add_plan('123', { 'create' => 0, 'update' => 0, 'delete' => 0 })
+        subject.add_plan('123', { 'create' => 1, 'update' => 2, 'delete' => 3 })
 
         expect(subject.plans).to eq({
-          'tfplan.json' => { 'create' => 0, 'update' => 1, 'delete' => 0 }
+          '123' => { 'create' => 1, 'update' => 2, 'delete' => 3 }
         })
       end
     end

spec/lib/gitlab/usage_data_spec.rb

@@ -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_terraform_reports]).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[:issues_created_from_gitlab_error_tracking_ui]).to eq(1)
       expect(count_data[:issues_with_associated_zoom_link]).to eq(2)

spec/models/ci/build_spec.rb

@@ -4068,10 +4068,11 @@ describe Ci::Build do
 
           expect(terraform_reports.plans).to match(
             a_hash_including(
-              'tfplan.json' => a_hash_including(
+              build.id.to_s => a_hash_including(
                 'create' => 0,
                 'update' => 1,
-                'delete' => 0
+                'delete' => 0,
+                'job_name' => build.options.dig(:artifacts, :name).to_s
               )
             )
           )

spec/services/ci/generate_terraform_reports_service_spec.rb

@@ -12,17 +12,25 @@ describe Ci::GenerateTerraformReportsService do
 
     context 'when head pipeline has terraform reports' 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)
 
+        pipeline.builds.each do |build|
           expect(result).to match(
             status: :parsed,
             data: match(
-            a_hash_including('tfplan.json' => a_hash_including('create' => 0, 'update' => 1, 'delete' => 0))
+              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
 
     context 'when head pipeline has corrupted terraform reports' do
       it 'returns status and error message' do