From b3a3357de38298887aaf83cecbdf4e25feaec045 Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Tue, 12 May 2020 17:09:38 +1200
Subject: [PATCH 1/6] Implement CI syntax for secrets

Adds `secrets` section in CI config for jobs. There is no support for
default values yet.

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/28321 and
https://gitlab.com/gitlab-org/gitlab/-/issues/218746.
---
 ee/lib/ee/gitlab/ci/config/entry/job.rb       |  27 +++++
 ee/lib/gitlab/ci/config/entry/secret.rb       |  27 +++++
 ee/lib/gitlab/ci/config/entry/secrets.rb      |  33 +++++
 ee/lib/gitlab/ci/config/entry/vault/engine.rb |  29 +++++
 ee/lib/gitlab/ci/config/entry/vault/secret.rb |  90 ++++++++++++++
 .../lib/gitlab/ci/config/entry/job_spec.rb    |  75 ++++++++++++
 .../lib/gitlab/ci/config/entry/secret_spec.rb |  59 +++++++++
 .../gitlab/ci/config/entry/secrets_spec.rb    |  70 +++++++++++
 .../ci/config/entry/vault/engine_spec.rb      |  51 ++++++++
 .../ci/config/entry/vault/secret_spec.rb      | 114 ++++++++++++++++++
 lib/gitlab/ci/config/entry/job.rb             |   4 +-
 spec/lib/gitlab/ci/config/entry/job_spec.rb   |   2 +-
 12 files changed, 579 insertions(+), 2 deletions(-)
 create mode 100644 ee/lib/ee/gitlab/ci/config/entry/job.rb
 create mode 100644 ee/lib/gitlab/ci/config/entry/secret.rb
 create mode 100644 ee/lib/gitlab/ci/config/entry/secrets.rb
 create mode 100644 ee/lib/gitlab/ci/config/entry/vault/engine.rb
 create mode 100644 ee/lib/gitlab/ci/config/entry/vault/secret.rb
 create mode 100644 ee/spec/lib/gitlab/ci/config/entry/secret_spec.rb
 create mode 100644 ee/spec/lib/gitlab/ci/config/entry/secrets_spec.rb
 create mode 100644 ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
 create mode 100644 ee/spec/lib/gitlab/ci/config/entry/vault/secret_spec.rb

diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb
new file mode 100644
index 00000000000000..6afc37267bc053
--- /dev/null
+++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Config
+        module Entry
+          module Job
+            extend ActiveSupport::Concern
+            extend ::Gitlab::Utils::Override
+
+            prepended do
+              entry :secrets, ::Gitlab::Ci::Config::Entry::Secrets,
+                description: 'Configured secrets for this job',
+                inherit: false
+            end
+
+            override :value
+            def value
+              super.merge({ secrets: secrets_value }.compact)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/lib/gitlab/ci/config/entry/secret.rb b/ee/lib/gitlab/ci/config/entry/secret.rb
new file mode 100644
index 00000000000000..48f309b003caec
--- /dev/null
+++ b/ee/lib/gitlab/ci/config/entry/secret.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents a secret definition.
+        #
+        class Secret < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Configurable
+          include ::Gitlab::Config::Entry::Attributable
+
+          ALLOWED_KEYS = %i[vault].freeze
+
+          attributes ALLOWED_KEYS
+
+          entry :vault, Entry::Vault::Secret, description: 'Vault secrets engine configuration'
+
+          validations do
+            validates :config, allowed_keys: ALLOWED_KEYS, required_keys: ALLOWED_KEYS
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/lib/gitlab/ci/config/entry/secrets.rb b/ee/lib/gitlab/ci/config/entry/secrets.rb
new file mode 100644
index 00000000000000..63760afc0ad220
--- /dev/null
+++ b/ee/lib/gitlab/ci/config/entry/secrets.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents a secrets definition.
+        #
+        class Secrets < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Configurable
+
+          def compose!(deps = nil)
+            super do
+              @config.each do |name, config|
+                factory = ::Gitlab::Config::Entry::Factory.new(Entry::Secret)
+                  .value(config || {})
+                  .with(key: name, parent: self, description: "#{name} secret definition") # rubocop:disable CodeReuse/ActiveRecord
+                  .metadata(name: name)
+
+                @entries[name] = factory.create!
+              end
+
+              @entries.each_value do |entry|
+                entry.compose!(deps)
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/lib/gitlab/ci/config/entry/vault/engine.rb b/ee/lib/gitlab/ci/config/entry/vault/engine.rb
new file mode 100644
index 00000000000000..dd3d0fc208373d
--- /dev/null
+++ b/ee/lib/gitlab/ci/config/entry/vault/engine.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        module Vault
+          ##
+          # Entry that represents Vault secret engine.
+          #
+          class Engine < ::Gitlab::Config::Entry::Node
+            include ::Gitlab::Config::Entry::Validatable
+            include ::Gitlab::Config::Entry::Attributable
+
+            ALLOWED_KEYS = %i[name path].freeze
+
+            attributes ALLOWED_KEYS
+
+            validations do
+              validates :config, type: Hash, allowed_keys: ALLOWED_KEYS
+              validates :name, presence: true, type: String
+              validates :path, presence: true, type: String
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/lib/gitlab/ci/config/entry/vault/secret.rb b/ee/lib/gitlab/ci/config/entry/vault/secret.rb
new file mode 100644
index 00000000000000..a8abca9cc01360
--- /dev/null
+++ b/ee/lib/gitlab/ci/config/entry/vault/secret.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        module Vault
+          ##
+          # Entry that represents Vault secret.
+          #
+          class Secret < ::Gitlab::Config::Entry::Simplifiable
+            strategy :StringStrategy, if: -> (config) { config.is_a?(String) }
+            strategy :HashStrategy, if: -> (config) { config.is_a?(Hash) }
+
+            class UnknownStrategy < ::Gitlab::Config::Entry::Node
+              def errors
+                ["#{location} should be a hash or a string"]
+              end
+            end
+
+            class StringStrategy < ::Gitlab::Config::Entry::Node
+              include ::Gitlab::Config::Entry::Validatable
+
+              validations do
+                validates :config, presence: true
+                validates :config, type: String
+              end
+
+              def value
+                {
+                  engine: {
+                    name: 'kv-v2', path: secret[:engine_path]
+                  },
+                  path: secret[:path],
+                  field: secret[:field]
+                }
+              end
+
+              private
+
+              def secret
+                @secret ||= begin
+                              path_and_field, _, engine_path = config.rpartition('@')
+                              if path_and_field == ""
+                                path_and_field = config
+                                engine_path = 'kv-v2'
+                              end
+
+                              path, _, field = path_and_field.rpartition('/')
+
+                              {
+                                engine_path: engine_path,
+                                path: path,
+                                field: field
+                              }
+                            end
+              end
+            end
+
+            class HashStrategy < ::Gitlab::Config::Entry::Node
+              include ::Gitlab::Config::Entry::Configurable
+              include ::Gitlab::Config::Entry::Attributable
+
+              ALLOWED_KEYS = %i[engine path field].freeze
+
+              attributes ALLOWED_KEYS
+
+              entry :engine, Entry::Vault::Engine, description: 'Vault secrets engine configuration'
+
+              validations do
+                validates :config, allowed_keys: ALLOWED_KEYS
+                validates :path, presence: true, type: String
+                validates :field, presence: true, type: String
+                validates :engine, presence: true, type: Hash
+              end
+
+              def value
+                {
+                  engine: engine_value,
+                  path: path,
+                  field: field
+                }
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
index 5a196e917575c1..a722a5eb93f8e6 100644
--- a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -10,6 +10,14 @@
       entry.compose!
     end
 
+    context 'when entry value is correct' do
+      context 'when has secrets' do
+        let(:config) { { script: 'echo', secrets: {} } }
+
+        it { expect(entry).to be_valid }
+      end
+    end
+
     context 'when entry value is not correct' do
       context 'when has needs' do
         context 'when needs is bridge type' do
@@ -27,6 +35,73 @@
           end
         end
       end
