Merge pull request #229 from swghosh/cm-424

CM-424: Allow experimental features to be turned on from `--unsupported-addon-features`

Author: openshift-merge-bot[bot]

api/operator/v1alpha1/features.go

@@ -0,0 +1,22 @@
+package v1alpha1
+
+import (
+	"k8s.io/component-base/featuregate"
+)
+
+var (
+	// TechPreview: v1.15
+	//
+	// IstioCSR enables the controller for istiocsr.operator.openshift.io resource,
+	// which extends cert-manager-operator to deploy and manage the istio-csr agent.
+	// OpenShift Service Mesh facilitates the integration and istio-csr is an agent that
+	// allows Istio workload and control plane components to be secured using cert-manager.
+	//
+	// For more details,
+	// https://github.com/openshift/enhancements/blob/master/enhancements/cert-manager/istio-csr-controller.md
+	FeatureIstioCSR featuregate.Feature = "IstioCSR"
+)
+
+var OperatorFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
+	FeatureIstioCSR: {Default: false, PreRelease: "TechPreview"},
+}

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

@@ -673,6 +673,7 @@ spec:
                 - --v=$(OPERATOR_LOG_LEVEL)
                 - --trusted-ca-configmap=$(TRUSTED_CA_CONFIGMAP_NAME)
                 - --cloud-credentials-secret=$(CLOUD_CREDENTIALS_SECRET_NAME)
+                - --unsupported-addon-features=$(UNSUPPORTED_ADDON_FEATURES)
                 command:
                 - /usr/bin/cert-manager-operator
                 env:
@@ -706,6 +707,7 @@ spec:
                   value: "2"
                 - name: TRUSTED_CA_CONFIGMAP_NAME
                 - name: CLOUD_CREDENTIALS_SECRET_NAME
+                - name: UNSUPPORTED_ADDON_FEATURES
                 image: openshift.io/cert-manager-operator:latest
                 imagePullPolicy: IfNotPresent
                 name: cert-manager-operator

config/manager/manager.yaml

@@ -62,6 +62,7 @@ spec:
             - '--v=$(OPERATOR_LOG_LEVEL)'
             - '--trusted-ca-configmap=$(TRUSTED_CA_CONFIGMAP_NAME)'
             - '--cloud-credentials-secret=$(CLOUD_CREDENTIALS_SECRET_NAME)'
+            - '--unsupported-addon-features=$(UNSUPPORTED_ADDON_FEATURES)'
           env:
             - name: WATCH_NAMESPACE
               valueFrom:
@@ -93,6 +94,7 @@ spec:
               value: '2'
             - name: TRUSTED_CA_CONFIGMAP_NAME
             - name: CLOUD_CREDENTIALS_SECRET_NAME
+            - name: UNSUPPORTED_ADDON_FEATURES
           image: controller:latest
           imagePullPolicy: IfNotPresent
           name: cert-manager-operator

pkg/cmd/operator/cmd.go

@@ -20,5 +20,16 @@ func NewOperator() *cobra.Command {
 	cmd.Short = "Start the cert-manager Operator"
 	cmd.Flags().StringVar(&operator.TrustedCAConfigMapName, "trusted-ca-configmap", "", "The name of the config map containing TLS CA(s) which should be trusted by the controller's containers. PEM encoded file under \"ca-bundle.crt\" key is expected.")
 	cmd.Flags().StringVar(&operator.CloudCredentialSecret, "cloud-credentials-secret", "", "The name of the secret containing cloud credentials for authenticating using cert-manager ambient credentials mode.")
+
+	cmd.Flags().StringVar(&operator.UnsupportedAddonFeatures, "unsupported-addon-features", "",
+		`List of unsupported addon features that the operator optionally enables.
+		
+eg. --unsupported-addon-features="IstioCSR=true"
+		
+Note: Technology Preview features are not supported with Red Hat production service level agreements (SLAs)
+and might not be functionally complete. Red Hat does not recommend using them in production.
+
+These features provide early access to upcoming product features,
+enabling customers to test functionality and provide feedback during the development process.`)
 	return cmd
 }

pkg/features/features.go

@@ -0,0 +1,24 @@
+package features
+
+import (
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/component-base/featuregate"
+
+	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
+)
+
+var (
+	// mutableFeatureGate is the top-level FeatureGate with mutability (read/write).
+	mutableFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()
+
+	// DefaultFeatureGate is the top-level shared global FeatureGate (read-only).
+	DefaultFeatureGate featuregate.FeatureGate = mutableFeatureGate
+)
+
+func init() {
+	utilruntime.Must(mutableFeatureGate.Add(v1alpha1.OperatorFeatureGates))
+}
+
+func SetupWithFlagValue(flagValue string) error {
+	return mutableFeatureGate.Set(flagValue)
+}

