diff --git a/app/finders/ci/pipelines_finder.rb b/app/finders/ci/pipelines_finder.rb index 4c47517299a0ac3aa820ece442a50ef84aa6c096..a2d1805286de8898f16baef97ca90d18d4f4d21e 100644 --- a/app/finders/ci/pipelines_finder.rb +++ b/app/finders/ci/pipelines_finder.rb @@ -155,8 +155,7 @@ def by_updated_at(items) def by_name(items) return items unless - Feature.enabled?(:pipeline_name, project) && - Feature.enabled?(:pipeline_name_search, project) && + Feature.enabled?(:pipeline_name_search, project) && params[:name].present? items.for_name(params[:name]) diff --git a/app/models/integrations/chat_message/pipeline_message.rb b/app/models/integrations/chat_message/pipeline_message.rb index 88db40bea7f7eede7f7c20f06d2915075e9d6791..f8a634be3367f98768e7330a88f50960d6f40b2b 100644 --- a/app/models/integrations/chat_message/pipeline_message.rb +++ b/app/models/integrations/chat_message/pipeline_message.rb @@ -151,7 +151,7 @@ def attachments_fields fields << failed_stages_field if failed_stages.any? fields << failed_jobs_field if failed_jobs.any? fields << yaml_error_field if pipeline.has_yaml_errors? - fields << pipeline_name_field if Feature.enabled?(:pipeline_name, project) && pipeline.name.present? + fields << pipeline_name_field if pipeline.name.present? fields end diff --git a/app/serializers/ci/pipeline_entity.rb b/app/serializers/ci/pipeline_entity.rb index 143017c5159094f23d1532f83372197b65f05eb4..5e6ae0986df97d8449941653de4d9868828e43f8 100644 --- a/app/serializers/ci/pipeline_entity.rb +++ b/app/serializers/ci/pipeline_entity.rb @@ -10,7 +10,7 @@ class Ci::PipelineEntity < Grape::Entity expose :iid expose :user, using: UserEntity expose :active?, as: :active - expose :name, if: -> (pipeline, _) { Feature.enabled?(:pipeline_name, pipeline.project) } + expose :name # Coverage isn't always necessary (e.g. when displaying project pipelines in # the UI). Instead of creating an entirely different entity we just allow the diff --git a/app/serializers/merge_requests/pipeline_entity.rb b/app/serializers/merge_requests/pipeline_entity.rb index 445914fe01c6f72d9628661024036d3adb196b3b..cf050b32d21aef1fa50c87fe209c232e5c2b5939 100644 --- a/app/serializers/merge_requests/pipeline_entity.rb +++ b/app/serializers/merge_requests/pipeline_entity.rb @@ -5,7 +5,7 @@ class MergeRequests::PipelineEntity < Grape::Entity expose :id expose :active?, as: :active - expose :name, if: -> (pipeline, _) { Feature.enabled?(:pipeline_name, pipeline.project) } + expose :name expose :path do |pipeline| project_pipeline_path(pipeline.project, pipeline) diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 1a079324a0f8f5c3bea7d4b0da2e65e365440bf7..8f7f0a15e693fd2de2d18ffa294c0224fb053038 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,4 +1,4 @@ -- if Feature.enabled?(:pipeline_name, @pipeline.project) && @pipeline.name +- if @pipeline.name .gl-border-t.gl-p-5.gl-px-0 %h3.gl-m-0.gl-text-body = @pipeline.name @@ -53,7 +53,7 @@ .well-segment{ 'data-testid': 'commit-row' } .icon-container.commit-icon = sprite_icon('commit', css_class: 'gl-top-0!') - - if Feature.enabled?(:pipeline_name, @pipeline.project) && @pipeline.name + - if @pipeline.name = markdown(commit.title, pipeline: :single_line) = clipboard_button(text: @pipeline.sha, title: _("Copy commit SHA")) = link_to commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha" diff --git a/config/feature_flags/development/pipeline_name.yml b/config/feature_flags/development/pipeline_name.yml deleted file mode 100644 index 9070e754fff1b5bdd566ad8c3c0d46f6ebbe0688..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/pipeline_name.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: pipeline_name -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97502 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/376095 -milestone: '15.5' -type: development -group: group::delivery -default_enabled: true diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index b768bab3180338814fe8cde7f543a61fcd6d64a4..50c97826c9cf16440bfcfedb5862a834873bc4c4 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -397,10 +397,7 @@ Use [`workflow`](workflow.md) to control pipeline behavior. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/372538) in GitLab 15.5 [with a flag](../../administration/feature_flags.md) named `pipeline_name`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/376095) in GitLab 15.7. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, -ask an administrator to [disable the feature flag](../../administration/feature_flags.md) named `pipeline_name`. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/376095) in GitLab 15.8. Feature flag `pipeline_name` removed. You can use `name` in `workflow:` to define a name for pipelines. diff --git a/doc/user/project/integrations/webhook_events.md b/doc/user/project/integrations/webhook_events.md index dcb42bf595be8540ea7e8d8c90d370257e20913a..0f462ad41b05e3f54bd7bf103bd37e2d7ac1b21d 100644 --- a/doc/user/project/integrations/webhook_events.md +++ b/doc/user/project/integrations/webhook_events.md @@ -1454,13 +1454,7 @@ has not been retried. `1` means that it's the first retry. ### Pipeline name -> `commit.name` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107963) in GitLab 15.8 [with a flag](../../../administration/feature_flags.md) named `pipeline_name`. Enabled by default. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, -ask an administrator to [disable the feature flag](../../../administration/feature_flags.md) named -`pipeline_name`. -On GitLab.com, this feature is available. +> `commit.name` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107963) in GitLab 15.8. You can set custom names for pipelines with [`workflow:name`](../../../ci/yaml/index.md#workflowname). If the pipeline has a name, that name is the value of `commit.name`. diff --git a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb index 89befb2a65b70067f4cf6735b36498d59af8cd7f..e7a9009f8f43481480e29d4b811d6b16f5779c4f 100644 --- a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb +++ b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb @@ -22,8 +22,7 @@ def break? private def set_pipeline_name - return if Feature.disabled?(:pipeline_name, pipeline.project) || - @command.yaml_processor_result.workflow_name.blank? + return if @command.yaml_processor_result.workflow_name.blank? name = @command.yaml_processor_result.workflow_name name = ExpandVariables.expand(name, -> { global_context.variables.sort_and_expand_all }) diff --git a/lib/gitlab/data_builder/build.rb b/lib/gitlab/data_builder/build.rb index 4843d981c106ee8293792b9e5dbb0dbacc69fb13..8fec5cf330389503a0763b9907865fbec47ea470 100644 --- a/lib/gitlab/data_builder/build.rb +++ b/lib/gitlab/data_builder/build.rb @@ -45,6 +45,7 @@ def build(build) commit: { # note: commit.id is actually the pipeline id id: commit.id, + name: commit.name, sha: commit.sha, message: commit.git_commit_message, author_name: commit.git_author_name, @@ -70,7 +71,6 @@ def build(build) } data[:retries_count] = build.retries_count if Feature.enabled?(:job_webhook_retries_count, project) - data[:commit][:name] = commit.name if Feature.enabled?(:pipeline_name, project) data end diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index c9c79680c1cf605507f3c1afa8f0b0027b330410..d5739386a30334426231059ae0d2946cba89ebfd 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -100,42 +100,16 @@ end end - context 'with pipeline_name feature flag enabled' do - before do - stub_feature_flags(pipeline_name: true) - end - - it 'displays pipeline name instead of commit title' do - visit_pipeline - - within 'h3' do - expect(page).to have_content(pipeline.name) - end - - within '.well-segment[data-testid="commit-row"]' do - expect(page).to have_content(project.commit.title) - expect(page).to have_content(project.commit.short_id) - end - end - end + it 'displays pipeline name instead of commit title' do + visit_pipeline - context 'with pipeline_name feature flag disabled' do - before do - stub_feature_flags(pipeline_name: false) + within 'h3' do + expect(page).to have_content(pipeline.name) end - it 'displays commit title' do - visit_pipeline - - within 'h3' do - expect(page).not_to have_content(pipeline.name) - expect(page).to have_content(project.commit.title) - end - - within '.well-segment[data-testid="commit-row"]' do - expect(page).not_to have_content(project.commit.title) - expect(page).to have_content(project.commit.short_id) - end + within '.well-segment[data-testid="commit-row"]' do + expect(page).to have_content(project.commit.title) + expect(page).to have_content(project.commit.short_id) end end diff --git a/spec/finders/ci/pipelines_finder_spec.rb b/spec/finders/ci/pipelines_finder_spec.rb index a2e8fe8df5ad90809e29769947adfdec9fb638a1..9ce3becf013eba070dda63ace9eeab485da61a36 100644 --- a/spec/finders/ci/pipelines_finder_spec.rb +++ b/spec/finders/ci/pipelines_finder_spec.rb @@ -260,16 +260,6 @@ end end - context 'when pipeline_name feature flag is off' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'ignores name parameter' do - is_expected.to contain_exactly(pipeline, pipeline_other) - end - end - context 'when pipeline_name_search feature flag is off' do before do stub_feature_flags(pipeline_name_search: false) diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb index 35e1c48a942776712a843ecdde79d9b46ad86d14..00200b57b1ea99edfb068c49d59d99cb46d5e009 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb @@ -54,94 +54,76 @@ def run_chain expect(step.break?).to be false end - context 'with feature flag disabled' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'does not build pipeline_metadata' do - run_chain + it 'builds pipeline_metadata' do + run_chain - expect(pipeline.pipeline_metadata).to be_nil - end + expect(pipeline.pipeline_metadata.name).to eq('Pipeline name') + expect(pipeline.pipeline_metadata.project).to eq(pipeline.project) + expect(pipeline.pipeline_metadata).not_to be_persisted end - context 'with feature flag enabled' do - before do - stub_feature_flags(pipeline_name: true) + context 'with empty name' do + let(:config) do + { workflow: { name: ' ' }, rspec: { script: 'rspec' } } end - it 'builds pipeline_metadata' do + it 'strips whitespace from name' do run_chain - expect(pipeline.pipeline_metadata.name).to eq('Pipeline name') - expect(pipeline.pipeline_metadata.project).to eq(pipeline.project) - expect(pipeline.pipeline_metadata).not_to be_persisted + expect(pipeline.pipeline_metadata).to be_nil end - context 'with empty name' do + context 'with empty name after variable substitution' do let(:config) do - { workflow: { name: ' ' }, rspec: { script: 'rspec' } } + { workflow: { name: '$VAR1' }, rspec: { script: 'rspec' } } end - it 'strips whitespace from name' do + it 'does not save empty name' do run_chain expect(pipeline.pipeline_metadata).to be_nil end - - context 'with empty name after variable substitution' do - let(:config) do - { workflow: { name: '$VAR1' }, rspec: { script: 'rspec' } } - end - - it 'does not save empty name' do - run_chain - - expect(pipeline.pipeline_metadata).to be_nil - end - end end + end - context 'with variables' do - let(:config) do - { - variables: { ROOT_VAR: 'value $WORKFLOW_VAR1' }, - workflow: { - name: 'Pipeline $ROOT_VAR $WORKFLOW_VAR2 $UNKNOWN_VAR', - rules: [{ variables: { WORKFLOW_VAR1: 'value1', WORKFLOW_VAR2: 'value2' } }] - }, - rspec: { script: 'rspec' } - } - end + context 'with variables' do + let(:config) do + { + variables: { ROOT_VAR: 'value $WORKFLOW_VAR1' }, + workflow: { + name: 'Pipeline $ROOT_VAR $WORKFLOW_VAR2 $UNKNOWN_VAR', + rules: [{ variables: { WORKFLOW_VAR1: 'value1', WORKFLOW_VAR2: 'value2' } }] + }, + rspec: { script: 'rspec' } + } + end - it 'substitutes variables' do - run_chain + it 'substitutes variables' do + run_chain - expect(pipeline.pipeline_metadata.name).to eq('Pipeline value value1 value2') - end + expect(pipeline.pipeline_metadata.name).to eq('Pipeline value value1 value2') end + end - context 'with invalid name' do - let(:config) do - { - variables: { ROOT_VAR: 'a' * 256 }, - workflow: { - name: 'Pipeline $ROOT_VAR' - }, - rspec: { script: 'rspec' } - } - end + context 'with invalid name' do + let(:config) do + { + variables: { ROOT_VAR: 'a' * 256 }, + workflow: { + name: 'Pipeline $ROOT_VAR' + }, + rspec: { script: 'rspec' } + } + end - it 'returns error and breaks chain' do - ret = run_chain + it 'returns error and breaks chain' do + ret = run_chain - expect(ret) - .to match_array(["Failed to build pipeline metadata! Name is too long (maximum is 255 characters)"]) - expect(pipeline.pipeline_metadata.errors.full_messages) - .to match_array(['Name is too long (maximum is 255 characters)']) - expect(step.break?).to be true - end + expect(ret) + .to match_array(["Failed to build pipeline metadata! Name is too long (maximum is 255 characters)"]) + expect(pipeline.pipeline_metadata.errors.full_messages) + .to match_array(['Name is too long (maximum is 255 characters)']) + expect(step.break?).to be true end end end diff --git a/spec/lib/gitlab/data_builder/build_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb index 8bf524f4f5998ae8f6143c329cf2d5b6efad49b6..92fef93bddb62a58170f92eab44493e20ddad108 100644 --- a/spec/lib/gitlab/data_builder/build_spec.rb +++ b/spec/lib/gitlab/data_builder/build_spec.rb @@ -85,25 +85,6 @@ end end - context 'when pipeline_name feature flag is disabled' do - before do - stub_feature_flags(pipeline_name: false) - - ci_build # Make sure the Ci::Build model is created before recording. - end - - it { expect(data[:commit]).not_to have_key(:name) } - - it 'does not exceed number of expected queries' do - control = ActiveRecord::QueryRecorder.new(skip_cached: false) do - b = Ci::Build.find(ci_build.id) - described_class.build(b) # Don't use ci_build variable here since it has all associations loaded into memory - end - - expect(control.count).to eq(13) - end - end - context 'commit author_url' do context 'when no commit present' do let(:build) { build(:ci_build) } diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index 413cb097327618fe1b1a21955b16ec8d099af8f6..4d371ca089957743325ce957c21feb9f5820db7f 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -80,18 +80,6 @@ expect(name_field[:value]).to eq('Build pipeline') end - context 'when pipeline_name feature flag is disabled' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'does not return pipeline name' do - name_field = subject.attachments.first[:fields].find { |a| a[:title] == 'Pipeline name' } - - expect(name_field).to be nil - end - end - context "when the pipeline failed" do before do args[:object_attributes][:status] = 'failed' diff --git a/spec/serializers/ci/pipeline_entity_spec.rb b/spec/serializers/ci/pipeline_entity_spec.rb index ff364918b4f38002e93a7e2f9b2953e685e706ea..ae992e478a623582b8ef7c9d2c265d813fa3f104 100644 --- a/spec/serializers/ci/pipeline_entity_spec.rb +++ b/spec/serializers/ci/pipeline_entity_spec.rb @@ -49,16 +49,6 @@ .to include :stuck, :auto_devops, :yaml_errors, :retryable, :cancelable, :merge_request end - - context 'when pipeline_name feature flag is disabled' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'does not return name' do - is_expected.not_to include(:name) - end - end end context 'when default branch not protected' do diff --git a/spec/serializers/merge_requests/pipeline_entity_spec.rb b/spec/serializers/merge_requests/pipeline_entity_spec.rb index a8f4fc44f1002aa0701dea61aa989580986fb7ef..414ce6653bc42b2339f5b64ee97c5688670f5000 100644 --- a/spec/serializers/merge_requests/pipeline_entity_spec.rb +++ b/spec/serializers/merge_requests/pipeline_entity_spec.rb @@ -51,15 +51,5 @@ expect(entity.as_json).not_to include(:coverage) end - - context 'when pipeline_name feature flag is disabled' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'does not return name' do - is_expected.not_to include(:name) - end - end end end