+
+      context 'when has invalid secrets' do
+        let(:config) { { script: 'echo', secrets: [] } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'secrets config should be a hash'
+        end
+      end
+    end
+  end
+
+  describe '.nodes' do
+    context 'when filtering all the entry/node names' do
+      subject(:nodes) { described_class.nodes }
+
+      it 'has "secrets" node' do
+        expect(nodes).to have_key(:secrets)
+      end
+    end
+  end
+
+  describe 'secrets' do
+    let(:config) { { script: 'echo', secrets: secrets } }
+    let(:secrets) do
+      {
+        DATABASE_PASSWORD: { vault: 'production/db/password' },
+        SSL_PRIVATE_KEY: { vault: 'production/ssl/private-key@ops' },
+        S3_SECRET_KEY: {
+          vault: {
+            engine: { name: 'kv-v2', path: 'aws' },
+            path: 'production/s3',
+            field: 'secret-key'
+          }
+        }
+      }
+    end
+
+    before do
+      entry.compose!
+    end
+
+    it 'includes secrets value' do
+      expect(entry.errors).to be_empty
+      expect(entry.value[:secrets]).to eq({
+        DATABASE_PASSWORD: {
+          vault: {
+            engine: { name: 'kv-v2', path: 'kv-v2' },
+            path: 'production/db',
+            field: 'password'
+          }
+        },
+        SSL_PRIVATE_KEY: {
+          vault: {
+            engine: { name: 'kv-v2', path: 'ops' },
+            path: 'production/ssl',
+            field: 'private-key'
+          }
+        },
+        S3_SECRET_KEY: {
+          vault: {
+            engine: { name: 'kv-v2', path: 'aws' },
+            path: 'production/s3',
+            field: 'secret-key'
+          }
+        }
+      })
     end
   end
 end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/secret_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/secret_spec.rb
new file mode 100644
index 00000000000000..58fb437ddaecdd
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/config/entry/secret_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Secret do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    before do
+      entry.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:config) do
+        {
+          vault: {
+            engine: { name: 'kv-v2', path: 'kv-v2' },
+            path: 'production/db',
+            field: 'password'
+          }
+        }
+      end
+
+      describe '#value' do
+        it 'returns secret configuration' do
+          expect(entry.value).to eq(config)
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+  end
+
+  context 'when entry value is not correct' do
+    describe '#errors' do
+      context 'when there is an unknown key present' do
+        let(:config) { { foo: {} } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'secret config contains unknown keys: foo'
+        end
+      end
+
+      context 'when there is no vault entry' do
+        let(:config) { {} }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'secret config missing required keys: vault'
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/secrets_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/secrets_spec.rb
new file mode 100644
index 00000000000000..6e24324d8711dd
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/config/entry/secrets_spec.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Secrets do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    before do
+      entry.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:config) { {} }
+
+      describe '#value' do
+        it 'returns secrets configuration' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when config is of incorrect type' do
+      let(:config) { [] }
+
+      it 'reports error' do
+        expect(entry.errors)
+          .to include 'secrets config should be a hash'
+      end
+    end
+  end
+
+  describe '#compose!' do
+    context 'when valid secret entries composed' do
+      let(:config) do
+        {
+          DATABASE_PASSWORD: {
+            vault: {
+              engine: { name: 'kv-v2', path: 'kv-v2' },
+              path: 'production/db',
+              field: 'password'
+            }
+          }
+        }
+      end
+
+      before do
+        entry.compose!
+      end
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq(config)
+        end
+      end
+
+      describe '#descendants' do
+        it 'creates valid descendant nodes' do
+          expect(entry.descendants).to all(be_a(Gitlab::Ci::Config::Entry::Secret))
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
new file mode 100644
index 00000000000000..9a2b477e0b97c0
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Vault::Engine do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    before do
+      entry.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:config) { { name: 'kv-v2', path: 'kv-v2' } }
+
+      describe '#value' do
+        it 'returns Vault secret engine configuration' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+  end
+
+  context 'when entry value is not correct' do
+    describe '#errors' do
+      context 'when there is an unknown key present' do
+        let(:config) { { foo: :bar } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'engine config contains unknown keys: foo'
+        end
+      end
+
+      context 'when name is not present' do
+        let(:config) { {} }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'engine name can\'t be blank'
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/vault/secret_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/vault/secret_spec.rb
new file mode 100644
index 00000000000000..7fe164eb4a1f31
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/config/entry/vault/secret_spec.rb
@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Vault::Secret do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    before do
+      entry.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:hash_config) do
+        {
+          engine: {
+            name: 'kv-v2',
+            path: 'some/path'
+          },
+          path: 'production/db',
+          field: 'password'
+        }
+      end
+
+      context 'when config is a hash' do
+        let(:config) { hash_config }
+
+        describe '#value' do
+          it 'returns Vault secret configuration' do
+            expect(entry.value).to eq(hash_config)
+          end
+        end
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        end
+      end
+
+      context 'when config is a string with engine path' do
+        let(:config) { 'production/db/password@some/path' }
+
+        describe '#value' do
+          it 'returns Vault secret configuration' do
+            expect(entry.value).to eq(hash_config)
+          end
+        end
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        end
+      end
+
+      context 'when config is a string without engine path' do
+        let(:config) { 'production/db/password' }
+
+        describe '#value' do
+          it 'returns Vault secret configuration' do
+            expect(entry.value).to eq(hash_config.deep_merge(engine: { path: 'kv-v2' }))
+          end
+        end
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        end
+      end
+    end
+  end
+
+  context 'when entry value is not correct' do
+    describe '#errors' do
+      context 'when there is an unknown key present' do
+        let(:config) { { foo: :bar } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'hash strategy config contains unknown keys: foo'
+        end
+      end
+
+      context 'when path is not present' do
+        let(:config) { {} }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'hash strategy path can\'t be blank'
+        end
+      end
+
+      context 'when field is not present' do
+        let(:config) { {} }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'hash strategy field can\'t be blank'
+        end
+      end
+
+      context 'when engine is not a hash' do
+        let(:config) { { engine: [] } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include 'hash strategy engine should be a hash'
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 66050a7bbe0d46..a615cab1a802bb 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -15,7 +15,7 @@ class Job < ::Gitlab::Config::Entry::Node
                             allow_failure type when start_in artifacts cache
                             dependencies before_script needs after_script
                             environment coverage retry parallel interruptible timeout
-                            resource_group release].freeze
+                            resource_group release secrets].freeze
 
           REQUIRED_BY_NEEDS = %i[stage].freeze
 
