diff --git a/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go b/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go index 7c578bc36436..065ded632c79 100644 --- a/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go +++ b/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go @@ -68,9 +68,10 @@ func RunServiceAccountPullSecretsController(ctx ControllerContext) (bool, error) go dockercfgController.Run(5, ctx.Stop) dockerRegistryControllerOptions := serviceaccountcontrollers.DockerRegistryServiceControllerOptions{ - DockercfgController: dockercfgController, - DockerURLsInitialized: dockerURLsInitialized, - ClusterDNSSuffix: "cluster.local", + DockercfgController: dockercfgController, + DockerURLsInitialized: dockerURLsInitialized, + ClusterDNSSuffix: "cluster.local", + AdditionalRegistryURLs: ctx.OpenshiftControllerConfig.DockerPullSecret.RegistryURLs, } go serviceaccountcontrollers.NewDockerRegistryServiceController( ctx.ExternalKubeInformers.Core().V1().Secrets(), diff --git a/pkg/cmd/openshift-controller-manager/conversion.go b/pkg/cmd/openshift-controller-manager/conversion.go index 4d024f02189b..adcf4979633c 100644 --- a/pkg/cmd/openshift-controller-manager/conversion.go +++ b/pkg/cmd/openshift-controller-manager/conversion.go @@ -25,6 +25,14 @@ func ConvertMasterConfigToOpenshiftControllerConfig(input *configapi.MasterConfi // deep copy to make sure no linger references are shared in := input.DeepCopy() + registryURLs := []string{} + if len(in.ImagePolicyConfig.ExternalRegistryHostname) > 0 { + registryURLs = append(registryURLs, in.ImagePolicyConfig.ExternalRegistryHostname) + } + if len(in.ImagePolicyConfig.InternalRegistryHostname) > 0 { + registryURLs = append(registryURLs, in.ImagePolicyConfig.InternalRegistryHostname) + } + ret := &configapi.OpenshiftControllerConfig{ ClientConnectionOverrides: in.MasterClients.OpenShiftLoopbackClientConnectionOverrides, ServingInfo: &in.ServingInfo, @@ -56,6 +64,9 @@ func ConvertMasterConfigToOpenshiftControllerConfig(input *configapi.MasterConfi ServiceAccount: configapi.ServiceAccountControllerConfig{ ManagedNames: in.ServiceAccountConfig.ManagedNames, }, + DockerPullSecret: configapi.DockerPullSecretControllerConfig{ + RegistryURLs: registryURLs, + }, Network: configapi.NetworkControllerConfig{ ClusterNetworks: in.NetworkConfig.ClusterNetworks, NetworkPluginName: in.NetworkConfig.NetworkPluginName, diff --git a/pkg/cmd/server/apis/config/types.go b/pkg/cmd/server/apis/config/types.go index a7e2f8423fbc..12b1dc964811 100644 --- a/pkg/cmd/server/apis/config/types.go +++ b/pkg/cmd/server/apis/config/types.go @@ -1504,6 +1504,7 @@ type OpenshiftControllerConfig struct { Deployer DeployerControllerConfig Build BuildControllerConfig ServiceAccount ServiceAccountControllerConfig + DockerPullSecret DockerPullSecretControllerConfig Network NetworkControllerConfig Ingress IngressControllerConfig ImageImport ImageImportControllerConfig @@ -1555,6 +1556,11 @@ type ServiceAccountControllerConfig struct { ManagedNames []string } +type DockerPullSecretControllerConfig struct { + // RegistryURLs is a list of urls that the docker pull secrets should be valid for. + RegistryURLs []string +} + type ImageImportControllerConfig struct { // MaxScheduledImageImportsPerMinute is the maximum number of image streams that will be imported in the background per minute. // The default value is 60. Set to -1 for unlimited. diff --git a/pkg/cmd/server/apis/config/zz_generated.deepcopy.go b/pkg/cmd/server/apis/config/zz_generated.deepcopy.go index 3537ebe77cf6..6cad933875a7 100644 --- a/pkg/cmd/server/apis/config/zz_generated.deepcopy.go +++ b/pkg/cmd/server/apis/config/zz_generated.deepcopy.go @@ -455,6 +455,27 @@ func (in *DockerConfig) DeepCopy() *DockerConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DockerPullSecretControllerConfig) DeepCopyInto(out *DockerPullSecretControllerConfig) { + *out = *in + if in.RegistryURLs != nil { + in, out := &in.RegistryURLs, &out.RegistryURLs + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DockerPullSecretControllerConfig. +func (in *DockerPullSecretControllerConfig) DeepCopy() *DockerPullSecretControllerConfig { + if in == nil { + return nil + } + out := new(DockerPullSecretControllerConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EtcdConfig) DeepCopyInto(out *EtcdConfig) { *out = *in @@ -1683,6 +1704,7 @@ func (in *OpenshiftControllerConfig) DeepCopyInto(out *OpenshiftControllerConfig out.Deployer = in.Deployer in.Build.DeepCopyInto(&out.Build) in.ServiceAccount.DeepCopyInto(&out.ServiceAccount) + in.DockerPullSecret.DeepCopyInto(&out.DockerPullSecret) in.Network.DeepCopyInto(&out.Network) out.Ingress = in.Ingress out.ImageImport = in.ImageImport diff --git a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go index 631c748fab44..11ec20e826f9 100644 --- a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go +++ b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go @@ -200,6 +200,9 @@ func (e *DockercfgController) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer e.queue.ShutDown() + glog.Infof("Starting DockercfgController controller") + defer glog.Infof("Shutting down DockercfgController controller") + // Wait for the store to sync before starting any work in this controller. ready := make(chan struct{}) go e.waitForDockerURLs(ready, stopCh) @@ -208,18 +211,18 @@ func (e *DockercfgController) Run(workers int, stopCh <-chan struct{}) { case <-stopCh: return } + glog.V(1).Infof("urls found") // Wait for the stores to fill if !cache.WaitForCacheSync(stopCh, e.serviceAccountController.HasSynced, e.secretController.HasSynced) { return } + glog.V(1).Infof("caches synced") - glog.V(5).Infof("Starting workers") for i := 0; i < workers; i++ { go wait.Until(e.worker, time.Second, stopCh) } <-stopCh - glog.V(1).Infof("Shutting down") } func (c *DockercfgController) waitForDockerURLs(ready chan<- struct{}, stopCh <-chan struct{}) { diff --git a/pkg/serviceaccounts/controllers/deleted_dockercfg_secrets.go b/pkg/serviceaccounts/controllers/deleted_dockercfg_secrets.go index 0a051df180c2..4700aa8bad23 100644 --- a/pkg/serviceaccounts/controllers/deleted_dockercfg_secrets.go +++ b/pkg/serviceaccounts/controllers/deleted_dockercfg_secrets.go @@ -65,15 +65,16 @@ type DockercfgDeletedController struct { // Run processes the queue. func (e *DockercfgDeletedController) Run(stopCh <-chan struct{}) { defer utilruntime.HandleCrash() + glog.Infof("Starting DockercfgDeletedController controller") + defer glog.Infof("Shutting down DockercfgDeletedController controller") // Wait for the stores to fill if !cache.WaitForCacheSync(stopCh, e.secretController.HasSynced) { return } + glog.V(1).Infof("caches synced") - glog.V(5).Infof("Worker started") <-stopCh - glog.V(1).Infof("Shutting down") } // secretDeleted reacts to a Secret being deleted by looking to see if it's a dockercfg secret for a service account, in which case it diff --git a/pkg/serviceaccounts/controllers/deleted_token_secrets.go b/pkg/serviceaccounts/controllers/deleted_token_secrets.go index 9b2cb7bc102b..65157715ef17 100644 --- a/pkg/serviceaccounts/controllers/deleted_token_secrets.go +++ b/pkg/serviceaccounts/controllers/deleted_token_secrets.go @@ -62,15 +62,16 @@ type DockercfgTokenDeletedController struct { // Runs controller loops and returns on shutdown func (e *DockercfgTokenDeletedController) Run(stopCh <-chan struct{}) { defer utilruntime.HandleCrash() + glog.Infof("Starting DockercfgTokenDeletedController controller") + defer glog.Infof("Shutting down DockercfgTokenDeletedController controller") // Wait for the stores to fill if !cache.WaitForCacheSync(stopCh, e.secretController.HasSynced) { return } + glog.V(1).Infof("caches synced") - glog.V(5).Infof("Worker started") <-stopCh - glog.V(1).Infof("Shutting down") } // secretDeleted reacts to a token secret being deleted by looking for a corresponding dockercfg secret and deleting it if it exists diff --git a/pkg/serviceaccounts/controllers/docker_registry_service.go b/pkg/serviceaccounts/controllers/docker_registry_service.go index a948b5685b11..2db30ff3c3ac 100644 --- a/pkg/serviceaccounts/controllers/docker_registry_service.go +++ b/pkg/serviceaccounts/controllers/docker_registry_service.go @@ -34,6 +34,9 @@ type DockerRegistryServiceControllerOptions struct { DockercfgController *DockercfgController + // AdditionalRegistryURLs is a list of URLs that are always included + AdditionalRegistryURLs []string + // DockerURLsInitialized is used to send a signal to the DockercfgController that it has the correct set of docker urls DockerURLsInitialized chan struct{} } @@ -51,12 +54,13 @@ var serviceLocations = []serviceLocation{ // NewDockerRegistryServiceController returns a new *DockerRegistryServiceController. func NewDockerRegistryServiceController(secrets informers.SecretInformer, serviceInformer informers.ServiceInformer, cl kclientset.Interface, options DockerRegistryServiceControllerOptions) *DockerRegistryServiceController { e := &DockerRegistryServiceController{ - client: cl, - clusterDNSSuffix: options.ClusterDNSSuffix, - dockercfgController: options.DockercfgController, - registryLocationQueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), - secretsToUpdate: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), - dockerURLsInitialized: options.DockerURLsInitialized, + client: cl, + additionalRegistryURLs: options.AdditionalRegistryURLs, + clusterDNSSuffix: options.ClusterDNSSuffix, + dockercfgController: options.DockercfgController, + registryLocationQueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), + secretsToUpdate: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), + dockerURLsInitialized: options.DockerURLsInitialized, } // we're only watching two of these, but we already watch all services for the service serving cert signer @@ -104,6 +108,8 @@ type DockerRegistryServiceController struct { // clusterDNSSuffix is the suffix for in cluster DNS that can be added to service names clusterDNSSuffix string + // additionalRegistryURLs is a list of URLs that are always included + additionalRegistryURLs []string dockercfgController *DockercfgController @@ -134,6 +140,9 @@ func (e *DockerRegistryServiceController) Run(workers int, stopCh <-chan struct{ defer utilruntime.HandleCrash() defer e.registryLocationQueue.ShutDown() + glog.Infof("Starting DockerRegistryServiceController controller") + defer glog.Infof("Shutting down DockerRegistryServiceController controller") + // Wait for the store to sync before starting any work in this controller. ready := make(chan struct{}) go e.waitForDockerURLs(ready, stopCh) @@ -142,14 +151,13 @@ func (e *DockerRegistryServiceController) Run(workers int, stopCh <-chan struct{ case <-stopCh: return } + glog.V(1).Infof("caches synced") - glog.V(5).Infof("Starting workers") go wait.Until(e.watchForDockerURLChanges, time.Second, stopCh) for i := 0; i < workers; i++ { go wait.Until(e.watchForDockercfgSecretUpdates, time.Second, stopCh) } <-stopCh - glog.V(1).Infof("Shutting down") } // enqueue adds to our queue. We only have one entry, but we never have to check it since we already know the things @@ -225,10 +233,11 @@ func (e *DockerRegistryServiceController) watchForDockerURLChanges() { // getDockerRegistryLocations returns the dns form and the ip form of the secret func (e *DockerRegistryServiceController) getDockerRegistryLocations() []string { - ret := []string{} + ret := append([]string{}, e.additionalRegistryURLs...) for _, location := range serviceLocations { ret = append(ret, getDockerRegistryLocations(e.serviceLister, location, e.clusterDNSSuffix)...) } + glog.V(4).Infof("found docker registry urls: %v", ret) return ret } @@ -260,9 +269,10 @@ func (e *DockerRegistryServiceController) syncRegistryLocationChange() error { newDockerRegistryLocations := sets.NewString(newLocations...) existingURLs := e.getRegistryURLs() if existingURLs.Equal(newDockerRegistryLocations) && e.initialSecretsCheckDone { - glog.V(4).Infof("No effective update: %v", newDockerRegistryLocations) + glog.V(3).Infof("No effective update: %v", newDockerRegistryLocations) return nil } + glog.V(1).Infof("Updating registry URLs from %v to %v", existingURLs, newDockerRegistryLocations) // make sure that new dockercfg secrets get the correct locations e.dockercfgController.SetDockerURLs(newDockerRegistryLocations.List()...) diff --git a/pkg/serviceaccounts/controllers/docker_registry_service_test.go b/pkg/serviceaccounts/controllers/docker_registry_service_test.go index 5b1fa3ba6496..28a5c30be31e 100644 --- a/pkg/serviceaccounts/controllers/docker_registry_service_test.go +++ b/pkg/serviceaccounts/controllers/docker_registry_service_test.go @@ -123,7 +123,7 @@ func TestNoChangeNoOp(t *testing.T) { } } -func TestUpdateNewStyleSecretAndDNSSuffix(t *testing.T) { +func TestUpdateNewStyleSecretAndDNSSuffixAndAdditionalURLs(t *testing.T) { stopChannel := make(chan struct{}) defer close(stopChannel) received := make(chan bool) @@ -143,6 +143,8 @@ func TestUpdateNewStyleSecretAndDNSSuffix(t *testing.T) { kubeclient, fakeWatch, controller, informerFactory := controllerSetup([]runtime.Object{newStyleDockercfgSecret}, t, stopChannel) controller.clusterDNSSuffix = "something.else" + // this bit also tests the additional registryURL options + controller.additionalRegistryURLs = []string{"foo.bar.com"} controller.syncRegistryLocationHandler = wrapHandler(received, controller.syncRegistryLocationChange, t) controller.syncSecretHandler = wrapStringHandler(updatedSecret, controller.syncSecretUpdate, t) controller.initialSecretsCheckDone = false @@ -180,7 +182,7 @@ func TestUpdateNewStyleSecretAndDNSSuffix(t *testing.T) { } expectedDockercfgMap := credentialprovider.DockerConfig{} - for _, key := range []string{"172.16.123.123:1235", "docker-registry.default.svc:1235", "docker-registry.default.svc.something.else:1235"} { + for _, key := range []string{"foo.bar.com", "172.16.123.123:1235", "docker-registry.default.svc:1235", "docker-registry.default.svc.something.else:1235"} { expectedDockercfgMap[key] = credentialprovider.DockerConfigEntry{ Username: "serviceaccount", Password: newStyleDockercfgSecret.Annotations[ServiceAccountTokenValueAnnotation], diff --git a/test/integration/service_account_test.go b/test/integration/service_account_test.go index 17051db48723..c1d93ccae238 100644 --- a/test/integration/service_account_test.go +++ b/test/integration/service_account_test.go @@ -165,9 +165,15 @@ func TestAutomaticCreationOfPullSecrets(t *testing.T) { saNamespace := api.NamespaceDefault saName := serviceaccountadmission.DefaultServiceAccountName - masterConfig, clusterAdminConfig, err := testserver.StartTestMaster() + masterConfig, err := testserver.DefaultMasterOptions() if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("error creating config: %v", err) + } + masterConfig.ImagePolicyConfig.InternalRegistryHostname = "internal.registry.com:8080" + masterConfig.ImagePolicyConfig.ExternalRegistryHostname = "external.registry.com" + clusterAdminConfig, err := testserver.StartConfiguredMaster(masterConfig) + if err != nil { + t.Fatalf("error starting server: %v", err) } defer testserver.CleanupMasterEtcd(t, masterConfig) clusterAdminKubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminConfig) @@ -192,17 +198,23 @@ func TestAutomaticCreationOfPullSecrets(t *testing.T) { if len(saPullSecret) == 0 { t.Errorf("pull secret was not created") } + if !strings.Contains(saPullSecret, masterConfig.ImagePolicyConfig.InternalRegistryHostname) { + t.Errorf("missing %q in %v", masterConfig.ImagePolicyConfig.InternalRegistryHostname, saPullSecret) + } + if !strings.Contains(saPullSecret, masterConfig.ImagePolicyConfig.ExternalRegistryHostname) { + t.Errorf("missing %q in %v", masterConfig.ImagePolicyConfig.ExternalRegistryHostname, saPullSecret) + } } func waitForServiceAccountPullSecret(client kclientset.Interface, ns, name string, attempts int, interval time.Duration) (string, string, error) { for i := 0; i <= attempts; i++ { time.Sleep(interval) - secretName, token, err := getServiceAccountPullSecret(client, ns, name) + secretName, dockerCfg, err := getServiceAccountPullSecret(client, ns, name) if err != nil { return "", "", err } - if len(token) > 0 { - return secretName, token, nil + if len(dockerCfg) > 2 { + return secretName, dockerCfg, nil } } return "", "", nil @@ -317,9 +329,15 @@ func TestEnforcingServiceAccount(t *testing.T) { } func TestDockercfgTokenDeletedController(t *testing.T) { - masterConfig, clusterAdminConfig, err := testserver.StartTestMaster() + masterConfig, err := testserver.DefaultMasterOptions() if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("error creating config: %v", err) + } + masterConfig.ImagePolicyConfig.InternalRegistryHostname = "internal.registry.com:8080" + masterConfig.ImagePolicyConfig.ExternalRegistryHostname = "external.registry.com" + clusterAdminConfig, err := testserver.StartConfiguredMaster(masterConfig) + if err != nil { + t.Fatalf("error starting server: %v", err) } defer testserver.CleanupMasterEtcd(t, masterConfig) clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminConfig)