diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 7433b1694bb..800732bd7a4 100644 --- a/.github/workflows/static-analysis.yml +++ b/.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 diff --git a/Makefile b/Makefile index 47294d20be4..772210850c9 100644 --- a/Makefile +++ b/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 diff --git a/api/controller/caasmodelconfigmanager/client.go b/api/controller/caasmodelconfigmanager/client.go index 6b333416618..b871528be2e 100644 --- a/api/controller/caasmodelconfigmanager/client.go +++ b/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 +} diff --git a/apiserver/facades/client/controller/controller.go b/apiserver/facades/client/controller/controller.go index d9e9d9ac31e..e8cb6538082 100644 --- a/apiserver/facades/client/controller/controller.go +++ b/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) } diff --git a/apiserver/facades/client/controller/controller_test.go b/apiserver/facades/client/controller/controller_test.go index f8f96e7b190..e59f3078c2c 100644 --- a/apiserver/facades/client/controller/controller_test.go +++ b/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) diff --git a/apiserver/facades/controller/caasmodelconfigmanager/facade.go b/apiserver/facades/controller/caasmodelconfigmanager/facade.go index 5ac419ff5a6..7ff0dde2e8f 100644 --- a/apiserver/facades/controller/caasmodelconfigmanager/facade.go +++ b/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 +} diff --git a/apiserver/facades/controller/caasmodelconfigmanager/register.go b/apiserver/facades/controller/caasmodelconfigmanager/register.go index b32b07dd223..b3e0bee03c1 100644 --- a/apiserver/facades/controller/caasmodelconfigmanager/register.go +++ b/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 } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 39dca0bb48f..7cafd19fad8 100644 --- a/apiserver/facades/schema.json +++ b/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" + ] } } } diff --git a/caas/kubernetes/provider/bootstrap.go b/caas/kubernetes/provider/bootstrap.go index 1edae40a7ac..565bd52e17c 100644 --- a/caas/kubernetes/provider/bootstrap.go +++ b/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 } diff --git a/caas/kubernetes/provider/export_test.go b/caas/kubernetes/provider/export_test.go index bc0bb89abfd..e3a2750c635 100644 --- a/caas/kubernetes/provider/export_test.go +++ b/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 { diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go index 30dd36d1529..790f18174b5 100644 --- a/cmd/juju/commands/bootstrap.go +++ b/cmd/juju/commands/bootstrap.go @@ -41,6 +41,7 @@ import ( "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/core/series" + "github.com/juju/juju/docker" "github.com/juju/juju/environs" "github.com/juju/juju/environs/bootstrap" environscloudspec "github.com/juju/juju/environs/cloudspec" @@ -1497,6 +1498,21 @@ func (c *bootstrapCommand) bootstrapConfigs( return bootstrapConfigs{}, errors.Annotate(err, "constructing bootstrap config") } + // Pre-process controller attributes. + if _, ok := controllerConfigAttrs[controller.CAASOperatorImagePath]; ok { + return bootstrapConfigs{}, fmt.Errorf("%q is no longer supported controller configuration", + controller.CAASOperatorImagePath) + } + if v, ok := controllerConfigAttrs[controller.CAASImageRepo]; ok { + if v, ok := v.(string); ok { + repoDetails, err := docker.LoadImageRepoDetails(v) + if err != nil { + return bootstrapConfigs{}, errors.Annotatef(err, "processing %s", controller.CAASImageRepo) + } + controllerConfigAttrs[controller.CAASImageRepo] = repoDetails.Content() + } + } + controllerConfig, err := controller.NewConfig( controllerUUID.String(), bootstrapConfig.CACert, diff --git a/cmd/juju/commands/ssh_machine.go b/cmd/juju/commands/ssh_machine.go index 577a7a4f1d4..bf363c25bf8 100644 --- a/cmd/juju/commands/ssh_machine.go +++ b/cmd/juju/commands/ssh_machine.go @@ -98,8 +98,8 @@ func (t *resolvedTarget) isAgent() bool { return targetIsAgent(t.entity) } -// SSHPort is the TCP port used for SSH connections. -const SSHPort = 22 +// sshPort is the TCP port used for SSH connections. +const sshPort = 22 func (c *sshMachine) SetFlags(f *gnuflag.FlagSet) { f.BoolVar(&c.proxy, "proxy", false, "Proxy through the API server") @@ -577,7 +577,7 @@ func (c *sshMachine) reachableAddressGetter(entity string) (string, error) { } } - usable := network.NewMachineHostPorts(SSHPort, addresses...).HostPorts().FilterUnusable() + usable := network.NewMachineHostPorts(sshPort, addresses...).HostPorts().FilterUnusable() best, err := c.hostChecker.FindHost(usable, publicKeys) if err != nil { return "", errors.Trace(err) diff --git a/controller/config.go b/controller/config.go index 7107d5acad7..019b4c41d8a 100755 --- a/controller/config.go +++ b/controller/config.go @@ -487,6 +487,7 @@ var ( MigrationMinionWaitMax, ApplicationResourceDownloadLimit, ControllerResourceDownloadLimit, + CAASImageRepo, ) // DefaultAuditLogExcludeMethods is the default list of methods to @@ -940,27 +941,24 @@ func (c Config) CAASOperatorImagePath() (o docker.ImageRepoDetails) { return o } -func validateCAASImageRepo(imageRepo string) (string, error) { +func validateCAASImageRepo(imageRepo string) error { if imageRepo == "" { - return "", nil + return nil } imageDetails, err := docker.NewImageRepoDetails(imageRepo) if err != nil { - return "", errors.Trace(err) + return errors.Trace(err) } if err = imageDetails.Validate(); err != nil { - return "", errors.Trace(err) + return errors.Trace(err) } r, err := registry.New(*imageDetails) if err != nil { - return "", errors.Trace(err) + return errors.Trace(err) } defer func() { _ = r.Close() }() - if err = r.Ping(); err != nil { - return "", errors.Trace(err) - } - return r.ImageRepoDetails().Content(), nil + return nil } // CAASImageRepo sets the url of the docker repo @@ -1160,13 +1158,13 @@ func Validate(c Config) error { var err error if v, ok := c[CAASOperatorImagePath].(string); ok && v != "" { - if c[CAASOperatorImagePath], err = validateCAASImageRepo(v); err != nil { + if err = validateCAASImageRepo(v); err != nil { return errors.Trace(err) } } if v, ok := c[CAASImageRepo].(string); ok && v != "" { - if c[CAASImageRepo], err = validateCAASImageRepo(v); err != nil { + if err = validateCAASImageRepo(v); err != nil { return errors.Trace(err) } } diff --git a/controller/config_test.go b/controller/config_test.go index 2ba56149bd5..6a8b27a4166 100755 --- a/controller/config_test.go +++ b/controller/config_test.go @@ -6,8 +6,6 @@ package controller_test import ( "encoding/base64" "fmt" - "io/ioutil" - "net/http" stdtesting "testing" "time" @@ -657,42 +655,9 @@ func (s *ConfigSuite) TestConfigNoSpacesNilSpaceConfigPreserved(c *gc.C) { func (s *ConfigSuite) TestCAASImageRepo(c *gc.C) { ctrl := gomock.NewController(c) defer ctrl.Finish() + + // Ensure no requests are made from controller config code. mockRoundTripper := mocks.NewMockRoundTripper(ctrl) - gomock.InOrder( - mockRoundTripper.EXPECT().RoundTrip(gomock.Any()).DoAndReturn( - func(req *http.Request) (*http.Response, error) { - c.Assert(req.Method, gc.Equals, `GET`) - c.Assert(req.URL.String(), gc.Equals, `https://index.docker.io/v2`) - resps := &http.Response{ - StatusCode: http.StatusOK, - Body: ioutil.NopCloser(nil), - } - return resps, nil - }, - ), - mockRoundTripper.EXPECT().RoundTrip(gomock.Any()).DoAndReturn( - func(req *http.Request) (*http.Response, error) { - c.Assert(req.Method, gc.Equals, `GET`) - c.Assert(req.URL.String(), gc.Equals, `https://registry.foo.com/v2`) - resps := &http.Response{ - StatusCode: http.StatusOK, - Body: ioutil.NopCloser(nil), - } - return resps, nil - }, - ), - mockRoundTripper.EXPECT().RoundTrip(gomock.Any()).DoAndReturn( - func(req *http.Request) (*http.Response, error) { - c.Assert(req.Method, gc.Equals, `GET`) - c.Assert(req.URL.String(), gc.Equals, `https://ghcr.io/v2/`) - resps := &http.Response{ - StatusCode: http.StatusOK, - Body: ioutil.NopCloser(nil), - } - return resps, nil - }, - ), - ) s.PatchValue(®istry.DefaultTransport, mockRoundTripper) type tc struct { diff --git a/docker/auth.go b/docker/auth.go index d234d0d6575..b18cb5ec2ea 100644 --- a/docker/auth.go +++ b/docker/auth.go @@ -8,7 +8,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "io/ioutil" "os" "reflect" "time" @@ -110,10 +109,6 @@ func (ac *TokenAuthConfig) Validate() error { return nil } -func (ac *TokenAuthConfig) init() error { - return nil -} - // BasicAuthConfig contains authorization information for basic auth. type BasicAuthConfig struct { // Auth is the base64 encoded "username:password" string. @@ -136,16 +131,6 @@ func (ba *BasicAuthConfig) Validate() error { return nil } -func (ba *BasicAuthConfig) init() error { - if ba.Empty() { - return nil - } - if ba.Auth.Empty() { - ba.Auth = NewToken(base64.StdEncoding.EncodeToString([]byte(ba.Username + ":" + ba.Password))) - } - return nil -} - // ImageRepoDetails contains authorization information for connecting to a Registry. type ImageRepoDetails struct { BasicAuthConfig `json:",inline" yaml:",inline"` @@ -182,6 +167,10 @@ func (rid ImageRepoDetails) SecretData() ([]byte, error) { return nil, nil } rid.Repository = "" + if !rid.BasicAuthConfig.Empty() && rid.BasicAuthConfig.Auth.Empty() { + rid.BasicAuthConfig.Auth = NewToken( + base64.StdEncoding.EncodeToString([]byte(rid.BasicAuthConfig.Username + ":" + rid.BasicAuthConfig.Password))) + } o := dockerConfigData{ Auths: map[string]ImageRepoDetails{ rid.ServerAddress: rid, @@ -192,6 +181,12 @@ func (rid ImageRepoDetails) SecretData() ([]byte, error) { // Content returns the json marshalled string with raw credentials. func (rid ImageRepoDetails) Content() string { + copy := rid + copy.Repository = "" + if copy.Empty() { + // If only repository is set, return it. + return rid.Repository + } d, _ := json.Marshal(rid) return string(d) } @@ -214,16 +209,6 @@ func (rid *ImageRepoDetails) Validate() error { return nil } -func (rid *ImageRepoDetails) init() error { - if err := rid.BasicAuthConfig.init(); err != nil { - return errors.Annotatef(err, "initializing basic auth config for repository %q", rid.Repository) - } - if err := rid.TokenAuthConfig.init(); err != nil { - return errors.Annotatef(err, "initializing token auth config for repository %q", rid.Repository) - } - return nil -} - // Empty checks if the auth information is empty. func (rid ImageRepoDetails) Empty() bool { return rid == ImageRepoDetails{} @@ -240,8 +225,26 @@ func fileExists(p string) (bool, error) { return !info.IsDir(), nil } -// NewImageRepoDetails tries to parse a file path or file content and returns an instance of ImageRepoDetails. -func NewImageRepoDetails(contentOrPath string) (o *ImageRepoDetails, err error) { +// NewImageRepoDetails tries to parse as json or basic repository path and returns an instance of ImageRepoDetails. +func NewImageRepoDetails(repo string) (o *ImageRepoDetails, err error) { + if repo == "" { + return o, nil + } + data := []byte(repo) + o = &ImageRepoDetails{} + err = json.Unmarshal(data, o) + if err != nil { + logger.Tracef("unmarshalling %q, err %#v", repo, err) + return &ImageRepoDetails{Repository: repo}, nil + } + if err = o.Validate(); err != nil { + return nil, errors.Trace(err) + } + return o, nil +} + +// LoadImageRepoDetails tries to parse a file path or file content and returns an instance of ImageRepoDetails. +func LoadImageRepoDetails(contentOrPath string) (o *ImageRepoDetails, err error) { if contentOrPath == "" { return o, nil } @@ -249,23 +252,10 @@ func NewImageRepoDetails(contentOrPath string) (o *ImageRepoDetails, err error) isPath, err := fileExists(contentOrPath) if err == nil && isPath { logger.Debugf("reading image repository information from %q", contentOrPath) - data, err = ioutil.ReadFile(contentOrPath) + data, err = os.ReadFile(contentOrPath) if err != nil { return nil, errors.Trace(err) } } - o = &ImageRepoDetails{} - err = json.Unmarshal(data, o) - if err != nil { - logger.Tracef("unmarshalling %q, err %#v", contentOrPath, err) - return &ImageRepoDetails{Repository: contentOrPath}, nil - } - - if err = o.Validate(); err != nil { - return nil, errors.Trace(err) - } - if err = o.init(); err != nil { - return nil, errors.Trace(err) - } - return o, nil + return NewImageRepoDetails(string(data)) } diff --git a/docker/auth_test.go b/docker/auth_test.go index d1ad33b8b32..a955b1108a6 100644 --- a/docker/auth_test.go +++ b/docker/auth_test.go @@ -4,7 +4,6 @@ package docker_test import ( - "encoding/base64" "io/ioutil" "path/filepath" @@ -47,7 +46,7 @@ func (s *authSuite) TestNewImageRepoDetailsReadFromFile(c *gc.C) { fullpath := filepath.Join(dir, filename) err := ioutil.WriteFile(fullpath, []byte(quayContent), 0644) c.Assert(err, jc.ErrorIsNil) - imageRepoDetails, err := docker.NewImageRepoDetails(fullpath) + imageRepoDetails, err := docker.LoadImageRepoDetails(fullpath) c.Assert(err, jc.ErrorIsNil) c.Assert(imageRepoDetails, jc.DeepEquals, &docker.ImageRepoDetails{ Repository: "test-account", @@ -78,7 +77,6 @@ func (s *authSuite) TestNewImageRepoDetailsReadFromContent(c *gc.C) { BasicAuthConfig: docker.BasicAuthConfig{ Username: "aws_access_key_id", Password: "aws_secret_access_key", - Auth: docker.NewToken(base64.StdEncoding.EncodeToString([]byte("aws_access_key_id:aws_secret_access_key"))), }, TokenAuthConfig: docker.TokenAuthConfig{ IdentityToken: docker.NewToken("xxxxx=="), diff --git a/docker/registry/internal/acr.go b/docker/registry/internal/acr.go index 81c398dec94..de18f22f187 100644 --- a/docker/registry/internal/acr.go +++ b/docker/registry/internal/acr.go @@ -29,6 +29,10 @@ func normalizeRepoDetailsAzure(repoDetails *docker.ImageRepoDetails) { } } +func (c *azureContainerRegistry) String() string { + return "azurecr.io" +} + // Match checks if the repository details matches current provider format. func (c *azureContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "azurecr.io") diff --git a/docker/registry/internal/base_client.go b/docker/registry/internal/base_client.go index f1d2649eae9..c799f6eb1e5 100644 --- a/docker/registry/internal/base_client.go +++ b/docker/registry/internal/base_client.go @@ -77,9 +77,13 @@ func normalizeRepoDetailsCommon(repoDetails *docker.ImageRepoDetails) { } } +func (c *baseClient) String() string { + return "generic" +} + // ShouldRefreshAuth checks if the repoDetails should be refreshed. -func (c *baseClient) ShouldRefreshAuth() (bool, *time.Duration) { - return false, nil +func (c *baseClient) ShouldRefreshAuth() (bool, time.Duration) { + return false, time.Duration(0) } // RefreshAuth refreshes the repoDetails. diff --git a/docker/registry/internal/dockerhub.go b/docker/registry/internal/dockerhub.go index 2cbe9aa00ee..2517abfa35d 100644 --- a/docker/registry/internal/dockerhub.go +++ b/docker/registry/internal/dockerhub.go @@ -26,6 +26,10 @@ func newDockerhub(repoDetails docker.ImageRepoDetails, transport http.RoundTripp return &dockerhub{c} } +func (c *dockerhub) String() string { + return "docker.io" +} + // Match checks if the repository details matches current provider format. func (c *dockerhub) Match() bool { return c.repoDetails.ServerAddress == "" || strings.Contains(c.repoDetails.ServerAddress, "docker.io") diff --git a/docker/registry/internal/ecr.go b/docker/registry/internal/ecr.go index e95851656f9..aec2388d991 100644 --- a/docker/registry/internal/ecr.go +++ b/docker/registry/internal/ecr.go @@ -86,6 +86,10 @@ func normalizeRepoDetailsElasticContainerRegistry(repoDetails *docker.ImageRepoD } } +func (c *elasticContainerRegistry) String() string { + return "*.dkr.ecr.*.amazonaws.com" +} + // Match checks if the repository details matches current provider format. func (c *elasticContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "amazonaws.com") @@ -123,16 +127,15 @@ func (c *elasticContainerRegistry) refreshTokenForElasticContainerRegistry(image } // ShouldRefreshAuth checks if the repoDetails should be refreshed. -func (c *elasticContainerRegistry) ShouldRefreshAuth() (bool, *time.Duration) { +func (c *elasticContainerRegistry) ShouldRefreshAuth() (bool, time.Duration) { if c.repoDetails.Auth.Empty() || c.repoDetails.Auth.ExpiresAt == nil { - return true, nil + return true, time.Duration(0) } d := time.Until(*c.repoDetails.Auth.ExpiresAt) if d <= advanceExpiry { - return true, nil + return true, time.Duration(0) } - nextCheckDuration := d - advanceExpiry - return false, &nextCheckDuration + return false, d - advanceExpiry } // RefreshAuth refreshes the repoDetails. @@ -146,13 +149,23 @@ func (c *elasticContainerRegistry) elasticContainerRegistryTransport( if repoDetails.BasicAuthConfig.Empty() { return nil, errors.NewNotValid(nil, "empty credential for elastic container registry") } - if err := c.refreshTokenForElasticContainerRegistry(repoDetails); err != nil { - return nil, errors.Trace(err) + if repoDetails.Region == "" { + return nil, errors.NewNotValid(nil, "region is required") } - if repoDetails.Auth.Empty() { - return nil, errors.NewNotValid(nil, "empty identity token for elastic container registry") + if repoDetails.Username == "" || repoDetails.Password == "" { + return nil, errors.NewNotValid(nil, + fmt.Sprintf("username and password are required for registry %q", repoDetails.Repository), + ) } - return newBasicTransport(transport, "", "", repoDetails.Auth.Value), nil + return dynamicTransportFunc(func() (http.RoundTripper, error) { + if err := c.refreshTokenForElasticContainerRegistry(repoDetails); err != nil { + return nil, errors.Trace(err) + } + if repoDetails.Auth.Empty() { + return nil, errors.NewNotValid(nil, "empty identity token for elastic container registry") + } + return newBasicTransport(transport, "", "", repoDetails.Auth.Value), nil + }), nil } func (c *elasticContainerRegistry) WrapTransport(...TransportWrapper) (err error) { @@ -204,6 +217,10 @@ func newElasticContainerRegistryPublic(repoDetails docker.ImageRepoDetails, tran return &elasticContainerRegistryPublic{c} } +func (c *elasticContainerRegistryPublic) String() string { + return "public.ecr.aws" +} + // Match checks if the repository details matches current provider format. func (c *elasticContainerRegistryPublic) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "public.ecr.aws") diff --git a/docker/registry/internal/ecr_test.go b/docker/registry/internal/ecr_test.go index 49d929e0dde..bee53e3dc56 100644 --- a/docker/registry/internal/ecr_test.go +++ b/docker/registry/internal/ecr_test.go @@ -67,7 +67,7 @@ func (s *elasticContainerRegistrySuite) getRegistry(c *gc.C, ensureAsserts func( {AuthorizationToken: aws.String(`xxxx===`)}, }, }, nil, - ) + ).AnyTimes() } } @@ -153,7 +153,7 @@ func (s *elasticContainerRegistrySuite) TestShouldRefreshAuthAuthTokenMissing(c } setImageRepoDetails(c, reg, repoDetails) shouldRefreshAuth, tick := reg.ShouldRefreshAuth() - c.Assert(tick, gc.IsNil) + c.Assert(tick, gc.Equals, time.Duration(0)) c.Assert(shouldRefreshAuth, jc.IsTrue) } @@ -172,7 +172,7 @@ func (s *elasticContainerRegistrySuite) TestShouldRefreshNoExpireTime(c *gc.C) { repoDetails.Auth = docker.NewToken(`xxx===`) setImageRepoDetails(c, reg, repoDetails) shouldRefreshAuth, tick := reg.ShouldRefreshAuth() - c.Assert(tick, gc.IsNil) + c.Assert(tick, gc.Equals, time.Duration(0)) c.Assert(shouldRefreshAuth, jc.IsTrue) } @@ -196,7 +196,7 @@ func (s *elasticContainerRegistrySuite) TestShouldRefreshTokenExpired(c *gc.C) { } setImageRepoDetails(c, reg, repoDetails) shouldRefreshAuth, tick := reg.ShouldRefreshAuth() - c.Assert(tick, gc.IsNil) + c.Assert(tick, gc.Equals, time.Duration(0)) c.Assert(shouldRefreshAuth, jc.IsTrue) // // already expired. @@ -207,7 +207,7 @@ func (s *elasticContainerRegistrySuite) TestShouldRefreshTokenExpired(c *gc.C) { } setImageRepoDetails(c, reg, repoDetails) shouldRefreshAuth, tick = reg.ShouldRefreshAuth() - c.Assert(tick, gc.IsNil) + c.Assert(tick, gc.Equals, time.Duration(0)) c.Assert(shouldRefreshAuth, jc.IsTrue) } diff --git a/docker/registry/internal/gcr.go b/docker/registry/internal/gcr.go index 44e430cd6f8..90333251c5d 100644 --- a/docker/registry/internal/gcr.go +++ b/docker/registry/internal/gcr.go @@ -22,6 +22,10 @@ func newGoogleContainerRegistry(repoDetails docker.ImageRepoDetails, transport h return &googleContainerRegistry{c} } +func (c *googleContainerRegistry) String() string { + return "gcr.io" +} + // Match checks if the repository details matches current provider format. func (c *googleContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "gcr.io") diff --git a/docker/registry/internal/github.go b/docker/registry/internal/github.go index c34c1d4ac0f..a407dcc5f59 100644 --- a/docker/registry/internal/github.go +++ b/docker/registry/internal/github.go @@ -22,6 +22,10 @@ func newGithubContainerRegistry(repoDetails docker.ImageRepoDetails, transport h return &githubContainerRegistry{c} } +func (c *githubContainerRegistry) String() string { + return "ghcr.io" +} + // Match checks if the repository details matches current provider format. func (c *githubContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "ghcr.io") diff --git a/docker/registry/internal/gitlab.go b/docker/registry/internal/gitlab.go index 591043703fd..7addcbc4bd7 100644 --- a/docker/registry/internal/gitlab.go +++ b/docker/registry/internal/gitlab.go @@ -19,6 +19,10 @@ func newGitlabContainerRegistry(repoDetails docker.ImageRepoDetails, transport h return &gitlabContainerRegistry{c} } +func (c *gitlabContainerRegistry) String() string { + return "registry.gitlab.com" +} + // Match checks if the repository details matches current provider format. func (c *gitlabContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "registry.gitlab.com") diff --git a/docker/registry/internal/interface.go b/docker/registry/internal/interface.go index f37fc34a820..a047ccf3468 100644 --- a/docker/registry/internal/interface.go +++ b/docker/registry/internal/interface.go @@ -14,12 +14,13 @@ import ( // Registry provides APIs to interact with the OCI provider client. type Registry interface { + String() string Tags(string) (tools.Versions, error) GetArchitecture(imageName, tag string) (string, error) Close() error Ping() error ImageRepoDetails() docker.ImageRepoDetails - ShouldRefreshAuth() (bool, *time.Duration) + ShouldRefreshAuth() (bool, time.Duration) RefreshAuth() error } diff --git a/docker/registry/internal/provider.go b/docker/registry/internal/provider.go index 4b612b796c2..99954506e53 100644 --- a/docker/registry/internal/provider.go +++ b/docker/registry/internal/provider.go @@ -20,13 +20,13 @@ func NewBase(repoDetails docker.ImageRepoDetails, transport http.RoundTripper) * func Providers() []func(docker.ImageRepoDetails, http.RoundTripper) RegistryInternal { return []func(docker.ImageRepoDetails, http.RoundTripper) RegistryInternal{ newAzureContainerRegistry, - newDockerhub, newGitlabContainerRegistry, newGithubContainerRegistry, newQuayContainerRegistry, newGoogleContainerRegistry, newElasticContainerRegistry, newElasticContainerRegistryPublic, + newDockerhub, // DockerHub must be last as it matches on default domain. } } diff --git a/docker/registry/internal/quay.go b/docker/registry/internal/quay.go index 4466c45fb38..c58c5262d62 100644 --- a/docker/registry/internal/quay.go +++ b/docker/registry/internal/quay.go @@ -21,6 +21,10 @@ func newQuayContainerRegistry(repoDetails docker.ImageRepoDetails, transport htt return &quayContainerRegistry{c} } +func (c *quayContainerRegistry) String() string { + return "quay.io" +} + // Match checks if the repository details matches current provider format. func (c *quayContainerRegistry) Match() bool { return strings.Contains(c.repoDetails.ServerAddress, "quay.io") diff --git a/docker/registry/internal/transports.go b/docker/registry/internal/transports.go index 5d9450dcd08..a28957d5dd7 100644 --- a/docker/registry/internal/transports.go +++ b/docker/registry/internal/transports.go @@ -16,6 +16,17 @@ import ( "github.com/juju/errors" ) +type dynamicTransportFunc func() (http.RoundTripper, error) + +// RoundTrip executes a single HTTP transaction, returning a Response for the provided Request. +func (f dynamicTransportFunc) RoundTrip(req *http.Request) (*http.Response, error) { + transport, err := f() + if err != nil { + return nil, err + } + return transport.RoundTrip(req) +} + type basicTransport struct { transport http.RoundTripper username string diff --git a/docker/registry/mocks/registry_mock.go b/docker/registry/mocks/registry_mock.go index 87282ad68a6..05b72b19c72 100644 --- a/docker/registry/mocks/registry_mock.go +++ b/docker/registry/mocks/registry_mock.go @@ -108,11 +108,11 @@ func (mr *MockRegistryMockRecorder) RefreshAuth() *gomock.Call { } // ShouldRefreshAuth mocks base method. -func (m *MockRegistry) ShouldRefreshAuth() (bool, *time.Duration) { +func (m *MockRegistry) ShouldRefreshAuth() (bool, time.Duration) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ShouldRefreshAuth") ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(*time.Duration) + ret1, _ := ret[1].(time.Duration) return ret0, ret1 } @@ -122,6 +122,20 @@ func (mr *MockRegistryMockRecorder) ShouldRefreshAuth() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldRefreshAuth", reflect.TypeOf((*MockRegistry)(nil).ShouldRefreshAuth)) } +// String mocks base method. +func (m *MockRegistry) String() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "String") + ret0, _ := ret[0].(string) + return ret0 +} + +// String indicates an expected call of String. +func (mr *MockRegistryMockRecorder) String() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "String", reflect.TypeOf((*MockRegistry)(nil).String)) +} + // Tags mocks base method. func (m *MockRegistry) Tags(arg0 string) (tools.Versions, error) { m.ctrl.T.Helper() diff --git a/docker/registry/registry_test.go b/docker/registry/registry_test.go index 5f9865b65aa..63d9b9e81a1 100644 --- a/docker/registry/registry_test.go +++ b/docker/registry/registry_test.go @@ -4,12 +4,55 @@ package registry_test import ( - "github.com/juju/testing" + jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" + + "github.com/juju/juju/docker" + "github.com/juju/juju/docker/registry" ) type registrySuite struct { - testing.IsolationSuite } var _ = gc.Suite(®istrySuite{}) + +func (s *registrySuite) TestSelectsAWSPrivate(c *gc.C) { + reg, err := registry.New(docker.ImageRepoDetails{ + Repository: "123456.dkr.ecr.eu-west-1.amazonaws.com", + BasicAuthConfig: docker.BasicAuthConfig{ + Username: "access key id", + Password: "secret key", + }, + Region: "us-west-1", + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(reg, gc.NotNil) + c.Assert(reg.String(), gc.Equals, "*.dkr.ecr.*.amazonaws.com") +} + +func (s *registrySuite) TestSelectsDockerHub(c *gc.C) { + reg, err := registry.New(docker.ImageRepoDetails{ + Repository: "jujusolutions", + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(reg, gc.NotNil) + c.Assert(reg.String(), gc.Equals, "docker.io") +} + +func (s *registrySuite) TestSelectsGithubContainerRegistry(c *gc.C) { + reg, err := registry.New(docker.ImageRepoDetails{ + Repository: "ghcr.io/juju", + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(reg, gc.NotNil) + c.Assert(reg.String(), gc.Equals, "ghcr.io") +} + +func (s *registrySuite) TestSelectsAWSPublic(c *gc.C) { + reg, err := registry.New(docker.ImageRepoDetails{ + Repository: "public.ecr.aws/juju", + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(reg, gc.NotNil) + c.Assert(reg.String(), gc.Equals, "public.ecr.aws") +} diff --git a/make_functions.sh b/make_functions.sh index eeba157b49f..762a51af7a8 100755 --- a/make_functions.sh +++ b/make_functions.sh @@ -9,6 +9,7 @@ JUJUD_BIN_DIR=${JUJUD_BIN_DIR:-${BUILD_DIR}/bin} # Versioning variables JUJU_BUILD_NUMBER=${JUJU_BUILD_NUMBER:-} +JUJU_DB_VERSION=${JUJU_DB_VERSION:-} # Docker variables OCI_BUILDER=${OCI_BUILDER:-docker} @@ -149,6 +150,23 @@ build_push_operator_image() { fi } +seed_repository() { + set -x + docker pull "docker.io/jujusolutions/juju-db:${JUJU_DB_VERSION}" + docker tag "docker.io/jujusolutions/juju-db:${JUJU_DB_VERSION}" "${DOCKER_USERNAME}/juju-db:${JUJU_DB_VERSION}" + docker push "${DOCKER_USERNAME}/juju-db:${JUJU_DB_VERSION}" + + # copy all the lts that are available + for (( i = 18; ; i += 2 )); do + if docker pull "docker.io/jujusolutions/charm-base:ubuntu-$i.04" ; then + docker tag "docker.io/jujusolutions/charm-base:ubuntu-$i.04" "${DOCKER_USERNAME}/charm-base:ubuntu-$i.04" + docker push "${DOCKER_USERNAME}/charm-base:ubuntu-$i.04" + else + break + fi + done +} + wait_for_dpkg() { # Just in case, wait for cloud-init. cloud-init status --wait 2> /dev/null || true diff --git a/provider/equinix/environ.go b/provider/equinix/environ.go index 146456f91e5..11230cca46b 100644 --- a/provider/equinix/environ.go +++ b/provider/equinix/environ.go @@ -26,7 +26,6 @@ import ( "github.com/juju/juju/cloudconfig/cloudinit" "github.com/juju/juju/cloudconfig/instancecfg" "github.com/juju/juju/cloudconfig/providerinit" - "github.com/juju/juju/cmd/juju/commands" "github.com/juju/juju/core/constraints" "github.com/juju/juju/core/instance" "github.com/juju/juju/core/network" @@ -46,6 +45,8 @@ import ( var logger = loggo.GetLogger("juju.provider.equinix") +const sshPort = 22 + type environConfig struct { config *config.Config attrs map[string]interface{} @@ -286,8 +287,8 @@ func getCloudConfig(args environs.StartInstanceParams) (cloudinit.CloudConfig, e "iptables -A INPUT -i lo -j ACCEPT", "iptables -A OUTPUT -o lo -j ACCEPT", "iptables -P INPUT ! -i lo -s 127.0.0.0/8 -j REJECT", - fmt.Sprintf("iptables -A OUTPUT -p tcp --sport %d -m conntrack --ctstate ESTABLISHED -j ACCEPT", commands.SSHPort), - fmt.Sprintf(acceptInputPort, commands.SSHPort), + fmt.Sprintf("iptables -A OUTPUT -p tcp --sport %d -m conntrack --ctstate ESTABLISHED -j ACCEPT", sshPort), + fmt.Sprintf(acceptInputPort, sshPort), } if args.InstanceConfig.IsController() { for _, port := range []int{ diff --git a/tests/README.md b/tests/README.md index 23ee577fcd1..3603541dda4 100644 --- a/tests/README.md +++ b/tests/README.md @@ -58,7 +58,7 @@ sudo snap install expect The static analysis tests also require `golangci-lint`: ``` -go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.0 +go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.2 ``` To get started, it's best to quickly look at the help command from the runner. diff --git a/tests/suites/static_analysis/lint_go.sh b/tests/suites/static_analysis/lint_go.sh index cea4754b975..059870bc032 100644 --- a/tests/suites/static_analysis/lint_go.sh +++ b/tests/suites/static_analysis/lint_go.sh @@ -1,7 +1,7 @@ run_go() { VER=$(golangci-lint --version | tr -s ' ' | cut -d ' ' -f 4 | cut -d '.' -f 1,2) - if [[ ${VER} != "1.53" ]] && [[ ${VER} != "v1.53" ]]; then - (echo >&2 -e '\nError: golangci-lint version does not match 1.53. Please upgrade/downgrade to the right version.') + if [[ ${VER} != "1.54" ]] && [[ ${VER} != "v1.54" ]]; then + (echo >&2 -e '\nError: golangci-lint version does not match 1.54. Please upgrade/downgrade to the right version.') exit 1 fi OUT=$(golangci-lint run -c .github/golangci-lint.config.yaml 2>&1) diff --git a/worker/caasmodelconfigmanager/mocks/facade_mock.go b/worker/caasmodelconfigmanager/mocks/facade_mock.go index 1df25fded1f..4f2ef40d481 100644 --- a/worker/caasmodelconfigmanager/mocks/facade_mock.go +++ b/worker/caasmodelconfigmanager/mocks/facade_mock.go @@ -8,6 +8,7 @@ import ( reflect "reflect" controller "github.com/juju/juju/controller" + watcher "github.com/juju/juju/core/watcher" gomock "go.uber.org/mock/gomock" ) @@ -48,3 +49,18 @@ func (mr *MockFacadeMockRecorder) ControllerConfig() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerConfig", reflect.TypeOf((*MockFacade)(nil).ControllerConfig)) } + +// WatchControllerConfig mocks base method. +func (m *MockFacade) WatchControllerConfig() (watcher.NotifyWatcher, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WatchControllerConfig") + ret0, _ := ret[0].(watcher.NotifyWatcher) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// WatchControllerConfig indicates an expected call of WatchControllerConfig. +func (mr *MockFacadeMockRecorder) WatchControllerConfig() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WatchControllerConfig", reflect.TypeOf((*MockFacade)(nil).WatchControllerConfig)) +} diff --git a/worker/caasmodelconfigmanager/worker.go b/worker/caasmodelconfigmanager/worker.go index b71c0ff837e..ac6c851ae67 100644 --- a/worker/caasmodelconfigmanager/worker.go +++ b/worker/caasmodelconfigmanager/worker.go @@ -4,6 +4,7 @@ package caasmodelconfigmanager import ( + "reflect" "time" "github.com/juju/clock" @@ -16,10 +17,16 @@ import ( "github.com/juju/juju/api/base" api "github.com/juju/juju/api/controller/caasmodelconfigmanager" "github.com/juju/juju/controller" + "github.com/juju/juju/core/watcher" "github.com/juju/juju/docker" "github.com/juju/juju/docker/registry" ) +const ( + retryDuration = 1 * time.Second + refreshDuration = 30 * time.Second +) + // Logger represents the methods used by the worker to log details. type Logger interface { Debugf(string, ...interface{}) @@ -34,6 +41,7 @@ type Logger interface { //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/facade_mock.go github.com/juju/juju/worker/caasmodelconfigmanager Facade type Facade interface { ControllerConfig() (controller.Config, error) + WatchControllerConfig() (watcher.NotifyWatcher, error) } //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/broker_mock.go github.com/juju/juju/worker/caasmodelconfigmanager CAASBroker @@ -85,10 +93,6 @@ type manager struct { clock clock.Clock registryFunc func(docker.ImageRepoDetails) (registry.Registry, error) - reg registry.Registry - - nextTickDuration *time.Duration - ticker clock.Timer } // NewFacade returns a facade for caasapplicationprovisioner worker to use. @@ -129,78 +133,102 @@ func (w *manager) Wait() error { } func (w *manager) loop() (err error) { - defer func() { - if w.ticker != nil && !w.ticker.Stop() { - select { - case <-w.ticker.Chan(): - default: - } - } - }() - controllerConfig, err := w.config.Facade.ControllerConfig() + watcher, err := w.config.Facade.WatchControllerConfig() if err != nil { return errors.Trace(err) } - repoDetails := controllerConfig.CAASImageRepo() - if !repoDetails.IsPrivate() { - // No ops for public registry config. - return nil - } - w.reg, err = w.registryFunc(repoDetails) + err = w.catacomb.Add(watcher) if err != nil { return errors.Trace(err) } - if err = w.reg.Ping(); err != nil { - return errors.Trace(err) - } - if err := w.ensureImageRepoSecret(true); err != nil { - return errors.Trace(err) - } + + var ( + refresh <-chan struct{} + timeout <-chan time.Time + deadline time.Time + reg registry.Registry + lastRepoDetails docker.ImageRepoDetails + ) + first := false + signal := make(chan struct{}) + close(signal) + defer func() { + if reg != nil { + _ = reg.Close() + } + }() for { select { case <-w.catacomb.Dying(): return w.catacomb.ErrDying() - case <-w.getTickerChan(): - if err := w.ensureImageRepoSecret(false); err != nil { + case <-watcher.Changes(): + controllerConfig, err := w.config.Facade.ControllerConfig() + if err != nil { return errors.Trace(err) } - } - } -} - -func (w *manager) getTickerChan() <-chan time.Time { - d := w.getTickerDuration() - if w.ticker == nil { - w.ticker = w.clock.NewTimer(d) - } else { - if !w.ticker.Stop() { - select { - case <-w.ticker.Chan(): - default: + repoDetails := controllerConfig.CAASImageRepo() + if reflect.DeepEqual(repoDetails, lastRepoDetails) { + continue + } + lastRepoDetails = repoDetails + if !repoDetails.IsPrivate() { + timeout = nil + refresh = nil + continue + } + if reg != nil { + _ = reg.Close() + } + reg, err = w.registryFunc(repoDetails) + if err != nil { + return errors.Trace(err) + } + if err = reg.Ping(); err != nil { + return errors.Trace(err) + } + first = true + refresh = signal + case <-timeout: + timeout = nil + if refresh == nil { + refresh = signal + } + case <-refresh: + refresh = nil + next, err := w.ensureImageRepoSecret(reg, first) + if err != nil { + w.logger.Errorf("failed to update repository secret: %s", err.Error()) + next = retryDuration + } else { + first = false + } + if nextDeadline := w.clock.Now().Add(next); timeout == nil || nextDeadline.Before(deadline) { + deadline = nextDeadline + timeout = w.clock.After(next) } } - w.ticker.Reset(d) } - return w.ticker.Chan() } -func (w *manager) getTickerDuration() time.Duration { - if w.nextTickDuration != nil { - return *w.nextTickDuration +func (w *manager) ensureImageRepoSecret(reg registry.Registry, force bool) (time.Duration, error) { + shouldRefresh, nextRefresh := reg.ShouldRefreshAuth() + if nextRefresh == time.Duration(0) { + nextRefresh = refreshDuration + } + if !shouldRefresh && !force { + return nextRefresh, nil } - return 30 * time.Second -} -func (w *manager) ensureImageRepoSecret(isFirstCall bool) error { - var shouldRefresh bool - if shouldRefresh, w.nextTickDuration = w.reg.ShouldRefreshAuth(); !shouldRefresh && !isFirstCall { - return nil + w.logger.Debugf("refreshing auth token for %q", w.name) + if err := reg.RefreshAuth(); err != nil { + return time.Duration(0), errors.Annotatef(err, "refreshing registry auth token for %q", w.name) } - if err := w.reg.RefreshAuth(); err != nil { - return errors.Annotatef(err, "refreshing registry auth token for %q", w.name) + + w.logger.Debugf("applying refreshed auth token for %q", w.name) + err := w.config.Broker.EnsureImageRepoSecret(reg.ImageRepoDetails()) + if err != nil { + return time.Duration(0), errors.Annotatef(err, "ensuring image repository secret for %q", w.name) } - w.logger.Debugf("auth token for %q has been refreshed, applying to the secret now", w.name) - err := w.config.Broker.EnsureImageRepoSecret(w.reg.ImageRepoDetails()) - return errors.Annotatef(err, "ensuring image repository secret for %q", w.name) + return nextRefresh, nil } diff --git a/worker/caasmodelconfigmanager/worker_test.go b/worker/caasmodelconfigmanager/worker_test.go index 64fb81cbb90..1a708630d14 100644 --- a/worker/caasmodelconfigmanager/worker_test.go +++ b/worker/caasmodelconfigmanager/worker_test.go @@ -4,7 +4,6 @@ package caasmodelconfigmanager_test import ( - "encoding/base64" "time" "github.com/juju/clock/testclock" @@ -18,6 +17,8 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/controller" + "github.com/juju/juju/core/watcher" + "github.com/juju/juju/core/watcher/watchertest" "github.com/juju/juju/docker" "github.com/juju/juju/docker/registry" registrymocks "github.com/juju/juju/docker/registry/mocks" @@ -37,7 +38,7 @@ type workerSuite struct { facade *mocks.MockFacade broker *mocks.MockCAASBroker reg *registrymocks.MockRegistry - clock *testclock.Clock + clock testclock.AdvanceableClock controllerConfig controller.Config } @@ -46,7 +47,7 @@ func (s *workerSuite) SetUpTest(c *gc.C) { s.modelTag = names.NewModelTag("ffffffff-ffff-ffff-ffff-ffffffffffff") s.logger = loggo.GetLogger("test") s.controllerConfig = coretesting.FakeControllerConfig() - s.clock = testclock.NewClock(time.Time{}) + s.clock = testclock.NewDilatedWallClock(testing.ShortWait) } func (s *workerSuite) TearDownTest(c *gc.C) { @@ -121,7 +122,7 @@ func (s *workerSuite) getWorkerStarter(c *gc.C) (func(...*gomock.Call) worker.Wo Broker: s.broker, Clock: s.clock, RegistryFunc: func(i docker.ImageRepoDetails) (registry.Registry, error) { - c.Assert(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) + c.Check(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) return s.reg, nil }, } @@ -129,9 +130,6 @@ func (s *workerSuite) getWorkerStarter(c *gc.C) (func(...*gomock.Call) worker.Wo gomock.InOrder(calls...) w, err := caasmodelconfigmanager.NewWorker(cfg) c.Assert(err, jc.ErrorIsNil) - s.AddCleanup(func(c *gc.C) { - workertest.CleanKill(c, w) - }) return w }, ctrl } @@ -153,54 +151,53 @@ func (s *workerSuite) TestWorkerTokenRefreshRequired(c *gc.C) { startWorker, ctrl := s.getWorkerStarter(c) defer ctrl.Finish() - _ = startWorker( + controllerConfigChangedChan := make(chan struct{}, 1) + w := startWorker( + s.facade.EXPECT().WatchControllerConfig().DoAndReturn(func() (watcher.NotifyWatcher, error) { + controllerConfigChangedChan <- struct{}{} + return watchertest.NewMockNotifyWatcher(controllerConfigChangedChan), nil + }), // 1st round. s.facade.EXPECT().ControllerConfig().Return(s.controllerConfig, nil), s.reg.EXPECT().Ping().Return(nil), - s.reg.EXPECT().ShouldRefreshAuth().Return(true, nil), + s.reg.EXPECT().ShouldRefreshAuth().Return(true, time.Duration(0)), s.reg.EXPECT().RefreshAuth().Return(nil), - s.reg.EXPECT().ImageRepoDetails().DoAndReturn( - func() docker.ImageRepoDetails { - o := s.controllerConfig.CAASImageRepo() - c.Assert(o, gc.DeepEquals, docker.ImageRepoDetails{ - ServerAddress: "66668888.dkr.ecr.eu-west-1.amazonaws.com", - Repository: "66668888.dkr.ecr.eu-west-1.amazonaws.com", - Region: "ap-southeast-2", - BasicAuthConfig: docker.BasicAuthConfig{ - Username: "aws_access_key_id", - Password: "aws_secret_access_key", - Auth: docker.NewToken(base64.StdEncoding.EncodeToString([]byte("aws_access_key_id:aws_secret_access_key"))), - }, - }) - return o - }, - ), - s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn( - func(i docker.ImageRepoDetails) error { - c.Assert(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) - return nil - }, - ), + s.reg.EXPECT().ImageRepoDetails().DoAndReturn(func() docker.ImageRepoDetails { + o := s.controllerConfig.CAASImageRepo() + c.Check(o, gc.DeepEquals, docker.ImageRepoDetails{ + ServerAddress: "66668888.dkr.ecr.eu-west-1.amazonaws.com", + Repository: "66668888.dkr.ecr.eu-west-1.amazonaws.com", + Region: "ap-southeast-2", + BasicAuthConfig: docker.BasicAuthConfig{ + Username: "aws_access_key_id", + Password: "aws_secret_access_key", + }, + }) + return o + }), + s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn(func(i docker.ImageRepoDetails) error { + c.Check(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) + return nil + }), // 2nd round. - s.reg.EXPECT().ShouldRefreshAuth().Return(true, nil), + s.reg.EXPECT().ShouldRefreshAuth().Return(true, time.Duration(0)), s.reg.EXPECT().RefreshAuth().Return(nil), s.reg.EXPECT().ImageRepoDetails().Return(refreshed), - s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn( - func(i docker.ImageRepoDetails) error { - c.Assert(i, gc.DeepEquals, refreshed) - close(done) - return nil - }, - ), + s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn(func(i docker.ImageRepoDetails) error { + c.Check(i, gc.DeepEquals, refreshed) + close(done) + return nil + }), + s.reg.EXPECT().Close().Return(nil), ) - err := s.clock.WaitAdvance(30*time.Second, coretesting.ShortWait, 1) - c.Assert(err, jc.ErrorIsNil) select { case <-done: case <-time.After(coretesting.LongWait): c.Fatalf("timed out waiting for worker to start") } + + workertest.CleanKill(c, w) } func (s *workerSuite) TestWorkerTokenRefreshNotRequiredThenRetry(c *gc.C) { @@ -217,63 +214,59 @@ func (s *workerSuite) TestWorkerTokenRefreshNotRequiredThenRetry(c *gc.C) { startWorker, ctrl := s.getWorkerStarter(c) defer ctrl.Finish() - _ = startWorker( + controllerConfigChangedChan := make(chan struct{}, 1) + w := startWorker( + s.facade.EXPECT().WatchControllerConfig().DoAndReturn(func() (watcher.NotifyWatcher, error) { + controllerConfigChangedChan <- struct{}{} + return watchertest.NewMockNotifyWatcher(controllerConfigChangedChan), nil + }), // 1st round. s.facade.EXPECT().ControllerConfig().Return(s.controllerConfig, nil), s.reg.EXPECT().Ping().Return(nil), - s.reg.EXPECT().ShouldRefreshAuth().Return(true, nil), + s.reg.EXPECT().ShouldRefreshAuth().Return(true, time.Duration(0)), s.reg.EXPECT().RefreshAuth().Return(nil), - s.reg.EXPECT().ImageRepoDetails().DoAndReturn( - func() docker.ImageRepoDetails { - o := s.controllerConfig.CAASImageRepo() - c.Assert(o, gc.DeepEquals, docker.ImageRepoDetails{ - ServerAddress: "66668888.dkr.ecr.eu-west-1.amazonaws.com", - Repository: "66668888.dkr.ecr.eu-west-1.amazonaws.com", - Region: "ap-southeast-2", - BasicAuthConfig: docker.BasicAuthConfig{ - Username: "aws_access_key_id", - Password: "aws_secret_access_key", - Auth: docker.NewToken(base64.StdEncoding.EncodeToString([]byte("aws_access_key_id:aws_secret_access_key"))), - }, - }) - return o - }, - ), - s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn( - func(i docker.ImageRepoDetails) error { - c.Assert(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) - return nil - }, - ), + s.reg.EXPECT().ImageRepoDetails().DoAndReturn(func() docker.ImageRepoDetails { + o := s.controllerConfig.CAASImageRepo() + c.Check(o, gc.DeepEquals, docker.ImageRepoDetails{ + ServerAddress: "66668888.dkr.ecr.eu-west-1.amazonaws.com", + Repository: "66668888.dkr.ecr.eu-west-1.amazonaws.com", + Region: "ap-southeast-2", + BasicAuthConfig: docker.BasicAuthConfig{ + Username: "aws_access_key_id", + Password: "aws_secret_access_key", + }, + }) + return o + }), + s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn(func(i docker.ImageRepoDetails) error { + c.Check(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) + return nil + }), // 2nd round. - s.reg.EXPECT().ShouldRefreshAuth().DoAndReturn(func() (bool, *time.Duration) { - nextTick := 7 * time.Minute - return false, &nextTick + s.reg.EXPECT().ShouldRefreshAuth().DoAndReturn(func() (bool, time.Duration) { + return false, 1 * time.Second }), // 3rd round. - s.reg.EXPECT().ShouldRefreshAuth().DoAndReturn(func() (bool, *time.Duration) { - return true, nil + s.reg.EXPECT().ShouldRefreshAuth().DoAndReturn(func() (bool, time.Duration) { + return true, time.Duration(0) }), s.reg.EXPECT().RefreshAuth().Return(nil), s.reg.EXPECT().ImageRepoDetails().Return(s.controllerConfig.CAASImageRepo()), - s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn( - func(i docker.ImageRepoDetails) error { - c.Assert(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) - close(done) - return nil - }, - ), + s.broker.EXPECT().EnsureImageRepoSecret(gomock.Any()).DoAndReturn(func(i docker.ImageRepoDetails) error { + c.Check(i, gc.DeepEquals, s.controllerConfig.CAASImageRepo()) + close(done) + return nil + }), + s.reg.EXPECT().Close().Return(nil), ) - err := s.clock.WaitAdvance(30*time.Second, coretesting.ShortWait, 1) - c.Assert(err, jc.ErrorIsNil) - err = s.clock.WaitAdvance(7*time.Minute, coretesting.ShortWait, 1) - c.Assert(err, jc.ErrorIsNil) select { case <-done: case <-time.After(coretesting.LongWait): c.Fatalf("timed out waiting for worker to start") } + + workertest.CleanKill(c, w) } func (s *workerSuite) TestWorkerNoOpsForPublicRepo(c *gc.C) { @@ -288,7 +281,12 @@ func (s *workerSuite) TestWorkerNoOpsForPublicRepo(c *gc.C) { startWorker, ctrl := s.getWorkerStarter(c) defer ctrl.Finish() - _ = startWorker( + controllerConfigChangedChan := make(chan struct{}, 1) + w := startWorker( + s.facade.EXPECT().WatchControllerConfig().DoAndReturn(func() (watcher.NotifyWatcher, error) { + controllerConfigChangedChan <- struct{}{} + return watchertest.NewMockNotifyWatcher(controllerConfigChangedChan), nil + }), s.facade.EXPECT().ControllerConfig().DoAndReturn(func() (controller.Config, error) { close(done) return s.controllerConfig, nil @@ -300,4 +298,6 @@ func (s *workerSuite) TestWorkerNoOpsForPublicRepo(c *gc.C) { case <-time.After(coretesting.LongWait): c.Fatalf("timed out waiting for worker to start") } + + workertest.CleanKill(c, w) }