Add latest changes from gitlab-org/gitlab@master

app/services/ci/register_job_service.rb

@@ -42,7 +42,7 @@ module Ci
       if !db_all_caught_up && !result.build
         metrics.increment_queue_operation(:queue_replication_lag)
 
-        ::Ci::RegisterJobService::Result.new(nil, false) # rubocop:disable Cop/AvoidReturnFromBlocks
+        ::Ci::RegisterJobService::Result.new(nil, nil, false) # rubocop:disable Cop/AvoidReturnFromBlocks
       else
         result
       end

app/views/projects/network/show.html.haml

@@ -6,8 +6,7 @@
     .controls.gl-bg-gray-50.gl-p-2.gl-font-base.gl-text-gray-400.gl-border-b-1.gl-border-b-solid.gl-border-b-gray-300
       = form_tag project_network_path(@project, @id), method: :get, class: 'form-inline network-form' do |f|
         = text_field_tag :extended_sha1, @options[:extended_sha1], placeholder: _("Git revision"), class: 'search-input form-control gl-form-input input-mx-250 search-sha gl-mr-2'
-        = button_tag class: 'btn gl-button btn-confirm btn-icon' do
-          = sprite_icon('search')
+        = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, icon: 'search')
         .inline.gl-ml-5
           .form-check.light
             = check_box_tag :filter_ref, 1, @options[:filter_ref], class: 'form-check-input'

doc/ci/environments/index.md

@@ -383,6 +383,11 @@ To retry or rollback a deployment:
    - To retry a deployment, select **Re-deploy to environment**.
    - To roll back to a deployment, next to a previously successful deployment, select **Rollback environment**.
 
