From 42a2d94bac523ec8cbd000609c49a38b84a72b46 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Thu, 11 Apr 2024 15:40:12 +0000
Subject: [PATCH] Protected packages: Show protected label in packages list
MR includes:
- Feature flag switch to enable / disable the protected label
- Add badge to package in package list
- Extend graphql api to include package protection rules
in graphql type Types::Packages::PackageBaseType
- Add association in model Packages::Package
for matching package protection rules
- This commit is only a proof of concept
in order to see if this is going in the right direction
Changelog: added
---
.../components/list/package_list_row.vue | 25 ++++++
.../fragments/package_data.fragment.graphql | 1 +
.../projects/packages/packages_controller.rb | 10 +++
.../types/packages/package_base_type.rb | 12 +++
app/models/packages/package.rb | 1 +
doc/api/graphql/reference/index.md | 3 +
locale/gitlab.pot | 3 +
.../graphql/packages/package_details.json | 3 +
.../components/list/package_list_row_spec.js | 59 ++++++++++++-
.../package_registry/mock_data.js | 2 +
spec/models/packages/package_spec.rb | 21 +++++
.../api/graphql/project/packages_spec.rb | 84 +++++++++++++++++++
.../packages/packages_controller_spec.rb | 33 ++++++++
13 files changed, 256 insertions(+), 1 deletion(-)
create mode 100644 spec/requests/projects/packages/packages_controller_spec.rb
diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_list_row.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_list_row.vue
index 126eb9c099173..892e8ac79f5ea 100644
--- a/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_list_row.vue
+++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_list_row.vue
@@ -7,6 +7,8 @@ import {
GlSprintf,
GlTruncate,
GlLink,
+ GlBadge,
+ GlTooltipDirective,
} from '@gitlab/ui';
import { s__, __ } from '~/locale';
import ListItem from '~/vue_shared/components/registry/list_item.vue';
@@ -23,6 +25,7 @@ import PackageTags from '~/packages_and_registries/shared/components/package_tag
import PublishMethod from '~/packages_and_registries/package_registry/components/list/publish_method.vue';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default {
name: 'PackageListRow',
@@ -34,11 +37,16 @@ export default {
GlSprintf,
GlTruncate,
GlLink,
+ GlBadge,
PackageTags,
PublishMethod,
ListItem,
TimeagoTooltip,
},
+ directives: {
+ GlTooltip: GlTooltipDirective,
+ },
+ mixins: [glFeatureFlagsMixin()],
inject: ['isGroupPage', 'canDeletePackages'],
props: {
packageEntity: {
@@ -98,6 +106,12 @@ export default {
showTags() {
return Boolean(this.packageEntity.tags?.nodes?.length);
},
+ showBadgeProtected() {
+ return (
+ Boolean(this.glFeatures.packagesProtectedPackages) &&
+ Boolean(this.packageEntity.packageProtectionRuleExists)
+ );
+ },
nonDefaultRow() {
return this.packageEntity.status && this.packageEntity.status !== PACKAGE_DEFAULT_STATUS;
},
@@ -114,6 +128,7 @@ export default {
errorPublishing: ERROR_PUBLISHING,
warning: WARNING_TEXT,
moreActions: __('More actions'),
+ badgeProtectedTooltipText: s__('PackageRegistry|A protection rule exists for this package.'),
},
};
</script>
@@ -148,6 +163,16 @@ export default {
hide-label
:tag-display-limit="1"
/>
+
+ <gl-badge
+ v-if="showBadgeProtected"
+ v-gl-tooltip="{ title: $options.i18n.badgeProtectedTooltipText }"
+ class="gl-ml-3"
+ icon-size="sm"
+ size="sm"
+ variant="neutral"
+ >{{ __('protected') }}</gl-badge
+ >
</div>
</template>
<template #left-secondary>
diff --git a/app/assets/javascripts/packages_and_registries/package_registry/graphql/fragments/package_data.fragment.graphql b/app/assets/javascripts/packages_and_registries/package_registry/graphql/fragments/package_data.fragment.graphql
index fe0dbecfafcb1..bad9dc77523c9 100644
--- a/app/assets/javascripts/packages_and_registries/package_registry/graphql/fragments/package_data.fragment.graphql
+++ b/app/assets/javascripts/packages_and_registries/package_registry/graphql/fragments/package_data.fragment.graphql
@@ -30,6 +30,7 @@ fragment PackageData on Package {
fullPath
webUrl
}
+ packageProtectionRuleExists
_links {
webPath
}
diff --git a/app/controllers/projects/packages/packages_controller.rb b/app/controllers/projects/packages/packages_controller.rb
index f045bae5c9694..96fea1340d957 100644
--- a/app/controllers/projects/packages/packages_controller.rb
+++ b/app/controllers/projects/packages/packages_controller.rb
@@ -8,10 +8,20 @@ class PackagesController < Projects::ApplicationController
feature_category :package_registry
urgency :low
+ before_action :set_feature_flag_packages_protected_packages, only: :index
+
+ def index; end
+
# The show action renders index to allow frontend routing to work on page refresh
def show
render :index
end
+
+ private
+
+ def set_feature_flag_packages_protected_packages
+ push_frontend_feature_flag(:packages_protected_packages, project)
+ end
end
end
end
diff --git a/app/graphql/types/packages/package_base_type.rb b/app/graphql/types/packages/package_base_type.rb
index 5102e4ebcd594..ef2cdf983ec5b 100644
--- a/app/graphql/types/packages/package_base_type.rb
+++ b/app/graphql/types/packages/package_base_type.rb
@@ -28,6 +28,12 @@ class PackageBaseType < ::Types::BaseObject
null: true,
description: 'Package metadata.'
field :name, GraphQL::Types::String, null: false, description: 'Name of the package.'
+ field :package_protection_rule_exists, GraphQL::Types::Boolean,
+ null: false,
+ alpha: { milestone: '16.11' },
+ description:
+ 'Whether any matching package protection rule exists for this package. ' \
+ 'Available only when feature flag `packages_protected_packages` is enabled.'
field :package_type, Types::Packages::PackageTypeEnum, null: false, description: 'Package type.'
field :project, Types::ProjectType, null: false, description: 'Project where the package is stored.'
field :status, Types::Packages::PackageStatusEnum, null: false, description: 'Package status.'
@@ -44,6 +50,12 @@ def can_destroy
Ability.allowed?(current_user, :destroy_package, object)
end
+ def package_protection_rule_exists
+ return false if Feature.disabled?(:packages_protected_packages, object.project)
+
+ object.matching_package_protection_rules.exists?
+ end
+
# NOTE: This method must be kept in sync with the union
# type: `Types::Packages::MetadataType`.
#
diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb
index 6be77509f8a1e..0558e1b338d60 100644
--- a/app/models/packages/package.rb
+++ b/app/models/packages/package.rb
@@ -58,6 +58,7 @@ class Packages::Package < ApplicationRecord
has_many :pipelines, through: :build_infos, disable_joins: true
has_one :debian_publication, inverse_of: :package, class_name: 'Packages::Debian::Publication'
has_one :debian_distribution, through: :debian_publication, source: :distribution, inverse_of: :packages, class_name: 'Packages::Debian::ProjectDistribution'
+ has_many :matching_package_protection_rules, -> (package) { where(package_type: package.package_type).for_package_name(package.name) }, through: :project, source: :package_protection_rules
accepts_nested_attributes_for :conan_metadatum
accepts_nested_attributes_for :debian_publication
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 54f0655e7e853..e4b2f56ac72ce 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -24777,6 +24777,7 @@ Represents a package with pipelines in the Package Registry.
| <a id="packageid"></a>`id` | [`PackagesPackageID!`](#packagespackageid) | ID of the package. |
| <a id="packagemetadata"></a>`metadata` | [`PackageMetadata`](#packagemetadata) | Package metadata. |
| <a id="packagename"></a>`name` | [`String!`](#string) | Name of the package. |
+| <a id="packagepackageprotectionruleexists"></a>`packageProtectionRuleExists` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 16.11. **Status**: Experiment. Whether any matching package protection rule exists for this package. Available only when feature flag `packages_protected_packages` is enabled. |
| <a id="packagepackagetype"></a>`packageType` | [`PackageTypeEnum!`](#packagetypeenum) | Package type. |
| <a id="packagepipelines"></a>`pipelines` | [`PipelineConnection`](#pipelineconnection) | Pipelines that built the package. Max page size 20. (see [Connections](#connections)) |
| <a id="packageproject"></a>`project` | [`Project!`](#project) | Project where the package is stored. |
@@ -24801,6 +24802,7 @@ Represents a package in the Package Registry.
| <a id="packagebaseid"></a>`id` | [`PackagesPackageID!`](#packagespackageid) | ID of the package. |
| <a id="packagebasemetadata"></a>`metadata` | [`PackageMetadata`](#packagemetadata) | Package metadata. |
| <a id="packagebasename"></a>`name` | [`String!`](#string) | Name of the package. |
+| <a id="packagebasepackageprotectionruleexists"></a>`packageProtectionRuleExists` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 16.11. **Status**: Experiment. Whether any matching package protection rule exists for this package. Available only when feature flag `packages_protected_packages` is enabled. |
| <a id="packagebasepackagetype"></a>`packageType` | [`PackageTypeEnum!`](#packagetypeenum) | Package type. |
| <a id="packagebaseproject"></a>`project` | [`Project!`](#project) | Project where the package is stored. |
| <a id="packagebasestatus"></a>`status` | [`PackageStatus!`](#packagestatus) | Package status. |
@@ -24871,6 +24873,7 @@ Represents a package details in the Package Registry.
| <a id="packagedetailstypenpmurl"></a>`npmUrl` | [`String`](#string) | Url of the NPM project endpoint. |
| <a id="packagedetailstypenugeturl"></a>`nugetUrl` | [`String`](#string) | Url of the Nuget project endpoint. |
| <a id="packagedetailstypepackagefiles"></a>`packageFiles` | [`PackageFileConnection`](#packagefileconnection) | Package files. (see [Connections](#connections)) |
+| <a id="packagedetailstypepackageprotectionruleexists"></a>`packageProtectionRuleExists` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 16.11. **Status**: Experiment. Whether any matching package protection rule exists for this package. Available only when feature flag `packages_protected_packages` is enabled. |
| <a id="packagedetailstypepackagetype"></a>`packageType` | [`PackageTypeEnum!`](#packagetypeenum) | Package type. |
| <a id="packagedetailstypepipelines"></a>`pipelines` | [`PipelineConnection`](#pipelineconnection) | Pipelines that built the package. Max page size 20. (see [Connections](#connections)) |
| <a id="packagedetailstypeproject"></a>`project` | [`Project!`](#project) | Project where the package is stored. |
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a85f9159dec31..cf83f409a6946 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -35887,6 +35887,9 @@ msgstr ""
msgid "PackageRegistry|%{name} version %{version} was first created %{datetime}"
msgstr ""
+msgid "PackageRegistry|A protection rule exists for this package."
+msgstr ""
+
msgid "PackageRegistry|Add Conan Remote"
msgstr ""
diff --git a/spec/fixtures/api/schemas/graphql/packages/package_details.json b/spec/fixtures/api/schemas/graphql/packages/package_details.json
index 1acc3f6ad7d79..d01d75fcd46dd 100644
--- a/spec/fixtures/api/schemas/graphql/packages/package_details.json
+++ b/spec/fixtures/api/schemas/graphql/packages/package_details.json
@@ -262,6 +262,9 @@
"null"
]
},
+ "packageProtectionRuleExists": {
+ "type": "boolean"
+ },
"_links": {
"type": "object",
"additionalProperties": false,
diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js
index 499b5cb6fe1cc..7c742326a6b81 100644
--- a/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js
+++ b/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js
@@ -1,8 +1,9 @@
-import { GlFormCheckbox, GlSprintf, GlTruncate } from '@gitlab/ui';
+import { GlFormCheckbox, GlSprintf, GlTruncate, GlBadge } from '@gitlab/ui';
import Vue from 'vue';
import VueRouter from 'vue-router';
import { RouterLinkStub } from '@vue/test-utils';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import PackagesListRow from '~/packages_and_registries/package_registry/components/list/package_list_row.vue';
import PackageTags from '~/packages_and_registries/shared/components/package_tags.vue';
@@ -59,11 +60,15 @@ describe('packages_list_row', () => {
GlSprintf,
TimeagoTooltip,
RouterLink: RouterLinkStub,
+ GlBadge,
},
propsData: {
packageEntity,
selected,
},
+ directives: {
+ GlTooltip: createMockDirective('gl-tooltip'),
+ },
});
};
@@ -316,4 +321,56 @@ describe('packages_list_row', () => {
);
});
});
+
+ describe('badge "protected"', () => {
+ const mountComponentForBadgeProtected = ({
+ packageEntityPackageProtectionRuleExists = true,
+ glFeaturesPackagesProtectedPackages = true,
+ } = {}) =>
+ mountComponent({
+ packageEntity: {
+ ...packageWithoutTags,
+ packageProtectionRuleExists: packageEntityPackageProtectionRuleExists,
+ },
+ provide: {
+ ...defaultProvide,
+ glFeatures: { packagesProtectedPackages: glFeaturesPackagesProtectedPackages },
+ },
+ });
+
+ const findBadgeProtected = () => wrapper.findComponent(GlBadge);
+
+ describe('when package is protected', () => {
+ it('shows badge', () => {
+ mountComponentForBadgeProtected();
+
+ expect(findBadgeProtected().text()).toBe('protected');
+ });
+
+ it('binds tooltip directive', () => {
+ mountComponentForBadgeProtected();
+
+ const badgeProtectedTooltipBinding = getBinding(findBadgeProtected().element, 'gl-tooltip');
+ expect(badgeProtectedTooltipBinding.value).toMatchObject({
+ title: 'A protection rule exists for this package.',
+ });
+ });
+ });
+
+ describe('when package is not protected', () => {
+ it('does not show badge', () => {
+ mountComponentForBadgeProtected({ packageEntityPackageProtectionRuleExists: false });
+
+ expect(findBadgeProtected().exists()).toBe(false);
+ });
+ });
+
+ describe('when feature flag ":packages_protected_packages" disabled', () => {
+ it('does not show badge', () => {
+ mountComponentForBadgeProtected({ glFeaturesPackagesProtectedPackages: false });
+
+ expect(findBadgeProtected().exists()).toBe(false);
+ });
+ });
+ });
});
diff --git a/spec/frontend/packages_and_registries/package_registry/mock_data.js b/spec/frontend/packages_and_registries/package_registry/mock_data.js
index 4b3506d1452d4..9dc0770a26f42 100644
--- a/spec/frontend/packages_and_registries/package_registry/mock_data.js
+++ b/spec/frontend/packages_and_registries/package_registry/mock_data.js
@@ -431,6 +431,7 @@ export const packagesListQuery = ({
nodes: [
{
...packageData(),
+ packageProtectionRuleExists: false,
...linksData,
project: packageProject(),
tags: { nodes: packageTags() },
@@ -440,6 +441,7 @@ export const packagesListQuery = ({
},
{
...packageData(),
+ packageProtectionRuleExists: false,
project: packageProject(),
tags: { nodes: [] },
pipelines: { nodes: [] },
diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb
index 345ff2099db9c..e3569c7caa95c 100644
--- a/spec/models/packages/package_spec.rb
+++ b/spec/models/packages/package_spec.rb
@@ -26,6 +26,7 @@
it { is_expected.to have_one(:rpm_metadatum).inverse_of(:package) }
it { is_expected.to have_one(:terraform_module_metadatum).inverse_of(:package) }
it { is_expected.to have_many(:nuget_symbols).inverse_of(:package) }
+ it { is_expected.to have_many(:matching_package_protection_rules).through(:project).source(:package_protection_rules) }
end
describe '.with_debian_codename' do
@@ -1202,6 +1203,26 @@
end
end
+ describe '#matching_package_protection_rules' do
+ let_it_be(:package) do
+ create(:npm_package, name: 'npm-package')
+ end
+
+ let_it_be(:package_protection_rule) do
+ create(:package_protection_rule, project: package.project, package_name_pattern: package.name, package_type: :npm,
+ push_protected_up_to_access_level: :maintainer)
+ end
+
+ let_it_be(:package_protection_rule_no_match) do
+ create(:package_protection_rule, project: package.project, package_name_pattern: "other-#{package.name}", package_type: :npm,
+ push_protected_up_to_access_level: :maintainer)
+ end
+
+ subject { package.matching_package_protection_rules }
+
+ it { is_expected.to eq [package_protection_rule] }
+ end
+
describe '#tag_names' do
let_it_be(:package) { create(:nuget_package) }
diff --git a/spec/requests/api/graphql/project/packages_spec.rb b/spec/requests/api/graphql/project/packages_spec.rb
index 3413b80e8b4da..ad94d285182a8 100644
--- a/spec/requests/api/graphql/project/packages_spec.rb
+++ b/spec/requests/api/graphql/project/packages_spec.rb
@@ -13,4 +13,88 @@
let(:resource_type) { :project }
it_behaves_like 'group and project packages query'
+
+ describe 'packageProtectionRuleExists' do
+ let_it_be(:maven_package) { create(:maven_package, project: project1, name: 'maven-package') }
+ let_it_be(:npm_package) { create(:npm_package, project: project1, name: 'npm-package') }
+ let_it_be(:npm_package_no_match) { create(:npm_package, project: project1, name: 'other-npm-package') }
+
+ let_it_be(:npm_package_protection_rule) do
+ create(:package_protection_rule, project: resource, package_name_pattern: npm_package.name, package_type: :npm,
+ push_protected_up_to_access_level: :maintainer)
+ end
+
+ let(:query) do
+ graphql_query_for(
+ resource_type,
+ { 'fullPath' => resource.full_path },
+ query_graphql_field('packages', {}, fields)
+ )
+ end
+
+ let(:fields) do
+ <<~QUERY
+ nodes {
+ name
+ packageType
+ packageProtectionRuleExists
+ }
+ QUERY
+ end
+
+ context 'when package protection rule for package and user exists' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:current_user_access_level, :expected_package_protection_rule_exists) do
+ :reporter | true
+ :developer | true
+ :maintainer | true
+ :owner | true
+ end
+
+ with_them do
+ before do
+ resource.send("add_#{current_user_access_level}", current_user)
+
+ post_graphql(query, current_user: current_user)
+ end
+
+ it_behaves_like 'a working graphql query that returns data'
+
+ it 'returns package protection rules' do
+ expect(graphql_data_at(resource_type, :packages, :nodes)).to include(
+ hash_including(
+ 'name' => npm_package.name,
+ 'packageType' => npm_package.package_type.upcase,
+ 'packageProtectionRuleExists' =>
+ expected_package_protection_rule_exists
+ ),
+ hash_including(
+ 'name' => maven_package.name,
+ 'packageType' => maven_package.package_type.upcase,
+ 'packageProtectionRuleExists' => false
+ )
+ )
+ end
+ end
+ end
+
+ context "when feature flag ':packages_protected_packages' disabled" do
+ before_all do
+ resource.add_maintainer(current_user)
+ end
+
+ before do
+ stub_feature_flags(packages_protected_packages: false)
+
+ post_graphql(query, current_user: current_user)
+ end
+
+ it 'returns no package protection rules' do
+ graphql_data_at(resource_type, :packages, :nodes).each do |package|
+ expect(package['packageProtectionRuleExists']).to eq false
+ end
+ end
+ end
+ end
end
diff --git a/spec/requests/projects/packages/packages_controller_spec.rb b/spec/requests/projects/packages/packages_controller_spec.rb
new file mode 100644
index 0000000000000..5a3dcbfe72cc9
--- /dev/null
+++ b/spec/requests/projects/packages/packages_controller_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::Packages::PackagesController, feature_category: :package_registry do
+ let_it_be(:project) { create(:project, :public) }
+
+ describe 'GET #index' do
+ let(:get_namespace_project_packages_path) do
+ get namespace_project_packages_path(namespace_id: project.namespace, project_id: project)
+ end
+
+ subject { response.body }
+
+ context 'when feature flag "packages_protected_packages" is enabled' do
+ before do
+ get_namespace_project_packages_path
+ end
+
+ it { is_expected.to have_pushed_frontend_feature_flags(packagesProtectedPackages: true) }
+ end
+
+ context 'when feature flag "packages_protected_packages" is disabled' do
+ before do
+ stub_feature_flags(packages_protected_packages: false)
+
+ get_namespace_project_packages_path
+ end
+
+ it { is_expected.to have_pushed_frontend_feature_flags(packagesProtectedPackages: false) }
+ end
+ end
+end
--
GitLab