Add latest changes from gitlab-org/gitlab@master

16 files changed, 177 insertions, 35 deletions

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index a7892e477b0..cc8fe9bb8fc 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -2560,7 +2560,6 @@ Style/OpenStructUse:
     - 'ee/spec/helpers/ee/blob_helper_spec.rb'
     - 'ee/spec/lib/gitlab/auth/group_saml/failure_handler_spec.rb'
     - 'ee/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb'
-    - 'ee/spec/requests/api/ldap_spec.rb'
     - 'lib/api/wikis.rb'
     - 'lib/gitlab/git/diff_collection.rb'
     - 'lib/gitlab/import_export/after_export_strategies/base_after_export_strategy.rb'
diff --git a/app/services/clusters/cleanup/project_namespace_service.rb b/app/services/clusters/cleanup/project_namespace_service.rb
index 0173f93f625..80192aa14ab 100644
--- a/app/services/clusters/cleanup/project_namespace_service.rb
+++ b/app/services/clusters/cleanup/project_namespace_service.rb
@@ -26,8 +26,10 @@ module Clusters
 
           begin
             kubeclient_delete_namespace(kubernetes_namespace)
-          rescue Kubeclient::HttpError
-            next
+          rescue Kubeclient::HttpError => e
+            # unauthorized, forbidden: GitLab's access has been revoked
+            # certificate verify failed: Cluster is probably gone forever
+            raise unless e.message =~ /unauthorized|forbidden|certificate verify failed/i
           end
 
           kubernetes_namespace.destroy!
diff --git a/app/services/clusters/cleanup/service_account_service.rb b/app/services/clusters/cleanup/service_account_service.rb
index 53f968cd409..dce41d2a39c 100644
--- a/app/services/clusters/cleanup/service_account_service.rb
+++ b/app/services/clusters/cleanup/service_account_service.rb
@@ -24,6 +24,10 @@ module Clusters
         # The resources have already been deleted, possibly on a previous attempt that timed out
       rescue Gitlab::UrlBlocker::BlockedUrlError
         # User gave an invalid cluster from the start, or deleted the endpoint before this job ran
+      rescue Kubeclient::HttpError => e
+        # unauthorized, forbidden: GitLab's access has been revoked
+        # certificate verify failed: Cluster is probably gone forever
+        raise unless e.message =~ /unauthorized|forbidden|certificate verify failed/i
       end
     end
   end
diff --git a/config/feature_flags/ops/feature_flag_state_logs.yml b/config/feature_flags/ops/feature_flag_state_logs.yml
new file mode 100644
index 00000000000..7153aca3860
--- /dev/null
+++ b/config/feature_flags/ops/feature_flag_state_logs.yml
@@ -0,0 +1,8 @@
+---
+name: feature_flag_state_logs
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73729
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345888
+milestone: '14.6'
+type: ops
+group: group::incubation
+default_enabled: false
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index 4eb1d987d84..6b0c196dc5d 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -49,8 +49,8 @@ for them.
 MARKDOWN
 
 def group_not_available_template(slack_channel, gitlab_group)
-  <<~TEMPLATE
-    No engineer is available for automated assignment, please reach out to `#{slack_channel}` slack channel or mention `#{gitlab_group}` for assistance.
+  <<~TEMPLATE.strip
+    No engineer is available for automated assignment, please reach out to the `#{slack_channel}` Slack channel or mention `#{gitlab_group}` for assistance.
   TEMPLATE
 end
 