@@ -191,3 +191,5 @@ def value
     end
   end
 end
+
+::Gitlab::Ci::Config::Entry::Job.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Job')
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index b62794854268cc..a721f86bb49257 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -33,7 +33,7 @@
            inherit]
       end
 
-      it { is_expected.to match_array result }
+      it { is_expected.to include(*result) }
     end
   end
 
-- 
GitLab


From 3a355bb330a150a9cd5e286fc2fc917d6806489d Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Wed, 10 Jun 2020 13:08:00 +1200
Subject: [PATCH 2/6] Tighten up requirements for Vault engine syntax

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/28321 and
https://gitlab.com/gitlab-org/gitlab/-/issues/218746.
---
 ee/lib/gitlab/ci/config/entry/vault/engine.rb    |  2 +-
 .../gitlab/ci/config/entry/vault/engine_spec.rb  | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/ee/lib/gitlab/ci/config/entry/vault/engine.rb b/ee/lib/gitlab/ci/config/entry/vault/engine.rb
index dd3d0fc208373d..0b186f18dcacfa 100644
--- a/ee/lib/gitlab/ci/config/entry/vault/engine.rb
+++ b/ee/lib/gitlab/ci/config/entry/vault/engine.rb
@@ -17,7 +17,7 @@ class Engine < ::Gitlab::Config::Entry::Node
             attributes ALLOWED_KEYS
 
             validations do
-              validates :config, type: Hash, allowed_keys: ALLOWED_KEYS
+              validates :config, type: Hash, allowed_keys: ALLOWED_KEYS, required_keys: ALLOWED_KEYS
               validates :name, presence: true, type: String
               validates :path, presence: true, type: String
             end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
index 9a2b477e0b97c0..f8e17d59072065 100644
--- a/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
+++ b/ee/spec/lib/gitlab/ci/config/entry/vault/engine_spec.rb
@@ -38,12 +38,22 @@
         end
       end
 
-      context 'when name is not present' do
+      context 'when name and path are missing' do
         let(:config) { {} }
 
         it 'reports error' do
