refactor: move TLS profile deployment hook into controller/common, along with some PR comments addressed.

Author: Arun Maurya

.golangci.yaml

@@ -130,11 +130,11 @@ linters:
         linters:
           - godot
       # Exclude false positive hardcoded credential warnings (these are paths, not credentials)
-      - path: pkg/controller/deployment/credentials_request.go
+      - path: pkg/controller/certmanager/credentials_request.go
         text: "gcpCredentialsDir|cloudCredentialsVolumeName"
         linters:
           - gosec
-      - path: pkg/controller/deployment/deployment_overrides.go
+      - path: pkg/controller/certmanager/deployment_overrides.go
         text: "boundSATokenDir"
         linters:
           - gosec
@@ -150,7 +150,7 @@ linters:
       # Exclude unparam for functions that may be part of interfaces
       - linters:
           - unparam
-        path: pkg/controller/deployment/
+        path: pkg/controller/certmanager/
   settings:
     wrapcheck:
       # Ignore errors returned from these packages - they are either internal or

README.md

@@ -5,9 +5,9 @@ This repository contains Cert Manager Operator designed for OpenShift. The opera
 ## The operator architecture and design assumptions
 
 The Operator uses the [upstream deployment manifests](https://github.com/cert-manager/cert-manager/releases). It divides them into separate files and deploys using 3 controllers:
-- [cert_manager_cainjector_deployment.go](pkg/controller/deployment/cert_manager_cainjector_deployment.go)
-- [cert_manager_controller_deployment.go](pkg/controller/deployment/cert_manager_controller_deployment.go)
-- [cert_manager_webhook_deployment.go](pkg/controller/deployment/cert_manager_webhook_deployment.go)
+- [cert_manager_cainjector_deployment.go](pkg/controller/certmanager/cert_manager_cainjector_deployment.go)
+- [cert_manager_controller_deployment.go](pkg/controller/certmanager/cert_manager_controller_deployment.go)
+- [cert_manager_webhook_deployment.go](pkg/controller/certmanager/cert_manager_webhook_deployment.go)
 
 The Operator automatically deploys a cluster-scoped `CertManager` object named `cluster` if it's missing (with default values).
 

pkg/controller/certmanager/generic_deployment_controller.go

@@ -12,7 +12,7 @@ import (
 	"github.com/openshift/library-go/pkg/operator/status"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
 
-	deploymentpkg "github.com/openshift/cert-manager-operator/pkg/controller/deployment"
+	"github.com/openshift/cert-manager-operator/pkg/controller/common"
 	"github.com/openshift/cert-manager-operator/pkg/operator/assets"
 	certmanoperatorinformers "github.com/openshift/cert-manager-operator/pkg/operator/informers/externalversions"
 	"github.com/openshift/cert-manager-operator/pkg/operator/utils"
@@ -69,7 +69,7 @@ func newGenericDeploymentController(
 
 		informers = append(informers, infraInformerFactory.Config().V1().Infrastructures().Informer())
 
-		hooks = append(hooks, deploymentpkg.WithClusterTLSProfileFromAPIServer(infraInformerFactory.Config().V1().APIServers()))
+		hooks = append(hooks, common.WithClusterTLSProfileFromAPIServer(infraInformerFactory.Config().V1().APIServers()))
 		informers = append(informers, infraInformerFactory.Config().V1().APIServers().Informer())
 	}
 

pkg/controller/common/container_args.go

@@ -11,7 +11,7 @@ const argKeyValSeparator = "="
 // MergeContainerArgs merges source args with override values using a map that
 // tracks unique keys for each arg of the form key[=value].
 func MergeContainerArgs(sourceArgs []string, overrideArgs []string) []string {
-	destArgMap := map[string]string{}
+	destArgMap := make(map[string]string)
 	ParseArgMap(destArgMap, sourceArgs)
 	ParseArgMap(destArgMap, overrideArgs)
 

…ontroller/deployment/tls_profile_hook.go pkg/controller/common/tls_profile_hook.gopkg/controller/deployment/tls_profile_hook.go renamed to pkg/controller/common/tls_profile_hook.go pkg/controller/deployment/tls_profile_hook.go renamed to pkg/controller/common/tls_profile_hook.go

@@ -1,4 +1,4 @@
-package deployment
+package common
 
 import (
 	"fmt"
@@ -11,7 +11,6 @@ import (
 	configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
 	libgocrypto "github.com/openshift/library-go/pkg/crypto"
 
-	"github.com/openshift/cert-manager-operator/pkg/controller/common"
 	"github.com/openshift/cert-manager-operator/pkg/tlsprofile"
 )
 
@@ -64,9 +63,9 @@ func WithClusterTLSProfileFromAPIServer(apiServerInformer configinformersv1.APIS
 		container := deployment.Spec.Template.Spec.Containers[0]
 		sourceArgs := container.Args
 		if effective.MinTLSVersion == configv1.VersionTLS13 {
-			sourceArgs = common.StripArgsByKeys(sourceArgs, common.ArgKeysSet(tlsprofile.CertManagerCipherSuiteArgKeys))
+			sourceArgs = StripArgsByKeys(sourceArgs, ArgKeysSet(tlsprofile.CertManagerCipherSuiteArgKeys))
 		}
-		container.Args = common.MergeContainerArgs(sourceArgs, extra)
+		container.Args = MergeContainerArgs(sourceArgs, extra)
 		deployment.Spec.Template.Spec.Containers[0] = container
 		return nil
 	}

…ller/deployment/tls_profile_hook_test.go …ntroller/common/tls_profile_hook_test.gopkg/controller/deployment/tls_profile_hook_test.go renamed to pkg/controller/common/tls_profile_hook_test.go pkg/controller/deployment/tls_profile_hook_test.go renamed to pkg/controller/common/tls_profile_hook_test.go

@@ -1,4 +1,4 @@
-package deployment
+package common
 
 import (
 	"fmt"

test/e2e/overrides_test.go

@@ -50,7 +50,7 @@ var _ = Describe("Overrides test", Ordered, Label("Platform:Generic"), func() {
 			By("Adding cert-manager controller override args to the cert-manager operator object")
 			args := []string{
 				// good-have to sync these args updated with the args present in
-				// pkg/controller/deployment/deployment_overrides_validation.go,
+				// pkg/controller/certmanager/deployment_overrides_validation.go,
 				// so the e2e is self-aware of overrideArgs.
 
 				"--acme-http01-solver-resource-limits-cpu=150m",