pkg/features/features_test.go

@@ -0,0 +1,93 @@
+package features
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"k8s.io/component-base/featuregate"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// expectedDefaultFeatureState is a map of features with known states
+//
+// Always update this test map when adding a new FeatureName to the api,
+// a test is added below to enforce this.
+var expectedDefaultFeatureState = map[bool][]featuregate.Feature{
+	// features ENABLED by default,
+	// list of features which are expected to be enabled at runtime.
+	true: {},
+
+	// features DISABLED by default,
+	// list of features which are expected to be disabled at runtime.
+	false: {
+		featuregate.Feature("IstioCSR"),
+	},
+}
+
+func TestFeatureGates(t *testing.T) {
+	t.Run("runtime features to test should be identical with operator features", func(t *testing.T) {
+		testFeatureNames := make([]featuregate.Feature, 0)
+		for _, featureNames := range expectedDefaultFeatureState {
+			testFeatureNames = append(testFeatureNames, featureNames...)
+		}
+
+		knownOperatorFeatures := make([]featuregate.Feature, 0)
+		feats := mutableFeatureGate.GetAll()
+		for feat := range feats {
+			// skip "AllBeta", "AllAlpha": our operator does not use those
+			if feat == "AllBeta" || feat == "AllAlpha" {
+				continue
+			}
+			knownOperatorFeatures = append(knownOperatorFeatures, feat)
+		}
+
+		assert.Equal(t, knownOperatorFeatures, testFeatureNames,
+			`the list of features known to the operator differ from what is being tested here,
+			it could be that there was a new Feature added to the api which wasn't added to the tests.
+			Please verify "api/operator/v1alpha1" and "pkg/operator/features" have identical features.`)
+	})
+
+	t.Run("all runtime features should honor expected default feature state", func(t *testing.T) {
+		for expectedDefaultState, features := range expectedDefaultFeatureState {
+			for _, featureName := range features {
+				assert.Equal(t, expectedDefaultState, DefaultFeatureGate.Enabled(featureName),
+					"violated by feature=%s", featureName)
+			}
+		}
+	})
+
+	t.Run("all TechPreview features should be disabled by default", func(t *testing.T) {
+		feats := mutableFeatureGate.GetAll()
+		for feat, spec := range feats {
+			// skip "AllBeta", "AllAlpha": our operator does not use those
+			if feat == "AllBeta" || feat == "AllAlpha" {
+				continue
+			}
+
+			assert.Equal(t, spec.PreRelease == "TechPreview", !spec.Default,
+				"prerelease TechPreview %q feature should default to disabled",
+				feat)
+		}
+	})
+
+	t.Run("runtime features should allow enabling all feature gates that are disabled by default", func(t *testing.T) {
+		// enable all disabled features
+		defaultDisabledFeatures := expectedDefaultFeatureState[false]
+
+		featVals := make([]string, len(defaultDisabledFeatures))
+		for i := range featVals {
+			featVals[i] = fmt.Sprintf("%s=true", defaultDisabledFeatures[i])
+		}
+
+		err := mutableFeatureGate.Set(strings.Join(featVals, ","))
+		assert.NoError(t, err)
+
+		// check if all those features were enabled successfully
+		for _, featureName := range defaultDisabledFeatures {
+			assert.Equal(t, true, DefaultFeatureGate.Enabled(featureName),
+				"violated by feature=%s", featureName)
+		}
+	})
+}

pkg/operator/starter.go

