Merge branch 'ci-add-extra-properties-to-external-validation-payload' into 'master'

Add extra fields to the pipeline validation payload

See merge request gitlab-org/gitlab!56969

changelogs/unreleased/ci-add-extra-properties-to-external-validation-payload.yml

+---
+title: Add extra fields to the external pipeline validation payload
+merge_request: 56969
+author:
+type: changed

doc/administration/external_pipeline_validation.md

@@ -38,18 +38,21 @@ Set the `EXTERNAL_VALIDATION_SERVICE_URL` to the external service URL and enable
     "project",
     "user",
     "pipeline",
-    "builds"
+    "builds",
+    "namespace"
   ],
   "properties" : {
     "project": {
       "type": "object",
       "required": [
         "id",
-        "path"
+        "path",
+        "created_at"
       ],
       "properties": {
         "id": { "type": "integer" },
-        "path": { "type": "string" }
+        "path": { "type": "string" },
+        "created_at": { "type": ["string", "null"], "format": "date-time" }
       }
     },
     "user": {
@@ -57,12 +60,14 @@ Set the `EXTERNAL_VALIDATION_SERVICE_URL` to the external service URL and enable
       "required": [
         "id",
         "username",
-        "email"
+        "email",
+        "created_at"
       ],
       "properties": {
         "id": { "type": "integer" },
         "username": { "type": "string" },
-        "email": { "type": "string" }
+        "email": { "type": "string" },
+        "created_at": { "type": ["string", "null"], "format": "date-time" }
       }
     },
     "pipeline": {
@@ -103,8 +108,18 @@ Set the `EXTERNAL_VALIDATION_SERVICE_URL` to the external service URL and enable
           }
         }
       }
+    },
+    "namespace": {
+      "type": "object",
+      "required": [
+        "plan",
+        "trial"
+      ],
+      "properties": {
+        "plan": { "type": "string" },
+        "trial": { "type": "boolean" }
+      }
     }
-  },
-  "additionalProperties": false
+  }
 }
 ```

ee/lib/ee/gitlab/ci/pipeline/chain/validate/external.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Pipeline
+        module Chain
+          module Validate
+            module External
+              extend ::Gitlab::Utils::Override
+
+              private
+
+              delegate :namespace, to: :project
+
+              override :validation_service_payload
+              def validation_service_payload
+                super.deep_merge(
+                  namespace: {
+                    plan: namespace.actual_plan_name,
+                    trial: namespace.trial_active?
+                  }
+                )
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/fixtures/api/schemas/external_validation.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../spec/fixtures/api/schemas/external_validation.json" },
+    {
+      "required" : [
+        "namespace"
+      ],
+      "properties": {
+        "namespace": {
+          "type": "object",
+          "required": [
+            "plan",
+            "trial"
+          ],
+          "properties": {
+            "plan": { "type": "string" },
+            "trial": { "type": "boolean" }
+          }
+        }
+      }
+    }
+  ]
+}

ee/spec/lib/ee/gitlab/ci/pipeline/chain/validate/external_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:subscription) { create(:gitlab_subscription, :default, namespace: user.namespace) }
+  let(:pipeline) { build(:ci_empty_pipeline, user: user, project: project) }
+
+  let(:ci_yaml) do
+    <<-CI_YAML
+    job:
+      script: ls
+      parallel: 5
+    CI_YAML
+  end
+
+  let(:yaml_processor_result) do
+    ::Gitlab::Ci::YamlProcessor.new(
+      ci_yaml, {
+        project: project,
+        sha: pipeline.sha,
+        user: user
+      }
+    ).execute
+  end
+
+  let(:command) do
+    Gitlab::Ci::Pipeline::Chain::Command.new(
+      project: project, current_user: user, yaml_processor_result: yaml_processor_result, save_incompleted: true
+    )
+  end
+
+  let(:step) { described_class.new(pipeline, command) }
+  let(:validation_service_url) { 'https://validation-service.external/' }
+
+  describe '#validation_service_payload' do
+    before do
+      stub_env('EXTERNAL_VALIDATION_SERVICE_URL', validation_service_url)
+    end
+
+    it 'respects the defined schema' do
+      expect(::Gitlab::HTTP).to receive(:post) do |_url, params|
+        expect(params[:body]).to match_schema('/external_validation', dir: 'ee')
+      end
+
+      step.perform!
+    end
+
+    it 'does not fire N+1 SQL queries' do
+      stub_request(:post, validation_service_url)
+
+      expect { step.perform! }.not_to exceed_query_limit(4)
+    end
+  end
+end

lib/gitlab/ci/pipeline/chain/validate/external.rb

@@ -21,13 +21,13 @@ module Gitlab
               pipeline_authorized = validate_external
 
               log_message = pipeline_authorized ? 'authorized' : 'not authorized'
-              Gitlab::AppLogger.info(message: "Pipeline #{log_message}", project_id: @pipeline.project.id, user_id: @pipeline.user.id)
+              Gitlab::AppLogger.info(message: "Pipeline #{log_message}", project_id: project.id, user_id: current_user.id)
 
               error('External validation failed', drop_reason: :external_validation_failure) unless pipeline_authorized
             end
 
             def break?
-              @pipeline.errors.any?
+              pipeline.errors.any?
             end
 
             private
@@ -71,7 +71,7 @@ module Gitlab
             def validate_service_request
               Gitlab::HTTP.post(
                 validation_service_url, timeout: VALIDATION_REQUEST_TIMEOUT,
-                body: validation_service_payload(@pipeline, @command.yaml_processor_result.stages_attributes)
+                body: validation_service_payload.to_json
               )
             end
 
@@ -79,28 +79,30 @@ module Gitlab
               ENV['EXTERNAL_VALIDATION_SERVICE_URL']
             end
 
-            def validation_service_payload(pipeline, stages_attributes)
+            def validation_service_payload
               {
                 project: {
-                  id: pipeline.project.id,
-                  path: pipeline.project.full_path
+                  id: project.id,
+                  path: project.full_path,
+                  created_at: project.created_at&.iso8601
                 },
                 user: {
-                  id: pipeline.user.id,
-                  username: pipeline.user.username,
-                  email: pipeline.user.email
+                  id: current_user.id,
+                  username: current_user.username,
+                  email: current_user.email,
+                  created_at: current_user.created_at&.iso8601
                 },
                 pipeline: {
                   sha: pipeline.sha,
                   ref: pipeline.ref,
                   type: pipeline.source
                 },
-                builds: builds_validation_payload(stages_attributes)
-              }.to_json
+                builds: builds_validation_payload
+              }
             end
 
-            def builds_validation_payload(stages_attributes)
-              stages_attributes.map { |stage| stage[:builds] }.flatten
+            def builds_validation_payload
+              stages_attributes.flat_map { |stage| stage[:builds] }
                 .map(&method(:build_validation_payload))
             end
 
@@ -117,9 +119,15 @@ module Gitlab
                 ].flatten.compact
               }
             end
+
+            def stages_attributes
+              command.yaml_processor_result.stages_attributes
+            end
           end
         end
       end
     end
   end
 end
+
+Gitlab::Ci::Pipeline::Chain::Validate::External.prepend_if_ee('EE::Gitlab::Ci::Pipeline::Chain::Validate::External')

spec/fixtures/api/schemas/external_validation.json

@@ -11,11 +11,13 @@
       "type": "object",
       "required": [
         "id",
-        "path"
+        "path",
+        "created_at"
       ],
       "properties": {
         "id": { "type": "integer" },
-        "path": { "type": "string" }
+        "path": { "type": "string" },
+        "created_at": { "type": ["string", "null"], "format": "date-time" }
       }
     },
     "user": {
@@ -23,12 +25,14 @@
       "required": [
         "id",
         "username",
-        "email"
+        "email",
+        "created_at"
       ],
       "properties": {
         "id": { "type": "integer" },
         "username": { "type": "string" },
-        "email": { "type": "string" }
+        "email": { "type": "string" },
+        "created_at": { "type": ["string", "null"], "format": "date-time" }
       }
     },
     "pipeline": {
@@ -70,6 +74,5 @@
         }
       }
     }
-  },
-  "additionalProperties": false
+  }
 }

spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
-  let(:project) { create(:project) }
-  let(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
   let(:pipeline) { build(:ci_empty_pipeline, user: user, project: project) }
   let!(:step) { described_class.new(pipeline, command) }
 
@@ -59,6 +59,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
       allow(Gitlab).to receive(:com?).and_return(dot_com)
     end
 
+    it 'respects the defined payload schema' do
+      expect(::Gitlab::HTTP).to receive(:post) do |_url, params|
+        expect(params[:body]).to match_schema('/external_validation')
+      end
+
+      perform!
+    end
+
     shared_examples 'successful external authorization' do
       it 'does not drop the pipeline' do
         perform!
@@ -224,16 +232,4 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
       end
     end
   end
-
-  describe '#validation_service_payload' do
-    subject(:validation_service_payload) { step.send(:validation_service_payload, pipeline, command.yaml_processor_result.stages_attributes) }
-
-    it 'respects the defined schema' do
-      expect(validation_service_payload).to match_schema('/external_validation')
-    end
-
-    it 'does not fire sql queries' do
-      expect { validation_service_payload }.not_to exceed_query_limit(1)
-    end
-  end
 end