Automatic merge of gitlab-org/gitlab master

app/graphql/mutations/clusters/agent_tokens/revoke.rb

+# frozen_string_literal: true
+
+module Mutations
+  module Clusters
+    module AgentTokens
+      class Revoke < BaseMutation
+        graphql_name 'ClusterAgentTokenRevoke'
+
+        authorize :admin_cluster
+
+        TokenID = ::Types::GlobalIDType[::Clusters::AgentToken]
+
+        argument :id, TokenID,
+                 required: true,
+                 description: 'Global ID of the agent token that will be revoked.'
+
+        def resolve(id:)
+          token = authorized_find!(id: id)
+          token.update(status: token.class.statuses[:revoked])
+
+          { errors: errors_on_object(token) }
+        end
+
+        private
+
+        def find_object(id:)
+          # TODO: remove this line when the compatibility layer is removed
+          # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+          id = TokenID.coerce_isolated_input(id)
+          GitlabSchema.find_by_gid(id)
+        end
+      end
+    end
+  end
+end

app/graphql/types/mutation_type.rb

@@ -36,6 +36,7 @@ module Types
     mount_mutation Mutations::Clusters::Agents::Delete
     mount_mutation Mutations::Clusters::AgentTokens::Create
     mount_mutation Mutations::Clusters::AgentTokens::Delete
+    mount_mutation Mutations::Clusters::AgentTokens::Revoke
     mount_mutation Mutations::Commits::Create, calls_gitaly: true
     mount_mutation Mutations::CustomEmoji::Create, feature_flag: :custom_emoji
     mount_mutation Mutations::CustomEmoji::Destroy, feature_flag: :custom_emoji

app/models/concerns/import_state/sidekiq_job_tracker.rb

@@ -15,7 +15,7 @@ module ImportState
       def refresh_jid_expiration
         return unless jid
 
-        Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2)
+        Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
       end
 
       def self.jid_by(project_id:, status:)

config/feature_flags/development/log_implicit_sidekiq_status_calls.yml → config/feature_flags/development/opt_in_sidekiq_status.yml

 ---
-name: log_implicit_sidekiq_status_calls
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74815
+name: opt_in_sidekiq_status
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77349
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343964
-milestone: '14.6'
+milestone: '14.7'
 type: development
 group: group::scalability
 default_enabled: false

doc/api/graphql/reference/index.md

@@ -970,6 +970,24 @@ Input type: `ClusterAgentTokenDeleteInput`
 | <a id="mutationclusteragenttokendeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
 | <a id="mutationclusteragenttokendeleteerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 
