Merge pull request #584 from sector2000/centralize-patterns-operator-config

Cantralize PatternsOperator config

Author: openshift-merge-bot[bot]

cmd/main.go

@@ -86,7 +86,7 @@ func main() {
 	setupLog.Info("detected operator namespace", "namespace", controllers.DetectOperatorNamespace())
 
 	// Create initial config map for gitops
-	err := createGitOpsConfigMap()
+	err := createPatternsOperatorConfigMap()
 	if err != nil {
 		setupLog.Error(err, "unable to create config map")
 	}
@@ -165,7 +165,7 @@ func printVersion() {
 // Creates the patterns operator configmap
 // This will include configuration parameters that
 // will allow operator configuration
-func createGitOpsConfigMap() error {
+func createPatternsOperatorConfigMap() error {
 	config, err := ctrl.GetConfig()
 	if err != nil {
 		return fmt.Errorf("failed to get config: %s", err)

internal/controller/argo.go

@@ -911,7 +911,7 @@ func newArgoApplication(p *api.Pattern) *argoapi.Application {
 	return targetApp
 }
 
-func newArgoGiteaApplication(p *api.Pattern) *argoapi.Application {
+func newArgoGiteaApplication(p *api.Pattern, patternsOperatorConfig PatternsOperatorConfig) *argoapi.Application {
 	consoleHref := fmt.Sprintf("https://%s-%s.%s", GiteaRouteName, GiteaNamespace, p.Status.AppClusterDomain)
 	parameters := []argoapi.HelmParameter{
 		{
@@ -934,9 +934,9 @@ func newArgoGiteaApplication(p *api.Pattern) *argoapi.Application {
 		},
 		Project: "default",
 		Source: &argoapi.ApplicationSource{
-			RepoURL:        PatternsOperatorConfig.getValueWithDefault("gitea.helmRepoUrl"),
-			TargetRevision: PatternsOperatorConfig.getValueWithDefault("gitea.chartVersion"),
-			Chart:          PatternsOperatorConfig.getValueWithDefault("gitea.chartName"),
+			RepoURL:        patternsOperatorConfig.getValueWithDefault("gitea.helmRepoUrl"),
+			TargetRevision: patternsOperatorConfig.getValueWithDefault("gitea.chartVersion"),
+			Chart:          patternsOperatorConfig.getValueWithDefault("gitea.chartName"),
 			Helm: &argoapi.ApplicationSourceHelm{
 				Parameters: parameters,
 			},

internal/controller/argo_test.go

@@ -1420,9 +1420,12 @@ var _ = Describe("getChildApplications", func() {
 
 var _ = Describe("NewArgoGiteaApplication", func() {
 	var pattern *api.Pattern
+	var patternsOperatorConfig PatternsOperatorConfig
+	var app *argoapi.Application
+
 	BeforeEach(func() {
 		tmpFalse := false
-		PatternsOperatorConfig = DefaultPatternOperatorConfig
+		patternsOperatorConfig = DefaultPatternsOperatorConfig
 		pattern = &api.Pattern{
 			ObjectMeta: metav1.ObjectMeta{Name: "test-pattern", Namespace: defaultNamespace},
 			TypeMeta:   metav1.TypeMeta{Kind: "Pattern", APIVersion: api.GroupVersion.String()},
@@ -1444,10 +1447,11 @@ var _ = Describe("NewArgoGiteaApplication", func() {
 				ClusterDomain:    "hub-cluster.validatedpatterns.io",
 			},
 		}
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
+
 	})
 
 	It("should create a gitea application with correct properties", func() {
-		app := newArgoGiteaApplication(pattern)
 		Expect(app).ToNot(BeNil())
 		Expect(app.Name).To(Equal(GiteaApplicationName))
 		Expect(app.Namespace).To(Equal(getClusterWideArgoNamespace()))
@@ -2001,6 +2005,8 @@ var _ = Describe("removeApplication", func() {
 
 var _ = Describe("newArgoGiteaApplication", func() {
 	var pattern *api.Pattern
+	var patternsOperatorConfig PatternsOperatorConfig
+	var app *argoapi.Application
 
 	BeforeEach(func() {
 		tmpFalse := false
@@ -2028,37 +2034,37 @@ var _ = Describe("newArgoGiteaApplication", func() {
 				ClusterVersion:   "4.14.0",
 			},
 		}
-		PatternsOperatorConfig = GitOpsConfig{}
+		patternsOperatorConfig = DefaultPatternsOperatorConfig
 	})
 
 	It("should create the Gitea application with correct name", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Name).To(Equal(GiteaApplicationName))
 		Expect(app.Namespace).To(Equal(getClusterWideArgoNamespace()))
 	})
 
 	It("should set the pattern label", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Labels).To(HaveKeyWithValue("validatedpatterns.io/pattern", "test-pattern"))
 	})
 
 	It("should set the destination namespace to GiteaNamespace", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.Destination.Namespace).To(Equal(GiteaNamespace))
 	})
 
 	It("should set destination to in-cluster", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.Destination.Name).To(Equal("in-cluster"))
 	})
 
 	It("should set the project to default", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.Project).To(Equal("default"))
 	})
 
 	It("should include helm parameters for gitea admin secret and console href", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.Source).ToNot(BeNil())
 		Expect(app.Spec.Source.Helm).ToNot(BeNil())
 
@@ -2074,19 +2080,19 @@ var _ = Describe("newArgoGiteaApplication", func() {
 	})
 
 	It("should have the foreground propagation finalizer", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(controllerutil.ContainsFinalizer(app, argoapi.ForegroundPropagationPolicyFinalizer)).To(BeTrue())
 	})
 
 	It("should set a sync policy when not manual sync", func() {
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.SyncPolicy).ToNot(BeNil())
 		Expect(app.Spec.SyncPolicy.Automated).ToNot(BeNil())
 	})
 
 	It("should have nil sync policy when manual sync is enabled", func() {
 		pattern.Spec.GitOpsConfig.ManualSync = true
-		app := newArgoGiteaApplication(pattern)
+		app = newArgoGiteaApplication(pattern, patternsOperatorConfig)
 		Expect(app.Spec.SyncPolicy).To(BeNil())
 	})
 })

internal/controller/defaults.go

@@ -86,7 +86,7 @@ const (
 // Experimental Capabilities that can be enabled
 // Currently none
 
-var DefaultPatternOperatorConfig = map[string]string{
+var DefaultPatternsOperatorConfig = map[string]string{
 	"gitops.catalogSource":       GitOpsDefaultCatalogSource,
 	"gitops.channel":             GitOpsDefaultChannel,
 	"gitops.sourceNamespace":     GitOpsDefaultCatalogSourceNamespace,
@@ -99,15 +99,13 @@ var DefaultPatternOperatorConfig = map[string]string{
 	"catalog.image":              "",
 }
 
-type GitOpsConfig map[string]string
+type PatternsOperatorConfig map[string]string
 
-var PatternsOperatorConfig GitOpsConfig
-
-func (g GitOpsConfig) getValueWithDefault(k string) string {
+func (g PatternsOperatorConfig) getValueWithDefault(k string) string {
 	if v, present := g[k]; present {
 		return v
 	}
-	if defaultValue, present := DefaultPatternOperatorConfig[k]; present {
+	if defaultValue, present := DefaultPatternsOperatorConfig[k]; present {
 		return defaultValue
 	}
 	return ""

internal/controller/defaults_test.go

@@ -5,10 +5,10 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("GitOpsConfig getValueWithDefault", func() {
+var _ = Describe("PatternsOperatorConfig getValueWithDefault", func() {
 	Context("when the key exists in the config", func() {
 		It("should return the config value", func() {
-			config := GitOpsConfig{
+			config := PatternsOperatorConfig{
 				"gitops.channel": "custom-channel",
 			}
 			Expect(config.getValueWithDefault("gitops.channel")).To(Equal("custom-channel"))
@@ -17,56 +17,56 @@ var _ = Describe("GitOpsConfig getValueWithDefault", func() {
 
 	Context("when the key does not exist in config but exists in defaults", func() {
 		It("should return the default value for gitops.channel", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitops.channel")).To(Equal(GitOpsDefaultChannel))
 		})
 
 		It("should return the default value for gitops.catalogSource", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitops.catalogSource")).To(Equal(GitOpsDefaultCatalogSource))
 		})
 
 		It("should return the default value for gitops.sourceNamespace", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitops.sourceNamespace")).To(Equal(GitOpsDefaultCatalogSourceNamespace))
 		})
 
 		It("should return the default value for gitops.installApprovalPlan", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitops.installApprovalPlan")).To(Equal(GitOpsDefaultApprovalPlan))
 		})
 
 		It("should return the default value for gitea.chartName", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitea.chartName")).To(Equal(GiteaChartName))
 		})
 
 		It("should return the default value for gitea.helmRepoUrl", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitea.helmRepoUrl")).To(Equal(GiteaHelmRepoUrl))
 		})
 
 		It("should return the default value for gitea.chartVersion", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("gitea.chartVersion")).To(Equal(GiteaDefaultChartVersion))
 		})
 
 		It("should return the default value for analytics.enabled", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("analytics.enabled")).To(Equal("true"))
 		})
 	})
 
 	Context("when the key does not exist in config or defaults", func() {
 		It("should return an empty string", func() {
-			config := GitOpsConfig{}
+			config := PatternsOperatorConfig{}
 			Expect(config.getValueWithDefault("nonexistent.key")).To(Equal(""))
 		})
 	})
 
 	Context("when config overrides a default value", func() {
 		It("should return the overridden value, not the default", func() {
-			config := GitOpsConfig{
+			config := PatternsOperatorConfig{
 				"gitops.channel": "gitops-1.99",
 			}
 			Expect(config.getValueWithDefault("gitops.channel")).To(Equal("gitops-1.99"))
@@ -75,13 +75,13 @@ var _ = Describe("GitOpsConfig getValueWithDefault", func() {
 
 	Context("when config is nil", func() {
 		It("should return the default value", func() {
-			var config GitOpsConfig
+			var config PatternsOperatorConfig
 			Expect(config.getValueWithDefault("gitops.channel")).To(Equal(GitOpsDefaultChannel))
 		})
 	})
 })
 
-var _ = Describe("DefaultPatternOperatorConfig", func() {
+var _ = Describe("DefaultPatternsOperatorConfig", func() {
 	It("should contain all expected keys", func() {
 		expectedKeys := []string{
 			"gitops.catalogSource",
@@ -94,14 +94,14 @@ var _ = Describe("DefaultPatternOperatorConfig", func() {
 			"analytics.enabled",
 		}
 		for _, key := range expectedKeys {
-			Expect(DefaultPatternOperatorConfig).To(HaveKey(key))
+			Expect(DefaultPatternsOperatorConfig).To(HaveKey(key))
 		}
 	})
 
 	It("should have correct default values", func() {
-		Expect(DefaultPatternOperatorConfig["gitops.catalogSource"]).To(Equal("redhat-operators"))
-		Expect(DefaultPatternOperatorConfig["gitops.sourceNamespace"]).To(Equal("openshift-marketplace"))
-		Expect(DefaultPatternOperatorConfig["gitops.installApprovalPlan"]).To(Equal("Automatic"))
-		Expect(DefaultPatternOperatorConfig["analytics.enabled"]).To(Equal("true"))
+		Expect(DefaultPatternsOperatorConfig["gitops.catalogSource"]).To(Equal("redhat-operators"))
+		Expect(DefaultPatternsOperatorConfig["gitops.sourceNamespace"]).To(Equal("openshift-marketplace"))
+		Expect(DefaultPatternsOperatorConfig["gitops.installApprovalPlan"]).To(Equal("Automatic"))
+		Expect(DefaultPatternsOperatorConfig["analytics.enabled"]).To(Equal("true"))
 	})
 })

internal/controller/pattern_controller.go

@@ -150,6 +150,17 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return reconcile.Result{}, err
 	}
 
+	patternsOperatorConfig := DefaultPatternsOperatorConfig
+
+	configCM, err := GetPatternsOperatorConfigMap()
+	// If we hit an error that is not related to the configmap not existing bubble it up
+	if err != nil && !kerrors.IsNotFound(err) {
+		return r.actionPerformed(instance, "failed to get the configuration ConfigMap", err)
+	}
+	if configCM != nil {
+		patternsOperatorConfig = configCM.Data
+	}
+
 	// Remove the ArgoCD application on deletion
 	if instance.DeletionTimestamp.IsZero() {
 		// Add finalizer when object is created
@@ -198,7 +209,7 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}
 
 	// -- GitOps Subscription
-	if done, result, subErr := r.reconcileGitOpsSubscription(qualifiedInstance); done {
+	if done, result, subErr := r.reconcileGitOpsSubscription(qualifiedInstance, patternsOperatorConfig); done {
 		return result, subErr
 	}
 	logOnce("subscription found")
@@ -262,7 +273,7 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 	// If you specified OriginRepo then we automatically spawn a gitea instance via a special argo gitea application
 	if qualifiedInstance.Spec.GitConfig.OriginRepo != "" {
-		giteaErr := r.createGiteaInstance(qualifiedInstance)
+		giteaErr := r.createGiteaInstance(qualifiedInstance, patternsOperatorConfig)
 		if giteaErr != nil {
 			return r.actionPerformed(qualifiedInstance, "error created gitea instance", giteaErr)
 		}
@@ -339,16 +350,12 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 // reconcileGitOpsSubscription ensures the GitOps operator subscription exists and is up-to-date.
 // It returns (done, result, err) — when done is true the caller should return result/err immediately.
-func (r *PatternReconciler) reconcileGitOpsSubscription(qualifiedInstance *api.Pattern) (done bool, result ctrl.Result, err error) {
+func (r *PatternReconciler) reconcileGitOpsSubscription(qualifiedInstance *api.Pattern, patternsOperatorConfig PatternsOperatorConfig) (done bool, result ctrl.Result, err error) {
 	// Only disable the default ArgoCD instance for non-legacy deployments.
 	// For legacy deployments, the gitops-operator's default instance is still in use.
 	disableDefault := !isLegacyArgoNamespace()
-	targetSub, err := newSubscriptionFromConfigMap(r.fullClient, disableDefault)
+	targetSub := newSubscription(patternsOperatorConfig, disableDefault)
 
-	if err != nil {
-		res, e := r.actionPerformed(qualifiedInstance, "error creating new subscription from configmap", err)
-		return true, res, e
-	}
 	subscriptionName, subscriptionNamespace := DetectGitOpsSubscription()
 	// If the pattern operator is installed to the new vp namespace we need to create a ns, operatorgroup for the new sub
 	if DetectOperatorNamespace() != LegacyOperatorNamespace {
@@ -392,7 +399,7 @@ func (r *PatternReconciler) reconcileGitOpsSubscription(qualifiedInstance *api.P
 		if err := controllerutil.RemoveOwnerReference(qualifiedInstance, currentSub, r.Scheme); err == nil {
 			changed = true
 		}
-		operatorConfigMap, cmErr := GetOperatorConfigmap()
+		operatorConfigMap, cmErr := GetPatternsOperatorConfigMap()
 		if cmErr == nil {
 			if err := controllerutil.RemoveOwnerReference(operatorConfigMap, currentSub, r.Scheme); err == nil {
 				changed = true
@@ -418,7 +425,7 @@ func (r *PatternReconciler) reconcileGitOpsSubscription(qualifiedInstance *api.P
 	return false, ctrl.Result{}, nil
 }
 
-func (r *PatternReconciler) createGiteaInstance(input *api.Pattern) error {
+func (r *PatternReconciler) createGiteaInstance(input *api.Pattern, patternsOperatorConfig PatternsOperatorConfig) error {
 	gitConfig := input.Spec.GitConfig
 	clusterWideNS := getClusterWideArgoNamespace()
 	// The reason we create the vp-gitea namespace and and the
@@ -449,7 +456,7 @@ func (r *PatternReconciler) createGiteaInstance(input *api.Pattern) error {
 	}
 
 	log.Printf("Origin repo is set, creating gitea instance: %s", gitConfig.OriginRepo)
-	giteaApp := newArgoGiteaApplication(input)
+	giteaApp := newArgoGiteaApplication(input, patternsOperatorConfig)
 	_ = controllerutil.SetOwnerReference(input, giteaApp, r.Scheme)
 	app, err := getApplication(r.argoClient, GiteaApplicationName, clusterWideNS)
 	if app == nil {