Merge pull request juju#16374 from hpidcock/fix-ecr-private-registry

juju#16374

- Ensure ecr private registry is used before docker registry. Until juju 3.3, we can't remove the default docker registry (i.e. using "jujusolutions" as a repo name instead of the fully qualified name "docker.io/jujusolutions"). This fixes an ordering issue with ecr private (and other repos where you can specify the repo without a subpath) being interpreted as a docker.io repos.
- Once you are using a private OCI registry for caas-image-repo, it is impossible to update the authentication details. This PR allows you to change those authentication details, but not change the registry/repository.

## QA steps

- Create private ECR registry
- Copy jujudb image + push jujud operator image
- Make sure to bootstrap with json repo info like:
- `juju bootstrap k8s --config caas-image-repo='{"repository":"12345.dkr.ecr.eu-west-1.amazonaws.com", "username":"AWS_KEY_ID", "password": "AWS_SECRET_KEY", "region":"eu-west-1" }'`
- Try updating the username and password:
- `juju controller-config caas-image-repo='{"repository":"12345.dkr.ecr.eu-west-1.amazonaws.com", "username":"NEW_AWS_KEY_ID_NEW", "password": "NEW_AWS_SECRET_KEY_NEW", "region":"eu-west-1" }'`
- Check image pull secrets are updated.
- Also try bootstrapping with invalid private ecr `juju bootstrap k8s --config caas-image-repo=12345.dkr.ecr.eu-west-1.amazonaws.com`. It should error `ERROR constructing controller config: empty credential for elastic container registry`.

## Documentation changes

N/A

## Links

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/2037744

**Jira card:** JUJU-4707

.github/workflows/static-analysis.yml

@@ -48,7 +48,7 @@ jobs:
         echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
         echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
 
-        curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.53.0
+        curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2
         sudo curl -sSfL https://github.com/mvdan/sh/releases/download/v3.7.0/shfmt_v3.7.0_linux_$(go env GOARCH) -o /usr/bin/shfmt
         sudo chmod +x /usr/bin/shfmt
         sudo DEBIAN_FRONTEND=noninteractive apt install -y expect

Makefile

@@ -410,7 +410,8 @@ endif
 endif
 
 WAIT_FOR_DPKG=bash -c '. "${PROJECT_DIR}/make_functions.sh"; wait_for_dpkg "$$@"' wait_for_dpkg
-JUJU_DB_CHANNEL=4.4/stable
+JUJU_DB_VERSION=4.4
+JUJU_DB_CHANNEL=${JUJU_DB_VERSION}/stable
 
 .PHONY: install-mongo-dependencies
 install-mongo-dependencies:
@@ -467,6 +468,7 @@ BUILD_OPERATOR_IMAGE=bash -c '. "${PROJECT_DIR}/make_functions.sh"; build_push_o
 OPERATOR_IMAGE_PATH=bash -c '. "${PROJECT_DIR}/make_functions.sh"; operator_image_path "$$@"' operator_image_path
 OPERATOR_IMAGE_RELEASE_PATH=bash -c '. "${PROJECT_DIR}/make_functions.sh"; operator_image_release_path "$$@"' operator_image_release_path
 UPDATE_MICROK8S_OPERATOR=bash -c '. "${PROJECT_DIR}/make_functions.sh"; microk8s_operator_update "$$@"' microk8s_operator_update
+SEED_REPOSITORY=bash -c '. "${PROJECT_DIR}/make_functions.sh"; seed_repository "$$@"' seed_repository
 
 image_check_prereq=image-check-build
 ifneq ($(OPERATOR_IMAGE_BUILD_SRC),true)
@@ -514,12 +516,16 @@ push-operator-image-undefined:
 push-operator-image: $(push_operator_image_prereq)
 ## push-operator-image: Push up the newly built operator image via docker
 
-
 .PHONY: push-release-operator-image
 push-release-operator-image: PUSH_IMAGE=true
 push-release-operator-image: operator-image
 ## push-release-operator-image: Push up the newly built release operator image via docker
 
+.PHONY: seed-repository
+seed-repository:
+## seed-repository: Copy required juju images from docker.io/jujusolutions
+	JUJU_DB_VERSION=$(JUJU_DB_VERSION) $(SEED_REPOSITORY)
+
 
 .PHONY: host-install
 host-install:
@@ -528,19 +534,20 @@ host-install:
 
 .PHONY: minikube-operator-update
 minikube-operator-update: host-install operator-image
