[release-4.7] Bug 1931856: Set registry routes in operand config

pkg/operator/configobservation/configobservercontroller/observe_config_controller.go

@@ -44,6 +44,7 @@ func NewConfigObserver(
 		},
 		[]factory.Informer{operatorConfigInformers.Operator().V1().OpenShiftControllerManagers().Informer()},
 		images.ObserveInternalRegistryHostname,
+		images.ObserveExternalRegistryHostnames,
 		builds.ObserveBuildControllerConfig,
 		network.ObserveExternalIPAutoAssignCIDRs,
 		deployimages.ObserveControllerManagerImagesConfig,

pkg/operator/configobservation/images/observe_images.go

@@ -52,3 +52,50 @@ func ObserveInternalRegistryHostname(genericListers configobserver.Listers, reco
 
 	return observedConfig, errs
 }
+
+// ObserveExternalRegistryHostnames observers information about registry external URLs,
+// aka Routes. It retrieves this information from cluster's Image config.
+func ObserveExternalRegistryHostnames(
+	genericListers configobserver.Listers,
+	recorder events.Recorder,
+	existingConfig map[string]interface{},
+) (map[string]interface{}, []error) {
+	var errs []error
+	prevObservedConfig := map[string]interface{}{}
+	listers := genericListers.(configobservation.Listers)
+
+	// first observe all the existing config values so that if we get any errors
+	// we can at least return those.
+	cfgpath := []string{"dockerPullSecret", "registryURLs"}
+	currentURLs, _, err := unstructured.NestedStringSlice(existingConfig, cfgpath...)
+	if err != nil {
+		return prevObservedConfig, append(errs, err)
+	}
+	if len(currentURLs) > 0 {
+		if err := unstructured.SetNestedStringSlice(
+			prevObservedConfig, currentURLs, cfgpath...,
+		); err != nil {
+			return prevObservedConfig, append(errs, err)
+		}
+	}
+
+	observedConfig := map[string]interface{}{}
+	configImage, err := listers.ImageConfigLister.Get("cluster")
+	if errors.IsNotFound(err) {
+		klog.V(2).Infof("images.config.openshift.io/cluster: not found")
+		return observedConfig, errs
+	} else if err != nil {
+		return prevObservedConfig, append(errs, err)
+	}
+
+	if len(configImage.Status.ExternalRegistryHostnames) == 0 {
+		return observedConfig, errs
+	}
+
+	if err := unstructured.SetNestedStringSlice(
+		observedConfig, configImage.Status.ExternalRegistryHostnames, cfgpath...,
+	); err != nil {
+		return prevObservedConfig, append(errs, err)
+	}
+	return observedConfig, errs
+}

pkg/operator/configobservation/images/observe_images_test.go

@@ -1,6 +1,8 @@
 package images
 
 import (
+	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/openshift/library-go/pkg/operator/events"
@@ -45,3 +47,178 @@ func TestObserveRegistryConfig(t *testing.T) {
 		t.Errorf("expected internal registry hostname: %s, got %s", expectedInternalRegistryHostname, internalRegistryHostname)
 	}
 }
+
+func TestObserveRegistryExternalHostnames(t *testing.T) {
+	for _, tt := range []struct {
+		name     string
+		err      string
+		config   *configv1.Image
+		expected map[string]interface{}
+		existing map[string]interface{}
+	}{
+		{
+			name:     "broken pre existing config",
+			err:      "accessor error: broken is of the type string",
+			config:   nil,
+			expected: map[string]interface{}{},
+			existing: map[string]interface{}{
+				"dockerPullSecret": "broken",
+			},
+		},
+		{
+			name:     "empty if no image config found",
+			err:      "",
+			config:   nil,
+			expected: map[string]interface{}{},
+			existing: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"old-registry.openshift.io",
+					},
+				},
+			},
+		},
+		{
+			name: "empty if no external hostnames in image config status",
+			err:  "",
+			config: &configv1.Image{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cluster",
+				},
+			},
+			expected: map[string]interface{}{},
+			existing: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"old-registry.openshift.io",
+					},
+				},
+			},
+		},
+		{
+			name: "empty if external hostnames only in image config spec",
+			err:  "",
+			config: &configv1.Image{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cluster",
+				},
+				Spec: configv1.ImageSpec{
+					ExternalRegistryHostnames: []string{
+						"hostname-in-spec.openshift.io",
+					},
+				},
+			},
+			expected: map[string]interface{}{},
+			existing: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"old-registry.openshift.io",
+					},
+				},
+			},
+		},
+		{
+			name: "using external hostnames from image config status",
+			err:  "",
+			config: &configv1.Image{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cluster",
+				},
+				Status: configv1.ImageStatus{
+					ExternalRegistryHostnames: []string{
+						"hostname-0-in-status.openshift.io",
+						"hostname-1-in-status.openshift.io",
+					},
+				},
+			},
+			expected: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"hostname-0-in-status.openshift.io",
+						"hostname-1-in-status.openshift.io",
+					},
+				},
+			},
+			existing: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"old-registry.openshift.io",
+					},
+				},
+			},
+		},
+		{
+			name: "ignoring hostnames from image config's spec",
+			err:  "",
+			config: &configv1.Image{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cluster",
+				},
+				Spec: configv1.ImageSpec{
+					ExternalRegistryHostnames: []string{
+						"hostname-0-in-status.openshift.io",
+						"hostname-1-in-status.openshift.io",
+					},
+				},
+				Status: configv1.ImageStatus{
+					ExternalRegistryHostnames: []string{
+						"hostname-2-in-status.openshift.io",
+						"hostname-3-in-status.openshift.io",
+					},
+				},
+			},
+			expected: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"hostname-2-in-status.openshift.io",
+						"hostname-3-in-status.openshift.io",
+					},
+				},
+			},
+			existing: map[string]interface{}{
+				"dockerPullSecret": map[string]interface{}{
+					"registryURLs": []interface{}{
+						"old-registry.openshift.io",
+					},
+				},
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
+			if tt.config != nil {
+				indexer.Add(tt.config)
+			}
+			listers := configobservation.Listers{
+				ImageConfigLister: configlistersv1.NewImageLister(indexer),
+			}
+
+			result, errs := ObserveExternalRegistryHostnames(
+				listers, events.NewInMemoryRecorder(""), tt.existing,
+			)
+			if len(errs) > 0 && len(tt.err) == 0 {
+				t.Errorf("unexpected error: %v", errs)
+			} else if len(errs) > 0 {
+				errstr := ""
+				for _, err := range errs {
+					errstr += err.Error()
+				}
+				if !strings.Contains(errstr, tt.err) {
+					t.Errorf(
+						"expecting error to have %q, %v received instead",
+						tt.err, errs,
+					)
+				}
+			} else if len(tt.err) > 0 {
+				t.Errorf("expecting errors to contain %q, nil received instead", tt.err)
+			}
+
+			if !reflect.DeepEqual(tt.expected, result) {
+				t.Errorf(
+					"expected config %+v, got %+v",
+					tt.expected, result,
+				)
+			}
+		})
+	}
+}