-          expect(entry.errors)
-            .to include 'engine name can\'t be blank'
+          expect(entry.errors).to include 'engine config missing required keys: name, path'
+        end
+      end
+
+      context 'when name and path are blank' do
+        let(:config) { { name: '', path: '' } }
+
+        it 'reports error' do
+          aggregate_failures do
+            expect(entry.errors).to include 'engine name can\'t be blank'
+            expect(entry.errors).to include 'engine path can\'t be blank'
+          end
         end
       end
     end
-- 
GitLab


From 89038acaae9dc824fb0c760f3c44fd0ae93819a6 Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Wed, 10 Jun 2020 13:30:28 +1200
Subject: [PATCH 3/6] Simplify parsing Vault secret from string

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/28321 and
https://gitlab.com/gitlab-org/gitlab/-/issues/218746.
---
 ee/lib/gitlab/ci/config/entry/vault/secret.rb | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/ee/lib/gitlab/ci/config/entry/vault/secret.rb b/ee/lib/gitlab/ci/config/entry/vault/secret.rb
index a8abca9cc01360..ebd2ae405f4f08 100644
--- a/ee/lib/gitlab/ci/config/entry/vault/secret.rb
+++ b/ee/lib/gitlab/ci/config/entry/vault/secret.rb
@@ -31,8 +31,8 @@ def value
                   engine: {
                     name: 'kv-v2', path: secret[:engine_path]
                   },
-                  path: secret[:path],
-                  field: secret[:field]
+                  path: secret[:secret_path],
+                  field: secret[:secret_field]
                 }
               end
 
@@ -40,21 +40,27 @@ def value
 
               def secret
                 @secret ||= begin
-                              path_and_field, _, engine_path = config.rpartition('@')
-                              if path_and_field == ""
-                                path_and_field = config
-                                engine_path = 'kv-v2'
-                              end
-
-                              path, _, field = path_and_field.rpartition('/')
+                              secret, engine_path = secret_and_engine
+                              secret_path, _, secret_field = secret.rpartition('/')
 
                               {
                                 engine_path: engine_path,
-                                path: path,
-                                field: field
+                                secret_path: secret_path,
+                                secret_field: secret_field
                               }
                             end
               end
+
+              def secret_and_engine
+                secret, _, engine = config.rpartition('@')
+
+                if secret == ""
+                  secret = config
+                  engine = 'kv-v2'
+                end
+
+                [secret, engine]
+              end
             end
 
             class HashStrategy < ::Gitlab::Config::Entry::Node
-- 
GitLab


From 08ca149691e2d81d966bb09d231c332795906237 Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Wed, 10 Jun 2020 16:13:26 +1200
Subject: [PATCH 4/6] Mixin Validatable in Entry::Secret

instead of Configurable.

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/28321 and
https://gitlab.com/gitlab-org/gitlab/-/issues/218746.
---
 ee/lib/gitlab/ci/config/entry/secrets.rb | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ee/lib/gitlab/ci/config/entry/secrets.rb b/ee/lib/gitlab/ci/config/entry/secrets.rb
index 63760afc0ad220..02438b4a6b22c4 100644
--- a/ee/lib/gitlab/ci/config/entry/secrets.rb
+++ b/ee/lib/gitlab/ci/config/entry/secrets.rb
@@ -8,7 +8,11 @@ module Entry
         # Entry that represents a secrets definition.
         #
         class Secrets < ::Gitlab::Config::Entry::Node
-          include ::Gitlab::Config::Entry::Configurable
+          include ::Gitlab::Config::Entry::Validatable
+
+          validations do
+            validates :config, type: Hash
+          end
 
           def compose!(deps = nil)
             super do
-- 
GitLab


From 96c1e65a7366d6859298077fd607027b8981ee12 Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Wed, 17 Jun 2020 17:14:59 +1200
Subject: [PATCH 5/6] Put CI secrets syntax behind feature flag