-## minikube-operator-update: Push up the newly built operator image for use with minikube
+## minikube-operator-update: Inject the newly built operator image into minikube
 	$(OCI_BUILDER) save "$(shell ${OPERATOR_IMAGE_PATH})" | minikube image load --overwrite=true -
 
 .PHONY: microk8s-operator-update
 microk8s-operator-update: host-install operator-image
-## microk8s-operator-update: Push up the newly built operator image for use with microk8s
+## microk8s-operator-update: Inject the newly built operator image into microk8s
 	@${UPDATE_MICROK8S_OPERATOR}
 
 .PHONY: k3s-operator-update
 k3s-operator-update: host-install operator-image
-## k3s-operator-update: Push up the newly built operator image for use with k3s
+## k3s-operator-update: Inject the newly built operator image into k3s
 	$(OCI_BUILDER) save "$(shell ${OPERATOR_IMAGE_PATH})" | sudo k3s ctr images import -
 
+
 .PHONY: check-k8s-model
 check-k8s-model:
 ## check-k8s-model: Check if k8s model is present in show-model

api/controller/caasmodelconfigmanager/client.go

@@ -8,6 +8,9 @@ import (
 
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/api/common"
+	apiwatcher "github.com/juju/juju/api/watcher"
+	"github.com/juju/juju/core/watcher"
+	"github.com/juju/juju/rpc/params"
 )
 
 // Client allows access to the CAAS model config manager API endpoint.
@@ -28,3 +31,15 @@ func NewClient(caller base.APICaller) (*Client, error) {
 		ControllerConfigAPI: common.NewControllerConfig(facadeCaller),
 	}, nil
 }
+
+// WatchControllerConfig provides a watcher for changes on controller config.
+func (c *Client) WatchControllerConfig() (watcher.NotifyWatcher, error) {
+	var result params.NotifyWatchResult
+	if err := c.facade.FacadeCall("WatchControllerConfig", nil, &result); err != nil {
+		return nil, err
+	}
+	if result.Error != nil {
+		return nil, result.Error
+	}
+	return apiwatcher.NewNotifyWatcher(c.facade.RawAPICaller(), result), nil
+}

apiserver/facades/client/controller/controller.go

