From cbb46ce4a8159c5e01ec37114a6fcc1762af346f Mon Sep 17 00:00:00 2001 From: janis Date: Tue, 21 Mar 2023 11:27:14 +0100 Subject: [PATCH 1/6] Add a "publish" instruction to the pages job This commit introduces the "publish" property to the CI job config entry. It will be used by the Pages job to dynamically set the pages root directory. Changelog: added --- lib/gitlab/ci/config/entry/job.rb | 11 ++++++++--- lib/gitlab/ci/config/entry/publish.rb | 24 ++++++++++++++++++++++++ lib/gitlab/ci/yaml_processor/result.rb | 3 ++- 3 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 lib/gitlab/ci/config/entry/publish.rb diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 2390ba05916319..e89de652e9bc4b 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -14,7 +14,7 @@ class Job < ::Gitlab::Config::Entry::Node ALLOWED_KEYS = %i[tags script image services start_in artifacts cache dependencies before_script after_script hooks environment coverage retry parallel interruptible timeout - release id_tokens].freeze + release id_tokens publish].freeze validations do validates :config, allowed_keys: Gitlab::Ci::Config::Entry::Job.allowed_keys + PROCESSABLE_ALLOWED_KEYS @@ -125,10 +125,14 @@ class Job < ::Gitlab::Config::Entry::Node inherit: false, metadata: { composable_class: ::Gitlab::Ci::Config::Entry::IdToken } + entry :publish, Entry::Publish, + description: 'Path to be published with Pages', + inherit: true + attributes :script, :tags, :when, :dependencies, :needs, :retry, :parallel, :start_in, :interruptible, :timeout, - :release, :allow_failure + :release, :allow_failure, :publish def self.matching?(name, config) !name.to_s.start_with?('.') && @@ -169,7 +173,8 @@ def value allow_failure_criteria: allow_failure_criteria, needs: needs_defined? ? needs_value : nil, scheduling_type: needs_defined? ? :dag : :stage, - id_tokens: id_tokens_value + id_tokens: id_tokens_value, + publish: publish ).compact end diff --git a/lib/gitlab/ci/config/entry/publish.rb b/lib/gitlab/ci/config/entry/publish.rb new file mode 100644 index 00000000000000..52a2487009e920 --- /dev/null +++ b/lib/gitlab/ci/config/entry/publish.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents the path to be published with Pages. + # + class Publish < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Validatable + + validations do + validates :config, type: String + end + + def self.default + 'public' + end + end + end + end + end +end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index d867439b10b9d5..6207b595fc6b05 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -123,7 +123,8 @@ def build_attributes(name) start_in: job[:start_in], trigger: job[:trigger], bridge_needs: job.dig(:needs, :bridge)&.first, - release: job[:release] + release: job[:release], + publish: job[:publish] }.compact }.compact end -- GitLab From 6148f22010354edb70148a4c022d012783628393 Mon Sep 17 00:00:00 2001 From: janis Date: Tue, 21 Mar 2023 14:12:00 +0100 Subject: [PATCH 2/6] Update the ci schema to reflect the new property --- app/assets/javascripts/editor/schema/ci.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index 701b40b102161a..b21c62a54c15b9 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -1890,6 +1890,16 @@ } }, "additionalProperties": false + }, + "publish": { + "description": "A path to a directory that contains the files to be published with Pages", + "anyOf": [ + { + "type": "string", + "minLength": 1, + "pattern": "^/.*" + } + ] } }, "oneOf": [ -- GitLab From ad2739252955f9a5c7b90428453be6402553bef7 Mon Sep 17 00:00:00 2001 From: janis Date: Tue, 21 Mar 2023 16:33:02 +0100 Subject: [PATCH 3/6] Add a validation to the job --- app/assets/javascripts/editor/schema/ci.json | 8 +------- lib/gitlab/ci/config/entry/job.rb | 8 +++++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index b21c62a54c15b9..bff782ac72af81 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -1893,13 +1893,7 @@ }, "publish": { "description": "A path to a directory that contains the files to be published with Pages", - "anyOf": [ - { - "type": "string", - "minLength": 1, - "pattern": "^/.*" - } - ] + "type": "string" } }, "oneOf": [ diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index e89de652e9bc4b..0a38b2a9af1493 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -45,6 +45,8 @@ class Job < ::Gitlab::Config::Entry::Node errors.add(:dependencies, "the #{missing_needs.join(", ")} should be part of needs") if missing_needs.any? end end + + validates :publish, absence: { message: "is only allowed in a job named `pages`" }, unless: -> { pages_job? } end entry :before_script, Entry::Commands, @@ -127,7 +129,7 @@ class Job < ::Gitlab::Config::Entry::Node entry :publish, Entry::Publish, description: 'Path to be published with Pages', - inherit: true + inherit: false attributes :script, :tags, :when, :dependencies, :needs, :retry, :parallel, :start_in, @@ -182,6 +184,10 @@ def ignored? allow_failure_defined? ? static_allow_failure : manual_action? end + def pages_job? + name === :pages + end + def self.allowed_keys ALLOWED_KEYS end -- GitLab From 2a78e179e796778e57e874080bd42849bebf85c2 Mon Sep 17 00:00:00 2001 From: janis Date: Tue, 21 Mar 2023 17:34:37 +0100 Subject: [PATCH 4/6] Add a spec for the publish job config entry --- lib/gitlab/ci/config/entry/job.rb | 2 +- spec/lib/gitlab/ci/config/entry/job_spec.rb | 33 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 0a38b2a9af1493..d294d934aad664 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -46,7 +46,7 @@ class Job < ::Gitlab::Config::Entry::Node end end - validates :publish, absence: { message: "is only allowed in a job named `pages`" }, unless: -> { pages_job? } + validates :publish, absence: { message: "can only be used within a `pages` job" }, unless: -> { pages_job? } end entry :before_script, Entry::Commands, diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index c8b4a8b8a0e02e..39a88fc7721083 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -595,6 +595,39 @@ end end end + + context 'when job is not a pages job' do + let(:name) { :rspec } + + context 'if the config contains a publish entry' do + let(:entry) { described_class.new({ script: 'echo', publish: 'foo' }, name: name) } + + it 'is invalid' do + expect(entry).not_to be_valid + expect(entry.errors).to include /job publish can only be used within a `pages` job/ + end + end + end + + context 'when job is a pages job' do + let(:name) { :pages } + + context 'when it does not have a publish entry' do + let(:entry) { described_class.new({ script: 'echo' }, name: name) } + + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when it has a publish entry' do + let(:entry) { described_class.new({ script: 'echo', publish: 'foo' }, name: name) } + + it 'is valid' do + expect(entry).to be_valid + end + end + end end describe '#relevant?' do -- GitLab From 77c117e99247dca9f4e8f60989e9a754c6b897f7 Mon Sep 17 00:00:00 2001 From: janis Date: Thu, 23 Mar 2023 16:53:14 +0100 Subject: [PATCH 5/6] Add spec for Publish config entry --- .../gitlab/ci/config/entry/publish_spec.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 spec/lib/gitlab/ci/config/entry/publish_spec.rb diff --git a/spec/lib/gitlab/ci/config/entry/publish_spec.rb b/spec/lib/gitlab/ci/config/entry/publish_spec.rb new file mode 100644 index 00000000000000..53ad868a05e9f7 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/publish_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Config::Entry::Publish, feature_category: :pages do + let(:publish) { described_class.new(config) } + + describe 'validations' do + context 'when publish config value is correct' do + let(:config) { 'dist/static' } + + describe '#config' do + it 'returns the publish directory' do + expect(publish.config).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(publish).to be_valid + end + end + end + + context 'when the value has a wrong type' do + let(:config) { { test: true } } + + it 'reports an error' do + expect(publish.errors) + .to include 'publish config should be a string' + end + end + end + + describe '.default' do + it 'returns the default value' do + expect(described_class.default).to eq 'public' + end + end +end -- GitLab From bbf29c37a00a15d29cef77fa8da5ea39975d4f9e Mon Sep 17 00:00:00 2001 From: janis Date: Wed, 29 Mar 2023 08:50:04 +0200 Subject: [PATCH 6/6] Use strict comparison in pages job identification --- lib/gitlab/ci/config/entry/job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index d294d934aad664..d31d1b366c3fe2 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -185,7 +185,7 @@ def ignored? end def pages_job? - name === :pages + name == :pages end def self.allowed_keys -- GitLab