Merge branch 'release/2.9'

* add missing changes to make updateDockerhubToDB32 idempotent

* fix(migration) make dockerhub registry migration idempotent EE-2042

* add tests for bad migrations

api/bolt/migrate_data.go

@@ -18,13 +18,16 @@ const beforePortainerVersionUpgradeBackup = "portainer.db.bak"
 var migrateLog = plog.NewScopedLog("bolt, migrate")
 
 // FailSafeMigrate backup and restore DB if migration fail
-func (store *Store) FailSafeMigrate(migrator *migrator.Migrator) error {
+func (store *Store) FailSafeMigrate(migrator *migrator.Migrator) (err error) {
 	defer func() {
-		if err := recover(); err != nil {
-			migrateLog.Info(fmt.Sprintf("Error during migration, recovering [%v]", err))
+		if e := recover(); e != nil {
 			store.Rollback(true)
+			err = fmt.Errorf("%v", e)
 		}
 	}()
+
+	// !Important: we must use a named return value in the function definition and not a local
+	// !variable referenced from the closure or else the return value will be incorrectly set
 	return migrator.Migrate()
 }
 

api/bolt/migrator/migrate_ce.go

@@ -301,7 +301,7 @@ func (m *Migrator) Migrate() error {
 		}
 	}
 
-	// Portainer 2.9.1
+	// Portainer 2.9.1, 2.9.2
 	if m.currentDBVersion < 33 {
 		err := m.migrateDBVersionToDB33()
 		if err != nil {
@@ -316,6 +316,13 @@ func (m *Migrator) Migrate() error {
 		}
 	}
 
+	// Portainer 2.9.3 (yep out of order, but 2.10 is EE only)
+	if m.currentDBVersion < 35 {
+		if err := m.migrateDBVersionToDB35(); err != nil {
+			return migrationError(err, "migrateDBVersionToDB35")
+		}
+	}
+
 	err = m.versionService.StoreDBVersion(portainer.DBVersion)
 	if err != nil {
 		return migrationError(err, "StoreDBVersion")

api/bolt/migrator/migrate_dbversion31.go

@@ -100,6 +100,32 @@ func (m *Migrator) updateDockerhubToDB32() error {
 		RegistryAccesses: portainer.RegistryAccesses{},
 	}
 
+	// The following code will make this function idempotent.
+	// i.e. if run again, it will not change the data.  It will ensure that
+	// we only have one migrated registry entry. Duplicates will be removed
+	// if they exist and which has been happening due to earlier migration bugs
+	migrated := false
+	registries, _ := m.registryService.Registries()
+	for _, r := range registries {
+		if r.Type == registry.Type &&
+			r.Name == registry.Name &&
+			r.URL == registry.URL &&
+			r.Authentication == registry.Authentication {
+
+			if !migrated {
+				// keep this one entry
+				migrated = true
+			} else {
+				// delete subsequent duplicates
+				m.registryService.DeleteRegistry(portainer.RegistryID(r.ID))
+			}
+		}
+	}
+
+	if migrated {
+		return nil
+	}
+
 	endpoints, err := m.endpointService.Endpoints()
 	if err != nil {
 		return err
@@ -218,8 +244,12 @@ func findResourcesToUpdateForDB32(dockerID string, volumesData map[string]interf
 		if !nameExist {
 			continue
 		}
+		createTime, createTimeExist := volume["CreatedAt"].(string)
+		if !createTimeExist {
+			continue
+		}
 
-		oldResourceID := fmt.Sprintf("%s%s", volumeName, volume["CreatedAt"].(string))
+		oldResourceID := fmt.Sprintf("%s%s", volumeName, createTime)
 		resourceControl, ok := volumeResourceControls[oldResourceID]
 
 		if ok {

api/bolt/migrator/migrate_dbversion34.go

@@ -0,0 +1,11 @@
+package migrator
+
+func (m *Migrator) migrateDBVersionToDB35() error {
+	// These should have been migrated already, but due to an earlier bug and a bunch of duplicates,
+	// calling it again will now fix the issue as the function has been repaired.
+	err := m.updateDockerhubToDB32()
+	if err != nil {
+		return err
+	}
+	return nil
+}

api/bolt/migrator/migrate_dbversion34_test.go

@@ -0,0 +1,108 @@
+package migrator
+
+import (
+	"os"
+	"path"
+	"testing"
+	"time"
+
+	"github.com/boltdb/bolt"
+	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/bolt/dockerhub"
+	"github.com/portainer/portainer/api/bolt/endpoint"
+	"github.com/portainer/portainer/api/bolt/internal"
+	"github.com/portainer/portainer/api/bolt/registry"
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	db35TestFile = "portainer-mig-35.db"
+	username     = "portainer"
+	password     = "password"
+)
+
+func setupDB35Test(t *testing.T) *Migrator {
+	is := assert.New(t)
+	dbConn, err := bolt.Open(path.Join(t.TempDir(), db35TestFile), 0600, &bolt.Options{Timeout: 1 * time.Second})
+	is.NoError(err, "failed to init testing DB connection")
+
+	// Create an old style dockerhub authenticated account
+	dockerhubService, err := dockerhub.NewService(&internal.DbConnection{DB: dbConn})
+	is.NoError(err, "failed to init testing registry service")
+	err = dockerhubService.UpdateDockerHub(&portainer.DockerHub{true, username, password})
+	is.NoError(err, "failed to create dockerhub account")
+
+	registryService, err := registry.NewService(&internal.DbConnection{DB: dbConn})
+	is.NoError(err, "failed to init testing registry service")
+
+	endpointService, err := endpoint.NewService(&internal.DbConnection{DB: dbConn})
+	is.NoError(err, "failed to init endpoint service")
+
+	m := &Migrator{
+		db:               dbConn,
+		dockerhubService: dockerhubService,
+		registryService:  registryService,
+		endpointService:  endpointService,
+	}
+
+	return m
+}
+
+// TestUpdateDockerhubToDB32 tests a normal upgrade
+func TestUpdateDockerhubToDB32(t *testing.T) {
+	is := assert.New(t)
+	m := setupDB35Test(t)
+	defer m.db.Close()
+	defer os.Remove(db35TestFile)
+
+	if err := m.updateDockerhubToDB32(); err != nil {
+		t.Errorf("failed to update settings: %v", err)
+	}
+
+	// Verify we have a single registry were created
+	registries, err := m.registryService.Registries()
+	is.NoError(err, "failed to read registries from the RegistryService")
+	is.Equal(len(registries), 1, "only one migrated registry expected")
+}
+
+// TestUpdateDockerhubToDB32_with_duplicate_migrations tests an upgrade where in earlier versions a broken migration
+// created a large number of duplicate "dockerhub migrated" registry entries.
+func TestUpdateDockerhubToDB32_with_duplicate_migrations(t *testing.T) {
+	is := assert.New(t)
+	m := setupDB35Test(t)
+	defer m.db.Close()
+	defer os.Remove(db35TestFile)
+
+	// Create lots of duplicate entries...
+	registry := &portainer.Registry{
+		Type:             portainer.DockerHubRegistry,
+		Name:             "Dockerhub (authenticated - migrated)",
+		URL:              "docker.io",
+		Authentication:   true,
+		Username:         "portainer",
+		Password:         "password",
+		RegistryAccesses: portainer.RegistryAccesses{},
+	}
+
+	for i := 1; i < 150; i++ {
+		err := m.registryService.CreateRegistry(registry)
+		assert.NoError(t, err, "create registry failed")
+	}
+
+	// Verify they were created
+	registries, err := m.registryService.Registries()
+	is.NoError(err, "failed to read registries from the RegistryService")
+	is.Condition(func() bool {
+		return len(registries) > 1
+	}, "expected multiple duplicate registry entries")
+
+	// Now run the migrator
+	if err := m.updateDockerhubToDB32(); err != nil {
+		t.Errorf("failed to update settings: %v", err)
+	}
+
+	// Verify we have a single registry were created
+	registries, err = m.registryService.Registries()
+	is.NoError(err, "failed to read registries from the RegistryService")
+	is.Equal(len(registries), 1, "only one migrated registry expected")
+}

api/bolt/stack/stack.go

@@ -192,8 +192,8 @@ func (service *Service) RefreshableStacks() ([]portainer.Stack, error) {
 		bucket := tx.Bucket([]byte(BucketName))
 		cursor := bucket.Cursor()
 
-		var stack portainer.Stack
 		for k, v := cursor.First(); k != nil; k, v = cursor.Next() {
+			stack := portainer.Stack{}
 			err := internal.UnmarshalObject(v, &stack)
 			if err != nil {
 				return err

api/http/handler/handler.go

@@ -74,7 +74,7 @@ type Handler struct {
 }
 
 // @title PortainerCE API
-// @version 2.9.1
+// @version 2.9.3
 // @description.markdown api-description.md
 // @termsOfService
 

api/http/handler/settings/settings_update.go

@@ -54,11 +54,8 @@ func (payload *settingsUpdatePayload) Validate(r *http.Request) error {
 	if payload.TemplatesURL != nil && *payload.TemplatesURL != "" && !govalidator.IsURL(*payload.TemplatesURL) {
 		return errors.New("Invalid external templates URL. Must correspond to a valid URL format")
 	}
-	if payload.HelmRepositoryURL != nil && *payload.HelmRepositoryURL != "" {
-		err := libhelm.ValidateHelmRepositoryURL(*payload.HelmRepositoryURL)
-		if err != nil {
-			return errors.Wrap(err, "Invalid Helm repository URL. Must correspond to a valid URL format")
-		}
+	if payload.HelmRepositoryURL != nil && *payload.HelmRepositoryURL != "" && !govalidator.IsURL(*payload.HelmRepositoryURL) {
+		return errors.New("Invalid Helm repository URL. Must correspond to a valid URL format")
 	}
 	if payload.UserSessionTimeout != nil {
 		_, err := time.ParseDuration(*payload.UserSessionTimeout)
@@ -114,7 +111,16 @@ func (handler *Handler) settingsUpdate(w http.ResponseWriter, r *http.Request) *
 	}
 
 	if payload.HelmRepositoryURL != nil {
-		settings.HelmRepositoryURL = strings.TrimSuffix(strings.ToLower(*payload.HelmRepositoryURL), "/")
+		newHelmRepo := strings.TrimSuffix(strings.ToLower(*payload.HelmRepositoryURL), "/")
+
+		if newHelmRepo != settings.HelmRepositoryURL && newHelmRepo != portainer.DefaultHelmRepositoryURL {
+			err := libhelm.ValidateHelmRepositoryURL(*payload.HelmRepositoryURL)
+			if err != nil {
+				return &httperror.HandlerError{http.StatusBadRequest, "Invalid Helm repository URL. Must correspond to a valid URL format", err}
+			}
+		}
+
+		settings.HelmRepositoryURL = newHelmRepo
 	}
 
 	if payload.BlackListedLabels != nil {

api/http/proxy/factory/kubernetes/namespaces.go

@@ -46,5 +46,19 @@ func (transport *baseTransport) proxyNamespaceDeleteOperation(request *http.Requ
 			}
 		}
 	}
+
+	stacks, err := transport.dataStore.Stack().Stacks()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, s := range stacks {
+		if s.Namespace == namespace && s.EndpointID == transport.endpoint.ID {
+			if err := transport.dataStore.Stack().DeleteStack(s.ID); err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	return transport.executeKubernetesRequest(request)
 }

api/kubernetes/cli/role.go

@@ -9,7 +9,7 @@ import (
 func getPortainerUserDefaultPolicies() []rbacv1.PolicyRule {
 	return []rbacv1.PolicyRule{
 		{
-			Verbs:     []string{"list"},
+			Verbs:     []string{"list", "get"},
 			Resources: []string{"namespaces", "nodes"},
 			APIGroups: []string{""},
 		},
@@ -18,6 +18,11 @@ func getPortainerUserDefaultPolicies() []rbacv1.PolicyRule {
 			Resources: []string{"storageclasses"},
 			APIGroups: []string{"storage.k8s.io"},
 		},
+		{
+			Verbs:     []string{"list", "get"},
+			Resources: []string{"namespaces", "pods", "nodes"},
+			APIGroups: []string{"metrics.k8s.io"},
+		},
 	}
 }
 

api/portainer.go

@@ -1470,9 +1470,9 @@ type (
 
 const (
 	// APIVersion is the version number of the Portainer API
-	APIVersion = "2.9.1"
+	APIVersion = "2.9.3"
 	// DBVersion is the version number of the Portainer database
-	DBVersion = 32
+	DBVersion = 35
 	// ComposeSyntaxMaxVersion is a maximum supported version of the docker compose syntax
 	ComposeSyntaxMaxVersion = "3.9"
 	// AssetsServerURL represents the URL of the Portainer asset server

app/docker/components/imageRegistry/por-image-registry.controller.js

@@ -69,13 +69,19 @@ class porImageRegistryController {
   async reloadRegistries() {
     return this.$async(async () => {
       try {
-        const registries = await this.EndpointService.registries(this.endpoint.Id, this.namespace);
-        this.registries = _.concat(this.defaultRegistry, registries);
+        let showDefaultRegistry = false;
+        this.registries = await this.EndpointService.registries(this.endpoint.Id, this.namespace);
+
+        // hide default(anonymous) dockerhub registry if user has an authenticated one
+        if (!this.registries.some((registry) => registry.Type === RegistryTypes.DOCKERHUB)) {
+          showDefaultRegistry = true;
+          this.registries.push(this.defaultRegistry);
+        }
 
         const id = this.model.Registry.Id;
         const registry = _.find(this.registries, { Id: id });
         if (!registry) {
-          this.model.Registry = this.defaultRegistry;
+          this.model.Registry = showDefaultRegistry ? this.defaultRegistry : this.registries[0];
         }
       } catch (err) {
         this.Notifications.error('Failure', err, 'Unable to retrieve registries');

app/docker/components/imageRegistry/por-image-registry.html

@@ -6,7 +6,7 @@
     </label>
     <div ng-class="$ctrl.inputClass">
       <select
-        ng-options="registry as registry.Name for registry in $ctrl.registries track by registry.Name"
+        ng-options="registry as registry.Name for registry in $ctrl.registries track by registry.Id"
         ng-model="$ctrl.model.Registry"
         id="image_registry"
         class="form-control"

app/docker/views/images/edit/image.html

@@ -82,7 +82,7 @@
           <!-- !tag-note -->
           <div class="form-group">
             <div class="col-sm-12">
-              <button type="button" class="btn btn-primary btn-sm" ng-disabled="!state.pullImageValidity || !formValues.RegistryModel.Image" ng-click="tagImage()">Tag</button>
+              <button type="button" class="btn btn-primary btn-sm" ng-disabled="!formValues.RegistryModel.Image" ng-click="tagImage()">Tag</button>
             </div>
           </div>
         </form>

app/docker/views/images/edit/imageController.js

@@ -17,6 +17,8 @@ angular.module('portainer.docker').controller('ImageController', [
   'FileSaver',
   'Blob',
   'endpoint',
+  'EndpointService',
+  'RegistryModalService',
   function (
     $async,
     $q,
@@ -32,7 +34,9 @@ angular.module('portainer.docker').controller('ImageController', [
     ModalService,
     FileSaver,
     Blob,
-    endpoint
+    endpoint,
+    EndpointService,
+    RegistryModalService
   ) {
     $scope.endpoint = endpoint;
     $scope.isAdmin = Authentication.isAdmin();
@@ -84,11 +88,13 @@ angular.module('portainer.docker').controller('ImageController', [
 
     async function pushTag(repository) {
       return $async(async () => {
-        $('#uploadResourceHint').show();
         try {
-          const registryModel = await RegistryService.retrievePorRegistryModelFromRepository(repository, endpoint.Id);
-          await ImageService.pushImage(registryModel);
-          Notifications.success('Image successfully pushed', repository);
+          const registryModel = await RegistryModalService.registryModal(repository, $scope.registries);
+          if (registryModel) {
+            $('#uploadResourceHint').show();
+            await ImageService.pushImage(registryModel);
+            Notifications.success('Image successfully pushed', repository);
+          }
         } catch (err) {
           Notifications.error('Failure', err, 'Unable to push image to repository');
         } finally {
@@ -100,11 +106,13 @@ angular.module('portainer.docker').controller('ImageController', [
     $scope.pullTag = pullTag;
     async function pullTag(repository) {
       return $async(async () => {
-        $('#downloadResourceHint').show();
         try {
-          const registryModel = await RegistryService.retrievePorRegistryModelFromRepository(repository, endpoint.Id);
-          await ImageService.pullImage(registryModel);
-          Notifications.success('Image successfully pulled', repository);
+          const registryModel = await RegistryModalService.registryModal(repository, $scope.registries);
+          if (registryModel) {
+            $('#downloadResourceHint').show();
+            await ImageService.pullImage(registryModel);
+            Notifications.success('Image successfully pulled', repository);
+          }
         } catch (err) {
           Notifications.error('Failure', err, 'Unable to pull image from repository');
         } finally {
@@ -171,8 +179,15 @@ angular.module('portainer.docker').controller('ImageController', [
       });
     };
 
-    function initView() {
+    async function initView() {
       HttpRequestHelper.setPortainerAgentTargetHeader($transition$.params().nodeName);
+
+      try {
+        $scope.registries = await RegistryService.loadRegistriesForDropdown(endpoint.Id);
+      } catch (err) {
+        this.Notifications.error('Failure', err, 'Unable to load registries');
+      }
+
       $q.all({
         image: ImageService.image($transition$.params().id),
         history: ImageService.history($transition$.params().id),

app/kubernetes/views/applications/create/createApplication.html

@@ -1771,7 +1771,7 @@
                   ng-if="ctrl.state.appType === ctrl.KubernetesDeploymentTypes.APPLICATION_FORM"
                   type="button"
                   class="btn btn-primary btn-sm"
-                  ng-disabled="!kubernetesApplicationCreationForm.$valid || ctrl.isDeployUpdateButtonDisabled() || !ctrl.state.pullImageValidity"
+                  ng-disabled="!kubernetesApplicationCreationForm.$valid || ctrl.isDeployUpdateButtonDisabled() || !ctrl.imageValidityIsValid()"
                   ng-click="ctrl.deployApplication()"
                   button-spinner="ctrl.state.actionInProgress"
                   data-cy="k8sAppCreate-deployButton"

app/kubernetes/views/applications/create/createApplicationController.js

@@ -2,6 +2,7 @@ import angular from 'angular';
 import _ from 'lodash-es';
 import filesizeParser from 'filesize-parser';
 import * as JsonPatch from 'fast-json-patch';
+import { RegistryTypes } from '@/portainer/models/registryTypes';
 
 import {
   KubernetesApplicationDataAccessPolicies,
@@ -193,6 +194,10 @@ class KubernetesCreateApplicationController {
     this.state.pullImageValidity = validity;
   }
 
+  imageValidityIsValid() {
+    return this.state.pullImageValidity || this.formValues.ImageModel.Registry.Type !== RegistryTypes.DOCKERHUB;
+  }
+
   onChangeName() {
     const existingApplication = _.find(this.applications, { Name: this.formValues.Name });
     this.state.alreadyExists = (this.state.isEdit && existingApplication && this.application.Id !== existingApplication.Id) || (!this.state.isEdit && existingApplication);

app/portainer/services/api/registryService.js

@@ -22,6 +22,8 @@ angular.module('portainer.app').factory('RegistryService', [
       createRegistry,
       createGitlabRegistries,
       retrievePorRegistryModelFromRepository,
+      retrievePorRegistryModelFromRepositoryWithRegistries,
+      loadRegistriesForDropdown,
     };
 
     function registries() {
@@ -107,17 +109,45 @@ angular.module('portainer.app').factory('RegistryService', [
       return url;
     }
 
+    // findBestMatchRegistry finds out the best match registry for repository
+    // matching precedence:
+    // 1. registryId matched
+    // 2. both domain name and username matched (for dockerhub only)
+    // 3. only URL matched
+    // 4. pick up the first dockerhub registry
+    function findBestMatchRegistry(repository, registries, registryId) {
+      let match2, match3, match4;
+
+      for (const registry of registries) {
+        if (registry.Id == registryId) {
+          return registry;
+        }
+
+        if (registry.Type === RegistryTypes.DOCKERHUB) {
+          // try to match repository examples:
+          //   <USERNAME>/nginx:latest
+          //   docker.io/<USERNAME>/nginx:latest
+          if (repository.startsWith(registry.Username + '/') || repository.startsWith(getURL(registry) + '/' + registry.Username + '/')) {
+            match2 = registry;
+          }
+
+          // try to match repository examples:
+          //   portainer/portainer-ee:latest
+          //   <NON-USERNAME>/portainer-ee:latest
+          match4 = match4 || registry;
+        }
+
+        if (_.includes(repository, getURL(registry))) {
+          match3 = registry;
+        }
+      }
+
+      return match2 || match3 || match4;
+    }
+
     function retrievePorRegistryModelFromRepositoryWithRegistries(repository, registries, registryId) {
       const model = new PorImageRegistryModel();
-      const registry = registries.find((reg) => {
-        if (registryId) {
-          return reg.Id === registryId;
-        }
-        if (reg.Type === RegistryTypes.DOCKERHUB) {
-          return _.includes(repository, reg.Username);
-        }
-        return _.includes(repository, getURL(reg));
-      });
+      const registry = findBestMatchRegistry(repository, registries, registryId);
       if (registry) {
         const url = getURL(registry);
         let lastIndex = repository.lastIndexOf(url);
@@ -148,5 +178,22 @@ angular.module('portainer.app').factory('RegistryService', [
         }
       });
     }
+
+    function loadRegistriesForDropdown(endpointId, namespace) {
+      return $async(async () => {
+        try {
+          const registries = await EndpointService.registries(endpointId, namespace);
+
+          // hide default(anonymous) dockerhub registry if user has an authenticated one
+          if (!registries.some((registry) => registry.Type === RegistryTypes.DOCKERHUB)) {
+            registries.push(new DockerHubViewModel());
+          }
+
+          return registries;
+        } catch (err) {
+          throw { msg: 'Unable to retrieve the registries', err: err };
+        }
+      });
+    }
   },
 ]);

app/portainer/services/modalService.js

@@ -308,6 +308,17 @@ angular.module('portainer.app').factory('ModalService', [
       );
     };
 
+    service.selectRegistry = function (options) {
+      var box = bootbox.prompt({
+        title: 'Which registry do you want to use?',
+        inputType: 'select',
+        value: options.defaultValue,
+        inputOptions: options.options,
+        callback: options.callback,
+      });
+      applyBoxCSS(box);
+    };
+
     return service;
   },
 ]);

app/portainer/services/registryModalService.js

@@ -0,0 +1,39 @@
+import _ from 'lodash';
+
+angular.module('portainer.app').factory('RegistryModalService', ModalServiceFactory);
+
+function ModalServiceFactory($q, ModalService, RegistryService) {
+  const service = {};
+
+  function registries2Options(registries) {
+    return registries.map((r) => ({
+      text: r.Name,
+      value: String(r.Id),
+    }));
+  }
+
+  service.registryModal = async function (repository, registries) {
+    const deferred = $q.defer();
+
+    const options = registries2Options(registries);
+    const registryModel = RegistryService.retrievePorRegistryModelFromRepositoryWithRegistries(repository, registries);
+    const defaultValue = String(_.get(registryModel, 'Registry.Id', '0'));
+
+    ModalService.selectRegistry({
+      options,
+      defaultValue,
+      callback: (registryId) => {
+        if (registryId) {
+          const registryModel = RegistryService.retrievePorRegistryModelFromRepositoryWithRegistries(repository, registries, registryId);
+          deferred.resolve(registryModel);
+        } else {
+          deferred.resolve(null);
+        }
+      },
+    });
+
+    return deferred.promise;
+  };
+
+  return service;
+}

package.json

@@ -2,7 +2,7 @@
   "author": "Portainer.io",
   "name": "portainer",
   "homepage": "http://portainer.io",
-  "version": "2.9.1",
+  "version": "2.9.3",
   "repository": {
     "type": "git",
     "url": "git@github.com:portainer/portainer.git"
@@ -176,4 +176,4 @@
     "*.js": "eslint --cache --fix",
     "*.{js,css,md,html}": "prettier --write"
   }
-}
+}