Refactor config and substitution

controllers/clusteroperator_controller.go

@@ -22,7 +22,8 @@ import (
 
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud"
-	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/openstack"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/substitution"
 	"github.com/openshift/library-go/pkg/cloudprovider"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -100,7 +101,7 @@ func (r *CloudOperatorReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
-	platform, err := getProviderFromInfrastructure(infra)
+	platform, err := config.GetProviderFromInfrastructure(infra)
 	if err != nil {
 		klog.Errorf("Unable to determine platform from infrastructure: %s", err)
 		// Ignoring error here as infrastructure resource needs to be reconciled externally
@@ -133,7 +134,7 @@ func (r *CloudOperatorReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
 		return ctrl.Result{}, nil
 	}
 
-	config, err := r.composeConfig(platform)
+	config, err := config.ComposeConfig(platform, r.ImagesFile, r.ManagedNamespace)
 	if err != nil {
 		klog.Errorf("Unable to build operator config %s", err)
 		if err := r.setStatusDegraded(ctx, err); err != nil {
@@ -160,10 +161,10 @@ func (r *CloudOperatorReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
 	return ctrl.Result{}, nil
 }
 
-func (r *CloudOperatorReconciler) sync(ctx context.Context, config operatorConfig) error {
+func (r *CloudOperatorReconciler) sync(ctx context.Context, config config.OperatorConfig) error {
 	// Deploy resources for platform
-	templates := getResources(config.Platform)
-	resources := fillConfigValues(config, templates)
+	templates := cloud.GetResources(config.Platform)
+	resources := substitution.FillConfigValues(config, templates)
 
 	updated, err := r.applyResources(ctx, resources)
 	if err != nil {
@@ -220,19 +221,6 @@ func (r *CloudOperatorReconciler) applyResources(ctx context.Context, resources
 	return updated, nil
 }
 
-func getResources(platform configv1.PlatformType) []client.Object {
-	switch platform {
-	case configv1.AWSPlatformType:
-		return cloud.GetAWSResources()
-	case configv1.OpenStackPlatformType:
-		return openstack.GetResources()
-	default:
-		klog.Warning("No recognized cloud provider platform found in infrastructure")
-	}
-
-	return nil
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *CloudOperatorReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	watcher, err := NewObjectWatcher(WatcherOptions{

controllers/clusteroperator_controller_test.go

@@ -9,7 +9,8 @@ import (
 	. "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud"
-	openstack "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/openstack"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/substitution"
 	"github.com/openshift/library-go/pkg/cloudprovider"
 	"github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
 	appsv1 "k8s.io/api/apps/v1"
@@ -269,7 +270,7 @@ var _ = Describe("Component sync controller", func() {
 	type testCase struct {
 		status          *configv1.InfrastructureStatus
 		featureGateSpec *configv1.FeatureGateSpec
-		config          operatorConfig
+		config          config.OperatorConfig
 		expected        []client.Object
 	}
 
@@ -290,7 +291,7 @@ var _ = Describe("Component sync controller", func() {
 
 			watchMap := watcher.getWatchedResources()
 
-			operands = fillConfigValues(tc.config, tc.expected)
+			operands = substitution.FillConfigValues(tc.config, tc.expected)
 			for _, obj := range operands {
 				Expect(watchMap[obj.GetName()]).ToNot(BeNil())
 
@@ -317,11 +318,11 @@ var _ = Describe("Component sync controller", func() {
 				},
 			},
 			featureGateSpec: externalFeatureGateSpec,
-			config: operatorConfig{
+			config: config.OperatorConfig{
 				ManagedNamespace: testManagementNamespace,
 				ControllerImage:  "registry.ci.openshift.org/openshift:aws-cloud-controller-manager",
 			},
-			expected: cloud.GetAWSResources(),
+			expected: cloud.GetResources(configv1.AWSPlatformType),
 		}),
 		Entry("Should provision OpenStack resources", testCase{
 			status: &configv1.InfrastructureStatus{
@@ -330,12 +331,12 @@ var _ = Describe("Component sync controller", func() {
 					Type: configv1.OpenStackPlatformType,
 				},
 			},
-			config: operatorConfig{
+			config: config.OperatorConfig{
 				ManagedNamespace: testManagementNamespace,
 				ControllerImage:  "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager",
 			},
 			featureGateSpec: externalFeatureGateSpec,
-			expected:        openstack.GetResources(),
+			expected:        cloud.GetResources(configv1.OpenStackPlatformType),
 		}),
 		Entry("Should not provision resources for currently unsupported platform", testCase{
 			status: &configv1.InfrastructureStatus{
@@ -395,7 +396,7 @@ var _ = Describe("Apply resources should", func() {
 	})
 
 	It("Expect update when resources are not found", func() {
-		resources = append(resources, cloud.GetAWSResources()...)
+		resources = append(resources, cloud.GetResources(configv1.AWSPlatformType)...)
 
 		updated, err := reconciler.applyResources(context.TODO(), resources)
 		Expect(err).ShouldNot(HaveOccurred())
@@ -405,7 +406,7 @@ var _ = Describe("Apply resources should", func() {
 
 	It("Expect update when deployment generation have changed", func() {
 		var dep *appsv1.Deployment
-		for _, res := range cloud.GetAWSResources() {
+		for _, res := range cloud.GetResources(configv1.AWSPlatformType) {
 			if deployment, ok := res.(*appsv1.Deployment); ok {
 				dep = deployment
 				break
@@ -432,7 +433,7 @@ var _ = Describe("Apply resources should", func() {
 	})
 
 	It("Expect error when object requsted is incorrect", func() {
-		objects := cloud.GetAWSResources()
+		objects := cloud.GetResources(configv1.AWSPlatformType)
 		objects[0].SetNamespace("non-existent")
 
 		updated, err := reconciler.applyResources(context.TODO(), objects)
@@ -442,7 +443,7 @@ var _ = Describe("Apply resources should", func() {
 	})
 
 	It("Expect no update when resources are applied twice", func() {
-		resources = append(resources, openstack.GetResources()...)
+		resources = append(resources, cloud.GetResources(configv1.OpenStackPlatformType)...)
 
 		updated, err := reconciler.applyResources(context.TODO(), resources)
 		Expect(err).ShouldNot(HaveOccurred())

pkg/cloud/aws/aws.go

@@ -0,0 +1,34 @@
+package aws
+
+import (
+	"embed"
+
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/common"
+	appsv1 "k8s.io/api/apps/v1"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var (
+	//go:embed assets/*
+	awsFS      embed.FS
+	awsSources = []common.ObjectSource{
+		{Object: &appsv1.Deployment{}, Path: "assets/deployment.yaml"},
+	}
+	awsResources []client.Object
+)
+
+func init() {
+	var err error
+	awsResources, err = common.ReadResources(awsFS, awsSources)
+	utilruntime.Must(err)
+}
+
+func GetResources() []client.Object {
+	resources := make([]client.Object, len(awsResources))
+	for i := range awsResources {
+		resources[i] = awsResources[i].DeepCopyObject().(client.Object)
+	}
+
+	return resources
+}

pkg/cloud/aws/aws_test.go

@@ -0,0 +1,21 @@
+package aws
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetResources(t *testing.T) {
+	resources := GetResources()
+	assert.Len(t, resources, 1)
+
+	var names, kinds []string
+	for _, r := range resources {
+		names = append(names, r.GetName())
+		kinds = append(kinds, r.GetObjectKind().GroupVersionKind().Kind)
+	}
+
+	assert.Contains(t, names, "aws-cloud-controller-manager")
+	assert.Contains(t, kinds, "Deployment")
+}

pkg/cloud/cloud.go

@@ -0,0 +1,21 @@
+package cloud
+
+import (
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/aws"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/openstack"
+	"k8s.io/klog"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func GetResources(platform configv1.PlatformType) []client.Object {
+	switch platform {
+	case configv1.AWSPlatformType:
+		return aws.GetResources()
+	case configv1.OpenStackPlatformType:
+		return openstack.GetResources()
+	default:
+		klog.Warningf("Unrecognized platform type %q found in infrastructure", platform)
+		return nil
+	}
+}

pkg/cloud/cloud_test.go

@@ -0,0 +1,73 @@
+package cloud
+
+import (
+	"testing"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/aws"
+	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/openstack"
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestGetResources(t *testing.T) {
+	tc := []struct {
+		name     string
+		platform configv1.PlatformType
+		expected []client.Object
+	}{{
+		name:     "AWS resources returned as expected",
+		platform: configv1.AWSPlatformType,
+		expected: aws.GetResources(),
+	}, {
+		name:     "OpenStack resources returned as expected",
+		platform: configv1.OpenStackPlatformType,
+		expected: openstack.GetResources(),
+	}, {
+		name:     "GCP resources are empty, as the platform is not yet supported",
+		platform: configv1.GCPPlatformType,
+	}, {
+		name:     "Azure resources are empty, as the platform is not yet supported",
+		platform: configv1.AzurePlatformType,
+	}, {
+		name:     "VSphere resources are empty, as the platform is not yet supported",
+		platform: configv1.VSpherePlatformType,
+	}, {
+		name:     "OVirt resources are empty, as the platform is not yet supported",
+		platform: configv1.OvirtPlatformType,
+	}, {
+		name:     "IBMCloud resources are empty, as the platform is not yet supported",
+		platform: configv1.IBMCloudPlatformType,
+	}, {
+		name:     "Libvirt resources are empty",
+		platform: configv1.LibvirtPlatformType,
+	}, {
+		name:     "Kubevirt resources are empty",
+		platform: configv1.KubevirtPlatformType,
+	}, {
+		name:     "BareMetal resources are empty",
+		platform: configv1.BareMetalPlatformType,
+	}, {
+		name:     "None platform resources are empty",
+		platform: configv1.NonePlatformType,
+	}}
+
+	for _, tc := range tc {
+		t.Run(tc.name, func(t *testing.T) {
+			resources := GetResources(tc.platform)
+
+			assert.Equal(t, len(tc.expected), len(resources))
+			assert.EqualValues(t, tc.expected, resources)
+
+			// Edit and repeat procedure to ensure modification in place is not present
+			if len(resources) > 0 {
+				resources[0].SetName("different")
+				newResources := GetResources(tc.platform)
+
+				assert.Equal(t, len(tc.expected), len(newResources))
+				assert.EqualValues(t, tc.expected, newResources)
+				assert.NotEqualValues(t, resources, newResources)
+			}
+		})
+	}
+}

pkg/cloud/common/_testdata/assets/deployment.yaml

@@ -0,0 +1,22 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: sample
+  namespace: sample
+spec:
+  selector:
+    matchLabels:
+      k8s-app: aws-cloud-controller-manager
+  template:
+    metadata:
+      annotations:
+        target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
+      labels:
+        k8s-app: aws-cloud-controller-manager
+    spec:
+      containers:
+      - args:
+        - -v=2
+        image: test:image
+        imagePullPolicy: IfNotPresent
+        name: cloud-controller-manager

pkg/cloud/common/_testdata/foo

@@ -0,0 +1 @@
+Sample file undecodable as yaml or json