Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/clusters_list/components/agent_table.vue

@@ -1,12 +1,12 @@
 <script>
 import { GlLink, GlTable, GlIcon, GlSprintf, GlTooltip, GlPopover } from '@gitlab/ui';
-import { s__, __ } from '~/locale';
+import { s__ } from '~/locale';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
 import { helpPagePath } from '~/helpers/help_page_helper';
 import { AGENT_STATUSES } from '../constants';
 import { getAgentConfigPath } from '../clusters_util';
-import AgentOptions from './agent_options.vue';
+import DeleteAgentButton from './delete_agent_button.vue';
 
 export default {
   i18n: {
@@ -14,7 +14,6 @@ export default {
     statusLabel: s__('ClusterAgents|Connection status'),
     lastContactLabel: s__('ClusterAgents|Last contact'),
     configurationLabel: s__('ClusterAgents|Configuration'),
-    optionsLabel: __('Options'),
     troubleshootingText: s__('ClusterAgents|Learn how to troubleshoot'),
     neverConnectedText: s__('ClusterAgents|Never'),
   },
@@ -26,7 +25,7 @@ export default {
     GlTooltip,
     GlPopover,
     TimeAgoTooltip,
-    AgentOptions,
+    DeleteAgentButton,
   },
   mixins: [timeagoMixin],
   AGENT_STATUSES,
@@ -75,7 +74,7 @@ export default {
         },
         {
           key: 'options',
-          label: this.$options.i18n.optionsLabel,
+          label: '',
           tdClass,
         },
       ];
@@ -155,7 +154,7 @@ export default {
     </template>
 
     <template #cell(options)="{ item }">
-      <agent-options
+      <delete-agent-button
         :agent="item"
         :default-branch-name="defaultBranchName"
         :max-agents="maxAgents"

app/assets/javascripts/clusters_list/components/clusters_actions.vue

@@ -1,5 +1,5 @@
 <script>
-import { GlDropdown, GlDropdownItem, GlModalDirective } from '@gitlab/ui';
+import { GlDropdown, GlDropdownItem, GlModalDirective, GlTooltipDirective } from '@gitlab/ui';
 import { INSTALL_AGENT_MODAL_ID, CLUSTERS_ACTIONS } from '../constants';
 
 export default {
@@ -11,8 +11,15 @@ export default {
   },
   directives: {
     GlModalDirective,
+    GlTooltip: GlTooltipDirective,
+  },
+  inject: ['newClusterPath', 'addClusterPath', 'canAddCluster'],
+  computed: {
+    tooltip() {
+      const { connectWithAgent, dropdownDisabledHint } = this.$options.i18n;
+      return this.canAddCluster ? connectWithAgent : dropdownDisabledHint;
+    },
   },
-  inject: ['newClusterPath', 'addClusterPath'],
 };
 </script>
 
@@ -20,10 +27,12 @@ export default {
   <div class="nav-controls gl-ml-auto">
     <gl-dropdown
       ref="dropdown"
-      v-gl-modal-directive="$options.INSTALL_AGENT_MODAL_ID"
+      v-gl-modal-directive="canAddCluster && $options.INSTALL_AGENT_MODAL_ID"
+      v-gl-tooltip="tooltip"
       category="primary"
       variant="confirm"
       :text="$options.i18n.actionsButton"
+      :disabled="!canAddCluster"
       split
       right
     >

app/assets/javascripts/clusters_list/components/clusters_view_all.vue

@@ -8,6 +8,7 @@ import {
   GlBadge,
   GlLoadingIcon,
   GlModalDirective,
+  GlTooltipDirective,
 } from '@gitlab/ui';
 import { mapState } from 'vuex';
 import {
@@ -33,6 +34,7 @@ export default {
   },
   directives: {
     GlModalDirective,
+    GlTooltip: GlTooltipDirective,
   },
   MAX_CLUSTERS_LIST,
   INSTALL_AGENT_MODAL_ID,
@@ -40,7 +42,7 @@ export default {
     agent: AGENT_CARD_INFO,
     certificate: CERTIFICATE_BASED_CARD_INFO,
   },
-  inject: ['addClusterPath'],
+  inject: ['addClusterPath', 'canAddCluster'],
   props: {
     defaultBranchName: {
       default: '.noBranch',
@@ -91,6 +93,14 @@ export default {
 
       return cardTitle;
     },
+    installAgentTooltip() {
+      return this.canAddCluster ? '' : this.$options.i18n.agent.installAgentDisabledHint;
+    },
+    connectExistingClusterTooltip() {
+      return this.canAddCluster
+        ? ''
+        : this.$options.i18n.certificate.connectExistingClusterDisabledHint;
+    },
   },
   methods: {
     cardFooterNumber(number) {
@@ -166,13 +176,22 @@ export default {
             ><gl-sprintf :message="$options.i18n.agent.footerText"
               ><template #number>{{ cardFooterNumber(totalAgents) }}</template></gl-sprintf
             ></gl-link
-          ><gl-button
-            v-gl-modal-directive="$options.INSTALL_AGENT_MODAL_ID"
-            class="gl-ml-4"
-            category="secondary"
-            variant="confirm"
-            >{{ $options.i18n.agent.actionText }}</gl-button
           >
+          <div
+            v-gl-tooltip="installAgentTooltip"
+            class="gl-display-inline-block"
+            tabindex="-1"
+            data-testid="install-agent-button-tooltip"
+          >
+            <gl-button
+              v-gl-modal-directive="$options.INSTALL_AGENT_MODAL_ID"
+              class="gl-ml-4"
+              category="secondary"
+              variant="confirm"
+              :disabled="!canAddCluster"
+              >{{ $options.i18n.agent.actionText }}</gl-button
+            >
+          </div>
         </template>
       </gl-card>
 
@@ -206,14 +225,23 @@ export default {
             ><gl-sprintf :message="$options.i18n.certificate.footerText"
               ><template #number>{{ cardFooterNumber(totalClusters) }}</template></gl-sprintf
             ></gl-link
-          ><gl-button
-            category="secondary"
-            data-qa-selector="connect_existing_cluster_button"
-            variant="confirm"
-            class="gl-ml-4"
-            :href="addClusterPath"
-            >{{ $options.i18n.certificate.actionText }}</gl-button
           >
+          <div
+            v-gl-tooltip="connectExistingClusterTooltip"
+            class="gl-display-inline-block"
+            tabindex="-1"
+            data-testid="connect-existing-cluster-button-tooltip"
+          >
+            <gl-button
+              category="secondary"
+              data-qa-selector="connect_existing_cluster_button"
+              variant="confirm"
+              class="gl-ml-4"
+              :href="addClusterPath"
+              :disabled="!canAddCluster"
+              >{{ $options.i18n.certificate.actionText }}</gl-button
+            >
+          </div>
         </template>
       </gl-card>
     </div>

app/assets/javascripts/clusters_list/components/delete_agent_button.vue

@@ -1,36 +1,23 @@
 <script>
 import {
-  GlDropdown,
+  GlButton,
-  GlDropdownItem,
   GlModal,
   GlModalDirective,
   GlSprintf,
   GlFormGroup,
   GlFormInput,
+  GlTooltipDirective,
 } from '@gitlab/ui';
-import { s__, __, sprintf } from '~/locale';
+import { sprintf } from '~/locale';
-import { DELETE_AGENT_MODAL_ID } from '../constants';
+import { DELETE_AGENT_BUTTON, DELETE_AGENT_MODAL_ID } from '../constants';
 import deleteAgent from '../graphql/mutations/delete_agent.mutation.graphql';
 import getAgentsQuery from '../graphql/queries/get_agents.query.graphql';
 import { removeAgentFromStore } from '../graphql/cache_update';
 
 export default {
-  i18n: {
+  i18n: DELETE_AGENT_BUTTON,
-    dropdownText: __('More options'),
-    deleteButton: s__('ClusterAgents|Delete agent'),
-    modalTitle: __('Are you sure?'),
-    modalBody: s__(
-      'ClusterAgents|Are you sure you want to delete this agent? You cannot undo this.',
-    ),
-    modalInputLabel: s__('ClusterAgents|To delete the agent, type %{name} to confirm:'),
-    modalAction: s__('ClusterAgents|Delete'),
-    modalCancel: __('Cancel'),
-    successMessage: s__('ClusterAgents|%{name} successfully deleted'),
-    defaultError: __('An error occurred. Please try again.'),
-  },
   components: {
-    GlDropdown,
+    GlButton,
-    GlDropdownItem,
     GlModal,
     GlSprintf,
     GlFormGroup,
@@ -38,8 +25,9 @@ export default {
   },
   directives: {
     GlModalDirective,
+    GlTooltip: GlTooltipDirective,
   },
-  inject: ['projectPath'],
+  inject: ['projectPath', 'canAdminCluster'],
   props: {
     agent: {
       required: true,
@@ -66,6 +54,13 @@ export default {
     };
   },
   computed: {
+    deleteButtonDisabled() {
+      return this.loading || !this.canAdminCluster;
+    },
+    deleteButtonTooltip() {
+      const { deleteButton, disabledHint } = this.$options.i18n;
+      return this.deleteButtonDisabled ? disabledHint : deleteButton;
+    },
     getAgentsQueryVariables() {
       return {
         defaultBranchName: this.defaultBranchName,
@@ -159,19 +154,22 @@ export default {
 
 <template>
   <div>
-    <gl-dropdown
+    <div
-      icon="ellipsis_v"
+      v-gl-tooltip="deleteButtonTooltip"
-      right
+      class="gl-display-inline-block"
-      :disabled="loading"
+      tabindex="-1"
-      :text="$options.i18n.dropdownText"
+      data-testid="delete-agent-button-tooltip"
-      text-sr-only
-      category="tertiary"
-      no-caret
     >
-      <gl-dropdown-item v-gl-modal-directive="modalId">
+      <gl-button
-        {{ $options.i18n.deleteButton }}
+        ref="deleteAgentButton"
-      </gl-dropdown-item>
+        v-gl-modal-directive="modalId"
-    </gl-dropdown>
+        icon="remove"
+        category="secondary"
+        variant="danger"
+        :disabled="deleteButtonDisabled"
+        :aria-label="$options.i18n.deleteButton"
+      />
+    </div>
 
     <gl-modal
       ref="modal"

app/assets/javascripts/clusters_list/constants.js

@@ -190,6 +190,9 @@ export const AGENT_CARD_INFO = {
   },
   actionText: s__('ClusterAgents|Install new Agent'),
   footerText: sprintf(s__('ClusterAgents|View all %{number} agents')),
+  installAgentDisabledHint: s__(
+    'ClusterAgents|Requires a Maintainer or greater role to install new agents',
+  ),
 };
 
 export const CERTIFICATE_BASED_CARD_INFO = {
@@ -201,6 +204,9 @@ export const CERTIFICATE_BASED_CARD_INFO = {
   actionText: s__('ClusterAgents|Connect existing cluster'),
   footerText: sprintf(s__('ClusterAgents|View all %{number} clusters')),
   badgeText: s__('ClusterAgents|Deprecated'),
+  connectExistingClusterDisabledHint: s__(
+    'ClusterAgents|Requires a maintainer or greater role to connect existing clusters',
+  ),
 };
 
 export const MAX_CLUSTERS_LIST = 6;
@@ -226,8 +232,23 @@ export const CLUSTERS_TABS = [
 export const CLUSTERS_ACTIONS = {
   actionsButton: s__('ClusterAgents|Actions'),
   createNewCluster: s__('ClusterAgents|Create a new cluster'),
-  connectWithAgent: s__('ClusterAgents|Connect with Agent'),
+  connectWithAgent: s__('ClusterAgents|Connect with agent'),
   connectExistingCluster: s__('ClusterAgents|Connect with a certificate'),
+  dropdownDisabledHint: s__(
+    'ClusterAgents|Requires a Maintainer or greater role to perform these actions',
+  ),
+};
+
+export const DELETE_AGENT_BUTTON = {
+  deleteButton: s__('ClusterAgents|Delete agent'),
+  disabledHint: s__('ClusterAgents|Requires a Maintainer or greater role to delete agents'),
+  modalTitle: __('Are you sure?'),
+  modalBody: s__('ClusterAgents|Are you sure you want to delete this agent? You cannot undo this.'),
+  modalInputLabel: s__('ClusterAgents|To delete the agent, type %{name} to confirm:'),
+  modalAction: s__('ClusterAgents|Delete'),
+  modalCancel: __('Cancel'),
+  successMessage: s__('ClusterAgents|%{name} successfully deleted'),
+  defaultError: __('An error occurred. Please try again.'),
 };
 
 export const AGENT = 'agent';

app/assets/javascripts/clusters_list/load_main_view.js

@@ -1,5 +1,6 @@
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
+import { parseBoolean } from '~/lib/utils/common_utils';
 import createDefaultClient from '~/lib/graphql';
 import ClustersMainView from './components/clusters_main_view.vue';
 import { createStore } from './store';
@@ -24,6 +25,8 @@ export default () => {
     addClusterPath,
     emptyStateHelpText,
     clustersEmptyStateImage,
+    canAddCluster,
+    canAdminCluster,
   } = el.dataset;
 
   return new Vue({
@@ -37,6 +40,8 @@ export default () => {
       addClusterPath,
       emptyStateHelpText,
       clustersEmptyStateImage,
+      canAddCluster: parseBoolean(canAddCluster),
+      canAdminCluster: parseBoolean(canAdminCluster),
     },
     store: createStore(el.dataset),
     render(createElement) {

app/assets/stylesheets/highlight/themes/dark.scss

@@ -36,6 +36,7 @@ $dark-cm: #969896;
 $dark-cp: #969896;
 $dark-c1: #969896;
 $dark-cs: #969896;
+$dark-cd: #969896;
 $dark-gd: #c66;
 $dark-gh: #8abeb7;
 $dark-gi: #b5bd68;
@@ -236,6 +237,7 @@ $dark-il: #de935f;
   .cp { color: $dark-cp; } /* Comment.Preproc */
   .c1 { color: $dark-c1; } /* Comment.Single */
   .cs { color: $dark-cs; } /* Comment.Special */
+  .cd { color: $dark-cd; } /* Comment.Doc */
   .gd { color: $dark-gd; } /* Generic.Deleted */
   .ge { font-style: italic; } /* Generic.Emph */
   .gh { /* Generic.Heading */

app/assets/stylesheets/highlight/themes/monokai.scss

@@ -38,6 +38,7 @@ $monokai-cm: #75715e;
 $monokai-cp: #75715e;
 $monokai-c1: #75715e;
 $monokai-cs: #75715e;
+$monokai-cd: #75715e;
 $monokai-kc: #66d9ef;
 $monokai-kd: #66d9ef;
 $monokai-kn: #f92672;
@@ -240,6 +241,7 @@ $monokai-gh: #75715e;
   .cp { color: $monokai-cp; } /* Comment.Preproc */
   .c1 { color: $monokai-c1; } /* Comment.Single */
   .cs { color: $monokai-cs; } /* Comment.Special */
+  .cd { color: $monokai-cd; } /* Comment.Doc */
   .ge { font-style: italic; } /* Generic.Emph */
   .gs { font-weight: $gl-font-weight-bold; } /* Generic.Strong */
   .kc { color: $monokai-kc; } /* Keyword.Constant */

app/assets/stylesheets/highlight/themes/none.scss

@@ -204,6 +204,7 @@
   .cp { color: $gl-text-color; } /* Comment.Preproc */
   .c1 { color: $gl-text-color; } /* Comment.Single */
   .cs { color: $gl-text-color; } /* Comment.Special */
+  .cd { color: $gl-text-color; } /* Comment.Doc */
   .ge { color: $gl-text-color; } /* Generic.Emph */
   .gr { color: $gl-text-color; } /* Generic.Error */
   .gh { color: $gl-text-color; } /* Generic.Heading */

app/assets/stylesheets/highlight/themes/solarized-dark.scss

@@ -35,6 +35,7 @@ $solarized-dark-cm: #586e75;
 $solarized-dark-cp: #859900;
 $solarized-dark-c1: #586e75;
 $solarized-dark-cs: #859900;
+$solarized-dark-cd: #586e75;
 $solarized-dark-gd: #2aa198;
 $solarized-dark-ge: #93a1a1;
 $solarized-dark-gr: #dc322f;
@@ -258,6 +259,7 @@ $solarized-dark-il: #2aa198;
   .cp { color: $solarized-dark-cp; } /* Comment.Preproc */
   .c1 { color: $solarized-dark-c1; } /* Comment.Single */
   .cs { color: $solarized-dark-cs; } /* Comment.Special */
+  .cd { color: $solarized-dark-cd; } /* Comment.Doc */
   .gd { color: $solarized-dark-gd; } /* Generic.Deleted */
   .ge { /* Generic.Emph */
     color: $solarized-dark-ge;

app/assets/stylesheets/highlight/themes/solarized-light.scss

@@ -37,6 +37,7 @@ $solarized-light-cm: #93a1a1;
 $solarized-light-cp: #859900;
 $solarized-light-c1: #93a1a1;
 $solarized-light-cs: #859900;
+$solarized-light-cd: #93a1a1;
 $solarized-light-gd: #2aa198;
 $solarized-light-ge: #586e75;
 $solarized-light-gr: #dc322f;
@@ -266,6 +267,7 @@ $solarized-light-il: #2aa198;
   .cp { color: $solarized-light-cp; } /* Comment.Preproc */
   .c1 { color: $solarized-light-c1; } /* Comment.Single */
   .cs { color: $solarized-light-cs; } /* Comment.Special */
+  .cd { color: $solarized-light-cd; } /* Comment.Doc */
   .gd { color: $solarized-light-gd; } /* Generic.Deleted */
   .ge { /* Generic.Emph */
     color: $solarized-light-ge;

app/assets/stylesheets/highlight/white_base.scss

@@ -18,6 +18,7 @@ $white-cm: #998;
 $white-cp: #999;
 $white-c1: #998;
 $white-cs: #999;
+$white-cd: #998;
 $white-gd: $black;
 $white-gd-bg: #fdd;
 $white-gd-x: $black;
@@ -290,6 +291,9 @@ span.highlight_word {
   font-weight: $gl-font-weight-bold;
   font-style: italic; }
 
+.cd { color: $white-cd;
+  font-style: italic; }
+
 .gd {
   color: $white-gd;
   background-color: $white-gd-bg;

app/assets/stylesheets/mailers/highlighted_diff_email.scss

@@ -22,6 +22,7 @@ $highlighted-cm: #998;
 $highlighted-cp: #999;
 $highlighted-c1: #998;
 $highlighted-cs: #999;
+$highlighted-cd: #998;
 $highlighted-gd: #000;
 $highlighted-gd-bg: #fdd;
 $highlighted-gd-x: #000;
@@ -173,6 +174,9 @@ span.highlight_word {
   font-weight: $gl-font-weight-bold;
   font-style: italic; }
 
+.cd { color: $highlighted-cd;
+  font-style: italic; }
+
 .gd {
   color: $highlighted-gd;
   background-color: $highlighted-gd-bg;

app/controllers/clusters/base_controller.rb

@@ -4,7 +4,7 @@ class Clusters::BaseController < ApplicationController
   include RoutableActions
 
   skip_before_action :authenticate_user!
-  before_action :authorize_read_cluster!
+  before_action :authorize_admin_cluster!, except: [:show, :index, :new, :authorize_aws_role, :update]
 
   helper_method :clusterable
 
@@ -18,11 +18,11 @@ class Clusters::BaseController < ApplicationController
   end
 
   def authorize_update_cluster!
-    access_denied! unless can?(current_user, :update_cluster, cluster)
+    access_denied! unless can?(current_user, :update_cluster, clusterable)
   end
 
   def authorize_admin_cluster!
-    access_denied! unless can?(current_user, :admin_cluster, cluster)
+    access_denied! unless can?(current_user, :admin_cluster, clusterable)
   end
 
   def authorize_read_cluster!

app/controllers/clusters/clusters_controller.rb

@@ -10,9 +10,9 @@ class Clusters::ClustersController < Clusters::BaseController
   before_action :validate_gcp_token, only: [:new]
   before_action :gcp_cluster, only: [:new]
   before_action :user_cluster, only: [:new]
+  before_action :authorize_read_cluster!, only: [:show, :index]
   before_action :authorize_create_cluster!, only: [:new, :authorize_aws_role]
   before_action :authorize_update_cluster!, only: [:update]
-  before_action :authorize_admin_cluster!, only: [:destroy, :clear_cache]
   before_action :update_applications_status, only: [:cluster_status]
 
   helper_method :token_in_session

app/controllers/projects/cluster_agents_controller.rb

@@ -16,7 +16,7 @@ class Projects::ClusterAgentsController < Projects::ApplicationController
   private
 
   def authorize_can_read_cluster_agent!
-    return if can?(current_user, :admin_cluster, project)
+    return if can?(current_user, :read_cluster, project)
 
     access_denied!
   end

app/graphql/resolvers/clusters/agent_tokens_resolver.rb

@@ -25,7 +25,7 @@ module Resolvers
       private
 
       def can_read_agent_tokens?
-        current_user.can?(:admin_cluster, project)
+        current_user.can?(:read_cluster, project)
       end
     end
   end

app/graphql/resolvers/kas/agent_configurations_resolver.rb

@@ -21,7 +21,7 @@ module Resolvers
       private
 
       def can_read_agent_configuration?
-        current_user.can?(:admin_cluster, project)
+        current_user.can?(:read_cluster, project)
       end
 
       def kas_client

app/graphql/types/clusters/agent_activity_event_type.rb

@@ -5,7 +5,7 @@ module Types
     class AgentActivityEventType < BaseObject
       graphql_name 'ClusterAgentActivityEvent'
 
-      authorize :admin_cluster
+      authorize :read_cluster
 
       connection_type_class(Types::CountableConnectionType)
 

app/graphql/types/clusters/agent_token_type.rb

@@ -5,7 +5,7 @@ module Types
     class AgentTokenType < BaseObject
       graphql_name 'ClusterAgentToken'
 
-      authorize :admin_cluster
+      authorize :read_cluster
 
       connection_type_class(Types::CountableConnectionType)
 

app/graphql/types/clusters/agent_type.rb

@@ -5,7 +5,7 @@ module Types
     class AgentType < BaseObject
       graphql_name 'ClusterAgent'
 
-      authorize :admin_cluster
+      authorize :read_cluster
 
       connection_type_class(Types::CountableConnectionType)
 

app/helpers/clusters_helper.rb

@@ -28,7 +28,8 @@ module ClustersHelper
       clusters_empty_state_image: image_path('illustrations/empty-state/empty-state-clusters.svg'),
       empty_state_help_text: clusterable.empty_state_help_text,
       new_cluster_path: clusterable.new_path(tab: 'create'),
-      can_add_cluster: clusterable.can_add_cluster?.to_s
+      can_add_cluster: clusterable.can_add_cluster?.to_s,
+      can_admin_cluster: clusterable.can_admin_cluster?.to_s
     }
   end
 

app/models/concerns/resolvable_discussion.rb

@@ -99,6 +99,12 @@ module ResolvableDiscussion
     update { |notes| notes.unresolve! }
   end
 
+  def clear_memoized_values
+    self.class.memoized_values.each do |name|
+      clear_memoization(name)
+    end
+  end
+
   private
 
   def update
@@ -110,8 +116,6 @@ module ResolvableDiscussion
     # Set the notes array to the updated notes
     @notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
-    self.class.memoized_values.each do |name|
+    clear_memoized_values
-      clear_memoization(name)
-    end
   end
 end

app/models/merge_request.rb

@@ -1754,6 +1754,8 @@ class MergeRequest < ApplicationRecord
 
     paths = active_diff_discussions.flat_map { |n| n.diff_file.paths }.uniq
 
+    active_discussions_resolved = active_diff_discussions.all?(&:resolved?)
+
     service = Discussions::UpdateDiffPositionService.new(
       self.project,
       current_user,
@@ -1764,9 +1766,15 @@ class MergeRequest < ApplicationRecord
 
     active_diff_discussions.each do |discussion|
       service.execute(discussion)
+      discussion.clear_memoized_values
     end
 
-    if project.resolve_outdated_diff_discussions?
+    # If they were all already resolved, this method will have already been called.
+    # If they all don't get resolved, we don't need to call the method
+    # If they go from unresolved -> resolved, then we call the method
+    if !active_discussions_resolved &&
+        active_diff_discussions.all?(&:resolved?) &&
+        project.resolve_outdated_diff_discussions?
       MergeRequests::ResolvedDiscussionNotificationService
         .new(project: project, current_user: current_user)
         .execute(self)

app/policies/group_policy.rb

@@ -144,6 +144,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
     enable :developer_access
     enable :admin_crm_organization
     enable :admin_crm_contact
+    enable :read_cluster
   end
 
   rule { reporter }.policy do
@@ -166,7 +167,6 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
     enable :create_projects
     enable :admin_pipeline
     enable :admin_build
-    enable :read_cluster
     enable :add_cluster
     enable :create_cluster
     enable :update_cluster

app/policies/project_policy.rb

@@ -385,6 +385,7 @@ class ProjectPolicy < BasePolicy
     enable :destroy_environment
     enable :create_deployment
     enable :update_deployment
+    enable :read_cluster
     enable :create_release
     enable :update_release
     enable :destroy_release
@@ -433,7 +434,6 @@ class ProjectPolicy < BasePolicy
     enable :read_pages
     enable :update_pages
     enable :remove_pages
-    enable :read_cluster
     enable :add_cluster
     enable :create_cluster
     enable :update_cluster

app/presenters/clusterable_presenter.rb

@@ -16,6 +16,10 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated
     can?(current_user, :add_cluster, clusterable)
   end
 
+  def can_admin_cluster?
+    can?(current_user, :admin_cluster, clusterable)
+  end
+
   def can_create_cluster?
     can?(current_user, :create_cluster, clusterable)
   end

doc/development/contributing/merge_request_workflow.md

@@ -71,6 +71,9 @@ request is as follows:
    1. The MR must include *Before* and *After* screenshots if UI changes are made.
    1. Include any steps or setup required to ensure reviewers can view the changes you've made (for example, include any information about feature flags).
 1. If you're allowed to, set a relevant milestone and [labels](issue_workflow.md).
+   MR labels should generally match the corresponding issue (if there is one).
+   The group label should reflect the group that executed or coached the work,
+   not necessarily the group that owns the feature.
 1. UI changes should use available components from the GitLab Design System,
    [Pajamas](https://design.gitlab.com/).
 1. If the MR changes CSS classes, please include the list of affected pages, which

doc/user/application_security/sast/index.md

@@ -639,15 +639,33 @@ variables:
 
 ### Pre-compilation
 
-If your project requires custom build configurations, it can be preferable to avoid
+Most GitLab SAST analyzers directly scan your source code without compiling it first.
-compilation during your SAST execution and instead pass all job artifacts from an
+However, for technical reasons, some analyzers can only scan compiled code.
-earlier stage in the pipeline. This is the current strategy when requiring
-a `before_script` execution to prepare your scan job.
 
-To pass your project's dependencies as artifacts, the dependencies must be included
+By default, these analyzers automatically attempt to fetch dependencies and compile your code so it can be scanned.
-in the project's working directory and specified using the `artifacts:path` configuration.
+Automatic compilation can fail if:
-If all dependencies are present, the `COMPILE=false` CI/CD variable can be provided to the
+
-analyzer and compilation is skipped:
+- your project requires custom build configurations.
+- you use language versions that aren't built into the analyzer.
+
+To resolve these issues, you can skip the analyzer's compilation step and directly provide artifacts from an earlier stage in your pipeline instead.
+This strategy is called _pre-compilation_.
+
+Pre-compilation is available for the analyzers that support the `COMPILE` CI/CD variable.
+See [Analyzer settings](#analyzer-settings) for the current list.
+
+To use pre-compilation:
+
+1. Output your project's dependencies to a directory in the project's working directory, then save that directory as an artifact by [setting the `artifacts: paths` configuration](../../../ci/yaml/index.md#artifactspaths).
+1. Provide the `COMPILE: "false"` CI/CD variable to the analyzer to disable automatic compilation.
+1. Add your compilation stage as a dependency for the analyzer job.
+
+To allow the analyzer to recognize the compiled artifacts, you must explicitly specify the path to
+the vendored directory.
+This configuration can vary per analyzer. For Maven projects, you can use `MAVEN_REPO_PATH`.
+See [Analyzer settings](#analyzer-settings) for the complete list of available options.
+
+The following example pre-compiles a Maven project and provides it to the SpotBugs SAST analyzer:
 
 ```yaml
 stages:
@@ -678,11 +696,6 @@ spotbugs-sast:
       sast: gl-sast-report.json
 ```
 
-To allow the analyzer to recognize the compiled artifacts, you must explicitly specify the path to
-the vendored directory. This configuration can vary per analyzer but in the case of Java above, you
-can use `MAVEN_REPO_PATH`. See
-[Analyzer settings](#analyzer-settings) for the complete list of available options.
-
 ### Available CI/CD variables
 
 SAST can be configured using the [`variables`](../../../ci/yaml/index.md#variables) parameter in

doc/user/permissions.md

@@ -78,6 +78,7 @@ The following table lists project permissions available for each role:
 | [CI/CD](../ci/index.md):<br>Use [environment terminals](../ci/environments/index.md#web-terminals-deprecated)                                                                             |          |          |           | ✓          | ✓     |
 | [CI/CD](../ci/index.md):<br>Delete pipelines                                                                                                                                              |          |          |           |            | ✓     |
 | [Clusters](infrastructure/clusters/index.md):<br>View [pod logs](project/clusters/kubernetes_pod_logs.md)                                                                                 |          |          | ✓         | ✓          | ✓     |
+| [Clusters](infrastructure/clusters/index.md):<br>View clusters                                                                                                                            |          |          | ✓         | ✓          | ✓     |
 | [Clusters](infrastructure/clusters/index.md):<br>Manage clusters                                                                                                                          |          |          |           | ✓          | ✓     |
 | [Container Registry](packages/container_registry/index.md):<br>Create, edit, delete cleanup policies                                                                                      |          |          | ✓         | ✓          | ✓     |
 | [Container Registry](packages/container_registry/index.md):<br>Remove a container registry image                                                                                          |          |          | ✓         | ✓          | ✓     |

locale/gitlab.pot

@@ -7605,10 +7605,10 @@ msgstr ""
 msgid "ClusterAgents|Connect existing cluster"
 msgstr ""
 
-msgid "ClusterAgents|Connect with Agent"
+msgid "ClusterAgents|Connect with a certificate"
 msgstr ""
 
-msgid "ClusterAgents|Connect with a certificate"
+msgid "ClusterAgents|Connect with agent"
 msgstr ""
 
 msgid "ClusterAgents|Connect with the GitLab Agent"
@@ -7725,6 +7725,18 @@ msgstr ""
 msgid "ClusterAgents|Registration token"
 msgstr ""
 
+msgid "ClusterAgents|Requires a Maintainer or greater role to delete agents"
+msgstr ""
+
+msgid "ClusterAgents|Requires a Maintainer or greater role to install new agents"
+msgstr ""
+
+msgid "ClusterAgents|Requires a Maintainer or greater role to perform these actions"
+msgstr ""
+
+msgid "ClusterAgents|Requires a maintainer or greater role to connect existing clusters"
+msgstr ""
+
 msgid "ClusterAgents|Security"
 msgstr ""
 
@@ -23243,9 +23255,6 @@ msgstr ""
 msgid "More information."
 msgstr ""
 
-msgid "More options"
-msgstr ""
-
 msgid "More than %{number_commits_distance} commits different with %{default_branch}"
 msgstr ""
 

spec/controllers/groups/clusters_controller_spec.rb

@@ -103,7 +103,7 @@ RSpec.describe Groups::ClustersController do
       it('is denied for admin when admin mode is disabled') { expect { go }.to be_denied_for(:admin) }
       it { expect { go }.to be_allowed_for(:owner).of(group) }
       it { expect { go }.to be_allowed_for(:maintainer).of(group) }
-      it { expect { go }.to be_denied_for(:developer).of(group) }
+      it { expect { go }.to be_allowed_for(:developer).of(group) }
       it { expect { go }.to be_denied_for(:reporter).of(group) }
       it { expect { go }.to be_denied_for(:guest).of(group) }
       it { expect { go }.to be_denied_for(:user) }
@@ -673,7 +673,7 @@ RSpec.describe Groups::ClustersController do
       it('is denied for admin when admin mode is disabled') { expect { go }.to be_denied_for(:admin) }
       it { expect { go }.to be_allowed_for(:owner).of(group) }
       it { expect { go }.to be_allowed_for(:maintainer).of(group) }
-      it { expect { go }.to be_denied_for(:developer).of(group) }
+      it { expect { go }.to be_allowed_for(:developer).of(group) }
       it { expect { go }.to be_denied_for(:reporter).of(group) }
       it { expect { go }.to be_denied_for(:guest).of(group) }
       it { expect { go }.to be_denied_for(:user) }

spec/controllers/projects/clusters_controller_spec.rb

@@ -101,7 +101,7 @@ RSpec.describe Projects::ClustersController do
 
       it { expect { go }.to be_allowed_for(:owner).of(project) }
       it { expect { go }.to be_allowed_for(:maintainer).of(project) }
-      it { expect { go }.to be_denied_for(:developer).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
       it { expect { go }.to be_denied_for(:reporter).of(project) }
       it { expect { go }.to be_denied_for(:guest).of(project) }
       it { expect { go }.to be_denied_for(:user) }
@@ -711,7 +711,7 @@ RSpec.describe Projects::ClustersController do
       end
       it { expect { go }.to be_allowed_for(:owner).of(project) }
       it { expect { go }.to be_allowed_for(:maintainer).of(project) }
-      it { expect { go }.to be_denied_for(:developer).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
       it { expect { go }.to be_denied_for(:reporter).of(project) }
       it { expect { go }.to be_denied_for(:guest).of(project) }
       it { expect { go }.to be_denied_for(:user) }

spec/features/monitor_sidebar_link_spec.rb

@@ -117,9 +117,8 @@ RSpec.describe 'Monitor dropdown sidebar', :aggregate_failures do
       expect(page).to have_link('Error Tracking', href: project_error_tracking_index_path(project))
       expect(page).to have_link('Product Analytics', href: project_product_analytics_path(project))
       expect(page).to have_link('Logs', href: project_logs_path(project))
-
+      expect(page).to have_link('Serverless', href: project_serverless_functions_path(project))
-      expect(page).not_to have_link('Serverless', href: project_serverless_functions_path(project))
+      expect(page).to have_link('Kubernetes', href: project_clusters_path(project))
-      expect(page).not_to have_link('Kubernetes', href: project_clusters_path(project))
     end
 
     it_behaves_like 'shows Monitor menu based on the access level'

spec/features/projects/jobs_spec.rb

@@ -615,7 +615,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
           end
 
           context 'when the user is not able to view the cluster' do
-            let(:user_access_level) { :developer }
+            let(:user_access_level) { :reporter }
 
             it 'includes only the name of the cluster without a link' do
               expect(page).to have_content 'using cluster the-cluster'

spec/finders/clusters/agents_finder_spec.rb

@@ -15,7 +15,11 @@ RSpec.describe Clusters::AgentsFinder do
     it { is_expected.to contain_exactly(matching_agent) }
 
     context 'user does not have permission' do
-      let(:user) { create(:user, developer_projects: [project]) }
+      let(:user) { create(:user) }
+
+      before do
+        project.add_reporter(user)
+      end
 
       it { is_expected.to be_empty }
     end

spec/frontend/clusters_list/components/agent_table_spec.js

@@ -1,6 +1,6 @@
 import { GlLink, GlIcon } from '@gitlab/ui';
 import AgentTable from '~/clusters_list/components/agent_table.vue';
-import AgentOptions from '~/clusters_list/components/agent_options.vue';
+import DeleteAgentButton from '~/clusters_list/components/delete_agent_button.vue';
 import { ACTIVE_CONNECTION_TIME } from '~/clusters_list/constants';
 import { mountExtended } from 'helpers/vue_test_utils_helper';
 import { stubComponent } from 'helpers/stub_component';
@@ -56,7 +56,7 @@ const propsData = {
   ],
 };
 
-const AgentOptionsStub = stubComponent(AgentOptions, {
+const DeleteAgentButtonStub = stubComponent(DeleteAgentButton, {
   template: `<div></div>`,
 });
 
@@ -69,14 +69,14 @@ describe('AgentTable', () => {
   const findLastContactText = (at) => wrapper.findAllByTestId('cluster-agent-last-contact').at(at);
   const findConfiguration = (at) =>
     wrapper.findAllByTestId('cluster-agent-configuration-link').at(at);
-  const findAgentOptions = () => wrapper.findAllComponents(AgentOptions);
+  const findDeleteAgentButton = () => wrapper.findAllComponents(DeleteAgentButton);
 
   beforeEach(() => {
     wrapper = mountExtended(AgentTable, {
       propsData,
       provide: provideData,
       stubs: {
-        AgentOptions: AgentOptionsStub,
+        DeleteAgentButton: DeleteAgentButtonStub,
       },
     });
   });
@@ -128,7 +128,7 @@ describe('AgentTable', () => {
     });
 
     it('displays actions menu for each agent', () => {
-      expect(findAgentOptions()).toHaveLength(3);
+      expect(findDeleteAgentButton()).toHaveLength(3);
     });
   });
 });

spec/frontend/clusters_list/components/clusters_actions_spec.js

@@ -10,9 +10,10 @@ describe('ClustersActionsComponent', () => {
   const newClusterPath = 'path/to/create/cluster';
   const addClusterPath = 'path/to/connect/existing/cluster';
 
-  const provideData = {
+  const defaultProvide = {
     newClusterPath,
     addClusterPath,
+    canAddCluster: true,
   };
 
   const findDropdown = () => wrapper.findComponent(GlDropdown);
@@ -21,13 +22,21 @@ describe('ClustersActionsComponent', () => {
   const findConnectClusterLink = () => wrapper.findByTestId('connect-cluster-link');
   const findConnectNewAgentLink = () => wrapper.findByTestId('connect-new-agent-link');
 
-  beforeEach(() => {
+  const createWrapper = (provideData = {}) => {
     wrapper = shallowMountExtended(ClustersActions, {
-      provide: provideData,
+      provide: {
+        ...defaultProvide,
+        ...provideData,
+      },
       directives: {
         GlModalDirective: createMockDirective(),
+        GlTooltip: createMockDirective(),
       },
     });
+  };
+
+  beforeEach(() => {
+    createWrapper();
   });
 
   afterEach(() => {
@@ -52,4 +61,24 @@ describe('ClustersActionsComponent', () => {
 
     expect(binding.value).toBe(INSTALL_AGENT_MODAL_ID);
   });
+
+  it('shows tooltip', () => {
+    const tooltip = getBinding(findDropdown().element, 'gl-tooltip');
+    expect(tooltip.value).toBe(CLUSTERS_ACTIONS.connectWithAgent);
+  });
+
+  describe('when user cannot add clusters', () => {
+    beforeEach(() => {
+      createWrapper({ canAddCluster: false });
+    });
+
+    it('disables dropdown', () => {
+      expect(findDropdown().props('disabled')).toBe(true);
+    });
+
+    it('shows tooltip explaining why dropdown is disabled', () => {
+      const tooltip = getBinding(findDropdown().element, 'gl-tooltip');
+      expect(tooltip.value).toBe(CLUSTERS_ACTIONS.dropdownDisabledHint);
+    });
+  });
 });

spec/frontend/clusters_list/components/clusters_view_all_spec.js

@@ -32,8 +32,9 @@ describe('ClustersViewAllComponent', () => {
     defaultBranchName,
   };
 
-  const provideData = {
+  const defaultProvide = {
     addClusterPath,
+    canAddCluster: true,
   };
 
   const entryData = {
@@ -45,31 +46,43 @@ describe('ClustersViewAllComponent', () => {
   const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
   const findAgentsComponent = () => wrapper.findComponent(Agents);
   const findClustersComponent = () => wrapper.findComponent(Clusters);
+  const findInstallAgentButtonTooltip = () => wrapper.findByTestId('install-agent-button-tooltip');
+  const findConnectExistingClusterButtonTooltip = () =>
+    wrapper.findByTestId('connect-existing-cluster-button-tooltip');
   const findCardsContainer = () => wrapper.findByTestId('clusters-cards-container');
   const findAgentCardTitle = () => wrapper.findByTestId('agent-card-title');
   const findRecommendedBadge = () => wrapper.findComponent(GlBadge);
   const findClustersCardTitle = () => wrapper.findByTestId('clusters-card-title');
   const findFooterButton = (line) => findCards().at(line).findComponent(GlButton);
+  const getTooltipText = (el) => {
+    const binding = getBinding(el, 'gl-tooltip');
+
+    return binding.value;
+  };
 
   const createStore = (initialState) =>
     new Vuex.Store({
       state: initialState,
     });
 
-  const createWrapper = ({ initialState }) => {
+  const createWrapper = ({ initialState = entryData, provideData } = {}) => {
     wrapper = shallowMountExtended(ClustersViewAll, {
       store: createStore(initialState),
       propsData,
-      provide: provideData,
+      provide: {
+        ...defaultProvide,
+        ...provideData,
+      },
       directives: {
         GlModalDirective: createMockDirective(),
+        GlTooltip: createMockDirective(),
       },
       stubs: { GlCard, GlSprintf },
     });
   };
 
   beforeEach(() => {
-    createWrapper({ initialState: entryData });
+    createWrapper();
   });
 
   afterEach(() => {
@@ -125,15 +138,20 @@ describe('ClustersViewAllComponent', () => {
       expect(findAgentsComponent().props('defaultBranchName')).toBe(defaultBranchName);
     });
 
+    it('should show install new Agent button in the footer', () => {
+      expect(findFooterButton(0).exists()).toBe(true);
+      expect(findFooterButton(0).props('disabled')).toBe(false);
+    });
+
+    it('does not show tooltip for install new Agent button', () => {
+      expect(getTooltipText(findInstallAgentButtonTooltip().element)).toBe('');
+    });
+
     describe('when there are no agents', () => {
       it('should show the empty title', () => {
         expect(findAgentCardTitle().text()).toBe(AGENT_CARD_INFO.emptyTitle);
       });
 
-      it('should show install new Agent button in the footer', () => {
-        expect(findFooterButton(0).exists()).toBe(true);
-      });
-
       it('should render correct modal id for the agent link', () => {
         const binding = getBinding(findFooterButton(0).element, 'gl-modal-directive');
 
@@ -173,6 +191,22 @@ describe('ClustersViewAllComponent', () => {
         });
       });
     });
+
+    describe('when the user cannot add clusters', () => {
+      beforeEach(() => {
+        createWrapper({ provideData: { canAddCluster: false } });
+      });
+
+      it('should disable the button', () => {
+        expect(findFooterButton(0).props('disabled')).toBe(true);
+      });
+
+      it('should show a tooltip explaining why the button is disabled', () => {
+        expect(getTooltipText(findInstallAgentButtonTooltip().element)).toBe(
+          AGENT_CARD_INFO.installAgentDisabledHint,
+        );
+      });
+    });
   });
 
   describe('clusters tab', () => {
@@ -189,13 +223,34 @@ describe('ClustersViewAllComponent', () => {
         expect(findClustersCardTitle().text()).toBe(CERTIFICATE_BASED_CARD_INFO.emptyTitle);
       });
 
-      it('should show install new Agent button in the footer', () => {
+      it('should show install new cluster button in the footer', () => {
         expect(findFooterButton(1).exists()).toBe(true);
+        expect(findFooterButton(1).props('disabled')).toBe(false);
       });
 
       it('should render correct href for the button in the footer', () => {
         expect(findFooterButton(1).attributes('href')).toBe(addClusterPath);
       });
+
+      it('does not show tooltip for install new cluster button', () => {
+        expect(getTooltipText(findConnectExistingClusterButtonTooltip().element)).toBe('');
+      });
+    });
+
+    describe('when the user cannot add clusters', () => {
+      beforeEach(() => {
+        createWrapper({ provideData: { canAddCluster: false } });
+      });
+
+      it('should disable the button', () => {
+        expect(findFooterButton(1).props('disabled')).toBe(true);
+      });
+
+      it('should show a tooltip explaining why the button is disabled', () => {
+        expect(getTooltipText(findConnectExistingClusterButtonTooltip().element)).toBe(
+          CERTIFICATE_BASED_CARD_INFO.connectExistingClusterDisabledHint,
+        );
+      });
     });
 
     describe('when the clusters are present', () => {

spec/frontend/clusters_list/components/delete_agent_button_spec.js

@@ -1,4 +1,4 @@
-import { GlDropdown, GlDropdownItem, GlModal, GlFormInput } from '@gitlab/ui';
+import { GlButton, GlModal, GlFormInput } from '@gitlab/ui';
 import Vue, { nextTick } from 'vue';
 import VueApollo from 'vue-apollo';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
@@ -7,8 +7,9 @@ import getAgentsQuery from '~/clusters_list/graphql/queries/get_agents.query.gra
 import deleteAgentMutation from '~/clusters_list/graphql/mutations/delete_agent.mutation.graphql';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
-import AgentOptions from '~/clusters_list/components/agent_options.vue';
+import DeleteAgentButton from '~/clusters_list/components/delete_agent_button.vue';
-import { MAX_LIST_COUNT } from '~/clusters_list/constants';
+import { MAX_LIST_COUNT, DELETE_AGENT_BUTTON } from '~/clusters_list/constants';
+import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
 import { getAgentResponse, mockDeleteResponse, mockErrorDeleteResponse } from '../mocks/apollo';
 
 Vue.use(VueApollo);
@@ -22,18 +23,23 @@ const agent = {
   webPath: 'agent-webPath',
 };
 
-describe('AgentOptions', () => {
+describe('DeleteAgentButton', () => {
   let wrapper;
   let toast;
   let apolloProvider;
   let deleteResponse;
 
   const findModal = () => wrapper.findComponent(GlModal);
-  const findDropdown = () => wrapper.findComponent(GlDropdown);
+  const findDeleteBtn = () => wrapper.findComponent(GlButton);
-  const findDeleteBtn = () => wrapper.findComponent(GlDropdownItem);
   const findInput = () => wrapper.findComponent(GlFormInput);
   const findPrimaryAction = () => findModal().props('actionPrimary');
   const findPrimaryActionAttributes = (attr) => findPrimaryAction().attributes[0][attr];
+  const findDeleteAgentButtonTooltip = () => wrapper.findByTestId('delete-agent-button-tooltip');
+  const getTooltipText = (el) => {
+    const binding = getBinding(el, 'gl-tooltip');
+
+    return binding.value;
+  };
 
   const createMockApolloProvider = ({ mutationResponse }) => {
     deleteResponse = jest.fn().mockResolvedValue(mutationResponse);
@@ -54,10 +60,14 @@ describe('AgentOptions', () => {
     });
   };
 
-  const createWrapper = async ({ mutationResponse = mockDeleteResponse } = {}) => {
+  const createWrapper = async ({
+    mutationResponse = mockDeleteResponse,
+    provideData = {},
+  } = {}) => {
     apolloProvider = createMockApolloProvider({ mutationResponse });
-    const provide = {
+    const defaultProvide = {
       projectPath,
+      canAdminCluster: true,
     };
     const propsData = {
       defaultBranchName,
@@ -67,9 +77,15 @@ describe('AgentOptions', () => {
 
     toast = jest.fn();
 
-    wrapper = shallowMountExtended(AgentOptions, {
+    wrapper = shallowMountExtended(DeleteAgentButton, {
       apolloProvider,
-      provide,
+      provide: {
+        ...defaultProvide,
+        ...provideData,
+      },
+      directives: {
+        GlTooltip: createMockDirective(),
+      },
       propsData,
       mocks: { $toast: { show: toast } },
       stubs: { GlModal },
@@ -100,7 +116,13 @@ describe('AgentOptions', () => {
 
   describe('delete agent action', () => {
     it('displays a delete button', () => {
-      expect(findDeleteBtn().text()).toBe('Delete agent');
+      expect(findDeleteBtn().attributes('aria-label')).toBe(DELETE_AGENT_BUTTON.deleteButton);
+    });
+
+    it('shows a tooltip for the button', () => {
+      expect(getTooltipText(findDeleteAgentButtonTooltip().element)).toBe(
+        DELETE_AGENT_BUTTON.deleteButton,
+      );
     });
 
     describe('when clicking the delete button', () => {
@@ -113,6 +135,22 @@ describe('AgentOptions', () => {
       });
     });
 
+    describe('when user cannot delete clusters', () => {
+      beforeEach(() => {
+        createWrapper({ provideData: { canAdminCluster: false } });
+      });
+
+      it('disables the button', () => {
+        expect(findDeleteBtn().attributes('disabled')).toBe('true');
+      });
+
+      it('shows a disabled tooltip', () => {
+        expect(getTooltipText(findDeleteAgentButtonTooltip().element)).toBe(
+          DELETE_AGENT_BUTTON.disabledHint,
+        );
+      });
+    });
+
     describe.each`
       condition                                   | agentName       | isDisabled | mutationCalled
       ${'the input with agent name is missing'}   | ${''}           | ${true}    | ${false}
@@ -191,14 +229,14 @@ describe('AgentOptions', () => {
       await submitAgentToDelete();
     });
 
-    it('reenables the options dropdown', async () => {
+    it('reenables the button', async () => {
       expect(findPrimaryActionAttributes('loading')).toBe(true);
-      expect(findDropdown().attributes('disabled')).toBe('true');
+      expect(findDeleteBtn().attributes('disabled')).toBe('true');
 
       await findModal().vm.$emit('hide');
 
       expect(findPrimaryActionAttributes('loading')).toBe(false);
-      expect(findDropdown().attributes('disabled')).toBeUndefined();
+      expect(findDeleteBtn().attributes('disabled')).toBeUndefined();
     });
 
     it('clears the agent name input', async () => {

spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do
 
   describe '#resolve' do
     let(:agent) { create(:cluster_agent) }
-    let(:user) { create(:user, maintainer_projects: [agent.project]) }
+    let(:user) { create(:user, developer_projects: [agent.project]) }
     let(:ctx) { Hash(current_user: user) }
 
     let!(:matching_token1) { create(:cluster_agent_token, agent: agent, last_used_at: 5.days.ago) }
@@ -33,7 +33,11 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do
     end
 
     context 'user does not have permission' do
-      let(:user) { create(:user, developer_projects: [agent.project]) }
+      let(:user) { create(:user) }
+
+      before do
+        agent.project.add_reporter(user)
+      end
 
       it { is_expected.to be_empty }
     end

spec/graphql/resolvers/clusters/agents_resolver_spec.rb

@@ -15,10 +15,14 @@ RSpec.describe Resolvers::Clusters::AgentsResolver do
 
   describe '#resolve' do
     let_it_be(:project) { create(:project) }
-    let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) }
+    let_it_be(:maintainer) { create(:user, developer_projects: [project]) }
-    let_it_be(:developer) { create(:user, developer_projects: [project]) }
+    let_it_be(:reporter) { create(:user) }
     let_it_be(:agents) { create_list(:cluster_agent, 2, project: project) }
 
+    before do
+      project.add_reporter(reporter)
+    end
+
     let(:ctx) { { current_user: current_user } }
 
     subject { resolve_agents }
@@ -32,7 +36,7 @@ RSpec.describe Resolvers::Clusters::AgentsResolver do
     end
 
     context 'the current user does not have access to clusters' do
-      let(:current_user) { developer }
+      let(:current_user) { reporter }
 
       it 'returns an empty result' do
         expect(subject).to be_empty

spec/graphql/types/clusters/agent_activity_event_type_spec.rb

@@ -6,6 +6,6 @@ RSpec.describe GitlabSchema.types['ClusterAgentActivityEvent'] do
   let(:fields) { %i[recorded_at kind level user agent_token] }
 
   it { expect(described_class.graphql_name).to eq('ClusterAgentActivityEvent') }
-  it { expect(described_class).to require_graphql_authorizations(:admin_cluster) }
+  it { expect(described_class).to require_graphql_authorizations(:read_cluster) }
   it { expect(described_class).to have_graphql_fields(fields) }
 end

spec/graphql/types/clusters/agent_token_type_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe GitlabSchema.types['ClusterAgentToken'] do
 
   it { expect(described_class.graphql_name).to eq('ClusterAgentToken') }
 
-  it { expect(described_class).to require_graphql_authorizations(:admin_cluster) }
+  it { expect(described_class).to require_graphql_authorizations(:read_cluster) }
 
   it { expect(described_class).to have_graphql_fields(fields) }
 end

spec/graphql/types/clusters/agent_type_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe GitlabSchema.types['ClusterAgent'] do
 
   it { expect(described_class.graphql_name).to eq('ClusterAgent') }
 
-  it { expect(described_class).to require_graphql_authorizations(:admin_cluster) }
+  it { expect(described_class).to require_graphql_authorizations(:read_cluster) }
 
   it { expect(described_class).to have_graphql_fields(fields) }
 end

spec/helpers/clusters_helper_spec.rb

@@ -93,8 +93,9 @@ RSpec.describe ClustersHelper do
     end
 
     context 'user has no permissions to create a cluster' do
-      it 'displays that user can\t add cluster' do
+      it 'displays that user can\'t add cluster' do
         expect(subject[:can_add_cluster]).to eq("false")
+        expect(subject[:can_admin_cluster]).to eq("false")
       end
     end
 
@@ -105,6 +106,7 @@ RSpec.describe ClustersHelper do
 
       it 'displays that the user can add cluster' do
         expect(subject[:can_add_cluster]).to eq("true")
+        expect(subject[:can_admin_cluster]).to eq("true")
       end
     end
 

spec/models/concerns/resolvable_discussion_spec.rb

@@ -584,4 +584,14 @@ RSpec.describe Discussion, ResolvableDiscussion do
       expect(subject.last_resolved_note).to eq(second_note)
     end
   end
+
+  describe '#clear_memoized_values' do
+    it 'resets the memoized values' do
+      described_class.memoized_values.each do |memo|
+        subject.instance_variable_set("@#{memo}", 'memoized')
+        expect { subject.clear_memoized_values }.to change { subject.instance_variable_get("@#{memo}") }
+          .from('memoized').to(nil)
+      end
+    end
+  end
 end

spec/models/merge_request_spec.rb

@@ -3577,21 +3577,38 @@ RSpec.describe MergeRequest, factory_default: :keep do
   end
 
   describe '#update_diff_discussion_positions' do
-    let(:discussion) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject).to_discussion }
+    subject { create(:merge_request, source_project: project) }
-    let(:commit) { subject.project.commit(sample_commit.id) }
-    let(:old_diff_refs) { subject.diff_refs }
 
-    before do
+    let(:project) { create(:project, :repository) }
-      # Update merge_request_diff so that #diff_refs will return commit.diff_refs
+    let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") }
-      allow(subject).to receive(:create_merge_request_diff) do
+    let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") }
-        subject.merge_request_diffs.create!(
+    let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") }
-          base_commit_sha: commit.parent_id,
+    let(:discussion) { create(:diff_note_on_merge_request, noteable: subject, project: project, position: old_position).to_discussion }
-          start_commit_sha: commit.parent_id,
+    let(:path) { "files/ruby/popen.rb" }
-          head_commit_sha: commit.sha
+    let(:new_line) { 9 }
-        )
 
-        subject.reload_merge_request_diff
+    let(:old_diff_refs) do
-      end
+      Gitlab::Diff::DiffRefs.new(
+        base_sha: create_commit.parent_id,
+        head_sha: modify_commit.sha
+      )
+    end
+
+    let(:new_diff_refs) do
+      Gitlab::Diff::DiffRefs.new(
+        base_sha: create_commit.parent_id,
+        head_sha: edit_commit.sha
+      )
+    end
+
+    let(:old_position) do
+      Gitlab::Diff::Position.new(
+        old_path: path,
+        new_path: path,
+        old_line: nil,
+        new_line: new_line,
+        diff_refs: old_diff_refs
+      )
     end
 
     it "updates diff discussion positions" do
@@ -3599,36 +3616,67 @@ RSpec.describe MergeRequest, factory_default: :keep do
         subject.project,
         subject.author,
         old_diff_refs: old_diff_refs,
-        new_diff_refs: commit.diff_refs,
+        new_diff_refs: new_diff_refs,
         paths: discussion.position.paths
       ).and_call_original
 
       expect_any_instance_of(Discussions::UpdateDiffPositionService).to receive(:execute).with(discussion).and_call_original
-      expect_any_instance_of(DiffNote).to receive(:save).once
 
       subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
-                                               new_diff_refs: commit.diff_refs,
+                                               new_diff_refs: new_diff_refs,
+                                               current_user: subject.author)
+    end
+
+    it 'does not call the resolve method' do
+      expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new)
+
+      subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
+                                               new_diff_refs: new_diff_refs,
                                                current_user: subject.author)
     end
 
     context 'when resolve_outdated_diff_discussions is set' do
-      let(:project) { create(:project, :repository) }
-
-      subject { create(:merge_request, source_project: project) }
-
       before do
         discussion
 
         subject.project.update!(resolve_outdated_diff_discussions: true)
       end
 
-      it 'calls MergeRequests::ResolvedDiscussionNotificationService' do
+      context 'when the active discussion is resolved in the update' do
-        expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService)
+        it 'calls MergeRequests::ResolvedDiscussionNotificationService' do
-          .to receive(:execute).with(subject)
+          expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService)
+            .to receive(:execute).with(subject)
 
-        subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
+          subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
-                                                 new_diff_refs: commit.diff_refs,
+                                                   new_diff_refs: new_diff_refs,
-                                                 current_user: subject.author)
+                                                   current_user: subject.author)
+        end
+      end
+
+      context 'when the active discussion does not have resolved in the update' do
+        let(:new_line) { 16 }
+
+        it 'does not call the resolve method' do
+          expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new)
+
+          subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
+                                                   new_diff_refs: new_diff_refs,
+                                                   current_user: subject.author)
+        end
+      end
+
+      context 'when the active discussion was already resolved' do
+        before do
+          discussion.resolve!(subject.author)
+        end
+
+        it 'does not call the resolve method' do
+          expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new)
+
+          subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs,
+                                                   new_diff_refs: new_diff_refs,
+                                                   current_user: subject.author)
+        end
       end
     end
   end

spec/policies/clusters/agent_token_policy_spec.rb

@@ -10,13 +10,22 @@ RSpec.describe Clusters::AgentTokenPolicy do
   let(:project) { token.agent.project }
 
   describe 'rules' do
+    context 'when reporter' do
+      before do
+        project.add_reporter(user)
+      end
+
+      it { expect(policy).to be_disallowed :admin_cluster }
+      it { expect(policy).to be_disallowed :read_cluster }
+    end
+
     context 'when developer' do
       before do
         project.add_developer(user)
       end
 
       it { expect(policy).to be_disallowed :admin_cluster }
-      it { expect(policy).to be_disallowed :read_cluster }
+      it { expect(policy).to be_allowed :read_cluster }
     end
 
     context 'when maintainer' do

spec/policies/clusters/agents/activity_event_policy_spec.rb

@@ -10,13 +10,22 @@ RSpec.describe Clusters::Agents::ActivityEventPolicy do
   let(:project) { event.agent.project }
 
   describe 'rules' do
+    context 'reporter' do
+      before do
+        project.add_reporter(user)
+      end
+
+      it { expect(policy).to be_disallowed :admin_cluster }
+      it { expect(policy).to be_disallowed :read_cluster }
+    end
+
     context 'developer' do
       before do
         project.add_developer(user)
       end
 
       it { expect(policy).to be_disallowed :admin_cluster }
-      it { expect(policy).to be_disallowed :read_cluster }
+      it { expect(policy).to be_allowed :read_cluster }
     end
 
     context 'maintainer' do

spec/presenters/clusterable_presenter_spec.rb

@@ -79,6 +79,30 @@ RSpec.describe ClusterablePresenter do
     end
   end
 
+  describe '#can_admin_cluster?' do
+    let(:user) { create(:user) }
+
+    subject { described_class.new(clusterable).can_admin_cluster? }
+
+    before do
+      clusterable.add_maintainer(user)
+
+      allow(clusterable).to receive(:current_user).and_return(user)
+    end
+
+    context 'when clusterable is a group' do
+      let(:clusterable) { create(:group) }
+
+      it_behaves_like 'appropriate member permissions'
+    end
+
+    context 'when clusterable is a project' do
+      let(:clusterable) { create(:project, :repository) }
+
+      it_behaves_like 'appropriate member permissions'
+    end
+  end
+
   describe '#environments_cluster_path' do
     subject { described_class.new(clusterable).environments_cluster_path(cluster) }
 

spec/requests/api/group_clusters_spec.rb

@@ -6,11 +6,11 @@ RSpec.describe API::GroupClusters do
   include KubernetesHelpers
 
   let(:current_user) { create(:user) }
-  let(:developer_user) { create(:user) }
+  let(:unauthorized_user) { create(:user) }
   let(:group) { create(:group, :private) }
 
   before do
-    group.add_developer(developer_user)
+    group.add_reporter(unauthorized_user)
     group.add_maintainer(current_user)
   end
 
@@ -24,7 +24,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        get api("/groups/#{group.id}/clusters", developer_user)
+        get api("/groups/#{group.id}/clusters", unauthorized_user)
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -68,7 +68,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        get api("/groups/#{group.id}/clusters/#{cluster_id}", developer_user)
+        get api("/groups/#{group.id}/clusters/#{cluster_id}", unauthorized_user)
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -183,7 +183,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params
+        post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -290,7 +290,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       before do
-        post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params
+        post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params
       end
 
       it 'responds with 403' do
@@ -364,7 +364,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        put api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: update_params
+        put api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: update_params
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -505,7 +505,7 @@ RSpec.describe API::GroupClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        delete api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: cluster_params
+        delete api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: cluster_params
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end

spec/requests/api/project_clusters_spec.rb

@@ -5,13 +5,15 @@ require 'spec_helper'
 RSpec.describe API::ProjectClusters do
   include KubernetesHelpers
 
-  let_it_be(:current_user) { create(:user) }
+  let_it_be(:maintainer_user) { create(:user) }
   let_it_be(:developer_user) { create(:user) }
+  let_it_be(:reporter_user) { create(:user) }
   let_it_be(:project) { create(:project) }
 
   before do
-    project.add_maintainer(current_user)
+    project.add_maintainer(maintainer_user)
     project.add_developer(developer_user)
+    project.add_reporter(reporter_user)
   end
 
   describe 'GET /projects/:id/clusters' do
@@ -24,7 +26,7 @@ RSpec.describe API::ProjectClusters do
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        get api("/projects/#{project.id}/clusters", developer_user)
+        get api("/projects/#{project.id}/clusters", reporter_user)
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -32,7 +34,7 @@ RSpec.describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        get api("/projects/#{project.id}/clusters", current_user)
+        get api("/projects/#{project.id}/clusters", developer_user)
       end
 
       it 'includes pagination headers' do
@@ -61,13 +63,13 @@ RSpec.describe API::ProjectClusters do
     let(:cluster) do
       create(:cluster, :project, :provided_by_gcp, :with_domain,
              platform_kubernetes: platform_kubernetes,
-             user: current_user,
+             user: maintainer_user,
              projects: [project])
     end
 
     context 'non-authorized user' do
       it 'responds with 403' do
-        get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user)
+        get api("/projects/#{project.id}/clusters/#{cluster_id}", reporter_user)
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -75,7 +77,7 @@ RSpec.describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        get api("/projects/#{project.id}/clusters/#{cluster_id}", current_user)
+        get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user)
       end
 
       it 'returns specific cluster' do
@@ -111,8 +113,8 @@ RSpec.describe API::ProjectClusters do
       it 'returns user information' do
         user = json_response['user']
 
-        expect(user['id']).to eq(current_user.id)
+        expect(user['id']).to eq(maintainer_user.id)
-        expect(user['username']).to eq(current_user.username)
+        expect(user['username']).to eq(maintainer_user.username)
       end
 
       it 'returns GCP provider information' do
@@ -156,7 +158,7 @@ RSpec.describe API::ProjectClusters do
     let(:management_project_id) { management_project.id }
 
     before do
-      management_project.add_maintainer(current_user)
+      management_project.add_maintainer(maintainer_user)
     end
 
     let(:platform_kubernetes_attributes) do
@@ -190,7 +192,7 @@ RSpec.describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params
+        post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params
       end
 
       context 'with valid params' do
@@ -317,7 +319,7 @@ RSpec.describe API::ProjectClusters do
         create(:cluster, :provided_by_gcp, :project,
                projects: [project])
 
-        post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params
+        post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params
       end
 
       it 'responds with 201' do
@@ -369,9 +371,9 @@ RSpec.describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        management_project.add_maintainer(current_user)
+        management_project.add_maintainer(maintainer_user)
 
-        put api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: update_params
+        put api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: update_params
 
         cluster.reload
       end
@@ -501,7 +503,7 @@ RSpec.describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        delete api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: cluster_params
+        delete api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: cluster_params
       end
 
       it 'deletes the cluster' do

spec/requests/projects/cluster_agents_controller_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Projects::ClusterAgentsController do
       let_it_be(:user) { create(:user) }
 
       before do
-        project.add_developer(user)
+        project.add_reporter(user)
         sign_in(user)
         subject
       end

spec/serializers/deployment_cluster_entity_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe DeploymentClusterEntity do
     subject { described_class.new(deployment, request: request).as_json }
 
     let(:maintainer) { create(:user) }
-    let(:developer) { create(:user) }
+    let(:reporter) { create(:user) }
     let(:current_user) { maintainer }
     let(:request) { double(:request, current_user: current_user) }
     let(:project) { create(:project) }
@@ -17,7 +17,7 @@ RSpec.describe DeploymentClusterEntity do
 
     before do
       project.add_maintainer(maintainer)
-      project.add_developer(developer)
+      project.add_reporter(reporter)
     end
 
     it 'matches deployment_cluster entity schema' do
@@ -31,7 +31,7 @@ RSpec.describe DeploymentClusterEntity do
     end
 
     context 'when the user does not have permission to view the cluster' do
-      let(:current_user) { developer }
+      let(:current_user) { reporter }
 
       it 'does not include the path nor the namespace' do
         expect(subject[:path]).to be_nil

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -47,6 +47,7 @@ RSpec.shared_context 'GroupPolicy context' do
         create_custom_emoji
         create_package
         create_package_settings
+        read_cluster
       ]
   end
 
@@ -54,7 +55,7 @@ RSpec.shared_context 'GroupPolicy context' do
     %i[
       destroy_package
       create_projects
-      read_cluster create_cluster update_cluster admin_cluster add_cluster
+      create_cluster update_cluster admin_cluster add_cluster
     ]
   end
 

spec/support/shared_examples/policies/clusterable_shared_examples.rb

@@ -6,12 +6,24 @@ RSpec.shared_examples 'clusterable policies' do
 
     subject { described_class.new(current_user, clusterable) }
 
+    context 'with a reporter' do
+      before do
+        clusterable.add_reporter(current_user)
+      end
+
+      it { expect_disallowed(:read_cluster) }
+      it { expect_disallowed(:add_cluster) }
+      it { expect_disallowed(:create_cluster) }
+      it { expect_disallowed(:update_cluster) }
+      it { expect_disallowed(:admin_cluster) }
+    end
+
     context 'with a developer' do
       before do
         clusterable.add_developer(current_user)
       end
 
-      it { expect_disallowed(:read_cluster) }
+      it { expect_allowed(:read_cluster) }
       it { expect_disallowed(:add_cluster) }
       it { expect_disallowed(:create_cluster) }
       it { expect_disallowed(:update_cluster) }