@@ -17,7 +17,9 @@ import (
 	"github.com/openshift/library-go/pkg/operator/status"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
 
+	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
 	"github.com/openshift/cert-manager-operator/pkg/controller/deployment"
+	"github.com/openshift/cert-manager-operator/pkg/features"
 	certmanoperatorclient "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned"
 	certmanoperatorinformers "github.com/openshift/cert-manager-operator/pkg/operator/informers/externalversions"
 	"github.com/openshift/cert-manager-operator/pkg/operator/operatorclient"
@@ -35,6 +37,10 @@ var TrustedCAConfigMapName string
 // used in ambient credentials mode, and is provided as a runtime arg
 var CloudCredentialSecret string
 
+// UnsupportedAddonFeatures is the user-specific list of unsupported addon features
+// that the operator optionally enables, and is provided as a runtime arg.
+var UnsupportedAddonFeatures string
+
 func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error {
 	kubeClient, err := kubernetes.NewForConfig(cc.ProtoKubeConfig)
 	if err != nil {
@@ -112,12 +118,21 @@ func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 		go controller.Run(ctx, 1)
 	}
 
-	manager, err := NewControllerManager()
+	err = features.SetupWithFlagValue(UnsupportedAddonFeatures)
 	if err != nil {
-		return fmt.Errorf("failed to create controller manager: %w", err)
+		return fmt.Errorf("failed to parse addon features: %w", err)
 	}
-	if err := manager.Start(ctrl.SetupSignalHandler()); err != nil {
-		return fmt.Errorf("failed to start istiocsr controller: %w", err)
+
+	// enable controller-runtime and istio-csr controller
+	// only when "IstioCSR" feature is turned on from --addon-features
+	if features.DefaultFeatureGate.Enabled(v1alpha1.FeatureIstioCSR) {
+		manager, err := NewControllerManager()
+		if err != nil {
+			return fmt.Errorf("failed to create controller manager: %w", err)
+		}
+		if err := manager.Start(ctrl.SetupSignalHandler()); err != nil {
+			return fmt.Errorf("failed to start istiocsr controller: %w", err)
+		}
 	}
 
 	<-ctx.Done()

test/e2e/certificates_test.go

@@ -205,8 +205,9 @@ var _ = Describe("ACME Certificate", Ordered, func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			By("setting cloud credential secret name in subscription object")
-			credentialSecret := "aws-creds"
-			err = patchSubscriptionWithCloudCredential(ctx, loader, credentialSecret)
+			err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{
+				"CLOUD_CREDENTIALS_SECRET_NAME": "aws-creds",
+			})
 			Expect(err).NotTo(HaveOccurred())
 
 			By("getting AWS zone from Infrastructure object")
@@ -297,8 +298,9 @@ var _ = Describe("ACME Certificate", Ordered, func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			By("setting cloud credential secret name in subscription object")
-			credentialSecret := "aws-creds"
-			err = patchSubscriptionWithCloudCredential(ctx, loader, credentialSecret)
+			err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{
+				"CLOUD_CREDENTIALS_SECRET_NAME": "aws-creds",
+			})
 			Expect(err).NotTo(HaveOccurred())
 
 			By("getting AWS zone from Infrastructure object")
@@ -496,7 +498,9 @@ var _ = Describe("ACME Certificate", Ordered, func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Configure cert-manager to use credential, setting this credential secret name in subscription object")
-			err = patchSubscriptionWithCloudCredential(ctx, loader, credentialSecret)
+			err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{
+				"CLOUD_CREDENTIALS_SECRET_NAME": credentialSecret,
+			})
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Getting GCP project ID from Infrastructure object")

test/e2e/istio_csr_test.go

@@ -46,6 +46,12 @@ var _ = Describe("Istio-CSR", Ordered, Label("TechPreview", "Feature:IstioCSR"),
 
 		dynamicClient, err = dynamic.NewForConfig(cfg)
 		Expect(err).Should(BeNil())
+
+		By("enable IstioCSR addon feature by patching subscription object")
+		err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{
+			"UNSUPPORTED_ADDON_FEATURES": "IstioCSR=true",
+		})
+		Expect(err).NotTo(HaveOccurred())
 	})
 
 	var ns *corev1.Namespace

test/e2e/utils_test.go

@@ -400,24 +400,28 @@ func getCertManagerOperatorSubscription(ctx context.Context, loader library.Dyna
 	return subName, nil
 }
 
-// patchSubscriptionWithCloudCredential uses the k8s dynamic client to patche the only Subscription object
-// in the cert-manager-operator namespace to inject CLOUD_CREDENTIALS_SECRET_NAME="aws-creds" env
-// into its spec.config.env
-func patchSubscriptionWithCloudCredential(ctx context.Context, loader library.DynamicResourceLoader, credentialSecret string) error {
+// patchSubscriptionWithEnvVars uses the k8s dynamic client to patch the only Subscription object
+// in the cert-manager-operator namespace, inject specified env vars into spec.config.env
+func patchSubscriptionWithEnvVars(ctx context.Context, loader library.DynamicResourceLoader, envVars map[string]string) error {
 	subName, err := getCertManagerOperatorSubscription(ctx, loader)
 	if err != nil {
 		return err
 	}
 
+	env := make([]interface{}, len(envVars))
+	i := 0
+	for k, v := range envVars {
+		env[i] = map[string]interface{}{
+			"name":  k,
+			"value": v,
+		}
+		i++
+	}
+
 	patch := map[string]interface{}{
 		"spec": map[string]interface{}{
 			"config": map[string]interface{}{
-				"env": []interface{}{
-					map[string]interface{}{
-						"name":  "CLOUD_CREDENTIALS_SECRET_NAME",
-						"value": credentialSecret,
-					},
-				},
+				"env": env,
 			},
 		},
 	}