Fixing magic strings on operator flags

pkg/api/defaults.go

@@ -1,5 +1,7 @@
 package api
 
+import "github.com/Azure/ARO-RP/pkg/operator"
+
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache License 2.0.
 
@@ -71,41 +73,35 @@ const flagFalse string = "false"
 // DefaultOperatorFlags returns flags for new clusters
 // and ones that have not been AdminUpdated.
 func DefaultOperatorFlags() OperatorFlags {
-	// TODO: Get rid of magic strings.
-	// We already have constants for all of the below strings.
-	// For example `controllerEnabled` in `github.com/Azure/ARO-RP/pkg/operator/controllers/machine`.
-	// But if we import packages with constants here we will have a cyclic import issue because controllers
-	// import this package. We should probably move this somewhere else.
-	// Maybe into a subpackage like `github.com/Azure/ARO-RP/pkg/api/defaults`?
 	return OperatorFlags{
-		"aro.alertwebhook.enabled":                 flagTrue,
-		"aro.azuresubnets.enabled":                 flagTrue,
-		"aro.azuresubnets.nsg.managed":             flagTrue,
-		"aro.azuresubnets.serviceendpoint.managed": flagTrue,
-		"aro.banner.enabled":                       flagFalse,
-		"aro.checker.enabled":                      flagTrue,
-		"aro.dnsmasq.enabled":                      flagTrue,
-		"aro.restartdnsmasq.enabled":               flagTrue,
-		"aro.genevalogging.enabled":                flagTrue,
-		"aro.imageconfig.enabled":                  flagTrue,
-		"aro.ingress.enabled":                      flagTrue,
-		"aro.machine.enabled":                      flagTrue,
-		"aro.machineset.enabled":                   flagTrue,
-		"aro.machinehealthcheck.enabled":           flagTrue,
-		"aro.machinehealthcheck.managed":           flagTrue,
-		"aro.monitoring.enabled":                   flagTrue,
-		"aro.nodedrainer.enabled":                  flagTrue,
-		"aro.pullsecret.enabled":                   flagTrue,
-		"aro.pullsecret.managed":                   flagTrue,
-		"aro.rbac.enabled":                         flagTrue,
-		"aro.routefix.enabled":                     flagTrue,
-		"aro.storageaccounts.enabled":              flagTrue,
-		"aro.workaround.enabled":                   flagTrue,
-		"aro.autosizednodes.enabled":               flagTrue,
-		"rh.srep.muo.enabled":                      flagTrue,
-		"rh.srep.muo.managed":                      flagTrue,
-		"aro.guardrails.enabled":                   flagFalse,
-		"aro.guardrails.deploy.managed":            flagFalse,
-		"aro.cloudproviderconfig.enabled":          flagTrue,
+		operator.AlertWebhookEnabled:                operator.FlagTrue,
+		operator.AzureSubnetsEnabled:                operator.FlagTrue,
+		operator.AzureSubnetsNsgManaged:             operator.FlagTrue,
+		operator.AzureSubnetsServiceEndpointManaged: operator.FlagTrue,
+		operator.BannerEnabled:                      operator.FlagFalse,
+		operator.CheckerEnabled:                     operator.FlagTrue,
+		operator.DnsMasqEnabled:                     operator.FlagTrue,
+		operator.RestartDnsMasqEnabled:              operator.FlagTrue,
+		operator.GenevaLoggingEnabled:               operator.FlagTrue,
+		operator.ImageConfigEnabled:                 operator.FlagTrue,
+		operator.IngressEnabled:                     operator.FlagTrue,
+		operator.MachineEnabled:                     operator.FlagTrue,
+		operator.MachineSetEnabled:                  operator.FlagTrue,
+		operator.MachineHealthCheckEnabled:          operator.FlagTrue,
+		operator.MachineHealthCheckManaged:          operator.FlagTrue,
+		operator.MonitoringEnabled:                  operator.FlagTrue,
+		operator.NodeDrainerEnabled:                 operator.FlagTrue,
+		operator.PullSecretEnabled:                  operator.FlagTrue,
+		operator.PullSecretManaged:                  operator.FlagTrue,
+		operator.RbacEnabled:                        operator.FlagTrue,
+		operator.RouteFixEnabled:                    operator.FlagTrue,
+		operator.StorageAccountsEnabled:             operator.FlagTrue,
+		operator.WorkaroundEnabled:                  operator.FlagTrue,
+		operator.AutosizedNodesEnabled:              operator.FlagTrue,
+		operator.MuoEnabled:                         operator.FlagTrue,
+		operator.MuoManaged:                         operator.FlagTrue,
+		operator.GuardrailsEnabled:                  operator.FlagFalse,
+		operator.GuardrailsDeployManaged:            operator.FlagFalse,
+		operator.CloudProviderConfigEnabled:         operator.FlagTrue,
 	}
 }