+### `Mutation.clusterAgentTokenRevoke`
+
+Input type: `ClusterAgentTokenRevokeInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationclusteragenttokenrevokeclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationclusteragenttokenrevokeid"></a>`id` | [`ClustersAgentTokenID!`](#clustersagenttokenid) | Global ID of the agent token that will be revoked. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationclusteragenttokenrevokeclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationclusteragenttokenrevokeerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+
 ### `Mutation.commitCreate`
 
 Input type: `CommitCreateInput`

ee/app/assets/javascripts/vue_shared/survey_banner/survey_banner.stories.js

+/* eslint-disable @gitlab/require-i18n-strings */
+import SurveyBanner from './survey_banner.vue';
+
+export default {
+  component: SurveyBanner,
+  title: 'vue_shared/components/survey_banner',
+};
+
+const Template = (args, { argTypes }) => ({
+  components: { SurveyBanner },
+  props: Object.keys(argTypes),
+  template: '<survey-banner v-bind="$props" />',
+});
+
+export const Default = Template.bind({});
+
+Default.args = {
+  surveyLink: 'testlink.test',
+  daysToAskLater: 7,
+  title: 'Shared Survey Banner Test Title',
+  buttonText: 'Shared Survey Banner Button Text',
+  description: 'Shared Survey Banner Test Description',
+  toastMessage: 'Shared Survey Banner Test ToastMessage',
+  storageKey: 'testStorageKey',
+  bannerId: 'testbannerID',
+  svgPath: 'https://gitlab-org.gitlab.io/gitlab-svgs/dist/illustrations/monitoring/tracing.svg',
+};

lib/gitlab/import/set_async_jid.rb

@@ -13,7 +13,7 @@ module Gitlab
       def self.set_jid(import_state)
         jid = generate_jid(import_state)
 
-        Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2)
+        Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
 
         import_state.update_column(:jid, jid)
       end

lib/gitlab/sidekiq_status.rb

@@ -29,16 +29,15 @@ module Gitlab
     # for most jobs.
     DEFAULT_EXPIRATION = 30.minutes.to_i
 
-    DEFAULT_VALUE = 1
-    DEFAULT_VALUE_MESSAGE = 'Keys using the default value for SidekiqStatus detected'
-
     # Starts tracking of the given job.
     #
     # jid - The Sidekiq job ID
     # expire - The expiration time of the Redis key.
-    def self.set(jid, expire = DEFAULT_EXPIRATION, value: DEFAULT_VALUE)
+    def self.set(jid, expire = DEFAULT_EXPIRATION)
+      return unless expire
+
       Sidekiq.redis do |redis|
-        redis.set(key_for(jid), value, ex: expire)
+        redis.set(key_for(jid), 1, ex: expire)
       end
     end
 
@@ -94,17 +93,10 @@ module Gitlab
       return [] if job_ids.empty?
 
       keys = job_ids.map { |jid| key_for(jid) }
-      results = Sidekiq.redis { |redis| redis.mget(*keys) }
-
-      if Feature.enabled?(:log_implicit_sidekiq_status_calls, default_enabled: :yaml)
-        to_log = keys.zip(results).select do |_key, result|
-          result == DEFAULT_VALUE.to_s
-        end.map(&:first)
-
-        Sidekiq.logger.info(message: DEFAULT_VALUE_MESSAGE, keys: to_log) if to_log.any?
-      end
 
-      results.map { |result| !result.nil? }
+      Sidekiq
+        .redis { |redis| redis.mget(*keys) }
+        .map { |result| !result.nil? }
     end
 
     # Returns the JIDs that are completed

lib/gitlab/sidekiq_status/client_middleware.rb

@@ -4,10 +4,14 @@ module Gitlab
   module SidekiqStatus
     class ClientMiddleware
       def call(_, job, _, _)
-        status_expiration = job['status_expiration'] || Gitlab::SidekiqStatus::DEFAULT_EXPIRATION
-        value = job['status_expiration'] ? 2 : Gitlab::SidekiqStatus::DEFAULT_VALUE
+        status_expiration = job['status_expiration']
+
+        unless ::Feature.enabled?(:opt_in_sidekiq_status, default_enabled: :yaml)
+          status_expiration ||= Gitlab::SidekiqStatus::DEFAULT_EXPIRATION
+        end
+
+        Gitlab::SidekiqStatus.set(job['jid'], status_expiration)
 
-        Gitlab::SidekiqStatus.set(job['jid'], status_expiration, value: value)
         yield
       end
     end

spec/graphql/mutations/clusters/agent_tokens/revoke_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::Clusters::AgentTokens::Revoke do
+  let_it_be(:token) { create(:cluster_agent_token) }
+  let_it_be(:user) { create(:user) }
+
+  let(:mutation) do
+    described_class.new(
+      object: double,
+      context: { current_user: user },
+      field: double
+    )
+  end
+
+  it { expect(described_class.graphql_name).to eq('ClusterAgentTokenRevoke') }
+  it { expect(described_class).to require_graphql_authorizations(:admin_cluster) }
+
+  describe '#resolve' do
+    let(:global_id) { token.to_global_id }
+
+    subject { mutation.resolve(id: global_id) }
+
+    context 'user does not have permission' do
+      it 'does not revoke the token' do
+        expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+
+        expect(token.reload).not_to be_revoked
+      end
+    end
+
+    context 'user has permission' do
+      before do
+        token.agent.project.add_maintainer(user)
+      end
+
+      it 'revokes the token' do
+        subject
+
+        expect(token.reload).to be_revoked
+      end
+
+      context 'supplied ID is invalid' do
+        let(:global_id) { token.id }
+
+        it 'raises a coercion error' do
+          expect { subject }.to raise_error(::GraphQL::CoercionError)
+
+          expect(token.reload).not_to be_revoked
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/import/set_async_jid_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Import::SetAsyncJid do
     it 'sets the JID in Redis' do
       expect(Gitlab::SidekiqStatus)
         .to receive(:set)
-              .with("async-import/project-import-state/#{project.id}", Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2)
+              .with("async-import/project-import-state/#{project.id}", Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
               .and_call_original
 
       described_class.set_jid(project.import_state)

spec/lib/gitlab/sidekiq_status/client_middleware_spec.rb

 # frozen_string_literal: true
 
-require 'fast_spec_helper'
+# This can use fast_spec_helper when the feature flag stubbing is removed.
+require 'spec_helper'
 
-RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware do
+RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware, :clean_gitlab_redis_queues do
   describe '#call' do
-    context 'when the job has status_expiration set' do
-      it 'tracks the job in Redis with a value of 2' do
-        expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 1.hour.to_i, value: 2)
+    context 'when opt_in_sidekiq_status is disabled' do
+      before do
+        stub_feature_flags(opt_in_sidekiq_status: false)
+      end
+
+      context 'when the job has status_expiration set' do
+        it 'tracks the job in Redis' do
+          expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 1.hour.to_i).and_call_original
+
+          described_class.new
+            .call('Foo', { 'jid' => '123', 'status_expiration' => 1.hour.to_i }, double(:queue), double(:pool)) { nil }
+
+          expect(Gitlab::SidekiqStatus.num_running(['123'])).to eq(1)
+        end
+      end
+
+      context 'when the job does not have status_expiration set' do
+        it 'tracks the job in Redis' do
+          expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 30.minutes.to_i).and_call_original
+
+          described_class.new
+            .call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil }
 
-        described_class.new
-          .call('Foo', { 'jid' => '123', 'status_expiration' => 1.hour.to_i }, double(:queue), double(:pool)) { nil }
+          expect(Gitlab::SidekiqStatus.num_running(['123'])).to eq(1)
+        end
       end
     end
 
-    context 'when the job does not have status_expiration set' do
-      it 'tracks the job in Redis with a value of 1' do
-        expect(Gitlab::SidekiqStatus).to receive(:set).with('123', Gitlab::SidekiqStatus::DEFAULT_EXPIRATION, value: 1)
+    context 'when opt_in_sidekiq_status is enabled' do
+      before do
+        stub_feature_flags(opt_in_sidekiq_status: true)
+      end
+
+      context 'when the job has status_expiration set' do
+        it 'tracks the job in Redis' do
+          expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 1.hour.to_i).and_call_original
+
+          described_class.new
+            .call('Foo', { 'jid' => '123', 'status_expiration' => 1.hour.to_i }, double(:queue), double(:pool)) { nil }
+
+          expect(Gitlab::SidekiqStatus.num_running(['123'])).to eq(1)
+        end
+      end
+
+      context 'when the job does not have status_expiration set' do
+        it 'does not track the job in Redis' do
+          described_class.new
+            .call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil }
 
-        described_class.new
-          .call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil }
+          expect(Gitlab::SidekiqStatus.num_running(['123'])).to be_zero
+        end
       end
     end
   end

spec/lib/gitlab/sidekiq_status_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
       Sidekiq.redis do |redis|
         expect(redis.exists(key)).to eq(true)
         expect(redis.ttl(key) > 0).to eq(true)
-        expect(redis.get(key)).to eq(described_class::DEFAULT_VALUE.to_s)
+        expect(redis.get(key)).to eq('1')
       end
     end
 
@@ -24,19 +24,17 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
       Sidekiq.redis do |redis|
         expect(redis.exists(key)).to eq(true)
         expect(redis.ttl(key) > described_class::DEFAULT_EXPIRATION).to eq(true)
-        expect(redis.get(key)).to eq(described_class::DEFAULT_VALUE.to_s)
+        expect(redis.get(key)).to eq('1')
       end
     end
 
-    it 'allows overriding the default value' do
-      described_class.set('123', value: 2)
+    it 'does not store anything with a nil expiry' do
+      described_class.set('123', nil)
 
       key = described_class.key_for('123')
 
       Sidekiq.redis do |redis|
-        expect(redis.exists(key)).to eq(true)
-        expect(redis.ttl(key) > 0).to eq(true)
-        expect(redis.get(key)).to eq('2')
+        expect(redis.exists(key)).to eq(false)
       end
     end
   end
@@ -138,33 +136,5 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
     it 'handles an empty array' do
       expect(described_class.job_status([])).to eq([])
     end
-
-    context 'when log_implicit_sidekiq_status_calls is enabled' do
-      it 'logs keys that contained the default value' do
-        described_class.set('123', value: 2)
-        described_class.set('456')
-        described_class.set('012')
-
-        expect(Sidekiq.logger).to receive(:info).with(message: described_class::DEFAULT_VALUE_MESSAGE,
-                                                      keys: [described_class.key_for('456'), described_class.key_for('012')])
-
-        expect(described_class.job_status(%w(123 456 789 012))).to eq([true, true, false, true])
-      end
-    end
-
-    context 'when log_implicit_sidekiq_status_calls is disabled' do
-      before do
-        stub_feature_flags(log_implicit_sidekiq_status_calls: false)
-      end
-
-      it 'does not perform any logging' do
-        described_class.set('123', value: 2)
-        described_class.set('456')
-
-        expect(Sidekiq.logger).not_to receive(:info)
-
-        expect(described_class.job_status(%w(123 456 789))).to eq([true, true, false])
-      end
-    end
   end
 end

spec/requests/api/merge_requests_spec.rb

@@ -3278,9 +3278,10 @@ RSpec.describe API::MergeRequests do
 
       context 'when skip_ci parameter is set' do
         it 'enqueues a rebase of the merge request with skip_ci flag set' do
-          allow(RebaseWorker).to receive(:with_status).and_return(RebaseWorker)
+          with_status = RebaseWorker.with_status
 
-          expect(RebaseWorker).to receive(:perform_async).with(merge_request.id, user.id, true).and_call_original
+          expect(RebaseWorker).to receive(:with_status).and_return(with_status)
+          expect(with_status).to receive(:perform_async).with(merge_request.id, user.id, true).and_call_original
 
           Sidekiq::Testing.fake! do
             expect do