From 11f14607e313dbcf64eee8c889756b62c590af4f Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Mon, 23 May 2022 17:09:15 +0300
Subject: [PATCH 1/6] Add pull-policy support for images on CI config

In the runner config.toml file, you can already define pull_policy
for runners. This config determines how to fetch the needed images.
In this change, we are adding this config to CI config YAML.
The config can be a string or array of strings.
The config will be filtered by the allowed_pull_policies config
in the Runner side.

It's behind a feature flag ci_docker_image_pull_policy
---
 .../ci_docker_image_pull_policy.yml           |  8 ++
 doc/ci/yaml/index.md                          | 39 +++++++++
 lib/api/entities/ci/job_request/image.rb      |  2 +
 lib/api/entities/ci/job_request/service.rb    |  5 +-
 lib/gitlab/ci/build/image.rb                  |  3 +-
 lib/gitlab/ci/config/entry/image.rb           | 23 +++--
 lib/gitlab/ci/config/entry/pull_policy.rb     | 34 ++++++++
 lib/gitlab/config/entry/node.rb               |  4 +
 .../api/entities/ci/job_request/image_spec.rb | 16 +++-
 spec/lib/gitlab/ci/build/image_spec.rb        |  9 +-
 spec/lib/gitlab/ci/config/entry/image_spec.rb | 31 ++++++-
 .../ci/config/entry/pull_policy_spec.rb       | 87 +++++++++++++++++++
 spec/lib/gitlab/ci/yaml_processor_spec.rb     | 33 ++++++-
 .../api/ci/runner/jobs_request_post_spec.rb   | 41 ++++++++-
 14 files changed, 322 insertions(+), 13 deletions(-)
 create mode 100644 config/feature_flags/development/ci_docker_image_pull_policy.yml
 create mode 100644 lib/gitlab/ci/config/entry/pull_policy.rb
 create mode 100644 spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb

diff --git a/config/feature_flags/development/ci_docker_image_pull_policy.yml b/config/feature_flags/development/ci_docker_image_pull_policy.yml
new file mode 100644
index 00000000000000..09e01fb5232491
--- /dev/null
+++ b/config/feature_flags/development/ci_docker_image_pull_policy.yml
@@ -0,0 +1,8 @@
+---
+name: ci_docker_image_pull_policy
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85588
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/363186
+milestone: '15.1'
+type: development
+group: group::pipeline authoring
+default_enabled: false
diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md
index e5c4eb45131df3..5f262e40739a07 100644
--- a/doc/ci/yaml/index.md
+++ b/doc/ci/yaml/index.md
@@ -1878,6 +1878,45 @@ image:
 
 - [Override the entrypoint of an image](../docker/using_docker_images.md#override-the-entrypoint-of-an-image).
 
+#### `image:pull_policy`
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21619) in GitLab 15.1 [with a flag](../../administration/feature_flags.md) named `ci_docker_image_pull_policy`. Disabled by default.
+> - Requires GitLab Runner 15.1 or later.
+
+FLAG:
+On self-managed GitLab, by default this feature is not available. To make it available,
+ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `ci_docker_image_pull_policy`.
+The feature is not ready for production use.
+
+The pull policy that the runner uses to fetch the Docker image.
+
+**Keyword type**: Job keyword. You can use it only as part of a job or in the [`default` section](#default).
+
+**Possible inputs**:
+
+- A string. (`always` or `if-not-present` or `never`)
+- An array of strings. (e.g. `[always, if-not-present]`).
+See [Using multiple pull policies](https://docs.gitlab.com/runner/executors/docker.html#using-multiple-pull-policies).
+
+**Examples of `image:pull_policy`**:
+
+```yaml
+image:
+  name: ruby:3.0
+  pull_policy: if-not-present
+```
+
+```yaml
+image:
+  name: ruby:3.0
+  pull_policy: [always, if-not-present]
+```
+
+**Related topics**:
+
+- [Run your CI/CD jobs in Docker containers](../docker/using_docker_images.md).
+- [Runner pull policy documentation](https://docs.gitlab.com/runner/executors/docker.html#how-pull-policies-work).
+
 ### `inherit`
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/207484) in GitLab 12.9.
diff --git a/lib/api/entities/ci/job_request/image.rb b/lib/api/entities/ci/job_request/image.rb
index 8e404a8fa02917..83f64da605023c 100644
--- a/lib/api/entities/ci/job_request/image.rb
+++ b/lib/api/entities/ci/job_request/image.rb
@@ -7,6 +7,8 @@ module JobRequest
         class Image < Grape::Entity
           expose :name, :entrypoint
           expose :ports, using: Entities::Ci::JobRequest::Port
+
+          expose :pull_policy, if: ->(_) { ::Feature.enabled?(:ci_docker_image_pull_policy) }
         end
       end
     end
diff --git a/lib/api/entities/ci/job_request/service.rb b/lib/api/entities/ci/job_request/service.rb
index 0dae5d5a93350b..d9da2c92ec7c99 100644
--- a/lib/api/entities/ci/job_request/service.rb
+++ b/lib/api/entities/ci/job_request/service.rb
@@ -4,7 +4,10 @@ module API
   module Entities
     module Ci
       module JobRequest
-        class Service < Entities::Ci::JobRequest::Image
+        class Service < Grape::Entity
+          expose :name, :entrypoint
+          expose :ports, using: Entities::Ci::JobRequest::Port
+
           expose :alias, :command
           expose :variables
         end
diff --git a/lib/gitlab/ci/build/image.rb b/lib/gitlab/ci/build/image.rb
index 8ddcf1d523e2d3..7dc375e05ebbab 100644
--- a/lib/gitlab/ci/build/image.rb
+++ b/lib/gitlab/ci/build/image.rb
@@ -4,7 +4,7 @@ module Gitlab
   module Ci
     module Build
       class Image
-        attr_reader :alias, :command, :entrypoint, :name, :ports, :variables
+        attr_reader :alias, :command, :entrypoint, :name, :ports, :variables, :pull_policy
 
         class << self
           def from_image(job)
@@ -34,6 +34,7 @@ def initialize(image)
             @name = image[:name]
             @ports = build_ports(image).select(&:valid?)
             @variables = build_variables(image)
+            @pull_policy = image[:pull_policy]
           end
         end
 
diff --git a/lib/gitlab/ci/config/entry/image.rb b/lib/gitlab/ci/config/entry/image.rb
index 21c42857895451..cb9ce26f3ad9ec 100644
--- a/lib/gitlab/ci/config/entry/image.rb
+++ b/lib/gitlab/ci/config/entry/image.rb
@@ -12,7 +12,7 @@ class Image < ::Gitlab::Config::Entry::Node
           include ::Gitlab::Config::Entry::Attributable
           include ::Gitlab::Config::Entry::Configurable
 
-          ALLOWED_KEYS = %i[name entrypoint ports].freeze
+          ALLOWED_KEYS = %i[name entrypoint ports pull_policy].freeze
 
           validations do
             validates :config, hash_or_string: true
@@ -26,7 +26,10 @@ class Image < ::Gitlab::Config::Entry::Node
           entry :ports, Entry::Ports,
             description: 'Ports used to expose the image'
 
-          attributes :ports
+          entry :pull_policy, Entry::PullPolicy,
+            description: 'Pull policy for the image'
+
+          attributes :ports, :pull_policy
 
           def name
             value[:name]
@@ -37,10 +40,18 @@ def entrypoint
           end
 
           def value
-            return { name: @config } if string?
-            return @config if hash?
-
-            {}
+            if string?
+              { name: @config }
+            elsif hash?
+              {
+                name: @config[:name],
+                entrypoint: @config[:entrypoint],
+                ports: ports_value,
+                pull_policy: pull_policy_value
+              }.compact
+            else
+              {}
+            end
           end
 
           def with_image_ports?
diff --git a/lib/gitlab/ci/config/entry/pull_policy.rb b/lib/gitlab/ci/config/entry/pull_policy.rb
new file mode 100644
index 00000000000000..f597134dd2c337
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/pull_policy.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents a configuration of the pull policies of an image.
+        #
+        class PullPolicy < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Validatable
+
+          ALLOWED_POLICIES = %w[always never if-not-present].freeze
+
+          validations do
+            validates :config, array_of_strings_or_string: true
+            validates :config,
+                      allowed_array_values: { in: ALLOWED_POLICIES },
+                      presence: true,
+                      if: :array?
+            validates :config,
+                      inclusion: { in: ALLOWED_POLICIES },
+                      if: :string?
+          end
+
+          def value
+            # We either return an array with policies or nothing
+            Array(@config).presence
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/config/entry/node.rb b/lib/gitlab/config/entry/node.rb
index 6ce7046262b546..40418a4c7976b6 100644
--- a/lib/gitlab/config/entry/node.rb
+++ b/lib/gitlab/config/entry/node.rb
@@ -106,6 +106,10 @@ def hash?
           @config.is_a?(Hash)
         end
 
+        def array?
+          @config.is_a?(Array)
+        end
+
         def string?
           @config.is_a?(String)
         end
diff --git a/spec/lib/api/entities/ci/job_request/image_spec.rb b/spec/lib/api/entities/ci/job_request/image_spec.rb
index 55aade03129a1f..a426e6831d771b 100644
--- a/spec/lib/api/entities/ci/job_request/image_spec.rb
+++ b/spec/lib/api/entities/ci/job_request/image_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe API::Entities::Ci::JobRequest::Image do
   let(:ports) { [{ number: 80, protocol: 'http', name: 'name' }]}
-  let(:image) { double(name: 'image_name', entrypoint: ['foo'], ports: ports)}
+  let(:image) { double(name: 'image_name', entrypoint: ['foo'], ports: ports, pull_policy: ["if-not-present"]) }
   let(:entity) { described_class.new(image) }
 
   subject { entity.as_json }
@@ -28,4 +28,18 @@
       expect(subject[:ports]).to be_nil
     end
   end
+
+  it 'returns the pull policy' do
+    expect(subject[:pull_policy]).to eq(['if-not-present'])
+  end
+
+  context 'when the FF ci_docker_image_pull_policy is disabled' do
+    before do
+      stub_feature_flags(ci_docker_image_pull_policy: false)
+    end
+
+    it 'does not return the pull policy' do
+      expect(subject).not_to have_key(:pull_policy)
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/build/image_spec.rb b/spec/lib/gitlab/ci/build/image_spec.rb
index 630dfcd06bb7e1..8f77a1f60ad2cf 100644
--- a/spec/lib/gitlab/ci/build/image_spec.rb
+++ b/spec/lib/gitlab/ci/build/image_spec.rb
@@ -28,8 +28,14 @@
 
       context 'when image is defined as hash' do
         let(:entrypoint) { '/bin/sh' }
+        let(:pull_policy) { %w[always if-not-present] }
 
-        let(:job) { create(:ci_build, options: { image: { name: image_name, entrypoint: entrypoint, ports: [80] } } ) }
+        let(:job) do
+          create(:ci_build, options: { image: { name: image_name,
+                                                entrypoint: entrypoint,
+                                                ports: [80],
+                                                pull_policy: pull_policy } } )
+        end
 
         it 'fabricates an object of the proper class' do
           is_expected.to be_kind_of(described_class)
@@ -38,6 +44,7 @@
         it 'populates fabricated object with the proper attributes' do
           expect(subject.name).to eq(image_name)
           expect(subject.entrypoint).to eq(entrypoint)
+          expect(subject.pull_policy).to eq(pull_policy)
         end
 
         it 'populates the ports' do
diff --git a/spec/lib/gitlab/ci/config/entry/image_spec.rb b/spec/lib/gitlab/ci/config/entry/image_spec.rb
index e16a9a7a74a90d..f32a135e744314 100644
--- a/spec/lib/gitlab/ci/config/entry/image_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/image_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 RSpec.describe Gitlab::Ci::Config::Entry::Image do
   let(:entry) { described_class.new(config) }
@@ -43,6 +43,12 @@
         expect(entry.ports).to be_nil
       end
     end
+
+    describe '#pull_policy' do
+      it "returns nil" do
+        expect(entry.pull_policy).to be_nil
+      end
+    end
   end
 
   context 'when configuration is a hash' do
@@ -109,6 +115,29 @@
         end
       end
     end
+
+    context 'when configuration has pull_policy' do
+      let(:config) { { name: 'image:1.0', pull_policy: 'if-not-present' } }
+
+      before do
+        entry.compose!
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+
+      describe '#value' do
+        it "returns value" do
+          expect(entry.value).to eq(
+            name: 'image:1.0',
+            pull_policy: ['if-not-present']
+          )
+        end
+      end
+    end
   end
 
   context 'when entry value is not correct' do
diff --git a/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb
new file mode 100644
index 00000000000000..27f699ec2ce6ee
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb
@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Ci::Config::Entry::PullPolicy do
+  let(:entry) { described_class.new(config) }
+
+  describe '#value' do
+    subject(:value) { entry.value }
+
+    context 'when retry value is nil' do
+      let(:config) { nil }
+
+      it { is_expected.to eq(nil) }
+    end
+
+    context 'when retry value is an empty array' do
+      let(:config) { [] }
+
+      it { is_expected.to eq(nil) }
+    end
+
+    context 'when retry value is string' do
+      let(:config) { "always" }
+
+      it { is_expected.to eq(%w[always]) }
+    end
+
+    context 'when retry value is array' do
+      let(:config) { %w[always if-not-present] }
+
+      it { is_expected.to eq(%w[always if-not-present]) }
+    end
+  end
+
+  describe 'validation' do
+    subject(:valid?) { entry.valid? }
+
+    context 'when retry value is nil' do
+      let(:config) { nil }
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when retry value is an empty array' do
+      let(:config) { [] }
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when retry value is a hash' do
+      let(:config) { {} }
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when retry value is string' do
+      let(:config) { "always" }
+
+      it { is_expected.to eq(true) }
+
+      context 'when it is an invalid policy' do
+        let(:config) { "invalid" }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when it is an empty string' do
+        let(:config) { "" }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+
+    context 'when retry value is array' do
+      let(:config) { %w[always if-not-present] }
+
+      it { is_expected.to eq(true) }
+
+      context 'when config contains an invalid policy' do
+        let(:config) { %w[always invalid] }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 1910057622b892..da359eb75c7c03 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -7,7 +7,7 @@ module Ci
     RSpec.describe YamlProcessor do
       include StubRequests
 
-      subject { described_class.new(config, user: nil).execute }
+      subject(:processor) { described_class.new(config, user: nil).execute }
 
       shared_examples 'returns errors' do |error_message|
         it 'adds a message when an error is encountered' do
@@ -965,6 +965,37 @@ module Ci
             })
           end
         end