diff --git a/doc/api/projects.md b/doc/api/projects.md
index d19019c9597..4caa4c4ffe6 100644
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@@ -1350,6 +1350,17 @@ where `password` is a public access key with the `api` scope enabled.
 PUT /projects/:id
 ```
 
+For example, to toggle the setting for
+[shared runners on a GitLab.com project](../user/gitlab_com/index.md#shared-runner-cloud-runners):
+
+```shell
+curl --request PUT --header "PRIVATE-TOKEN: <your-token>" \
+     --url 'https://gitlab.com/api/v4/projects/<your-project-ID>' \
+     --data "shared_runners_enabled=true" # to turn off: "shared_runners_enabled=false"
+```
+
+Supported attributes:
+
 | Attribute                                                   | Type           | Required               | Description |
 |-------------------------------------------------------------|----------------|------------------------|-------------|
 | `allow_merge_on_skipped_pipeline`                           | boolean        | **{dotted-circle}** No | Set whether or not merge requests can be merged with skipped jobs. |
diff --git a/doc/integration/mattermost/gitlab-mattermost.msc b/doc/integration/mattermost/gitlab-mattermost.msc
deleted file mode 100644
index f6d4bf7aa68..00000000000
--- a/doc/integration/mattermost/gitlab-mattermost.msc
+++ /dev/null
@@ -1,28 +0,0 @@
-msc {
-  # Use https://mscgen.js.org or mscgen to convert this into PNG
-  hscale="1.5",
-  wordwraparcs=on;
-
-  user [ label="User", textbgcolor="blue", textcolor="white" ],
-  mattermost [ label="Mattermost", textbgcolor="red", textcolor="white"],
-  gitlab [ label="GitLab", textbgcolor="indigo", textcolor="white"];
-
-  user=>mattermost [label="GET https://mm.domain.com"];
-  mattermost note gitlab [label="Obtain access code", textcolor="green"];
-  mattermost=>gitlab [label="GET https://gitlab.domain.com/oauth/authorize", textcolor="indigo"];
-  gitlab rbox user [label="GitLab user logs in (if necessary)"];
-  gitlab rbox gitlab [label="GitLab verifies client_id matches an OAuth application"];
-  gitlab=>user [label="GitLab asks user to authorize Mattermost OAuth app"];
-  user=>gitlab [label="User clicks 'Allow'"];
-  gitlab rbox gitlab [label="GitLab verifies redirect_uri matches list of valid URLs"];
-  gitlab=>user [label="302 Redirect: https://mm.domain.com/signup/gitlab/complete"];
-  user=>mattermost [label="GET https://mm.domain.com/signup/gitlab/complete", textcolor="red"];
-  mattermost note gitlab [label="Exchange access code for access token", textcolor="green"];
-  mattermost=>gitlab [label="POST http://gitlab.domain.com/oauth/token", textcolor="indigo"];
-  gitlab=>gitlab [label="Doorkeeper::TokensController#create"];
-  gitlab=>mattermost [label="Access token", textcolor="red"];
-  mattermost note gitlab [label="Mattermost looks up GitLab user", textcolor="green"];
-  mattermost=>gitlab [label="GET https://gitlab.domain.com/api/v4/user", textcolor="indigo"];
-  gitlab=>mattermost [label="User details", textcolor="red"];
-  mattermost=>user [label="Mattermost/GitLab user ready"];
-}
diff --git a/doc/integration/mattermost/img/gitlab-mattermost.png b/doc/integration/mattermost/img/gitlab-mattermost.png
deleted file mode 100644
index c3b019988d0..00000000000
--- a/doc/integration/mattermost/img/gitlab-mattermost.png
+++ /dev/null
diff --git a/doc/integration/mattermost/index.md b/doc/integration/mattermost/index.md
index 0489ccd431c..a532e94e838 100644
--- a/doc/integration/mattermost/index.md
+++ b/doc/integration/mattermost/index.md
@@ -476,7 +476,27 @@ The following image is a sequence diagram for how GitLab works as an OAuth2
 provider for Mattermost. You can use this to troubleshoot errors
 in getting the integration to work:
 
-![sequence diagram](img/gitlab-mattermost.png)
+```mermaid
+sequenceDiagram
+    User->>Mattermost: GET https://mm.domain.com
+    Note over Mattermost, GitLab: Obtain access code
+    Mattermost->>GitLab: GET https://gitlab.domain.com/oauth/authorize
+    Note over User, GitLab: GitLab user signs in (if necessary)
+    Note over GitLab: GitLab verifies client_id matches an OAuth application
+    GitLab->>User: GitLab asks user to authorize Mattermost OAuth app
+    User->>GitLab: User selects 'Allow'
+    Note over GitLab: GitLab verifies redirect_uri matches list of valid URLs
+    GitLab->>User: 302 redirect: https://mm.domain.com/signup/gitlab/complete
+    User->>Mattermost: GET https://mm.domain.com/signup/gitlab/complete
+    Note over Mattermost, GitLab: Exchange access code for access token
+    Mattermost->>GitLab: POST http://gitlab.domain.com/oauth/token
+    GitLab->>GitLab: Doorkeeper::TokensController#35;create
+    GitLab->>Mattermost: Access token
+    Note over Mattermost, GitLab: Mattermost looks up GitLab user
+    Mattermost->>GitLab: GET https://gitlab.domain.com/api/v4/user
+    GitLab->>Mattermost: User details
+    Mattermost->>User: Mattermost/GitLab user ready
+```
 
 ## Troubleshooting the Mattermost CLI
 
diff --git a/lib/feature.rb b/lib/feature.rb
index 8186fbc40fa..1a299323dda 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -83,7 +83,12 @@ class Feature
       # `persisted?` can potentially generate DB queries and also checks for inclusion
       # in an array of feature names (177 at last count), possibly reducing performance by half.
       # So we only perform the `persisted` check if `default_enabled: true`
-      !default_enabled || Feature.persisted_name?(feature.name) ? feature.enabled?(thing) : true
+      feature_value = !default_enabled || Feature.persisted_name?(feature.name) ? feature.enabled?(thing) : true
+
+      # If we don't filter out this flag here we will enter an infinite loop
+      log_feature_flag_state(key, feature_value) if log_feature_flag_states?(key)
+
+      feature_value
     end
 
     def disabled?(key, thing = nil, type: :development, default_enabled: false)
@@ -153,6 +158,18 @@ class Feature
       @logger ||= Feature::Logger.build
     end
 
+    def log_feature_flag_states?(key)
+      key != :feature_flag_state_logs && Feature.enabled?(:feature_flag_state_logs, type: :ops)
+    end
+
+    def log_feature_flag_state(key, feature_value)
+      logged_states[key] ||= feature_value
+    end
+
+    def logged_states
+      RequestStore.fetch(:feature_flag_events) { {} }
+    end
+
     private
 
     def flipper
diff --git a/lib/gitlab/lograge/custom_options.rb b/lib/gitlab/lograge/custom_options.rb
index e6c9ba0773c..84ead5119d5 100644
--- a/lib/gitlab/lograge/custom_options.rb
+++ b/lib/gitlab/lograge/custom_options.rb
@@ -32,6 +32,10 @@ module Gitlab
 
         ::Gitlab::ExceptionLogFormatter.format!(exception, payload)
 
+        if Feature.enabled?(:feature_flag_state_logs, type: :ops)
+          payload[:feature_flag_states] = Feature.logged_states.map { |key, state| "#{key}:#{state ? 1 : 0}" }
+        end
+
         payload
       end
     end
diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb
index 58e7292c125..2c4b0a85537 100644
--- a/spec/lib/feature_spec.rb
+++ b/spec/lib/feature_spec.rb
@@ -127,6 +127,10 @@ RSpec.describe Feature, stub_feature_flags: false do
   end
 
   describe '.enabled?' do
+    before do
+      allow(Feature).to receive(:log_feature_flag_states?).and_return(false)
+    end
+
     it 'returns false for undefined feature' do
       expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey
     end
@@ -179,6 +183,24 @@ RSpec.describe Feature, stub_feature_flags: false do
       expect(described_class.enabled?(:a_feature, default_enabled: fake_default)).to eq(fake_default)
     end
 
+    context 'logging is enabled', :request_store do
+      before do
+        allow(Feature).to receive(:log_feature_flag_states?).and_call_original
+        described_class.enable(:feature_flag_state_logs)
+        described_class.enable(:enabled_feature_flag)
+        described_class.enabled?(:enabled_feature_flag)
+      end
+
+      it 'does not log feature_flag_state_logs' do
+        expect(described_class.logged_states).not_to have_key("feature_flag_state_logs")
+      end
+
+      it 'logs other feature flags' do
+        expect(described_class.logged_states).to have_key(:enabled_feature_flag)
+        expect(described_class.logged_states[:enabled_feature_flag]).to be_truthy
+      end
+    end
+
     context 'cached feature flag', :request_store do
       let(:flag) { :some_feature_flag }
 
diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb
index d52ab3a8983..d9bf85460b3 100644
--- a/spec/lib/gitlab/experimentation/experiment_spec.rb
+++ b/spec/lib/gitlab/experimentation/experiment_spec.rb
@@ -16,6 +16,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do
   before do
     skip_feature_flags_yaml_validation
     skip_default_enabled_yaml_check
+    allow(Feature).to receive(:log_feature_flag_states?).and_return(false)
     feature = double('FeatureFlag', percentage_of_time_value: percentage, enabled?: true)
     allow(Feature).to receive(:get).with(:experiment_key_experiment_percentage).and_return(feature)
   end
diff --git a/spec/lib/gitlab/lograge/custom_options_spec.rb b/spec/lib/gitlab/lograge/custom_options_spec.rb
index a4ae39a835a..5c9cd86ce7d 100644
--- a/spec/lib/gitlab/lograge/custom_options_spec.rb
+++ b/spec/lib/gitlab/lograge/custom_options_spec.rb
@@ -95,5 +95,47 @@ RSpec.describe Gitlab::Lograge::CustomOptions do
         expect(subject[correlation_id_key]).to eq('123456')
       end
     end
+
+    context  'when feature flags are present', :request_store do
+      before do
+        allow(Feature::Definition).to receive(:valid_usage!).and_return(true)
+
+        allow(Feature).to receive(:log_feature_flag_states?).and_return(false)
+
+        Feature.enable(:enabled_feature)
+        Feature.disable(:disabled_feature)
+
+        allow(Feature).to receive(:log_feature_flag_states?).with(:enabled_feature).and_call_original
+        allow(Feature).to receive(:log_feature_flag_states?).with(:disabled_feature).and_call_original
+      end
+
+      context 'and :feature_flag_log_states is enabled' do
+        before do
+          Feature.enable(:feature_flag_state_logs)
+        end
+
+        it 'adds feature flag events' do
+          Feature.enabled?(:enabled_feature)
+          Feature.enabled?(:disabled_feature)
+
+          expect(subject).to have_key(:feature_flag_states)
+          expect(subject[:feature_flag_states]).to match_array(%w[enabled_feature:1 disabled_feature:0])
+        end
+      end
+
+      context 'and :feature_flag_log_states is disabled' do
+        before do
+          Feature.disable(:feature_flag_state_logs)
+        end
+
+        it 'does not track or add feature flag events' do
+          Feature.enabled?(:enabled_feature)
+          Feature.enabled?(:disabled_feature)
+
+          expect(subject).not_to have_key(:feature_flag_states)
+          expect(Feature).not_to receive(:log_feature_flag_state)
+        end
+      end
+    end
   end
 end
diff --git a/spec/services/clusters/cleanup/project_namespace_service_spec.rb b/spec/services/clusters/cleanup/project_namespace_service_spec.rb
index ec510b2e3c5..8d3ae217a9f 100644
--- a/spec/services/clusters/cleanup/project_namespace_service_spec.rb
+++ b/spec/services/clusters/cleanup/project_namespace_service_spec.rb
@@ -95,5 +95,31 @@ RSpec.describe Clusters::Cleanup::ProjectNamespaceService do
         subject
       end
     end
+
+    context 'when there is a Kubeclient::HttpError' do
+      let(:kubeclient_instance_double) do
+        instance_double(Gitlab::Kubernetes::KubeClient)
+      end
+
+      ['Unauthorized', 'forbidden', 'Certificate verify Failed'].each do |message|
+        it 'schedules ::ServiceAccountWorker with accepted errors' do
+          allow(kubeclient_instance_double)
+            .to receive(:delete_namespace)
+            .and_raise(Kubeclient::HttpError.new(401, message, nil))
+
+          expect(Clusters::Cleanup::ServiceAccountWorker).to receive(:perform_async).with(cluster.id)
+
+          subject
+        end
+      end
+
+      it 'raises error with unaccepted errors' do
+        allow(kubeclient_instance_double)
+          .to receive(:delete_namespace)
+          .and_raise(Kubeclient::HttpError.new(401, 'unexpected message', nil))
+
+        expect { subject }.to raise_error(Kubeclient::HttpError)
+      end
+    end
   end
 end
diff --git a/spec/services/clusters/cleanup/service_account_service_spec.rb b/spec/services/clusters/cleanup/service_account_service_spec.rb
index adcdbd84da0..769762237f9 100644
--- a/spec/services/clusters/cleanup/service_account_service_spec.rb
+++ b/spec/services/clusters/cleanup/service_account_service_spec.rb
@@ -52,5 +52,19 @@ RSpec.describe Clusters::Cleanup::ServiceAccountService do
         expect { subject }.to change { Clusters::Cluster.where(id: cluster.id).exists? }.from(true).to(false)
       end
     end
+
+    context 'when there is a Kubeclient::HttpError' do
+      ['Unauthorized', 'forbidden', 'Certificate verify Failed'].each do |message|
+        before do
+          allow(kubeclient_instance_double)
+            .to receive(:delete_service_account)
+            .and_raise(Kubeclient::HttpError.new(401, message, nil))
+        end
+
+        it 'destroys cluster' do
+          expect { subject }.to change { Clusters::Cluster.where(id: cluster.id).exists? }.from(true).to(false)
+        end
+      end
+    end
   end
 end