@@ -5,6 +5,7 @@ package controller
 
 import (
 	"encoding/json"
+	"fmt"
 	"sort"
 	"strings"
 
@@ -29,6 +30,7 @@ import (
 	coremigration "github.com/juju/juju/core/migration"
 	"github.com/juju/juju/core/multiwatcher"
 	"github.com/juju/juju/core/permission"
+	"github.com/juju/juju/docker"
 	"github.com/juju/juju/migration"
 	"github.com/juju/juju/pubsub/controller"
 	"github.com/juju/juju/rpc/params"
@@ -659,6 +661,50 @@ func (c *ControllerAPI) ConfigSet(args params.ControllerConfigSet) error {
 	if err := c.checkIsSuperUser(); err != nil {
 		return errors.Trace(err)
 	}
+
+	currentCfg, err := c.state.ControllerConfig()
+	if err != nil {
+		return errors.Trace(err)
+	}
+
+	// TODO(dqlite): move this business logic out of the facade.
+	if newValue, ok := args.Config[corecontroller.CAASImageRepo]; ok {
+		var newCAASImageRepo *docker.ImageRepoDetails
+		if v, ok := newValue.(string); ok {
+			newCAASImageRepo, err = docker.NewImageRepoDetails(v)
+			if err != nil {
+				return fmt.Errorf("cannot parse %s: %s%w", corecontroller.CAASImageRepo, err.Error(),
+					errors.Hide(errors.NotValid))
+			}
+		} else {
+			return fmt.Errorf("%s expected a string got %v%w", corecontroller.CAASImageRepo, v,
+				errors.Hide(errors.NotValid))
+		}
+
+		var currentCAASImageRepo *docker.ImageRepoDetails
+		if currentValue, ok := currentCfg[corecontroller.CAASImageRepo]; !ok {
+			return fmt.Errorf("cannot change %s as it is not currently set%w", corecontroller.CAASImageRepo,
+				errors.Hide(errors.NotValid))
+		} else if v, ok := currentValue.(string); !ok {
+			return fmt.Errorf("existing %s expected a string", corecontroller.CAASImageRepo)
+		} else {
+			currentCAASImageRepo, err = docker.NewImageRepoDetails(v)
+			if err != nil {
+				return fmt.Errorf("cannot parse existing %s: %w", corecontroller.CAASImageRepo, err)
+			}
+		}
+		// TODO: when podspec is removed, implement changing caas-image-repo.
+		if newCAASImageRepo.Repository != currentCAASImageRepo.Repository {
+			return fmt.Errorf("cannot change %s: repository read-only, only authentication can be updated", corecontroller.CAASImageRepo)
+		}
+		if !newCAASImageRepo.IsPrivate() && currentCAASImageRepo.IsPrivate() {
+			return fmt.Errorf("cannot change %s: unable to remove authentication details", corecontroller.CAASImageRepo)
+		}
+		if newCAASImageRepo.IsPrivate() && !currentCAASImageRepo.IsPrivate() {
+			return fmt.Errorf("cannot change %s: unable to add authentication details", corecontroller.CAASImageRepo)
+		}
+	}
+
 	if err := c.state.UpdateControllerConfig(args.Config, nil); err != nil {
 		return errors.Trace(err)
 	}

apiserver/facades/client/controller/controller_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/juju/juju/core/cache"
 	coremultiwatcher "github.com/juju/juju/core/multiwatcher"
 	"github.com/juju/juju/core/permission"
+	"github.com/juju/juju/docker"
 	"github.com/juju/juju/environs"
 	environscloudspec "github.com/juju/juju/environs/cloudspec"
 	"github.com/juju/juju/environs/config"
@@ -1081,6 +1082,52 @@ func (s *controllerSuite) TestConfigSetPublishesEvent(c *gc.C) {
 	c.Assert(config.Features().SortedValues(), jc.DeepEquals, []string{"bar", "foo"})
 }
 
+func (s *controllerSuite) TestConfigSetCAASImageRepo(c *gc.C) {
+	config, err := s.State.ControllerConfig()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(config.CAASImageRepo().Empty(), jc.IsTrue)
+
+	err = s.controller.ConfigSet(params.ControllerConfigSet{Config: map[string]interface{}{
+		"caas-image-repo": "juju-repo.local",
+	}})
+	c.Assert(err, gc.ErrorMatches, `cannot change caas-image-repo as it is not currently set`)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		"caas-image-repo": "jujusolutions",
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = s.controller.ConfigSet(params.ControllerConfigSet{Config: map[string]interface{}{
+		"caas-image-repo": "juju-repo.local",
+	}})
+	c.Assert(err, gc.ErrorMatches, `cannot change caas-image-repo: repository read-only, only authentication can be updated`)
+
+	err = s.controller.ConfigSet(params.ControllerConfigSet{Config: map[string]interface{}{
+		"caas-image-repo": `{"repository":"jujusolutions","username":"foo","password":"bar"}`,
+	}})
+	c.Assert(err, gc.ErrorMatches, `cannot change caas-image-repo: unable to add authentication details`)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		"caas-image-repo": `{"repository":"jujusolutions","username":"bar","password":"foo"}`,
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = s.controller.ConfigSet(params.ControllerConfigSet{Config: map[string]interface{}{
+		"caas-image-repo": `{"repository":"jujusolutions","username":"foo","password":"bar"}`,
+	}})
+	c.Assert(err, jc.ErrorIsNil)
+
+	config, err = s.State.ControllerConfig()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(config.CAASImageRepo(), gc.DeepEquals, docker.ImageRepoDetails{
+		Repository: "jujusolutions",
+		BasicAuthConfig: docker.BasicAuthConfig{
+			Username: "foo",
+			Password: "bar",
+		},
+	})
+}
+
 func (s *controllerSuite) TestMongoVersion(c *gc.C) {
 	result, err := s.controller.MongoVersion()
 	c.Assert(err, jc.ErrorIsNil)

apiserver/facades/controller/caasmodelconfigmanager/facade.go

@@ -7,16 +7,37 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/facade"
 	"github.com/juju/juju/rpc/params"
+	"github.com/juju/juju/state"
+	"github.com/juju/juju/state/watcher"
 )
 
 //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/context_mock.go github.com/juju/juju/apiserver/facade Authorizer,Context,Resources
 
+// State provides required state for the Facade.
+type State interface {
+	WatchControllerConfig() state.NotifyWatcher
+}
+
 // Facade allows model config manager clients to watch controller config changes and fetch controller config.
 type Facade struct {
-	auth                facade.Authorizer
+	auth      facade.Authorizer
+	resources facade.Resources
+
+	ctrlState           State
 	controllerConfigAPI *common.ControllerConfigAPI
 }
 
 func (f *Facade) ControllerConfig() (params.ControllerConfigResult, error) {
 	return f.controllerConfigAPI.ControllerConfig()
 }
+
+func (f *Facade) WatchControllerConfig() (params.NotifyWatchResult, error) {
+	result := params.NotifyWatchResult{}
+	w := f.ctrlState.WatchControllerConfig()
+	if _, ok := <-w.Changes(); ok {
+		result.NotifyWatcherId = f.resources.Register(w)
+	} else {
+		return result, watcher.EnsureErr(w)
+	}
+	return result, nil
+}

apiserver/facades/controller/caasmodelconfigmanager/register.go

@@ -26,6 +26,8 @@ func newFacade(ctx facade.Context) (*Facade, error) {
 	}
 	return &Facade{
 		auth:                authorizer,
+		resources:           ctx.Resources(),
 		controllerConfigAPI: common.NewStateControllerConfig(ctx.State()),
+		ctrlState:           ctx.State(),
 	}, nil
 }

apiserver/facades/schema.json

@@ -10607,6 +10607,14 @@
                             "$ref": "#/definitions/ControllerConfigResult"
                         }
                     }
