Merge pull request #19838 from deads2k/controller-25-pullsecret

add internal and external URL handling for the docker pull secret

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(),

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,

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.

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{}) {

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

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

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()...)

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],

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)