From f0fe61070f353deee5fccd0f4f07b1ac8e4f5856 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Mon, 11 Nov 2024 13:37:05 +0100
Subject: [PATCH] Protected containers: Remove field "Minimum access level for
delete"
- During rollout of the feature "Protected containers", we noticed
that the field "Minimum access level for delete" is visible, but
the aspect delete protection has not been implemented yet;
therefore, the existence / visibility of
the field "Minimum access level for delete" might lead to unnecessary
confusion.
- This MR intends to remove the field "Minimum access level for delete"
from the frontend as suggested here: https://gitlab.com/gitlab-org/gitlab/-/issues/480385#note_2193605295
Changelog: other
---
.../container_protection_rule_form.vue | 17 -----------
.../components/container_protection_rules.vue | 29 +------------------
...container_protection_rule.mutation.graphql | 1 -
...container_protection_rule.mutation.graphql | 1 -
..._registry_protection_rule.mutation.graphql | 1 -
...t_container_protection_rules.query.graphql | 1 -
.../container_protection_rules.md | 3 --
locale/gitlab.pot | 5 +---
.../container_protection_rule_form_spec.js | 28 +-----------------
.../container_protection_rules_spec.js | 23 ++-------------
.../settings/project/settings/mock_data.js | 3 --
11 files changed, 5 insertions(+), 107 deletions(-)
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rule_form.vue
index 5ec2508e2ac10b..af770e92d1814e 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rule_form.vue
@@ -12,7 +12,6 @@ import HelpPageLink from '~/vue_shared/components/help_page_link/help_page_link.
import createProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/create_container_protection_rule.mutation.graphql';
import { s__, __ } from '~/locale';
-const GRAPHQL_ACCESS_LEVEL_VALUE_NULL = null;
const GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER = 'MAINTAINER';
const GRAPHQL_ACCESS_LEVEL_VALUE_OWNER = 'OWNER';
const GRAPHQL_ACCESS_LEVEL_VALUE_ADMIN = 'ADMIN';
@@ -42,7 +41,6 @@ export default {
protectionRuleFormData: {
repositoryPathPattern: '',
minimumAccessLevelForPush: GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER,
- minimumAccessLevelForDelete: GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER,
},
updateInProgress: false,
alertErrorMessages: [],
@@ -66,12 +64,10 @@ export default {
projectPath: this.projectPath,
repositoryPathPattern: this.protectionRuleFormData.repositoryPathPattern,
minimumAccessLevelForPush: this.protectionRuleFormData.minimumAccessLevelForPush,
- minimumAccessLevelForDelete: this.protectionRuleFormData.minimumAccessLevelForDelete,
};
},
minimumAccessLevelOptions() {
return [
- { value: GRAPHQL_ACCESS_LEVEL_VALUE_NULL, text: __('Developer (default)') },
{ value: GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER, text: __('Maintainer') },
{ value: GRAPHQL_ACCESS_LEVEL_VALUE_OWNER, text: __('Owner') },
{ value: GRAPHQL_ACCESS_LEVEL_VALUE_ADMIN, text: __('Admin') },
@@ -170,19 +166,6 @@ export default {
/>
</gl-form-group>
- <gl-form-group
- :label="s__('ContainerRegistry|Minimum access level for delete')"
- label-for="input-minimum-access-level-for-delete"
- :disabled="isFieldDisabled"
- >
- <gl-form-select
- id="input-minimum-access-level-for-delete"
- v-model="protectionRuleFormData.minimumAccessLevelForDelete"
- :options="minimumAccessLevelOptions"
- :disabled="isFieldDisabled"
- />
- </gl-form-group>
-
<div class="gl-flex gl-justify-start">
<gl-button
variant="confirm"
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue
index b70029fcdbaebe..328cbb4e39b48a 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue
@@ -22,9 +22,6 @@ import { s__, __ } from '~/locale';
const PAGINATION_DEFAULT_PER_PAGE = 10;
const I18N_MINIMUM_ACCESS_LEVEL_FOR_PUSH = s__('ContainerRegistry|Minimum access level for push');
-const I18N_MINIMUM_ACCESS_LEVEL_FOR_DELETE = s__(
- 'ContainerRegistry|Minimum access level for delete',
-);
export default {
components: {
@@ -48,7 +45,7 @@ export default {
i18n: {
settingBlockTitle: s__('ContainerRegistry|Protected containers'),
settingBlockDescription: s__(
- 'ContainerRegistry|When a container is protected, only certain user roles can push and delete the protected container image, which helps to avoid tampering with the container image.',
+ 'ContainerRegistry|When a container is protected, only certain user roles can push the protected container image, which helps to avoid tampering with the container image.',
),
protectionRuleDeletionConfirmModal: {
title: s__('ContainerRegistry|Delete container protection rule?'),
@@ -60,7 +57,6 @@ export default {
),
},
minimumAccessLevelForPush: I18N_MINIMUM_ACCESS_LEVEL_FOR_PUSH,
- minimumAccessLevelForDelete: I18N_MINIMUM_ACCESS_LEVEL_FOR_DELETE,
},
apollo: {
protectionRulesQueryPayload: {
@@ -94,7 +90,6 @@ export default {
return this.protectionRulesQueryResult.map((protectionRule) => {
return {
id: protectionRule.id,
- minimumAccessLevelForDelete: protectionRule.minimumAccessLevelForDelete,
minimumAccessLevelForPush: protectionRule.minimumAccessLevelForPush,
repositoryPathPattern: protectionRule.repositoryPathPattern,
};
@@ -210,11 +205,6 @@ export default {
minimumAccessLevelForPush: protectionRule.minimumAccessLevelForPush,
});
},
- updateProtectionRuleMinimumAccessLevelForDelete(protectionRule) {
- this.updateProtectionRule(protectionRule, {
- minimumAccessLevelForDelete: protectionRule.minimumAccessLevelForDelete,
- });
- },
updateProtectionRule(protectionRule, updateData) {
this.clearAlertMessage();
@@ -259,11 +249,6 @@ export default {
label: I18N_MINIMUM_ACCESS_LEVEL_FOR_PUSH,
tdClass: '!gl-align-middle',
},
- {
- key: 'minimumAccessLevelForDelete',
- label: I18N_MINIMUM_ACCESS_LEVEL_FOR_DELETE,
- tdClass: '!gl-align-middle',
- },
{
key: 'rowActions',
label: __('Actions'),
@@ -326,18 +311,6 @@ export default {
/>
</template>
- <template #cell(minimumAccessLevelForDelete)="{ item }">
- <gl-form-select
- v-model="item.minimumAccessLevelForDelete"
- class="gl-max-w-34"
- required
- :aria-label="$options.i18n.minimumAccessLevelForDelete"
- :options="minimumAccessLevelOptions"
- :disabled="isProtectionRuleMinimumAccessLevelForPushFormSelectDisabled(item)"
- @change="updateProtectionRuleMinimumAccessLevelForDelete(item)"
- />
- </template>
-
<template #cell(rowActions)="{ item }">
<gl-button
v-gl-tooltip
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_container_protection_rule.mutation.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_container_protection_rule.mutation.graphql
index 224aee2040c75b..668258223903d6 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_container_protection_rule.mutation.graphql
+++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_container_protection_rule.mutation.graphql
@@ -4,7 +4,6 @@ mutation createContainerProtectionRule($input: CreateContainerRegistryProtection
id
repositoryPathPattern
minimumAccessLevelForPush
- minimumAccessLevelForDelete
}
errors
}
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql
index f92e126a5e4762..35c4290ba091e3 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql
+++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql
@@ -6,7 +6,6 @@ mutation deleteContainerRegistryProtectionRule(
id
repositoryPathPattern
minimumAccessLevelForPush
- minimumAccessLevelForDelete
}
errors
}
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/update_container_registry_protection_rule.mutation.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/update_container_registry_protection_rule.mutation.graphql
index 4aa907c83ddcbf..ae6dda75785347 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/update_container_registry_protection_rule.mutation.graphql
+++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/update_container_registry_protection_rule.mutation.graphql
@@ -6,7 +6,6 @@ mutation updateContainerRegistryProtectionRule(
id
repositoryPathPattern
minimumAccessLevelForPush
- minimumAccessLevelForDelete
}
errors
}
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql
index 371e08fbeff8d4..aa599b6769a343 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql
+++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql
@@ -14,7 +14,6 @@ query getProjectContainerProtectionRules(
id
repositoryPathPattern
minimumAccessLevelForPush
- minimumAccessLevelForDelete
}
pageInfo {
...PageInfo
diff --git a/doc/user/packages/container_registry/container_protection_rules.md b/doc/user/packages/container_registry/container_protection_rules.md
index 6bc3695acc7af3..6e9a96d37a01ea 100644
--- a/doc/user/packages/container_registry/container_protection_rules.md
+++ b/doc/user/packages/container_registry/container_protection_rules.md
@@ -31,7 +31,6 @@ When a container repository is protected, the default behavior enforces these re
| Protect a container repository and its container images | The Maintainer role. |
| Push or create a new image in a container repository | The role set in the [**Minimum access level for push**](#protect-a-container-repository-and-create-a-protection-rule) setting. |
| Push or update an existing image in a container repository | The role set in the [**Minimum access level for push**](#protect-a-container-repository-and-create-a-protection-rule) setting |
-| Delete an existing image from a container repository | The role set in the [**Minimum access level for delete**](#protect-a-container-repository-and-create-a-protection-rule) setting. |
You can use a wildcard (`*`) to protect multiple container repositories with the same container protection rule.
For example, you can protect different container repositories containing temporary container images built during a CI/CD pipeline.
@@ -65,8 +64,6 @@ To protect a container repository:
The pattern can include a wildcard (`*`).
- **Minimum access level for push** describes the minimum access level required
to push (create or update) to the protected container repository path.
- - **Minimum access level for delete** describes the minimum access level required
- to delete from the protected container repository path.
1. Select **Protect**.
The container protection rule is created, and appears in the settings.
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index efa19835aa78f4..011e7cc05000af 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -15248,9 +15248,6 @@ msgstr ""
msgid "ContainerRegistry|Manifest media type: %{mediaType}"
msgstr ""
-msgid "ContainerRegistry|Minimum access level for delete"
-msgstr ""
-
msgid "ContainerRegistry|Minimum access level for push"
msgstr ""
@@ -15442,7 +15439,7 @@ msgstr ""
msgid "ContainerRegistry|We are having trouble connecting to the Container Registry. Please try refreshing the page. If this error persists, please review %{docLinkStart}the troubleshooting documentation%{docLinkEnd}."
msgstr ""
-msgid "ContainerRegistry|When a container is protected, only certain user roles can push and delete the protected container image, which helps to avoid tampering with the container image."
+msgid "ContainerRegistry|When a container is protected, only certain user roles can push the protected container image, which helps to avoid tampering with the container image."
msgstr ""
msgid "ContainerRegistry|While the rename is in progress, new uploads to the container registry are blocked. Ongoing uploads may fail and need to be retried."
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rule_form_spec.js
index 1321549354d01d..1a5c5ce3809e9d 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rule_form_spec.js
@@ -27,8 +27,6 @@ describe('container Protection Rule Form', () => {
wrapper.findByRole('textbox', { name: /repository path pattern/i });
const findMinimumAccessLevelForPushSelect = () =>
wrapper.findByRole('combobox', { name: /minimum access level for push/i });
- const findMinimumAccessLevelForDeleteSelect = () =>
- wrapper.findByRole('combobox', { name: /minimum access level for delete/i });
const findSubmitButton = () => wrapper.findByRole('button', { name: /add rule/i });
const mountComponent = ({ config, provide = defaultProvidedValues } = {}) => {
@@ -58,7 +56,7 @@ describe('container Protection Rule Form', () => {
.findAll('option')
.wrappers.map((option) => option.element.value);
- it.each(['', 'MAINTAINER', 'OWNER', 'ADMIN'])(
+ it.each(['MAINTAINER', 'OWNER', 'ADMIN'])(
'includes the access level "%s" as an option',
(accessLevel) => {
mountComponent();
@@ -80,7 +78,6 @@ describe('container Protection Rule Form', () => {
expect(findSubmitButton().props('disabled')).toBe(true);
expect(findRepositoryPathPatternInput().attributes('disabled')).toBe('disabled');
expect(findMinimumAccessLevelForPushSelect().attributes('disabled')).toBe('disabled');
- expect(findMinimumAccessLevelForDeleteSelect().attributes('disabled')).toBe('disabled');
});
it('displays a loading spinner', () => {
@@ -165,29 +162,6 @@ describe('container Protection Rule Form', () => {
});
});
- it('dispatches correct apollo mutation when no minimumAccessLevelForPush is selected', async () => {
- const mutationResolver = jest
- .fn()
- .mockResolvedValue(createContainerProtectionRuleMutationPayload());
-
- mountComponentWithApollo({ mutationResolver });
-
- await findRepositoryPathPatternInput().setValue(
- createContainerProtectionRuleMutationInput.repositoryPathPattern,
- );
- await findMinimumAccessLevelForPushSelect().setValue('');
-
- await submitForm();
-
- expect(mutationResolver).toHaveBeenCalledWith({
- input: {
- projectPath: 'path',
- ...createContainerProtectionRuleMutationInput,
- minimumAccessLevelForPush: null,
- },
- });
- });
-
it('emits event "submit" when apollo mutation successful', async () => {
const mutationResolver = jest
.fn()
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js
index 39af02bbc1c1ef..662f77b8c479bf 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js
@@ -113,28 +113,10 @@ describe('Container protection rules project settings', () => {
(protectionRule, i) => {
expect(findTableRowCell(i, 0).text()).toBe(protectionRule.repositoryPathPattern);
expect(findTableRowCellComboboxSelectedOption(i, 1).text).toBe('Maintainer');
- expect(findTableRowCellComboboxSelectedOption(i, 2).text).toBe('Maintainer');
},
);
});
- it('renders table with container protection rule with blank minimumAccessLevelForDelete', async () => {
- const containerProtectionRuleQueryResolver = jest.fn().mockResolvedValue(
- containerProtectionRuleQueryPayload({
- nodes: [{ ...containerProtectionRulesData[0], minimumAccessLevelForDelete: null }],
- }),
- );
- createComponent({ containerProtectionRuleQueryResolver });
-
- await waitForPromises();
-
- expect(findTableRowCell(0, 0).text()).toBe(
- containerProtectionRulesData[0].repositoryPathPattern,
- );
- expect(findTableRowCellComboboxSelectedOption(0, 1).text).toBe('Maintainer');
- expect(findTableRowCellComboboxSelectedOption(0, 2).text).toBe('Developer (default)');
- });
-
it('displays table in busy state and shows loading icon inside table', async () => {
createComponent();
@@ -298,9 +280,8 @@ describe('Container protection rules project settings', () => {
});
describe.each`
- comboboxName | minimumAccessLevelAttribute
- ${'Minimum access level for push'} | ${'minimumAccessLevelForPush'}
- ${'Minimum access level for delete'} | ${'minimumAccessLevelForDelete'}
+ comboboxName | minimumAccessLevelAttribute
+ ${'Minimum access level for push'} | ${'minimumAccessLevelForPush'}
`(
'column "$comboboxName" with selectbox (combobox)',
({ comboboxName, minimumAccessLevelAttribute }) => {
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
index 913109d4028bce..a50bc8768aa6d1 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
@@ -171,13 +171,11 @@ export const containerProtectionRulesData = [
id: `gid://gitlab/ContainerRegistry::Protection::Rule/${i}`,
repositoryPathPattern: `@flight/flight/maintainer-${i}-*`,
minimumAccessLevelForPush: 'MAINTAINER',
- minimumAccessLevelForDelete: 'MAINTAINER',
})),
{
id: 'gid://gitlab/ContainerRegistry::Protection::Rule/16',
repositoryPathPattern: '@flight/flight/owner-16-*',
minimumAccessLevelForPush: 'OWNER',
- minimumAccessLevelForDelete: 'OWNER',
},
];
@@ -218,7 +216,6 @@ export const createContainerProtectionRuleMutationPayload = ({ override, errors
export const createContainerProtectionRuleMutationInput = {
repositoryPathPattern: `@flight/flight-maintainer-14-*`,
minimumAccessLevelForPush: 'MAINTAINER',
- minimumAccessLevelForDelete: 'MAINTAINER',
};
export const createContainerProtectionRuleMutationPayloadErrors = [
--
GitLab