From be95579bf607fcc42d7afe09b48f1a92815cc36d Mon Sep 17 00:00:00 2001 From: Gerardo Date: Mon, 5 Jun 2023 15:41:58 +0200 Subject: [PATCH 1/5] feat: Protected packages: Project settings ui package protection rules This MR includes: - Adds frontend ui for the package protection rule in project settings - Hide project setting for package protection rules when relevant feature flag is disabled Changelog: added --- .../components/packages_protection_rules.vue | 131 ++++++++++++++++++ .../components/registry_settings_app.vue | 13 ++ .../settings/project/constants.js | 15 ++ ...et_packages_protection_rules.query.graphql | 13 ++ .../packages_and_registries_controller.rb | 1 + locale/gitlab.pot | 15 ++ 6 files changed, 188 insertions(+) create mode 100644 app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue create mode 100644 app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue new file mode 100644 index 00000000000000..24ff753e1b5570 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -0,0 +1,131 @@ + + + diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue index 06af69ff250e1b..6120868b60e85e 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue @@ -8,6 +8,8 @@ import { } from '~/packages_and_registries/settings/project/constants'; import ContainerExpirationPolicy from '~/packages_and_registries/settings/project/components/container_expiration_policy.vue'; import PackagesCleanupPolicy from '~/packages_and_registries/settings/project/components/packages_cleanup_policy.vue'; +import PackagesProtectionRules from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { components: { @@ -18,7 +20,9 @@ export default { ), GlAlert, PackagesCleanupPolicy, + PackagesProtectionRules, }, + mixins: [glFeatureFlagsMixin()], inject: [ 'showContainerRegistrySettings', 'showPackageRegistrySettings', @@ -32,6 +36,12 @@ export default { showAlert: false, }; }, + // computed: { + // showSettingsForPackageProtectionRules() { + // console.log(this.glFeatures.packagesProtectedackages) + // return this.glFeatures.packagesProtectedackages + // } + // }, mounted() { this.checkAlert(); }, @@ -60,6 +70,9 @@ export default { > {{ $options.i18n.UPDATE_SETTINGS_SUCCESS_MESSAGE }} + diff --git a/app/assets/javascripts/packages_and_registries/settings/project/constants.js b/app/assets/javascripts/packages_and_registries/settings/project/constants.js index 05616a0a4f6ec3..396133ee0346c3 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/constants.js +++ b/app/assets/javascripts/packages_and_registries/settings/project/constants.js @@ -68,6 +68,21 @@ export const PACKAGES_CLEANUP_POLICY_TITLE = s__( export const PACKAGES_CLEANUP_POLICY_DESCRIPTION = s__( 'PackageRegistry|When a package with same name and version is uploaded to the registry, more assets are added to the package. To save storage space, keep only the most recent assets.', ); + +export const PACKAGES_PROTECTION_RULES_TITLE = s__('PackageRegistry| Protected packages'); +export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PACKAGE_NAME_PATTERN = s__( + 'PackageRegistry| Protection| Package Name Pattern', +); +export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PACKAGE_TYPE = s__( + 'PackageRegistry| Protection| Package Type', +); +export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PUSH_PROTECTED_UP_TO_ACCESS_LEVEL = s__( + 'PackageRegistry|Push protected up to access level', +); +export const PACKAGES_PROTECTION_RULES_DESCRIPTION = s__( + 'PackageRegistry| When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package.', +); + export const KEEP_N_DUPLICATED_PACKAGE_FILES_LABEL = s__( 'PackageRegistry|Number of duplicate assets to keep', ); diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql new file mode 100644 index 00000000000000..5b2eae1737d5de --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql @@ -0,0 +1,13 @@ +query getProjectPackageProtectionRules($projectPath: ID!) { + project(fullPath: $projectPath) { + id + packagesProtectionRules { + nodes { + id + packageNamePattern + packageType + pushProtectedUpToAccessLevel + } + } + } +} diff --git a/app/controllers/projects/settings/packages_and_registries_controller.rb b/app/controllers/projects/settings/packages_and_registries_controller.rb index 76c9cead360db1..fd4dbdab95ff0d 100644 --- a/app/controllers/projects/settings/packages_and_registries_controller.rb +++ b/app/controllers/projects/settings/packages_and_registries_controller.rb @@ -12,6 +12,7 @@ class PackagesAndRegistriesController < Projects::ApplicationController urgency :low def show + push_frontend_feature_flag(:packages_protected_packages, project) end def cleanup_tags diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ee4d1e9bcd7dc2..c4900a471a5971 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -34229,6 +34229,18 @@ msgstr "" msgid "Package type must be RubyGems" msgstr "" +msgid "PackageRegistry| Protected packages" +msgstr "" + +msgid "PackageRegistry| Protection| Package Name Pattern" +msgstr "" + +msgid "PackageRegistry| Protection| Package Type" +msgstr "" + +msgid "PackageRegistry| When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package." +msgstr "" + msgid "PackageRegistry|%{name} version %{version} was first created %{datetime}" msgstr "" @@ -34579,6 +34591,9 @@ msgstr "" msgid "PackageRegistry|Published to the %{project} Package Registry %{datetime}" msgstr "" +msgid "PackageRegistry|Push protected up to access level" +msgstr "" + msgid "PackageRegistry|PyPI" msgstr "" -- GitLab From b487147f4ec949436d0207008133ba019bea6d9f Mon Sep 17 00:00:00 2001 From: Rahul Chanila Date: Fri, 22 Dec 2023 15:29:40 +0000 Subject: [PATCH 2/5] refactor: Apply suggestion from @rchanila This commit includes: - Moving external string inside the component and from the `contants.js` - Added pagination to table only showing 10 protection rules per page - Added tests for new components --- .../components/packages_protection_rules.vue | 81 ++++++++-------- .../components/registry_settings_app.vue | 10 +- .../settings/project/constants.js | 14 --- locale/gitlab.pot | 24 ++--- .../packages_protection_rules_spec.js | 94 +++++++++++++++++++ .../components/registry_settings_app_spec.js | 24 +++++ .../settings/project/settings/mock_data.js | 27 ++++++ 7 files changed, 197 insertions(+), 77 deletions(-) create mode 100644 spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue index 24ff753e1b5570..aef077d6bd9d82 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -1,60 +1,38 @@ diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue index 6120868b60e85e..e0689f04922693 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue @@ -8,7 +8,6 @@ import { } from '~/packages_and_registries/settings/project/constants'; import ContainerExpirationPolicy from '~/packages_and_registries/settings/project/components/container_expiration_policy.vue'; import PackagesCleanupPolicy from '~/packages_and_registries/settings/project/components/packages_cleanup_policy.vue'; -import PackagesProtectionRules from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { @@ -20,7 +19,8 @@ export default { ), GlAlert, PackagesCleanupPolicy, - PackagesProtectionRules, + PackagesProtectionRules: () => + import('~/packages_and_registries/settings/project/components/packages_protection_rules.vue'), }, mixins: [glFeatureFlagsMixin()], inject: [ @@ -36,12 +36,6 @@ export default { showAlert: false, }; }, - // computed: { - // showSettingsForPackageProtectionRules() { - // console.log(this.glFeatures.packagesProtectedackages) - // return this.glFeatures.packagesProtectedackages - // } - // }, mounted() { this.checkAlert(); }, diff --git a/app/assets/javascripts/packages_and_registries/settings/project/constants.js b/app/assets/javascripts/packages_and_registries/settings/project/constants.js index 396133ee0346c3..8b545ac1ab6084 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/constants.js +++ b/app/assets/javascripts/packages_and_registries/settings/project/constants.js @@ -69,20 +69,6 @@ export const PACKAGES_CLEANUP_POLICY_DESCRIPTION = s__( 'PackageRegistry|When a package with same name and version is uploaded to the registry, more assets are added to the package. To save storage space, keep only the most recent assets.', ); -export const PACKAGES_PROTECTION_RULES_TITLE = s__('PackageRegistry| Protected packages'); -export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PACKAGE_NAME_PATTERN = s__( - 'PackageRegistry| Protection| Package Name Pattern', -); -export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PACKAGE_TYPE = s__( - 'PackageRegistry| Protection| Package Type', -); -export const PACKAGES_PROTECTION_RULES_TABLE_COLUMN_PUSH_PROTECTED_UP_TO_ACCESS_LEVEL = s__( - 'PackageRegistry|Push protected up to access level', -); -export const PACKAGES_PROTECTION_RULES_DESCRIPTION = s__( - 'PackageRegistry| When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package.', -); - export const KEEP_N_DUPLICATED_PACKAGE_FILES_LABEL = s__( 'PackageRegistry|Number of duplicate assets to keep', ); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c4900a471a5971..27c3537f710e6a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -34229,18 +34229,6 @@ msgstr "" msgid "Package type must be RubyGems" msgstr "" -msgid "PackageRegistry| Protected packages" -msgstr "" - -msgid "PackageRegistry| Protection| Package Name Pattern" -msgstr "" - -msgid "PackageRegistry| Protection| Package Type" -msgstr "" - -msgid "PackageRegistry| When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package." -msgstr "" - msgid "PackageRegistry|%{name} version %{version} was first created %{datetime}" msgstr "" @@ -34549,6 +34537,12 @@ msgid_plural "PackageRegistry|Package has %{updatesCount} archived updates" msgstr[0] "" msgstr[1] "" +msgid "PackageRegistry|Package name pattern" +msgstr "" + +msgid "PackageRegistry|Package type" +msgstr "" + msgid "PackageRegistry|Package updated by commit %{link} on branch %{branch}, built by pipeline %{pipeline}, and published to the registry %{datetime}" msgstr "" @@ -34573,6 +34567,9 @@ msgstr "" msgid "PackageRegistry|Project-level" msgstr "" +msgid "PackageRegistry|Protected packages" +msgstr "" + msgid "PackageRegistry|Publish packages if their name or version matches this regex." msgstr "" @@ -34699,6 +34696,9 @@ msgstr "" msgid "PackageRegistry|Unable to load package" msgstr "" +msgid "PackageRegistry|When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package." +msgstr "" + msgid "PackageRegistry|When a package with same name and version is uploaded to the registry, more assets are added to the package. To save storage space, keep only the most recent assets." msgstr "" diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js new file mode 100644 index 00000000000000..9c0762846f9f31 --- /dev/null +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js @@ -0,0 +1,94 @@ +import { GlSprintf, GlTable, GlPagination } from '@gitlab/ui'; +import { shallowMount, mount } from '@vue/test-utils'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import { forEach } from 'lodash'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import component from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue'; +import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue'; +import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql'; + +import { packagesProtectionRuleQueryPayload, packagesProtectionRulesData } from '../mock_data'; + +Vue.use(VueApollo); + +describe('Packages protection rules project settings', () => { + let wrapper; + let fakeApollo; + + const defaultProvidedValues = { + projectPath: 'path', + }; + const findSettingsBlock = () => wrapper.findComponent(SettingsBlock); + const findTable = () => wrapper.findComponent(GlTable); + const findTableRows = () => findTable().find('tbody').findAll('tr'); + const findPagination = () => wrapper.findComponent(GlPagination); + + const mountComponent = (mountFn = shallowMount, provide = defaultProvidedValues, config) => { + wrapper = mountFn(component, { + stubs: { + GlSprintf, + SettingsBlock, + }, + provide, + ...config, + }); + }; + + const mountComponentWithApollo = ({ + mountFn = shallowMount, + provide = defaultProvidedValues, + resolver, + } = {}) => { + const requestHandlers = [[packagesProtectionRuleQuery, resolver]]; + + fakeApollo = createMockApollo(requestHandlers); + + mountComponent(mountFn, provide, { + apolloProvider: fakeApollo, + }); + }; + + it('renders the setting block with table', async () => { + mountComponentWithApollo({ + resolver: jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload()), + }); + + await waitForPromises(); + + expect(findSettingsBlock().exists()).toBe(true); + expect(findTable().exists()).toBe(true); + }); + + it('renders table with container registry protection rules', async () => { + mountComponentWithApollo({ + mountFn: mount, + resolver: jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload()), + }); + + await waitForPromises(); + + expect(findTable().exists()).toBe(true); + + // The table is paginated and it shows only 10 rows by default + expect(findTableRows().length).toBe(10); + + forEach(packagesProtectionRulesData.slice(0, 10), (protectionRule, i) => { + expect(findTableRows().at(i).text()).toContain(protectionRule.packageNamePattern); + expect(findTableRows().at(i).text()).toContain(protectionRule.packageType); + expect(findTableRows().at(i).text()).toContain(protectionRule.pushProtectedUpToAccessLevel); + }); + }); + + it('renders table with pagination', async () => { + mountComponentWithApollo({ + resolver: jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload()), + }); + + await waitForPromises(); + + expect(findTable().exists()).toBe(true); + expect(findPagination().exists()).toBe(true); + }); +}); diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/registry_settings_app_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/registry_settings_app_spec.js index dfcabd14489f1c..7b46662ce47387 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/registry_settings_app_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/registry_settings_app_spec.js @@ -6,6 +6,7 @@ import * as commonUtils from '~/lib/utils/common_utils'; import component from '~/packages_and_registries/settings/project/components/registry_settings_app.vue'; import ContainerExpirationPolicy from '~/packages_and_registries/settings/project/components/container_expiration_policy.vue'; import PackagesCleanupPolicy from '~/packages_and_registries/settings/project/components/packages_cleanup_policy.vue'; +import PackagesProtectionRules from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue'; import DependencyProxyPackagesSettings from 'ee_component/packages_and_registries/settings/project/components/dependency_proxy_packages_settings.vue'; import { SHOW_SETUP_SUCCESS_ALERT, @@ -19,6 +20,7 @@ describe('Registry Settings app', () => { const findContainerExpirationPolicy = () => wrapper.findComponent(ContainerExpirationPolicy); const findPackagesCleanupPolicy = () => wrapper.findComponent(PackagesCleanupPolicy); + const findPackagesProtectionRules = () => wrapper.findComponent(PackagesProtectionRules); const findDependencyProxyPackagesSettings = () => wrapper.findComponent(DependencyProxyPackagesSettings); const findAlert = () => wrapper.findComponent(GlAlert); @@ -29,6 +31,7 @@ describe('Registry Settings app', () => { showPackageRegistrySettings: true, showDependencyProxySettings: false, ...(IS_EE && { showDependencyProxySettings: true }), + glFeatures: { packagesProtectedPackages: true }, }; const mountComponent = (provide = defaultProvide) => { @@ -95,6 +98,7 @@ describe('Registry Settings app', () => { expect(findContainerExpirationPolicy().exists()).toBe(showContainerRegistrySettings); expect(findPackagesCleanupPolicy().exists()).toBe(showPackageRegistrySettings); + expect(findPackagesProtectionRules().exists()).toBe(showPackageRegistrySettings); }, ); @@ -108,5 +112,25 @@ describe('Registry Settings app', () => { expect(findDependencyProxyPackagesSettings().exists()).toBe(value); }); } + + describe('when feature flag "packagesProtectedPackages" is disabled', () => { + it.each` + showContainerRegistrySettings | showPackageRegistrySettings + ${true} | ${false} + ${true} | ${true} + `( + 'container cleanup policy $showContainerRegistrySettings and package cleanup policy is $showPackageRegistrySettings', + ({ showContainerRegistrySettings, showPackageRegistrySettings }) => { + mountComponent({ + ...defaultProvide, + showContainerRegistrySettings, + showPackageRegistrySettings, + glFeatures: { packagesProtectedPackages: false }, + }); + + expect(findPackagesProtectionRules().exists()).toBe(false); + }, + ); + }); }); }); 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 3204ca01f99d24..e16b4a3ae62ce7 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 @@ -79,3 +79,30 @@ export const packagesCleanupPolicyMutationPayload = ({ override, errors = [] } = }, }, }); + +export const packagesProtectionRulesData = [ + ...Array.from(Array(15)).map((_, i) => ({ + id: `gid://gitlab/Packages::Protection::Rule/${i}`, + packageNamePattern: `@flight/flight-maintainer-${i}-*`, + packageType: 'NPM', + pushProtectedUpToAccessLevel: 'MAINTAINER', + })), + { + id: 'gid://gitlab/Packages::Protection::Rule/16', + packageNamePattern: '@flight/flight-owner-16-*', + packageType: 'NPM', + pushProtectedUpToAccessLevel: 'OWNER', + }, +]; + +export const packagesProtectionRuleQueryPayload = ({ override, errors = [] } = {}) => ({ + data: { + project: { + id: '1', + packagesProtectionRules: { + nodes: override || packagesProtectionRulesData, + }, + errors, + }, + }, +}); -- GitLab From 0e1e88c04ea900adf2a7d5923a2b209c4e3fcdb8 Mon Sep 17 00:00:00 2001 From: Rahul Chanila Date: Thu, 28 Dec 2023 17:03:56 +0000 Subject: [PATCH 3/5] refactor: Apply suggestion from @rchanila This commit includes: - Removing / extracting the pagination logic into a follow-up MR - Limiting requested package protection rules to 10 entries in the GraphQL request --- .../components/packages_protection_rules.vue | 16 ++-------------- .../project/components/registry_settings_app.vue | 9 ++++++--- .../get_packages_protection_rules.query.graphql | 4 ++-- .../components/packages_protection_rules_spec.js | 13 ++++--------- .../components/registry_settings_app_spec.js | 11 +++-------- .../settings/project/settings/mock_data.js | 14 ++++++++++---- 6 files changed, 27 insertions(+), 40 deletions(-) diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue index aef077d6bd9d82..0aa93baa205505 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -1,22 +1,18 @@