pkg/cluster/defaults.go

@@ -7,6 +7,7 @@ import (
 	"context"
 
 	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/operator"
 )
 
 // ensureDefaults will ensure cluster documents has all default values
@@ -31,7 +32,7 @@ func (m *manager) ensurePreconfiguredNSG(ctx context.Context) error {
 		var err error
 		m.doc, err = m.db.PatchWithLease(ctx, m.doc.Key, func(doc *api.OpenShiftClusterDocument) error {
 			flags := doc.OpenShiftCluster.Properties.OperatorFlags
-			flags["aro.azuresubnets.nsg.managed"] = "false"
+			flags[operator.AzureSubnetsNsgManaged] = operator.FlagFalse
 			return nil
 		})
 		if err != nil {

pkg/monitor/cluster/clusterflagsandbanner_test.go

@@ -11,6 +11,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake"
 )
@@ -117,32 +118,32 @@ func TestEmitOperatorFlagsAndSupportBanner(t *testing.T) {
 		},
 		{
 			name:          "cluster with non-standard operator flags",
-			operatorFlags: generateNonStandardFlags([]string{"aro.imageconfig.enabled", "aro.dnsmasq.enabled", "aro.genevalogging.enabled", "aro.autosizednodes.enabled"}),
+			operatorFlags: generateNonStandardFlags([]string{operator.ImageConfigEnabled, operator.DnsMasqEnabled, operator.GenevaLoggingEnabled, operator.AutosizedNodesEnabled}),
 			clusterBanner: arov1alpha1.Banner{
 				Content: "",
 			},
 			expectFlagsMetricsValue: 1,
 			expectFlagsMetricsDims: map[string]string{
-				"aro.imageconfig.enabled":    "false",
-				"aro.dnsmasq.enabled":        "false",
-				"aro.genevalogging.enabled":  "false",
-				"aro.autosizednodes.enabled": "false",
+				operator.ImageConfigEnabled:    "false",
+				operator.DnsMasqEnabled:        "false",
+				operator.GenevaLoggingEnabled:  "false",
+				operator.AutosizedNodesEnabled: "false",
 			},
 			expectBannerMetricsValue: 0,
 			expectBannerMetricsDims:  nil,
 		},
 		{
 			name:          "cluster with missing operator flags",
-			operatorFlags: generateFlagsWithMissingEntries([]string{"aro.imageconfig.enabled", "aro.dnsmasq.enabled", "aro.genevalogging.enabled", "aro.autosizednodes.enabled"}),
+			operatorFlags: generateFlagsWithMissingEntries([]string{operator.ImageConfigEnabled, operator.DnsMasqEnabled, operator.GenevaLoggingEnabled, operator.AutosizedNodesEnabled}),
 			clusterBanner: arov1alpha1.Banner{
 				Content: "",
 			},
 			expectFlagsMetricsValue: 1,
 			expectFlagsMetricsDims: map[string]string{
-				"aro.imageconfig.enabled":    "DNE",
-				"aro.dnsmasq.enabled":        "DNE",
-				"aro.genevalogging.enabled":  "DNE",
-				"aro.autosizednodes.enabled": "DNE",
+				operator.ImageConfigEnabled:    "DNE",
+				operator.DnsMasqEnabled:        "DNE",
+				operator.GenevaLoggingEnabled:  "DNE",
+				operator.AutosizedNodesEnabled: "DNE",
 			},
 			expectBannerMetricsValue: 0,
 			expectBannerMetricsDims:  nil,
@@ -160,16 +161,16 @@ func TestEmitOperatorFlagsAndSupportBanner(t *testing.T) {
 		},
 		{
 			name:          "cluster with non-standard operator flags and activated support banner",
-			operatorFlags: generateNonStandardFlags([]string{"aro.imageconfig.enabled", "aro.dnsmasq.enabled", "aro.genevalogging.enabled", "aro.autosizednodes.enabled"}),
+			operatorFlags: generateNonStandardFlags([]string{operator.ImageConfigEnabled, operator.DnsMasqEnabled, operator.GenevaLoggingEnabled, operator.AutosizedNodesEnabled}),
 			clusterBanner: arov1alpha1.Banner{
 				Content: arov1alpha1.BannerContactSupport,
 			},
 			expectFlagsMetricsValue: 1,
 			expectFlagsMetricsDims: map[string]string{
-				"aro.imageconfig.enabled":    "false",
-				"aro.dnsmasq.enabled":        "false",
-				"aro.genevalogging.enabled":  "false",
-				"aro.autosizednodes.enabled": "false",
+				operator.ImageConfigEnabled:    operator.FlagFalse,
+				operator.DnsMasqEnabled:        operator.FlagFalse,
+				operator.GenevaLoggingEnabled:  operator.FlagFalse,
+				operator.AutosizedNodesEnabled: operator.FlagFalse,
 			},
 			expectBannerMetricsValue: 1,
 			expectBannerMetricsDims:  map[string]string{"msg": "contact support"},

pkg/operator/controllers/alertwebhook/alertwebhook_controller.go

@@ -17,13 +17,14 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 )
 
 const (
 	ControllerName = "Alertwebhook"
 
-	controllerEnabled = "aro.alertwebhook.enabled"
+	controllerEnabled = operator.AlertWebhookEnabled
 )
 
 var alertManagerName = types.NamespacedName{Name: "alertmanager-main", Namespace: "openshift-monitoring"}

pkg/operator/controllers/autosizednodes/autosizednodes_controller.go

@@ -19,6 +19,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 )
 
@@ -31,7 +32,7 @@ type Reconciler struct {
 const (
 	ControllerName = "AutoSizedNodes"
 
-	ControllerEnabled = "aro.autosizednodes.enabled"
+	ControllerEnabled = operator.AutosizedNodesEnabled
 	configName        = "dynamic-node"
 )
 

pkg/operator/controllers/banner/banner_controller.go

@@ -17,13 +17,14 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 )
 
 const (
 	ControllerName = "Banner"
 
-	controllerEnabled = "aro.banner.enabled"
+	controllerEnabled = operator.BannerEnabled
 )
 
 // BannerReconciler is the controller struct

pkg/operator/controllers/checkers/common/const.go

@@ -1,8 +1,10 @@
 package common
 
+import "github.com/Azure/ARO-RP/pkg/operator"
+
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache License 2.0.
 
 const (
-	ControllerEnabled = "aro.checker.enabled"
+	ControllerEnabled = operator.CheckerEnabled
 )

pkg/operator/controllers/cloudproviderconfig/cloudproviderconfig_controller.go

@@ -20,13 +20,14 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 )
 
 const (
 	ControllerName    = "CloudProviderConfig"
-	controllerEnabled = "aro.cloudproviderconfig.enabled"
+	controllerEnabled = operator.CloudProviderConfigEnabled
 )
 
 var cloudProviderConfigName = types.NamespacedName{Name: "cloud-provider-config", Namespace: "openshift-config"}