Feature flag is named ci_secrets_syntax and is enabled by default.

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/28321 and
https://gitlab.com/gitlab-org/gitlab/-/issues/218746.
---
 ee/lib/ee/gitlab/ci/config/entry/job.rb       | 10 ++++++
 .../lib/gitlab/ci/config/entry/job_spec.rb    | 33 +++++++++++++++----
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb
index 6afc37267bc053..44767353821a7e 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/job.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb
@@ -10,6 +10,12 @@ module Job
             extend ::Gitlab::Utils::Override
 
             prepended do
+              attributes :secrets
+
+              validations do
+                validates :secrets, absence: { message: 'feature is disabled' }, unless: :secrets_enabled?
+              end
+
               entry :secrets, ::Gitlab::Ci::Config::Entry::Secrets,
                 description: 'Configured secrets for this job',
                 inherit: false
@@ -19,6 +25,10 @@ module Job
             def value
               super.merge({ secrets: secrets_value }.compact)
             end
+
+            def secrets_enabled?
+              ::Feature.enabled?(:ci_secrets_syntax, default_enabled: true)
+            end
           end
         end
       end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
index a722a5eb93f8e6..cfb72efcdec71d 100644
--- a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -6,19 +6,40 @@
   let(:entry) { described_class.new(config, name: :rspec) }
 
   describe 'validations' do
-    before do
-      entry.compose!
-    end
-
     context 'when entry value is correct' do
       context 'when has secrets' do
-        let(:config) { { script: 'echo', secrets: {} } }
+        let(:config) { { script: 'echo', secrets: { DATABASE_PASSWORD: { vault: 'production/db/password' } } } }
+
+        context 'when ci_secrets_syntax feature flag is enabled' do
+          before do
+            stub_feature_flags(ci_secrets_syntax: true)
+            entry.compose!
+          end
+
+          it { expect(entry).to be_valid }
+        end
+
+        context 'when ci_secrets_syntax feature flag is disabled' do
+          before do
+            stub_feature_flags(ci_secrets_syntax: false)
+            entry.compose!
+          end
 
-        it { expect(entry).to be_valid }
+          it 'returns an error' do
+            aggregate_failures do
+              expect(entry).not_to be_valid
+              expect(entry.errors).to include 'job secrets feature is disabled'
+            end
+          end
+        end
       end
     end
 
     context 'when entry value is not correct' do
+      before do
+        entry.compose!
+      end
+
       context 'when has needs' do
         context 'when needs is bridge type' do
           let(:config) do
-- 
GitLab


From c25b0c70fd9a0f5f91d6972882262e3e51424cc9 Mon Sep 17 00:00:00 2001
From: Krasimir Angelov <kangelov@gitlab.com>
Date: Thu, 18 Jun 2020 11:47:10 +1200
Subject: [PATCH 6/6] Make ci_secrets_syntax flag disabled by default

and extract check in Gitlab::Ci::Features.
---
 ee/lib/ee/gitlab/ci/config/entry/job.rb |  2 +-
 ee/lib/ee/gitlab/ci/features.rb         | 17 +++++++++++++++++
 lib/gitlab/ci/features.rb               |  2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 ee/lib/ee/gitlab/ci/features.rb

diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb
index 44767353821a7e..2271c72a9ee9c9 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/job.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb
@@ -27,7 +27,7 @@ def value
             end
 
             def secrets_enabled?
-              ::Feature.enabled?(:ci_secrets_syntax, default_enabled: true)
+              ::Gitlab::Ci::Features.secrets_syntax_enabled?
             end
           end
         end
diff --git a/ee/lib/ee/gitlab/ci/features.rb b/ee/lib/ee/gitlab/ci/features.rb
new file mode 100644
index 00000000000000..33daaa938d9a78
--- /dev/null
+++ b/ee/lib/ee/gitlab/ci/features.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Features
+        extend ActiveSupport::Concern
+
+        prepended do
+          def self.secrets_syntax_enabled?
+            ::Feature.enabled?(:ci_secrets_syntax)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb
index 5d6cf54e610d90..89d0e31a05c392 100644
--- a/lib/gitlab/ci/features.rb
+++ b/lib/gitlab/ci/features.rb
@@ -44,3 +44,5 @@ def self.release_generation_enabled?
     end
   end
 end
+
+::Gitlab::Ci::Features.prepend_if_ee('::EE::Gitlab::Ci::Features')
-- 
GitLab