+                },
+                "WatchControllerConfig": {
+                    "type": "object",
+                    "properties": {
+                        "Result": {
+                            "$ref": "#/definitions/NotifyWatchResult"
+                        }
+                    }
                 }
             },
             "definitions": {
@@ -10627,6 +10635,46 @@
                     "required": [
                         "config"
                     ]
+                },
+                "Error": {
+                    "type": "object",
+                    "properties": {
+                        "code": {
+                            "type": "string"
+                        },
+                        "info": {
+                            "type": "object",
+                            "patternProperties": {
+                                ".*": {
+                                    "type": "object",
+                                    "additionalProperties": true
+                                }
+                            }
+                        },
+                        "message": {
+                            "type": "string"
+                        }
+                    },
+                    "additionalProperties": false,
+                    "required": [
+                        "message",
+                        "code"
+                    ]
+                },
+                "NotifyWatchResult": {
+                    "type": "object",
+                    "properties": {
+                        "NotifyWatcherId": {
+                            "type": "string"
+                        },
+                        "error": {
+                            "$ref": "#/definitions/Error"
+                        }
+                    },
+                    "additionalProperties": false,
+                    "required": [
+                        "NotifyWatcherId"
+                    ]
                 }
             }
         }

caas/kubernetes/provider/bootstrap.go

@@ -41,6 +41,7 @@ import (
 	"github.com/juju/juju/cloudconfig/podcfg"
 	k8sannotations "github.com/juju/juju/core/annotations"
 	"github.com/juju/juju/core/watcher"
+	"github.com/juju/juju/docker/registry"
 	"github.com/juju/juju/environs"
 	environsbootstrap "github.com/juju/juju/environs/bootstrap"
 	"github.com/juju/juju/juju/osenv"
@@ -301,9 +302,26 @@ func newcontrollerStack(
 
 	cs.pvcNameControllerPodStorage = "storage"
 
-	if cs.dockerAuthSecretData, err = pcfg.Controller.CAASImageRepo().SecretData(); err != nil {
-		return nil, errors.Trace(err)
+	// Initialize registry.
+	if repoDetails := pcfg.Controller.CAASImageRepo(); !repoDetails.Empty() {
+		reg, err := registry.New(repoDetails)
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		defer func() { _ = reg.Close() }()
+		err = reg.RefreshAuth()
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		err = reg.Ping()
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		if cs.dockerAuthSecretData, err = reg.ImageRepoDetails().SecretData(); err != nil {
+			return nil, errors.Trace(err)
+		}
 	}
+
 	return cs, nil
 }
 

caas/kubernetes/provider/export_test.go

@@ -98,7 +98,10 @@ func NewcontrollerStackForTest(
 	pcfg *podcfg.ControllerPodConfig,
 ) (ControllerStackerForTest, error) {
 	cs, err := newcontrollerStack(ctx, stackName, storageClass, broker, pcfg)
-	return cs.(*controllerStack), err
+	if err != nil {
+		return nil, err
+	}
+	return cs.(*controllerStack), nil
 }
 
 func Pod(u *workloadSpec) k8sspecs.PodSpecWithAnnotations {