pkg/operator/controllers/dnsmasq/cluster_controller.go

@@ -15,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
@@ -23,8 +24,8 @@ import (
 const (
 	ClusterControllerName = "DnsmasqCluster"
 
-	controllerEnabled     = "aro.dnsmasq.enabled"
-	restartDnsmasqEnabled = "aro.restartdnsmasq.enabled"
+	controllerEnabled     = operator.DnsMasqEnabled
+	restartDnsmasqEnabled = operator.RestartDnsMasqEnabled
 )
 
 type ClusterReconciler struct {

pkg/operator/controllers/genevalogging/genevalogging_controller.go

@@ -26,7 +26,7 @@ import (
 const (
 	ControllerName = "GenevaLogging"
 
-	controllerEnabled = "aro.genevalogging.enabled"
+	controllerEnabled = operator.GenevaLoggingEnabled
 	// full pullspec of fluentbit image
 	controllerFluentbitPullSpec = "aro.genevalogging.fluentbit.pullSpec"
 	// full pullspec of mdsd image

pkg/operator/controllers/guardrails/guardrails_config.go

@@ -11,16 +11,17 @@ import (
 	configv1 "github.com/openshift/api/config/v1"
 	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/guardrails/config"
 	"github.com/Azure/ARO-RP/pkg/util/version"
 )
 
 const (
 	ControllerName               = "GuardRails"
-	controllerEnabled            = "aro.guardrails.enabled"
+	controllerEnabled            = operator.GuardrailsEnabled
 	controllerNamespace          = "aro.guardrails.namespace"
-	controllerManaged            = "aro.guardrails.deploy.managed"
+	controllerManaged            = operator.GuardrailsDeployManaged
 	controllerPullSpec           = "aro.guardrails.deploy.pullspec"
 	controllerManagerRequestsCPU = "aro.guardrails.deploy.manager.requests.cpu"
 	controllerManagerRequestsMem = "aro.guardrails.deploy.manager.requests.mem"

pkg/operator/controllers/imageconfig/image_controller.go

@@ -19,6 +19,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 	"github.com/Azure/ARO-RP/pkg/util/azureclient"
@@ -27,7 +28,7 @@ import (
 const (
 	ControllerName = "ImageConfig"
 
-	controllerEnabled = "aro.imageconfig.enabled"
+	controllerEnabled = operator.ImageConfigEnabled
 
 	// Kubernetes object name
 	imageConfigResource = "cluster"

pkg/operator/controllers/ingress/ingress_controller.go

@@ -16,14 +16,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 )
 
 const (
 	ControllerName = "IngressControllerARO"
 
-	controllerEnabled                   = "aro.ingress.enabled"
+	controllerEnabled                   = operator.IngressEnabled
 	openshiftIngressControllerNamespace = "openshift-ingress-operator"
 	openshiftIngressControllerName      = "default"
 	minimumReplicas                     = 2

pkg/operator/controllers/ingress/ingress_controller_test.go

@@ -16,6 +16,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	_ "github.com/Azure/ARO-RP/pkg/util/scheme"
 	utilconditions "github.com/Azure/ARO-RP/test/util/conditions"
@@ -36,7 +37,7 @@ func TestReconciler(t *testing.T) {
 			},
 			Spec: arov1alpha1.ClusterSpec{
 				OperatorFlags: arov1alpha1.OperatorFlags{
-					"aro.ingress.enabled": controllerEnabledFlag,
+					operator.IngressEnabled: controllerEnabledFlag,
 				},
 			},
 		}

pkg/operator/controllers/machine/machine_controller.go

@@ -15,14 +15,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/util/conditions"
 )
 
 const (
 	ControllerName = "Machine"
 
-	controllerEnabled = "aro.machine.enabled"
+	controllerEnabled = operator.MachineEnabled
 )
 
 type Reconciler struct {

pkg/operator/controllers/machinehealthcheck/machinehealthcheck_controller.go

@@ -24,6 +24,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
@@ -37,8 +38,8 @@ var mhcremediationalertYaml []byte
 
 const (
 	ControllerName      string = "MachineHealthCheck"
-	managed             string = "aro.machinehealthcheck.managed"
-	enabled             string = "aro.machinehealthcheck.enabled"
+	managed             string = operator.MachineHealthCheckManaged
+	enabled             string = operator.MachineHealthCheckEnabled
 	MHCPausedAnnotation string = "cluster.x-k8s.io/paused"
 )
 

pkg/operator/controllers/machineset/machineset_controller.go

@@ -18,13 +18,14 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Azure/ARO-RP/pkg/operator"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/base"
 )
 
 const (
 	ControllerName = "MachineSet"
 
-	ControllerEnabled = "aro.machineset.enabled"
+	ControllerEnabled = operator.MachineSetEnabled
 )
 
 type Reconciler struct {