+NOTE:
+If you have [prevented outdated deployment jobs](deployment_safety.md#prevent-outdated-deployment-jobs) in your project,
+the rollback buttons might be hidden or disabled.
+In this case, see [how to rollback to an outdated deployment](deployment_safety.md#how-to-rollback-to-an-outdated-deployment).
+
 ### Environment URL
 
 > - [Fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/337417) to persist arbitrary URLs in GitLab 15.2 [with a flag](../../administration/feature_flags.md) named `soft_validation_on_external_url`. Disabled by default.

doc/ci/pipelines/settings.md

@@ -101,7 +101,7 @@ To avoid this scenario:
 1. Select the **Prevent outdated deployment jobs** checkbox.
 1. Select **Save changes**.
 
-For more information, see [Deployment safety](../environments/deployment_safety.md).
+For more information, see [Deployment safety](../environments/deployment_safety.md#prevent-outdated-deployment-jobs).
 
 ## Specify a custom CI/CD configuration file
 

lib/api/admin/ci/variables.rb

@@ -43,6 +43,7 @@ module API
               desc 'Create a new instance-level variable' do
                 success Entities::Ci::Variable
               end
+              route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
               params do
                 requires :key,
                   type: String,
@@ -80,6 +81,7 @@ module API
               desc 'Update an existing instance-variable' do
                 success Entities::Ci::Variable
               end
+              route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
               params do
                 optional :key,
                   type: String,

lib/api/api.rb

@@ -13,13 +13,14 @@ module API
     USER_REQUIREMENTS = { user_id: NO_SLASH_URL_PART_REGEX }.freeze
     LOG_FILTERS = ::Rails.application.config.filter_parameters + [/^output$/]
     LOG_FORMATTER = Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new
+    LOGGER = Logger.new(LOG_FILENAME)
 
     insert_before Grape::Middleware::Error,
                   GrapeLogging::Middleware::RequestLogger,
-                  logger: Logger.new(LOG_FILENAME),
+                  logger: LOGGER,
                   formatter: LOG_FORMATTER,
                   include: [
-                    GrapeLogging::Loggers::FilterParameters.new(LOG_FILTERS),
+                    Gitlab::GrapeLogging::Loggers::FilterParameters.new(LOG_FILTERS),
                     Gitlab::GrapeLogging::Loggers::ClientEnvLogger.new,
                     Gitlab::GrapeLogging::Loggers::RouteLogger.new,
                     Gitlab::GrapeLogging::Loggers::UserLogger.new,

lib/api/ci/variables.rb

@@ -49,6 +49,7 @@ module API
         desc 'Create a new variable in a project' do
           success Entities::Ci::Variable
         end
+        route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
         params do
           requires :key, type: String, desc: 'The key of the variable'
           requires :value, type: String, desc: 'The value of the variable'
@@ -74,6 +75,7 @@ module API
         desc 'Update an existing variable from a project' do
           success Entities::Ci::Variable
         end
+        route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
         params do
           optional :key, type: String, desc: 'The key of the variable'
           optional :value, type: String, desc: 'The value of the variable'

lib/api/group_variables.rb

@@ -43,6 +43,7 @@ module API
       desc 'Create a new variable in a group' do
         success Entities::Ci::Variable
       end
+      route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
       params do
         requires :key, type: String, desc: 'The key of the variable'
         requires :value, type: String, desc: 'The value of the variable'
@@ -74,6 +75,7 @@ module API
       desc 'Update an existing variable from a group' do
         success Entities::Ci::Variable
       end
+      route_setting :log_safety, { safe: %w[key], unsafe: %w[value] }
       params do
         optional :key, type: String, desc: 'The key of the variable'
         optional :value, type: String, desc: 'The value of the variable'

lib/gitlab/grape_logging/loggers/filter_parameters.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module GrapeLogging
+    module Loggers
+      # In the CI variables APIs, the POST or PUT parameters will always be
+      # literally 'key' and 'value'. Rails' default filters_parameters will
+      # always incorrectly mask the value of param 'key' when it should mask the
+      # value of the param 'value'.
+      # See: https://gitlab.com/gitlab-org/gitlab/-/issues/353857
+      class FilterParameters < ::GrapeLogging::Loggers::FilterParameters
+        private
+
+        def safe_parameters(request)
+          loggable_params = super
+          settings = request.env[Grape::Env::API_ENDPOINT]&.route&.settings
+
+          return loggable_params unless settings&.key?(:log_safety)
+
+          settings[:log_safety][:safe].each do |key|
+            loggable_params[key] = request.params[key] if loggable_params.key?(key)
+          end
+
+          settings[:log_safety][:unsafe].each do |key|
+            loggable_params[key] = @replacement if loggable_params.key?(key)
+          end
+
+          loggable_params
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/grape_logging/loggers/filter_parameters_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GrapeLogging::Loggers::FilterParameters do
+  subject { described_class.new }
+
+  describe ".parameters" do
+    let(:route) { instance_double('Grape::Router::Route', settings: settings) }
+    let(:endpoint) { instance_double('Grape::Endpoint', route: route) }
+
+    let(:env) do
+      { 'rack.input' => '', Grape::Env::API_ENDPOINT => endpoint }
+    end
+
+    let(:mock_request) { ActionDispatch::Request.new(env) }
+
+    before do
+      mock_request.params['key'] = 'some key'
+      mock_request.params['foo'] = 'wibble'
+      mock_request.params['value'] = 'some value'
+      mock_request.params['oof'] = 'wobble'
+      mock_request.params['other'] = 'Unaffected'
+    end
+
+    context 'when the log_safety setting is provided' do
+      let(:settings) { { log_safety: { safe: %w[foo bar key], unsafe: %w[oof rab value] } } }
+
+      it 'includes safe parameters, and filters unsafe ones' do
+        data = subject.parameters(mock_request, nil)
+
+        expect(data).to eq(
+          params: {
+            'key' => 'some key',
+            'foo' => 'wibble',
+            'value' => '[FILTERED]',
+            'oof' => '[FILTERED]',
+            'other' => 'Unaffected'
+          }
+        )
+      end
+    end
+
+    context 'when the log_safety is not provided' do
+      let(:settings) { {} }
+
+      it 'behaves like the normal parameter filter' do
+        data = subject.parameters(mock_request, nil)
+
+        expect(data).to eq(
+          params: {
+            'key' => '[FILTERED]',
+            'foo' => 'wibble',
+            'value' => 'some value',
+            'oof' => 'wobble',
+            'other' => 'Unaffected'
+          }
+        )
+      end
+    end
+  end
+end

spec/requests/api/admin/ci/variables_spec.rb

@@ -83,6 +83,15 @@ RSpec.describe ::API::Admin::Ci::Variables do
         expect(json_response['variable_type']).to eq('env_var')
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        post api("/admin/ci/variables", user),
+          params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true }
+      end
+
       it 'creates variable with optional attributes', :aggregate_failures do
         expect do
           post api('/admin/ci/variables', admin),
@@ -163,6 +172,15 @@ RSpec.describe ::API::Admin::Ci::Variables do
         expect(json_response['masked']).to be_truthy
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        put api("/admin/ci/variables/#{variable.key}", admin),
+          params: { value: 'SENSITIVE', protected: true, masked: true }
+      end
+
       it 'responds with 404 Not Found if requesting non-existing variable' do
         put api('/admin/ci/variables/non_existing_variable', admin)
 

spec/requests/api/ci/variables_spec.rb

@@ -126,9 +126,18 @@ RSpec.describe API::Ci::Variables do
         expect(json_response['variable_type']).to eq('env_var')
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        post api("/projects/#{project.id}/variables", user),
+          params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true }
+      end
+
       it 'creates variable with optional attributes' do
         expect do
-          post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file',  key: 'TEST_VARIABLE_2', value: 'VALUE_2' }
+          post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' }
         end.to change { project.variables.count }.by(1)
 
         expect(response).to have_gitlab_http_status(:created)
@@ -206,6 +215,15 @@ RSpec.describe API::Ci::Variables do
         expect(updated_variable.variable_type).to eq('file')
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'value' => '[FILTERED]', 'protected' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        put api("/projects/#{project.id}/variables/#{variable.key}", user),
+          params: { value: 'SENSITIVE', protected: true }
+      end
+
       it 'responds with 404 Not Found if requesting non-existing variable' do
         put api("/projects/#{project.id}/variables/non_existing_variable", user)
 

spec/requests/api/group_variables_spec.rb

@@ -102,6 +102,15 @@ RSpec.describe API::GroupVariables do
         expect(json_response['environment_scope']).to eq('*')
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        post api("/groups/#{group.id}/variables", user),
+          params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true }
+      end
+
       it 'creates variable with optional attributes' do
         expect do
           post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' }
@@ -164,6 +173,15 @@ RSpec.describe API::GroupVariables do
         expect(json_response['masked']).to be_truthy
       end
 
+      it 'masks the new value when logging' do
+        masked_params = { 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' }
+
+        expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params)))
+
+        put api("/groups/#{group.id}/variables/#{variable.key}", user),
+          params: { value: 'SENSITIVE', protected: true, masked: true }
+      end
+
       it 'responds with 404 Not Found if requesting non-existing variable' do
         put api("/groups/#{group.id}/variables/non_existing_variable", user)
 

spec/services/ci/register_job_service_spec.rb

@@ -14,25 +14,29 @@ module Ci
     let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) }
 
     describe '#execute' do
-      context 'checks database loadbalancing stickiness' do
-        subject { described_class.new(shared_runner).execute }
+      subject { described_class.new(shared_runner).execute }
 
+      context 'checks database loadbalancing stickiness' do
         before do
           project.update!(shared_runners_enabled: false)
         end
 
-        it 'result is valid if replica did caught-up' do
+        it 'result is valid if replica did caught-up', :aggregate_failures do
           expect(ApplicationRecord.sticking).to receive(:all_caught_up?)
             .with(:runner, shared_runner.id) { true }
 
           expect(subject).to be_valid
+          expect(subject.build).to be_nil
+          expect(subject.build_json).to be_nil
         end
 
-        it 'result is invalid if replica did not caught-up' do
+        it 'result is invalid if replica did not caught-up', :aggregate_failures do
           expect(ApplicationRecord.sticking).to receive(:all_caught_up?)
             .with(:runner, shared_runner.id) { false }
 
           expect(subject).not_to be_valid
+          expect(subject.build).to be_nil
+          expect(subject.build_json).to be_nil
         end
       end
 
@@ -954,6 +958,7 @@ module Ci
 
         expect(result).not_to be_valid
         expect(result.build).to be_nil
+        expect(result.build_json).to be_nil
       end
     end