diff --git a/cmd/main.go b/cmd/main.go index 6814e5ee2..0bb10de74 100644 --- a/cmd/main.go +++ b/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") } @@ -172,7 +172,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) diff --git a/hack/operator-build-deploy.sh b/hack/operator-build-deploy.sh index aaf599036..55573cd40 100755 --- a/hack/operator-build-deploy.sh +++ b/hack/operator-build-deploy.sh @@ -1,7 +1,7 @@ #!/bin/bash set -e -o pipefail -CATALOGSOURCE="test-pattern-operator" +CATALOGSOURCE="test-patterns-operator" DEFAULT_NS="patterns-operator" OPERATOR="patterns-operator" VERSION="${VERSION:-6.6.6}" diff --git a/internal/controller/argo.go b/internal/controller/argo.go index 61a10bf3e..e7faff715 100644 --- a/internal/controller/argo.go +++ b/internal/controller/argo.go @@ -56,10 +56,19 @@ const ( ConsoleLinkResource = "consolelinks" ) -func newArgoCD(name, namespace string) *argooperator.ArgoCD { - argoPolicy := `g, system:cluster-admins, role:admin -g, cluster-admins, role:admin -g, admin, role:admin` +func newArgoCD(name, namespace string, patternsOperatorConfig PatternsOperatorConfig) *argooperator.ArgoCD { + argoPolicies := []string{ + "g, system:cluster-admins, role:admin", + "g, cluster-admins, role:admin", + "g, admin, role:admin", + } + for argoAdmin := range strings.SplitSeq(patternsOperatorConfig.getValueWithDefault("gitops.additionalArgoAdmins"), ",") { + argoAdmin = strings.TrimSpace(argoAdmin) + if argoAdmin != "" { + argoPolicies = append(argoPolicies, "g, "+argoAdmin+", role:admin") + } + } + argoPolicy := strings.Join(argoPolicies, "\n") defaultPolicy := "role:readonly" argoScopes := "[groups,email]" trueBool := true @@ -403,8 +412,8 @@ func haveArgo(client dynamic.Interface, name, namespace string) bool { return err == nil } -func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interface, name, namespace string) error { - argo := newArgoCD(name, namespace) +func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interface, name, namespace string, patternsOperatorConfig PatternsOperatorConfig) error { + argo := newArgoCD(name, namespace, patternsOperatorConfig) gvr := schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource} var err error @@ -911,7 +920,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 +943,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, }, diff --git a/internal/controller/argo_test.go b/internal/controller/argo_test.go index bcb2f05b6..38d3a0217 100644 --- a/internal/controller/argo_test.go +++ b/internal/controller/argo_test.go @@ -824,15 +824,17 @@ var _ = Describe("NewApplicationValues", func() { var _ = Describe("NewArgoCD", func() { var ( - name string - namespace string - argoCD *argooperator.ArgoCD + name string + namespace string + argoCD *argooperator.ArgoCD + patternsOperatorConfig PatternsOperatorConfig ) BeforeEach(func() { name = "test-argocd" namespace = "test-namespace" - argoCD = newArgoCD(name, namespace) + patternsOperatorConfig = DefaultPatternsOperatorConfig + argoCD = newArgoCD(name, namespace, patternsOperatorConfig) }) Context("when creating a new ArgoCD object", func() { @@ -962,10 +964,11 @@ var _ = Describe("haveArgo", func() { var _ = Describe("CreateOrUpdateArgoCD", func() { var ( - dynamicClient dynamic.Interface - gvr schema.GroupVersionResource - name string - namespace string + dynamicClient dynamic.Interface + gvr schema.GroupVersionResource + name string + namespace string + patternsOperatorConfig PatternsOperatorConfig ) BeforeEach(func() { @@ -975,11 +978,12 @@ var _ = Describe("CreateOrUpdateArgoCD", func() { }) name = argoName namespace = argoNS + patternsOperatorConfig = DefaultPatternsOperatorConfig }) Context("when the ArgoCD instance does not exist", func() { It("should create a new ArgoCD instance", func() { - err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace) + err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace, patternsOperatorConfig) Expect(err).ToNot(HaveOccurred()) argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{}) @@ -1007,7 +1011,7 @@ var _ = Describe("CreateOrUpdateArgoCD", func() { }) It("should update the existing ArgoCD instance", func() { - err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace) + err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace, patternsOperatorConfig) Expect(err).ToNot(HaveOccurred()) argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{}) @@ -1038,7 +1042,7 @@ var _ = Describe("CreateOrUpdateArgoCD", func() { }) It("should propagate the error and not update the existing argocd", func() { - err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace) + err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace, patternsOperatorConfig) Expect(err).To(HaveOccurred()) argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{}) @@ -1420,9 +1424,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 +1451,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 +2009,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 +2038,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,52 +2084,54 @@ 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()) }) }) var _ = Describe("newArgoCD", func() { + var argo *argooperator.ArgoCD + It("should create an ArgoCD with the correct name and namespace", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Name).To(Equal("test-argo")) Expect(argo.Namespace).To(Equal("test-ns")) }) It("should have the argoproj.io/finalizer", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Finalizers).To(ContainElement("argoproj.io/finalizer")) }) It("should have HA disabled", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.HA.Enabled).To(BeFalse()) }) It("should have monitoring disabled", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.Monitoring.Enabled).To(BeFalse()) }) It("should have notifications disabled", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.Notifications.Enabled).To(BeFalse()) }) It("should have SSO configured with Dex provider", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.SSO).ToNot(BeNil()) Expect(argo.Spec.SSO.Provider).To(Equal(argooperator.SSOProviderTypeDex)) Expect(argo.Spec.SSO.Dex).ToNot(BeNil()) @@ -2127,37 +2139,45 @@ var _ = Describe("newArgoCD", func() { }) It("should have server route enabled with reencrypt TLS", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.Server.Route.Enabled).To(BeTrue()) Expect(argo.Spec.Server.Route.TLS).ToNot(BeNil()) Expect(argo.Spec.Server.Route.TLS.Termination).To(Equal(routev1.TLSTerminationReencrypt)) }) It("should have resource exclusions for tekton", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.ResourceExclusions).To(ContainSubstring("tekton.dev")) Expect(argo.Spec.ResourceExclusions).To(ContainSubstring("TaskRun")) Expect(argo.Spec.ResourceExclusions).To(ContainSubstring("PipelineRun")) }) It("should have resource health checks for Subscription", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.ResourceHealthChecks).To(HaveLen(1)) Expect(argo.Spec.ResourceHealthChecks[0].Group).To(Equal("operators.coreos.com")) Expect(argo.Spec.ResourceHealthChecks[0].Kind).To(Equal("Subscription")) }) It("should have init containers for CA cert fetching", func() { - argo := newArgoCD("test-argo", "test-ns") + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.Repo.InitContainers).To(HaveLen(1)) Expect(argo.Spec.Repo.InitContainers[0].Name).To(Equal("fetch-ca")) }) - It("should have correct RBAC policy", func() { - argo := newArgoCD("test-argo", "test-ns") + It("should have correct RBAC policy with defaults", func() { + argo = newArgoCD("test-argo", "test-ns", DefaultPatternsOperatorConfig) Expect(argo.Spec.RBAC.Policy).ToNot(BeNil()) Expect(*argo.Spec.RBAC.Policy).To(ContainSubstring("cluster-admins")) }) + + It("should have correct RBAC policy with additional admin", func() { + argo = newArgoCD("test-argo", "test-ns", PatternsOperatorConfig{"gitops.additionalArgoAdmins": "test-admins"}) + Expect(argo.Spec.RBAC.Policy).ToNot(BeNil()) + Expect(*argo.Spec.RBAC.Policy).To(ContainSubstring("cluster-admins")) + Expect(*argo.Spec.RBAC.Policy).To(ContainSubstring("test-admins")) + }) + }) var _ = Describe("commonSyncPolicy", func() { diff --git a/internal/controller/defaults.go b/internal/controller/defaults.go index 853f6a221..66fb66e06 100644 --- a/internal/controller/defaults.go +++ b/internal/controller/defaults.go @@ -86,28 +86,27 @@ const ( // Experimental Capabilities that can be enabled // Currently none -var DefaultPatternOperatorConfig = map[string]string{ - "gitops.catalogSource": GitOpsDefaultCatalogSource, - "gitops.channel": GitOpsDefaultChannel, - "gitops.sourceNamespace": GitOpsDefaultCatalogSourceNamespace, - "gitops.installApprovalPlan": GitOpsDefaultApprovalPlan, - "gitops.csv": GitOpsDefaultCSV, - "gitea.chartName": GiteaChartName, - "gitea.helmRepoUrl": GiteaHelmRepoUrl, - "gitea.chartVersion": GiteaDefaultChartVersion, - "analytics.enabled": "true", - "catalog.image": "", +var DefaultPatternsOperatorConfig = map[string]string{ + "gitops.catalogSource": GitOpsDefaultCatalogSource, + "gitops.channel": GitOpsDefaultChannel, + "gitops.sourceNamespace": GitOpsDefaultCatalogSourceNamespace, + "gitops.installApprovalPlan": GitOpsDefaultApprovalPlan, + "gitops.csv": GitOpsDefaultCSV, + "gitops.additionalArgoAdmins": "", + "gitea.chartName": GiteaChartName, + "gitea.helmRepoUrl": GiteaHelmRepoUrl, + "gitea.chartVersion": GiteaDefaultChartVersion, + "analytics.enabled": "true", + "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 "" diff --git a/internal/controller/defaults_test.go b/internal/controller/defaults_test.go index 338746d2f..d55ce8d42 100644 --- a/internal/controller/defaults_test.go +++ b/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,61 @@ 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 gitops.additionalArgoAdmins", func() { + config := PatternsOperatorConfig{} + Expect(config.getValueWithDefault("gitops.additionalArgoAdmins")).To(Equal("")) + }) + 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 +80,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 +99,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")) }) }) diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index f5b6b4bb6..862187c47 100644 --- a/internal/controller/pattern_controller.go +++ b/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") @@ -240,7 +251,7 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } // We only update the clusterwide argo instance so we can define our own 'initcontainers' section - err = createOrUpdateArgoCD(r.dynamicClient, r.fullClient, getClusterWideArgoName(), clusterWideNS) + err = createOrUpdateArgoCD(r.dynamicClient, r.fullClient, getClusterWideArgoName(), clusterWideNS, patternsOperatorConfig) if err != nil { return r.actionPerformed(qualifiedInstance, "created or updated clusterwide argo instance", err) } @@ -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 { diff --git a/internal/controller/subscription.go b/internal/controller/subscription.go index c928053b9..ab5244a8f 100644 --- a/internal/controller/subscription.go +++ b/internal/controller/subscription.go @@ -27,37 +27,25 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" ) -func newSubscriptionFromConfigMap(r kubernetes.Interface, disableDefaultInstance bool) (*operatorv1alpha1.Subscription, error) { +func newSubscription(patternsOperatorConfig PatternsOperatorConfig, disableDefaultInstance bool) *operatorv1alpha1.Subscription { var newSubscription *operatorv1alpha1.Subscription - // Check if the config map exists and read the config map values - cm, err := r.CoreV1().ConfigMaps(DetectOperatorNamespace()).Get(context.Background(), OperatorConfigMap, metav1.GetOptions{}) - // If we hit an error that is not related to the configmap not existing bubble it up - if err != nil && !apierrors.IsNotFound(err) { - return nil, err - } - - if cm != nil { - PatternsOperatorConfig = cm.Data - } - var installPlanApproval operatorv1alpha1.Approval - if PatternsOperatorConfig.getValueWithDefault("gitops.installApprovalPlan") == "Manual" { + if patternsOperatorConfig.getValueWithDefault("gitops.installApprovalPlan") == "Manual" { installPlanApproval = operatorv1alpha1.ApprovalManual } else { installPlanApproval = operatorv1alpha1.ApprovalAutomatic } spec := &operatorv1alpha1.SubscriptionSpec{ - CatalogSource: PatternsOperatorConfig.getValueWithDefault("gitops.catalogSource"), - CatalogSourceNamespace: PatternsOperatorConfig.getValueWithDefault("gitops.sourceNamespace"), + CatalogSource: patternsOperatorConfig.getValueWithDefault("gitops.catalogSource"), + CatalogSourceNamespace: patternsOperatorConfig.getValueWithDefault("gitops.sourceNamespace"), Package: GitOpsDefaultPackageName, - Channel: PatternsOperatorConfig.getValueWithDefault("gitops.channel"), - StartingCSV: PatternsOperatorConfig.getValueWithDefault("gitops.csv"), + Channel: patternsOperatorConfig.getValueWithDefault("gitops.channel"), + StartingCSV: patternsOperatorConfig.getValueWithDefault("gitops.csv"), InstallPlanApproval: installPlanApproval, Config: &operatorv1alpha1.SubscriptionConfig{ Env: newSubscriptionEnvVars(disableDefaultInstance), @@ -73,7 +61,7 @@ func newSubscriptionFromConfigMap(r kubernetes.Interface, disableDefaultInstance Spec: spec, } - return newSubscription, nil + return newSubscription } // newSubscriptionEnvVars returns the environment variables for the GitOps operator subscription. diff --git a/internal/controller/subscription_test.go b/internal/controller/subscription_test.go index b8b802a13..828ab5509 100644 --- a/internal/controller/subscription_test.go +++ b/internal/controller/subscription_test.go @@ -12,7 +12,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - kubeclient "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/testing" ) @@ -87,35 +86,16 @@ var _ = Describe("Subscription Functions", func() { }) }) - Context("newSubscriptionFromConfigMap (operator in suggested namespace patterns-operator)", func() { + Context("newSubscription (operator in suggested namespace patterns-operator)", func() { var testConfigMap *corev1.ConfigMap - var fakeClientSet *kubeclient.Clientset BeforeEach(func() { - fakeClientSet = kubeclient.NewSimpleClientset() cm := newDefaultTestSubConfigMap() testConfigMap = cm.DeepCopy() }) - It("should handle the absence of the ConfigMap gracefully", func() { - sub, err := newSubscriptionFromConfigMap(fakeClientSet, true) - Expect(err).ToNot(HaveOccurred()) - Expect(sub).NotTo(BeNil()) - Expect(sub.Spec.CatalogSource).To(Equal(GitOpsDefaultCatalogSource)) - Expect(sub.Spec.CatalogSourceNamespace).To(Equal(GitOpsDefaultCatalogSourceNamespace)) - Expect(sub.Spec.Package).To(Equal(GitOpsDefaultPackageName)) - Expect(sub.Spec.Channel).To(Equal(GitOpsDefaultChannel)) - Expect(sub.Spec.StartingCSV).To(BeEmpty()) - Expect(sub.Spec.InstallPlanApproval).To(Equal(operatorv1alpha1.ApprovalAutomatic)) - Expect(sub.Namespace).To(Equal(GitOpsDefaultSubscriptionNamespace)) - Expect(sub.Name).To(Equal(GitOpsDefaultPackageName)) - }) - It("should create a Subscription from a configmap", func() { - _, err := fakeClientSet.CoreV1().ConfigMaps(DetectOperatorNamespace()).Create(context.Background(), testConfigMap, metav1.CreateOptions{}) - Expect(err).ToNot(HaveOccurred()) - sub, err := newSubscriptionFromConfigMap(fakeClientSet, true) - Expect(err).ToNot(HaveOccurred()) + sub := newSubscription(testConfigMap.Data, true) Expect(sub).NotTo(BeNil()) Expect(sub.Spec.CatalogSource).To(Equal("foo-source")) Expect(sub.Spec.CatalogSourceNamespace).To(Equal("foo-source-namespace")) @@ -128,13 +108,11 @@ var _ = Describe("Subscription Functions", func() { }) }) - Context("newSubscriptionFromConfigMap (operator in legacy namespace openshift-operators)", func() { + Context("newSubscription (operator in legacy namespace openshift-operators)", func() { var testConfigMap *corev1.ConfigMap - var fakeClientSet *kubeclient.Clientset BeforeEach(func() { os.Setenv("OPERATOR_NAMESPACE", LegacyOperatorNamespace) - fakeClientSet = kubeclient.NewSimpleClientset() cm := newDefaultTestSubConfigMap() testConfigMap = cm.DeepCopy() }) @@ -144,10 +122,7 @@ var _ = Describe("Subscription Functions", func() { }) It("should create a Subscription to legacy operator ns", func() { - _, err := fakeClientSet.CoreV1().ConfigMaps(DetectOperatorNamespace()).Create(context.Background(), testConfigMap, metav1.CreateOptions{}) - Expect(err).ToNot(HaveOccurred()) - sub, err := newSubscriptionFromConfigMap(fakeClientSet, true) - Expect(err).ToNot(HaveOccurred()) + sub := newSubscription(testConfigMap.Data, true) Expect(sub).NotTo(BeNil()) Expect(sub.Spec.CatalogSource).To(Equal("foo-source")) Expect(sub.Spec.CatalogSourceNamespace).To(Equal("foo-source-namespace")) @@ -158,20 +133,6 @@ var _ = Describe("Subscription Functions", func() { Expect(sub.Namespace).To(Equal(GitOpsLegacySubscriptionNamespace)) Expect(sub.Name).To(Equal(GitOpsDefaultPackageName)) }) - - It("should handle the absence of the ConfigMap gracefully (legacy operator ns)", func() { - sub, err := newSubscriptionFromConfigMap(fakeClientSet, true) - Expect(err).ToNot(HaveOccurred()) - Expect(sub).NotTo(BeNil()) - Expect(sub.Spec.CatalogSource).To(Equal(GitOpsDefaultCatalogSource)) - Expect(sub.Spec.CatalogSourceNamespace).To(Equal(GitOpsDefaultCatalogSourceNamespace)) - Expect(sub.Spec.Package).To(Equal(GitOpsDefaultPackageName)) - Expect(sub.Spec.Channel).To(Equal(GitOpsDefaultChannel)) - Expect(sub.Spec.StartingCSV).To(BeEmpty()) - Expect(sub.Spec.InstallPlanApproval).To(Equal(operatorv1alpha1.ApprovalAutomatic)) - Expect(sub.Namespace).To(Equal(GitOpsLegacySubscriptionNamespace)) - Expect(sub.Name).To(Equal(GitOpsDefaultPackageName)) - }) }) }) diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 8fb602f17..6aef79a68 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -451,7 +451,7 @@ func IntOrZero(secret map[string][]byte, key string) (int64, error) { } // Gets the configmap for the Patterns Operator. (Used as an owner reference for the operator itself.) -func GetOperatorConfigmap() (*corev1.ConfigMap, error) { +func GetPatternsOperatorConfigMap() (*corev1.ConfigMap, error) { config, err := ctrl.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get config: %s", err) diff --git a/internal/controller/values_test.go b/internal/controller/values_test.go index 882378211..d5e35003f 100644 --- a/internal/controller/values_test.go +++ b/internal/controller/values_test.go @@ -406,38 +406,38 @@ var _ = Describe("CountApplicationsAndSets", func() { }) }) -var _ = Describe("GitOpsConfig getValueWithDefault", func() { +var _ = Describe("PatternsOperatorConfig getValueWithDefault", func() { Context("when value exists in config", func() { It("should return the configured value", func() { - config := GitOpsConfig{"key1": "value1"} + config := PatternsOperatorConfig{"key1": "value1"} Expect(config.getValueWithDefault("key1")).To(Equal("value1")) }) }) Context("when value does not exist in config but has a default", func() { It("should return the default value", func() { - config := GitOpsConfig{} + config := PatternsOperatorConfig{} Expect(config.getValueWithDefault("gitops.channel")).To(Equal(GitOpsDefaultChannel)) }) }) Context("when value does not exist anywhere", func() { It("should return empty string", func() { - config := GitOpsConfig{} + config := PatternsOperatorConfig{} Expect(config.getValueWithDefault("nonexistent.key")).To(Equal("")) }) }) Context("when config overrides a default", func() { It("should return the config value not the default", func() { - config := GitOpsConfig{"gitops.channel": "custom-channel"} + config := PatternsOperatorConfig{"gitops.channel": "custom-channel"} Expect(config.getValueWithDefault("gitops.channel")).To(Equal("custom-channel")) }) }) 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)) }) })