diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-28 00:14:06 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-28 00:14:06 +0000 |
commit | 3235221bc498ca3c80eeca505fb32bf9f237778a (patch) | |
tree | 30437bc7bd635e911cd6d35c33eb05cf68f3cfa9 | |
parent | ad928016f48a2f78168d1994c6012622af77f045 (diff) | |
download | gitlab-ce-3235221bc498ca3c80eeca505fb32bf9f237778a.tar.gz |
Add latest changes from gitlab-org/gitlab@master
57 files changed, 566 insertions, 196 deletions
diff --git a/app/assets/javascripts/clusters_list/components/agent_table.vue b/app/assets/javascripts/clusters_list/components/agent_table.vue index 695e16b7b4b..24310136953 100644 --- a/app/assets/javascripts/clusters_list/components/agent_table.vue +++ b/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" diff --git a/app/assets/javascripts/clusters_list/components/clusters_actions.vue b/app/assets/javascripts/clusters_list/components/clusters_actions.vue index 25f67462223..f6c94e2d330 100644 --- a/app/assets/javascripts/clusters_list/components/clusters_actions.vue +++ b/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 > diff --git a/app/assets/javascripts/clusters_list/components/clusters_view_all.vue b/app/assets/javascripts/clusters_list/components/clusters_view_all.vue index 0e312d21e4e..bd1a15dba8c 100644 --- a/app/assets/javascripts/clusters_list/components/clusters_view_all.vue +++ b/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> diff --git a/app/assets/javascripts/clusters_list/components/agent_options.vue b/app/assets/javascripts/clusters_list/components/delete_agent_button.vue index a364122ba56..6588d304d5c 100644 --- a/app/assets/javascripts/clusters_list/components/agent_options.vue +++ b/app/assets/javascripts/clusters_list/components/delete_agent_button.vue @@ -1,36 +1,23 @@ <script> import { - GlDropdown, - GlDropdownItem, + GlButton, GlModal, GlModalDirective, GlSprintf, GlFormGroup, GlFormInput, + GlTooltipDirective, } from '@gitlab/ui'; -import { s__, __, sprintf } from '~/locale'; -import { DELETE_AGENT_MODAL_ID } from '../constants'; +import { sprintf } from '~/locale'; +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: { - 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.'), - }, + i18n: DELETE_AGENT_BUTTON, components: { - GlDropdown, - GlDropdownItem, + GlButton, 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 - icon="ellipsis_v" - right - :disabled="loading" - :text="$options.i18n.dropdownText" - text-sr-only - category="tertiary" - no-caret + <div + v-gl-tooltip="deleteButtonTooltip" + class="gl-display-inline-block" + tabindex="-1" + data-testid="delete-agent-button-tooltip" > - <gl-dropdown-item v-gl-modal-directive="modalId"> - {{ $options.i18n.deleteButton }} - </gl-dropdown-item> - </gl-dropdown> + <gl-button + ref="deleteAgentButton" + v-gl-modal-directive="modalId" + icon="remove" + category="secondary" + variant="danger" + :disabled="deleteButtonDisabled" + :aria-label="$options.i18n.deleteButton" + /> + </div> <gl-modal ref="modal" diff --git a/app/assets/javascripts/clusters_list/constants.js b/app/assets/javascripts/clusters_list/constants.js index 310002d63c2..a012f930dc8 100644 --- a/app/assets/javascripts/clusters_list/constants.js +++ b/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'; diff --git a/app/assets/javascripts/clusters_list/load_main_view.js b/app/assets/javascripts/clusters_list/load_main_view.js index 08c99b46e16..1f2bd173c62 100644 --- a/app/assets/javascripts/clusters_list/load_main_view.js +++ b/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) { diff --git a/app/assets/stylesheets/highlight/themes/dark.scss b/app/assets/stylesheets/highlight/themes/dark.scss index 330f8d9be60..28878280d24 100644 --- a/app/assets/stylesheets/highlight/themes/dark.scss +++ b/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 */ diff --git a/app/assets/stylesheets/highlight/themes/monokai.scss b/app/assets/stylesheets/highlight/themes/monokai.scss index f406b150843..6faf1cffdef 100644 --- a/app/assets/stylesheets/highlight/themes/monokai.scss +++ b/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 */ diff --git a/app/assets/stylesheets/highlight/themes/none.scss b/app/assets/stylesheets/highlight/themes/none.scss index e0c9612f14a..9c28d9463dc 100644 --- a/app/assets/stylesheets/highlight/themes/none.scss +++ b/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 */ diff --git a/app/assets/stylesheets/highlight/themes/solarized-dark.scss b/app/assets/stylesheets/highlight/themes/solarized-dark.scss index 8cf3472890d..c9f889c79fc 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-dark.scss +++ b/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; diff --git a/app/assets/stylesheets/highlight/themes/solarized-light.scss b/app/assets/stylesheets/highlight/themes/solarized-light.scss index 54bde7c10ed..0108d7e496f 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-light.scss +++ b/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; diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 1c54077c4ac..91d8f4a1ba5 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/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; diff --git a/app/assets/stylesheets/mailers/highlighted_diff_email.scss b/app/assets/stylesheets/mailers/highlighted_diff_email.scss index 75c2428c1d4..fd212d14e30 100644 --- a/app/assets/stylesheets/mailers/highlighted_diff_email.scss +++ b/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; diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index b1ffdf00b87..f88d381b3bf 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/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! diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 15a261f572a..c12ceca9c3b 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/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 diff --git a/app/controllers/projects/cluster_agents_controller.rb b/app/controllers/projects/cluster_agents_controller.rb index 404d3907128..84bb01ee266 100644 --- a/app/controllers/projects/cluster_agents_controller.rb +++ b/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 diff --git a/app/graphql/resolvers/clusters/agent_tokens_resolver.rb b/app/graphql/resolvers/clusters/agent_tokens_resolver.rb index 8208fa56485..722fbab3bb7 100644 --- a/app/graphql/resolvers/clusters/agent_tokens_resolver.rb +++ b/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 diff --git a/app/graphql/resolvers/kas/agent_configurations_resolver.rb b/app/graphql/resolvers/kas/agent_configurations_resolver.rb index a1b1d3bfe4c..9db104287a6 100644 --- a/app/graphql/resolvers/kas/agent_configurations_resolver.rb +++ b/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 diff --git a/app/graphql/types/clusters/agent_activity_event_type.rb b/app/graphql/types/clusters/agent_activity_event_type.rb index 79a9fd70505..3484acfe25e 100644 --- a/app/graphql/types/clusters/agent_activity_event_type.rb +++ b/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) diff --git a/app/graphql/types/clusters/agent_token_type.rb b/app/graphql/types/clusters/agent_token_type.rb index 96fdb5f05c8..24489707698 100644 --- a/app/graphql/types/clusters/agent_token_type.rb +++ b/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) diff --git a/app/graphql/types/clusters/agent_type.rb b/app/graphql/types/clusters/agent_type.rb index 89316ed4728..546252b2285 100644 --- a/app/graphql/types/clusters/agent_type.rb +++ b/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) diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb index 93b6b4e8fe2..bc6a8bf25e6 100644 --- a/app/helpers/clusters_helper.rb +++ b/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 diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index aae338e9759..92a88d2f7c8 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/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_memoization(name) - end + clear_memoized_values end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index cf36e72a565..08bf4bcad1a 100644 --- a/app/models/merge_request.rb +++ b/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) diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index fee47fe0ae9..62111c55e83 100644 --- a/app/policies/group_policy.rb +++ b/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 diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 55f43cd9f7b..0a3f0c6d424 100644 --- a/app/policies/project_policy.rb +++ b/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 diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index 4b645510b51..cc466e0ff81 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/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 diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index cc6997e1a20..103dfb9a400 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/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 diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index de2f9af82c7..dfbc26593f2 100644 --- a/doc/user/application_security/sast/index.md +++ b/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 -compilation during your SAST execution and instead pass all job artifacts from an -earlier stage in the pipeline. This is the current strategy when requiring -a `before_script` execution to prepare your scan job. +Most GitLab SAST analyzers directly scan your source code without compiling it first. +However, for technical reasons, some analyzers can only scan compiled code. -To pass your project's dependencies as artifacts, the dependencies must be included -in the project's working directory and specified using the `artifacts:path` configuration. -If all dependencies are present, the `COMPILE=false` CI/CD variable can be provided to the -analyzer and compilation is skipped: +By default, these analyzers automatically attempt to fetch dependencies and compile your code so it can be scanned. +Automatic compilation can fail if: + +- 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 diff --git a/doc/user/permissions.md b/doc/user/permissions.md index f2f63aa5857..8ceedcec724 100644 --- a/doc/user/permissions.md +++ b/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 | | | ✓ | ✓ | ✓ | diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3b4c43d1cc6..d081abaf1e7 100644 --- a/locale/gitlab.pot +++ b/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 "" diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 93c560b4753..bc4eaf029e9 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/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) } diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 2a8feb09780..cf6bd70f2d2 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/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) } diff --git a/spec/features/monitor_sidebar_link_spec.rb b/spec/features/monitor_sidebar_link_spec.rb index bb5e581a034..fcef0fa0eff 100644 --- a/spec/features/monitor_sidebar_link_spec.rb +++ b/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).not_to have_link('Serverless', href: project_serverless_functions_path(project)) - expect(page).not_to have_link('Kubernetes', href: project_clusters_path(project)) + expect(page).to have_link('Serverless', href: project_serverless_functions_path(project)) + expect(page).to have_link('Kubernetes', href: project_clusters_path(project)) end it_behaves_like 'shows Monitor menu based on the access level' diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 7ccd5c51493..a65d2d15c12 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/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' diff --git a/spec/finders/clusters/agents_finder_spec.rb b/spec/finders/clusters/agents_finder_spec.rb index 0996d76b723..4ec798daa99 100644 --- a/spec/finders/clusters/agents_finder_spec.rb +++ b/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 diff --git a/spec/frontend/clusters_list/components/agent_table_spec.js b/spec/frontend/clusters_list/components/agent_table_spec.js index 887c17bb4ad..27de9b18f4e 100644 --- a/spec/frontend/clusters_list/components/agent_table_spec.js +++ b/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); }); }); }); diff --git a/spec/frontend/clusters_list/components/clusters_actions_spec.js b/spec/frontend/clusters_list/components/clusters_actions_spec.js index cb8303ca4b2..331690fc642 100644 --- a/spec/frontend/clusters_list/components/clusters_actions_spec.js +++ b/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); + }); + }); }); diff --git a/spec/frontend/clusters_list/components/clusters_view_all_spec.js b/spec/frontend/clusters_list/components/clusters_view_all_spec.js index 81084443889..2c1e3d909cc 100644 --- a/spec/frontend/clusters_list/components/clusters_view_all_spec.js +++ b/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', () => { diff --git a/spec/frontend/clusters_list/components/agent_options_spec.js b/spec/frontend/clusters_list/components/delete_agent_button_spec.js index 1d973ab39cc..82850b9dea4 100644 --- a/spec/frontend/clusters_list/components/agent_options_spec.js +++ b/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 { MAX_LIST_COUNT } from '~/clusters_list/constants'; +import DeleteAgentButton from '~/clusters_list/components/delete_agent_button.vue'; +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(GlDropdownItem); + const findDeleteBtn = () => wrapper.findComponent(GlButton); 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 () => { diff --git a/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb b/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb index 9b54d466681..866f4ce7b5a 100644 --- a/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb +++ b/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 diff --git a/spec/graphql/resolvers/clusters/agents_resolver_spec.rb b/spec/graphql/resolvers/clusters/agents_resolver_spec.rb index 70f40748e1d..152d7fa22c4 100644 --- a/spec/graphql/resolvers/clusters/agents_resolver_spec.rb +++ b/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(:developer) { create(:user, developer_projects: [project]) } + let_it_be(:maintainer) { 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 diff --git a/spec/graphql/types/clusters/agent_activity_event_type_spec.rb b/spec/graphql/types/clusters/agent_activity_event_type_spec.rb index 7773bad749d..cae75485846 100644 --- a/spec/graphql/types/clusters/agent_activity_event_type_spec.rb +++ b/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 diff --git a/spec/graphql/types/clusters/agent_token_type_spec.rb b/spec/graphql/types/clusters/agent_token_type_spec.rb index 3f0720cb4b5..1ca6d690c80 100644 --- a/spec/graphql/types/clusters/agent_token_type_spec.rb +++ b/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 diff --git a/spec/graphql/types/clusters/agent_type_spec.rb b/spec/graphql/types/clusters/agent_type_spec.rb index a1e5952bf73..3f4faccf15d 100644 --- a/spec/graphql/types/clusters/agent_type_spec.rb +++ b/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 diff --git a/spec/helpers/clusters_helper_spec.rb b/spec/helpers/clusters_helper_spec.rb index 51f111917d1..06be3c58e55 100644 --- a/spec/helpers/clusters_helper_spec.rb +++ b/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 diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index fc154738f11..7e08f47fb5a 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0b3fb80f97f..70dd1cbe240 100644 --- a/spec/models/merge_request_spec.rb +++ b/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 } - let(:commit) { subject.project.commit(sample_commit.id) } - let(:old_diff_refs) { subject.diff_refs } + subject { create(:merge_request, source_project: project) } - before do - # Update merge_request_diff so that #diff_refs will return commit.diff_refs - allow(subject).to receive(:create_merge_request_diff) do - subject.merge_request_diffs.create!( - base_commit_sha: commit.parent_id, - start_commit_sha: commit.parent_id, - head_commit_sha: commit.sha - ) + let(:project) { create(:project, :repository) } + let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") } + let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") } + let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } + let(:discussion) { create(:diff_note_on_merge_request, noteable: subject, project: project, position: old_position).to_discussion } + let(:path) { "files/ruby/popen.rb" } + let(:new_line) { 9 } + + let(:old_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: create_commit.parent_id, + head_sha: modify_commit.sha + ) + end - subject.reload_merge_request_diff - 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 - context 'when resolve_outdated_diff_discussions is set' do - let(:project) { create(:project, :repository) } + it 'does not call the resolve method' do + expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new) - subject { create(:merge_request, source_project: project) } + 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 before do discussion subject.project.update!(resolve_outdated_diff_discussions: true) end - it 'calls MergeRequests::ResolvedDiscussionNotificationService' do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService) - .to receive(:execute).with(subject) + context 'when the active discussion is resolved in the update' do + it 'calls MergeRequests::ResolvedDiscussionNotificationService' do + expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService) + .to receive(:execute).with(subject) + + 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 does not have resolved in the update' do + let(:new_line) { 16 } - subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, - new_diff_refs: commit.diff_refs, - current_user: subject.author) + 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 diff --git a/spec/policies/clusters/agent_token_policy_spec.rb b/spec/policies/clusters/agent_token_policy_spec.rb index 9ae99e66f59..f5ac8bd67e6 100644 --- a/spec/policies/clusters/agent_token_policy_spec.rb +++ b/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 diff --git a/spec/policies/clusters/agents/activity_event_policy_spec.rb b/spec/policies/clusters/agents/activity_event_policy_spec.rb index 1262fcfd9f2..365168de79f 100644 --- a/spec/policies/clusters/agents/activity_event_policy_spec.rb +++ b/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 diff --git a/spec/presenters/clusterable_presenter_spec.rb b/spec/presenters/clusterable_presenter_spec.rb index d19abd4e4d8..7c2e19728d5 100644 --- a/spec/presenters/clusterable_presenter_spec.rb +++ b/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) } diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb index f65f9384efa..c48b5007f91 100644 --- a/spec/requests/api/group_clusters_spec.rb +++ b/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 diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index 253b61e5865..b83b41a881a 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/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['username']).to eq(current_user.username) + expect(user['id']).to eq(maintainer_user.id) + 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 diff --git a/spec/requests/projects/cluster_agents_controller_spec.rb b/spec/requests/projects/cluster_agents_controller_spec.rb index e4c4f537699..914d5b17ba8 100644 --- a/spec/requests/projects/cluster_agents_controller_spec.rb +++ b/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 diff --git a/spec/serializers/deployment_cluster_entity_spec.rb b/spec/serializers/deployment_cluster_entity_spec.rb index 95f2f8ce6fc..419ae746b74 100644 --- a/spec/serializers/deployment_cluster_entity_spec.rb +++ b/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 diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 0dfd76de79c..76db2bd82f1 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/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 diff --git a/spec/support/shared_examples/policies/clusterable_shared_examples.rb b/spec/support/shared_examples/policies/clusterable_shared_examples.rb index b96aa71acbe..faf283f9059 100644 --- a/spec/support/shared_examples/policies/clusterable_shared_examples.rb +++ b/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) } |