+
+        context 'when image has pull_policy' do
+          let(:config) do
+            <<~YAML
+            image:
+              name: ruby:2.7
+              pull_policy: if-not-present
+
+            test:
+              script: exit 0
+            YAML
+          end
+
+          it "returns image and service when defined" do
+            expect(processor.stage_builds_attributes("test")).to contain_exactly({
+              stage: "test",
+              stage_idx: 2,
+              name: "test",
+              only: { refs: %w[branches tags] },
+              options: {
+                script: ["exit 0"],
+                image: { name: "ruby:2.7", pull_policy: ["if-not-present"] }
+              },
+              allow_failure: false,
+              when: "on_success",
+              job_variables: [],
+              root_variables_inheritance: true,
+              scheduling_type: :stage
+            })
+          end
+        end
       end
 
       describe 'Variables' do
diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb
index dbc5f0e74e20f3..3c6f9ac2816d34 100644
--- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb
+++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb
@@ -216,7 +216,7 @@
             expect(json_response['token']).to eq(job.token)
             expect(json_response['job_info']).to eq(expected_job_info)
             expect(json_response['git_info']).to eq(expected_git_info)
-            expect(json_response['image']).to eq({ 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [] })
+            expect(json_response['image']).to eq({ 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [], 'pull_policy' => nil })
             expect(json_response['services']).to eq([{ 'name' => 'postgres', 'entrypoint' => nil,
                                                        'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => nil },
                                                      { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh',
@@ -810,6 +810,45 @@
           end
         end
 
+        context 'when image has pull_policy' do
+          let(:job) { create(:ci_build, :pending, :queued, pipeline: pipeline, options: options) }
+
+          let(:options) do
+            {
+              image: {
+                name: 'ruby',
+                pull_policy: ['if-not-present']
+              }
+            }
+          end
+
+          it 'returns the image with pull policy' do
+            request_job
+
+            expect(response).to have_gitlab_http_status(:created)
+            expect(json_response).to include(
+              'id' => job.id,
+              'image' => { 'name' => 'ruby', 'pull_policy' => ['if-not-present'], 'entrypoint' => nil, 'ports' => [] }
+            )
+          end
+
+          context 'when the FF ci_docker_image_pull_policy is disabled' do
+            before do
+              stub_feature_flags(ci_docker_image_pull_policy: false)
+            end
+
+            it 'returns the image without pull policy' do
+              request_job
+
+              expect(response).to have_gitlab_http_status(:created)
+              expect(json_response).to include(
+                'id' => job.id,
+                'image' => { 'name' => 'ruby', 'entrypoint' => nil, 'ports' => [] }
+              )
+            end
+          end
+        end
+
         describe 'a job with excluded artifacts' do
           context 'when excluded paths are defined' do
             let(:job) do
-- 
GitLab


From e5768c292037adb8a36243d8e81b85b8a403db5a Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Tue, 24 May 2022 10:34:49 +0000
Subject: [PATCH 2/6] Apply 3 suggestion(s) to 2 file(s)

---
 spec/lib/api/entities/ci/job_request/image_spec.rb  | 2 +-
 spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/lib/api/entities/ci/job_request/image_spec.rb b/spec/lib/api/entities/ci/job_request/image_spec.rb
index a426e6831d771b..3ab14ffc3aead9 100644
--- a/spec/lib/api/entities/ci/job_request/image_spec.rb
+++ b/spec/lib/api/entities/ci/job_request/image_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe API::Entities::Ci::JobRequest::Image do
   let(:ports) { [{ number: 80, protocol: 'http', name: 'name' }]}
-  let(:image) { double(name: 'image_name', entrypoint: ['foo'], ports: ports, pull_policy: ["if-not-present"]) }
+  let(:image) { double(name: 'image_name', entrypoint: ['foo'], ports: ports, pull_policy: ['if-not-present']) }
   let(:entity) { described_class.new(image) }
 
   subject { entity.as_json }
diff --git a/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb
index 27f699ec2ce6ee..c35355b10c6a24 100644
--- a/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/pull_policy_spec.rb
@@ -8,10 +8,10 @@
   describe '#value' do
     subject(:value) { entry.value }
 
-    context 'when retry value is nil' do
+    context 'when config value is nil' do
       let(:config) { nil }
 
-      it { is_expected.to eq(nil) }
+      it { is_expected.to be_nil }
     end
 
     context 'when retry value is an empty array' do
-- 
GitLab


From 2cf1a967c1d4bad73d34aa72ea20f429977fed63 Mon Sep 17 00:00:00 2001
From: Marcel Amirault <mamirault@gitlab.com>
Date: Tue, 24 May 2022 10:35:55 +0000
Subject: [PATCH 3/6] Apply 3 suggestion(s) to 1 file(s)

---
 doc/ci/yaml/index.md | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md
index 5f262e40739a07..6b3eab227ebcc8 100644
--- a/doc/ci/yaml/index.md
+++ b/doc/ci/yaml/index.md
@@ -1894,28 +1894,30 @@ The pull policy that the runner uses to fetch the Docker image.
 
 **Possible inputs**:
 
-- A string. (`always` or `if-not-present` or `never`)
-- An array of strings. (e.g. `[always, if-not-present]`).
-See [Using multiple pull policies](https://docs.gitlab.com/runner/executors/docker.html#using-multiple-pull-policies).
+- A single pull policy, or multiple pull policies in an array.
+  Can be `always`, `if-not-present`, or `never`.
 
 **Examples of `image:pull_policy`**:
 
 ```yaml
-image:
-  name: ruby:3.0
-  pull_policy: if-not-present
-```
+job1:
+  script: echo "A single pull policy."
+  image:
+    name: ruby:3.0
+    pull_policy: if-not-present
 
-```yaml
-image:
-  name: ruby:3.0
-  pull_policy: [always, if-not-present]
+job2:
+  script: echo "Multiple pull policies."
+  image:
+    name: ruby:3.0
+    pull_policy: [always, if-not-present]
 ```
 
 **Related topics**:
 
 - [Run your CI/CD jobs in Docker containers](../docker/using_docker_images.md).
-- [Runner pull policy documentation](https://docs.gitlab.com/runner/executors/docker.html#how-pull-policies-work).
+- [How runner pull policies work](https://docs.gitlab.com/runner/executors/docker.html#how-pull-policies-work).
+- [Using multiple pull policies](https://docs.gitlab.com/runner/executors/docker.html#using-multiple-pull-policies).
 
 ### `inherit`
 
-- 
GitLab


From 3a957a3757b7a054226f9c36d598200ca60f2860 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Wed, 25 May 2022 13:46:20 +0300
Subject: [PATCH 4/6] Add schema ci.json changes

---
 app/assets/javascripts/editor/schema/ci.json | 27 ++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json
index 8262dae2b897f6..c8015f884b7d2d 100644
--- a/app/assets/javascripts/editor/schema/ci.json
+++ b/app/assets/javascripts/editor/schema/ci.json
@@ -374,6 +374,33 @@
               "type": "array",
               "description": "Command or script that should be executed as the container's entrypoint. It will be translated to Docker's --entrypoint option while creating the container. The syntax is similar to Dockerfile's ENTRYPOINT directive, where each shell token is a separate string in the array.",
               "minItems": 1
+            },
+            "pull_policy": {
+              "markdownDescription": "Specifies how to pull the image in Runner. It can be one of `always`, `never` or `if-not-present`. The default value is `always`. [Learn more](https://docs.gitlab.com/ee/ci/yaml/#imagepull_policy).",
+              "default": "always",
+              "oneOf": [
+                {
+                  "type": "string",
+                  "enum": [
+                    "always",
+                    "never",
+                    "if-not-present"
+                  ]
+                },
+                {
+                  "type": "array",
+                  "items": {
+                    "type": "string",
+                    "enum": [
+                      "always",
+                      "never",
+                      "if-not-present"
+                    ]
+                  },
+                  "minItems": 1,
+                  "uniqueItems": true
+                }
+              ]
             }
           },
           "required": ["name"]
-- 
GitLab


From f13555c412f2abc9bdd56867700392686b9c88c1 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 31 May 2022 19:29:23 +0300
Subject: [PATCH 5/6] Use the feature flag in config entry

---
 lib/gitlab/ci/config/entry/image.rb           | 10 ++++-
 spec/lib/gitlab/ci/config/entry/image_spec.rb | 43 +++++++++++++++++--
 spec/lib/gitlab/ci/yaml_processor_spec.rb     | 14 ++++++
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/image.rb b/lib/gitlab/ci/config/entry/image.rb
index cb9ce26f3ad9ec..79443f69b03f1a 100644
--- a/lib/gitlab/ci/config/entry/image.rb
+++ b/lib/gitlab/ci/config/entry/image.rb
@@ -13,10 +13,12 @@ class Image < ::Gitlab::Config::Entry::Node
           include ::Gitlab::Config::Entry::Configurable
 
           ALLOWED_KEYS = %i[name entrypoint ports pull_policy].freeze
+          LEGACY_ALLOWED_KEYS = %i[name entrypoint ports].freeze
 
           validations do
             validates :config, hash_or_string: true
-            validates :config, allowed_keys: ALLOWED_KEYS
+            validates :config, allowed_keys: ALLOWED_KEYS, if: :ci_docker_image_pull_policy_enabled?
+            validates :config, allowed_keys: LEGACY_ALLOWED_KEYS, unless: :ci_docker_image_pull_policy_enabled?
             validates :config, disallowed_keys: %i[ports], unless: :with_image_ports?
 
             validates :name, type: String, presence: true
@@ -47,7 +49,7 @@ def value
                 name: @config[:name],
                 entrypoint: @config[:entrypoint],
                 ports: ports_value,
-                pull_policy: pull_policy_value
+                pull_policy: (ci_docker_image_pull_policy_enabled? ? pull_policy_value : nil)
               }.compact
             else
               {}
@@ -58,6 +60,10 @@ def with_image_ports?
             opt(:with_image_ports)
           end
 
+          def ci_docker_image_pull_policy_enabled?
+            ::Feature.enabled?(:ci_docker_image_pull_policy)
+          end
+
           def skip_config_hash_validation?
             true
           end
diff --git a/spec/lib/gitlab/ci/config/entry/image_spec.rb b/spec/lib/gitlab/ci/config/entry/image_spec.rb
index f32a135e744314..bd1ab5d8c412e9 100644
--- a/spec/lib/gitlab/ci/config/entry/image_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/image_spec.rb
@@ -1,8 +1,16 @@
 # frozen_string_literal: true
 
 require 'fast_spec_helper'
+require 'support/helpers/stubbed_feature'
+require 'support/helpers/stub_feature_flags'
 
 RSpec.describe Gitlab::Ci::Config::Entry::Image do
+  include StubFeatureFlags
+
+  before do
+    stub_feature_flags(ci_docker_image_pull_policy: true)
+  end
+
   let(:entry) { described_class.new(config) }
 
   context 'when configuration is a string' do
@@ -119,23 +127,50 @@
     context 'when configuration has pull_policy' do
       let(:config) { { name: 'image:1.0', pull_policy: 'if-not-present' } }
 
-      before do
-        entry.compose!
-      end
-
       describe '#valid?' do
         it 'is valid' do
+          entry.compose!
+
           expect(entry).to be_valid
         end
+
+        context 'when the feature flag ci_docker_image_pull_policy is disabled' do
+          before do
+            stub_feature_flags(ci_docker_image_pull_policy: false)
+          end
+
+          it 'is not valid' do
+            entry.compose!
+
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include('image config contains unknown keys: pull_policy')
+          end
+        end
       end
 
       describe '#value' do
         it "returns value" do
+          entry.compose!
+
           expect(entry.value).to eq(
             name: 'image:1.0',
             pull_policy: ['if-not-present']
           )
         end
+
+        context 'when the feature flag ci_docker_image_pull_policy is disabled' do
+          before do
+            stub_feature_flags(ci_docker_image_pull_policy: false)
+          end
+
+          it 'is not valid' do
+            entry.compose!
+
+            expect(entry.value).to eq(
+              name: 'image:1.0'
+            )
+          end
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index da359eb75c7c03..3dd9ca35881635 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -978,6 +978,8 @@ module Ci
             YAML
           end
 
+          it { is_expected.to be_valid }
+
           it "returns image and service when defined" do
             expect(processor.stage_builds_attributes("test")).to contain_exactly({
               stage: "test",
@@ -995,6 +997,18 @@ module Ci
               scheduling_type: :stage
             })
           end
+
+          context 'when the feature flag ci_docker_image_pull_policy is disabled' do
+            before do
+              stub_feature_flags(ci_docker_image_pull_policy: false)
+            end
+
+            it { is_expected.not_to be_valid }
+
+            it "returns no job" do
+              expect(processor.jobs).to eq({})
+            end
+          end
         end
       end
 
-- 
GitLab


From e56151ad86b71c18de42187de17a091ea3cb8548 Mon Sep 17 00:00:00 2001
From: Marcel Amirault <mamirault@gitlab.com>
Date: Mon, 6 Jun 2022 10:00:42 +0000
Subject: [PATCH 6/6] Apply 1 suggestion(s) to 1 file(s)

---
 doc/ci/yaml/index.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md
index 6b3eab227ebcc8..13f0f4e50e0c2a 100644
--- a/doc/ci/yaml/index.md
+++ b/doc/ci/yaml/index.md
@@ -1913,6 +1913,11 @@ job2:
     pull_policy: [always, if-not-present]
 ```
 
+**Additional details**:
+
+- If the runner does not support the defined pull policy, the job fails with an error similar to:
+  `ERROR: Job failed (system failure): the configured PullPolicies ([always]) are not allowed by AllowedPullPolicies ([never])`.
+
 **Related topics**:
 
 - [Run your CI/CD jobs in Docker containers](../docker/using_docker_images.md).
-- 
GitLab