From 298e3d263e6f2401968e908366d51d83f6d18d64 Mon Sep 17 00:00:00 2001 From: LP B Date: Thu, 7 Apr 2022 22:58:26 +0200 Subject: [PATCH] feat(registry): enforce name uniqueness for registries (#6709) * feat(app/registries): add name uniqueness validation on registry creation * feat(api/registry): enforce name uniqueness on registry creation * feat(api/registry): enforce name uniqueness on registry update * feat(app/registry): enforce name uniqueness on registry update --- .../handler/registries/registry_create.go | 3 + .../handler/registries/registry_update.go | 18 ++- app/portainer/__module.js | 3 +- .../registry-form-ecr.html | 26 ++-- .../registry-form-ecr.js | 8 ++ .../registry-form-azure.html | 21 +-- .../registry-form-azure.js | 8 ++ .../registry-form-custom.html | 21 +-- .../registry-form-custom.js | 8 ++ .../registry-form-dockerhub.html | 17 +-- .../registry-form-dockerhub.js | 8 ++ .../registry-form-proget.html | 25 ++-- .../registry-form-proget.js | 8 ++ .../registries/create/createRegistry.html | 5 + .../create/createRegistryController.js | 35 ++++- .../views/registries/edit/registry.html | 59 +++++---- .../views/registries/edit/registry.js | 10 ++ .../registries/edit/registryController.js | 125 +++++++++++------- 18 files changed, 265 insertions(+), 143 deletions(-) create mode 100644 app/portainer/views/registries/edit/registry.js diff --git a/api/http/handler/registries/registry_create.go b/api/http/handler/registries/registry_create.go index 724b33aff..bb736fb50 100644 --- a/api/http/handler/registries/registry_create.go +++ b/api/http/handler/registries/registry_create.go @@ -126,6 +126,9 @@ func (handler *Handler) registryCreate(w http.ResponseWriter, r *http.Request) * return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve registries from the database", err} } for _, r := range registries { + if r.Name == registry.Name { + return &httperror.HandlerError{http.StatusConflict, "Another registry with the same name already exists", errors.New("A registry is already defined with this name")} + } if handler.registriesHaveSameURLAndCredentials(&r, registry) { return &httperror.HandlerError{http.StatusConflict, "Another registry with the same URL and credentials already exists", errors.New("A registry is already defined for this URL and credentials")} } diff --git a/api/http/handler/registries/registry_update.go b/api/http/handler/registries/registry_update.go index cf0a6a51a..3daa84afe 100644 --- a/api/http/handler/registries/registry_update.go +++ b/api/http/handler/registries/registry_update.go @@ -77,6 +77,11 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a registry with the specified identifier inside the database", err} } + registries, err := handler.DataStore.Registry().Registries() + if err != nil { + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve registries from the database", err} + } + var payload registryUpdatePayload err = request.DecodeAndValidateJSONPayload(r, &payload) if err != nil { @@ -86,6 +91,15 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * if payload.Name != nil { registry.Name = *payload.Name } + // enforce name uniqueness across registries + // check is performed even if Name didn't change (Name not in payload) as we need + // to enforce this rule on updates not performed with frontend (e.g. on direct API requests) + // see https://portainer.atlassian.net/browse/EE-2706 for more details + for _, r := range registries { + if r.ID != registry.ID && r.Name == registry.Name { + return &httperror.HandlerError{http.StatusConflict, "Another registry with the same name already exists", errors.New("A registry is already defined with this name")} + } + } if registry.Type == portainer.ProGetRegistry && payload.BaseURL != nil { registry.BaseURL = *payload.BaseURL @@ -129,10 +143,6 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * shouldUpdateSecrets = shouldUpdateSecrets || (*payload.URL != registry.URL) registry.URL = *payload.URL - registries, err := handler.DataStore.Registry().Registries() - if err != nil { - return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve registries from the database", err} - } for _, r := range registries { if r.ID != registry.ID && handler.registriesHaveSameURLAndCredentials(&r, registry) { diff --git a/app/portainer/__module.js b/app/portainer/__module.js index 028137979..b2f861728 100644 --- a/app/portainer/__module.js +++ b/app/portainer/__module.js @@ -401,8 +401,7 @@ angular url: '/:id', views: { 'content@': { - templateUrl: './views/registries/edit/registry.html', - controller: 'RegistryController', + component: 'editRegistry', }, }, }; diff --git a/app/portainer/components/forms/registry-form-aws-ecr/registry-form-ecr.html b/app/portainer/components/forms/registry-form-aws-ecr/registry-form-ecr.html index 93b11c9c0..529ecfe0e 100644 --- a/app/portainer/components/forms/registry-form-aws-ecr/registry-form-ecr.html +++ b/app/portainer/components/forms/registry-form-aws-ecr/registry-form-ecr.html @@ -1,4 +1,4 @@ -
+
Important notice
@@ -16,10 +16,11 @@
-
+
-
+

This field is required.

+

A registry with the same name already exists.

@@ -43,9 +44,9 @@ />
-
+
-
+

This field is required.

@@ -72,9 +73,9 @@
-
+
-
+

This field is required.

@@ -88,9 +89,9 @@
-
+
-
+

This field is required.

@@ -104,10 +105,11 @@
-
+
-
+

This field is required.

+

A registry with the same name already exists.

@@ -121,7 +123,7 @@
-
+
-
+

This field is required.

+

A registry with the same name already exists.

@@ -25,9 +26,9 @@
-
+
-
+

This field is required.

@@ -40,9 +41,9 @@
-
+
-
+

This field is required.

@@ -55,9 +56,9 @@
-
+
-
+

This field is required.

@@ -70,7 +71,7 @@
-
+
-
+

This field is required.

+

A registry with the same name already exists.

@@ -32,9 +33,9 @@
-
+
-
+

This field is required.

@@ -59,9 +60,9 @@
-
+
-
+

This field is required.

@@ -74,9 +75,9 @@
-
+
-
+

This field is required.

@@ -90,7 +91,7 @@
-
+
-
+

This field is required.

+

A registry with the same name already exists.

@@ -31,9 +32,9 @@
-
+
-
+

This field is required.

@@ -46,9 +47,9 @@
-
+
-
+

This field is required.

@@ -62,7 +63,7 @@ Cancel
diff --git a/app/portainer/views/registries/edit/registry.js b/app/portainer/views/registries/edit/registry.js new file mode 100644 index 000000000..c09ea410d --- /dev/null +++ b/app/portainer/views/registries/edit/registry.js @@ -0,0 +1,10 @@ +import angular from 'angular'; +import controller from './registryController'; + +angular.module('portainer.app').component('editRegistry', { + templateUrl: './registry.html', + controller, + bindings: { + $transition$: '<', + }, +}); diff --git a/app/portainer/views/registries/edit/registryController.js b/app/portainer/views/registries/edit/registryController.js index c518c4c30..f0642e5b0 100644 --- a/app/portainer/views/registries/edit/registryController.js +++ b/app/portainer/views/registries/edit/registryController.js @@ -1,61 +1,84 @@ +import _ from 'lodash'; import { RegistryTypes } from '@/portainer/models/registryTypes'; -angular.module('portainer.app').controller('RegistryController', [ - '$scope', - '$state', - 'RegistryService', - 'Notifications', - function ($scope, $state, RegistryService, Notifications) { - $scope.state = { +export default class RegistryController { + /* @ngInject */ + constructor($async, $state, RegistryService, Notifications) { + Object.assign(this, { $async, $state, RegistryService, Notifications }); + + this.RegistryTypes = RegistryTypes; + + this.state = { actionInProgress: false, + loading: false, }; - $scope.formValues = { + this.formValues = { Password: '', }; + } - $scope.RegistryTypes = RegistryTypes; - - $scope.passwordLabel = () => { - const type = $scope.registry.Type; - switch (type) { - case RegistryTypes.ECR: - return 'AWS Secret Access Key'; - case RegistryTypes.DOCKERHUB: - return 'Access token'; - default: - return 'Password'; - } - }; - - $scope.updateRegistry = function () { - var registry = $scope.registry; - registry.Password = $scope.formValues.Password; - $scope.state.actionInProgress = true; - RegistryService.updateRegistry(registry) - .then(function success() { - Notifications.success('Registry successfully updated'); - $state.go('portainer.registries'); - }) - .catch(function error(err) { - Notifications.error('Failure', err, 'Unable to update registry'); - }) - .finally(function final() { - $scope.state.actionInProgress = false; - }); - }; - - function initView() { - var registryID = $state.params.id; - RegistryService.registry(registryID) - .then(function success(data) { - $scope.registry = data; - }) - .catch(function error(err) { - Notifications.error('Failure', err, 'Unable to retrieve registry details'); - }); + passwordLabel() { + const type = this.registry.Type; + switch (type) { + case RegistryTypes.ECR: + return 'AWS Secret Access Key'; + case RegistryTypes.DOCKERHUB: + return 'Access token'; + default: + return 'Password'; } + }; - initView(); - }, -]); + updateRegistry() { + return this.$async(async () => { + try { + this.state.actionInProgress = true; + const registry = this.registry; + registry.Password = this.formValues.Password; + registry.Name = this.formValues.Name; + + await this.RegistryService.updateRegistry(registry); + this.Notifications.success('Registry successfully updated'); + this.$state.go('portainer.registries'); + } catch (err) { + this.Notifications.error('Failure', err, 'Unable to update registry'); + } finally { + this.state.actionInProgress = false; + } + }); + } + + onChangeName() { + this.state.nameAlreadyExists = _.includes(this.registriesNames, this.formValues.Name); + } + + isUpdateButtonDisabled() { + return ( + this.state.actionInProgress || + this.state.nameAlreadyExists || + !this.registry.Name || + !this.registry.URL || + (this.registry.Type == this.RegistryTypes.QUAY && this.registry.Quay.UseOrganisation && !this.registry.Quay.OrganisationName) + ); + } + + async $onInit() { + try { + this.state.loading = true; + + const registryId = this.$state.params.id; + const registry = await this.RegistryService.registry(registryId); + this.registry = registry; + this.formValues.Name = registry.Name; + + const registries = await this.RegistryService.registries(); + _.pullAllBy(registries, [registry], 'Id'); + this.registriesNames = _.map(registries, 'Name'); + } catch (err) { + this.Notifications.error('Failure', err, 'Unable to retrieve registry details'); + } finally { + this.state.loading = false; + } + } +}