chore(informer): Configure controller-runtime to not use shared list+watch informer for secrets

m8rmclaren · m8rmclaren · commit d229b82450c9 · 2024-12-28T13:49:18.000-07:00
Signed-off-by: Hayden Roszell &lt;hroszell@gmail.com&gt;
diff --git a/cmd/main.go b/cmd/main.go
@@ -28,10 +28,13 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/utils/clock"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -122,10 +125,18 @@ func main() {
 		}
 	}
 
+	var cacheOpts cache.Options
 	if secretAccessGrantedAtClusterLevel {
-		setupLog.Info("expecting secret access at cluster level")
+		setupLog.Info("expecting SA to have Get+List+Watch permissions for corev1 Secret resources at cluster level")
 	} else {
-		setupLog.Info(fmt.Sprintf("expecting secret access at namespace level (%s)", clusterResourceNamespace))
+		setupLog.Info(fmt.Sprintf("expecting SA to have Get+List+Watch permissions for corev1 Secret resources in the %q namespace", clusterResourceNamespace))
+		cacheOpts = cache.Options{
+			ByObject: map[client.Object]cache.ByObject{
+				&corev1.Secret{}: {
+					Namespaces: map[string]cache.Config{clusterResourceNamespace: cache.Config{}},
+				},
+			},
+		}
 	}
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
@@ -135,6 +146,7 @@ func main() {
 			SecureServing: secureMetrics,
 			TLSOpts:       tlsOpts,
 		},
+		Cache:                  cacheOpts,
 		WebhookServer:          webhookServer,
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
diff --git a/deploy/charts/command-cert-manager-issuer/README.md b/deploy/charts/command-cert-manager-issuer/README.md
@@ -85,5 +85,4 @@ The following table lists the configurable parameters of the `command-cert-manag
 | `resources`                                  | CPU/Memory resource requests/limits                                                                                                      | `{}` (with commented out options)                     |
 | `nodeSelector`                               | Node labels for pod assignment                                                                                                           | `{}`                                                  |
 | `tolerations`                                | Tolerations for pod assignment                                                                                                           | `[]`                                                  |
-| `secureMetrics.enabled`                      | Whether to enable and configure the kube-rbac-proxy sidecar for authorized and authenticated use of the /metrics endpoint by Prometheus. | `false`                                               |
 | `secretConfig.useClusterRoleForSecretAccess` | Specifies if the ServiceAccount should be granted access to the Secret resource using a ClusterRole                                      | `false`                                               |
diff --git a/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml b/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml
@@ -45,37 +45,3 @@ rules:
       - issuers/finalizers
     verbs:
       - update
-{{- if .Values.secureMetrics.enabled }}
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
-  labels:
-    {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
-  name: {{ include "command-cert-manager-issuer.name" . }}-proxy-role
-rules:
-  - apiGroups:
-      - authentication.k8s.io
-    resources:
-      - tokenreviews
-    verbs:
-      - create
-  - apiGroups:
-      - authorization.k8s.io
-    resources:
-      - subjectaccessreviews
-    verbs:
-      - create
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
-  labels:
-    {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
-  name: {{ include "command-cert-manager-issuer.name" . }}-metrics-reader
-rules:
-  - nonResourceURLs:
-      - /metrics
-    verbs:
-      - get
-{{- end }}
diff --git a/deploy/charts/command-cert-manager-issuer/templates/clusterrolebinding.yaml b/deploy/charts/command-cert-manager-issuer/templates/clusterrolebinding.yaml
@@ -12,20 +12,3 @@ subjects:
   - kind: ServiceAccount
     name: {{ include "command-cert-manager-issuer.serviceAccountName" . }}
     namespace: {{ .Release.Namespace }}
-{{- if .Values.secureMetrics.enabled }}
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
-  labels:
-    {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
-  name: {{ include "command-cert-manager-issuer.name" . }}-proxy-rolebinding
-roleRef:
-  apiGroup: rbac.authorization.k8s.io
-  kind: ClusterRole
-  name: {{ include "command-cert-manager-issuer.name" . }}-proxy-role
-subjects:
-  - kind: ServiceAccount
-    name: {{ include "command-cert-manager-issuer.serviceAccountName" . }}
-    namespace: {{ .Release.Namespace }}
-{{- end }}
diff --git a/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml b/deploy/charts/command-cert-manager-issuer/templates/deployment.yaml
@@ -17,6 +17,9 @@ spec:
       {{- end }}
       labels:
         {{- include "command-cert-manager-issuer.selectorLabels" . | nindent 8 }}
+        {{- with .Values.podLabels }}
+        {{- toYaml . | nindent 8 }}
+        {{- end }}
     spec:
       {{- with .Values.imagePullSecrets }}
       imagePullSecrets:
@@ -26,31 +29,6 @@ spec:
       securityContext:
         {{- toYaml .Values.podSecurityContext | nindent 8 }}
       containers:
-        {{- if .Values.secureMetrics.enabled }}
-        - args:
-            - --secure-listen-address=0.0.0.0:8443
-            - --upstream=http://127.0.0.1:8080/
-            - --logtostderr=true
-            - --v=0
-          image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.1
-          name: kube-rbac-proxy
-          ports:
-            - containerPort: 8443
-              name: https
-              protocol: TCP
-          resources:
-            limits:
-              cpu: 500m
-              memory: 128Mi
-            requests:
-              cpu: 5m
-              memory: 64Mi
-          securityContext:
-            allowPrivilegeEscalation: false
-            capabilities:
-              drop:
-                - ALL
-        {{- end }}
         - args:
             - --health-probe-bind-address=:8081
             - --metrics-bind-address=127.0.0.1:8080
diff --git a/deploy/charts/command-cert-manager-issuer/templates/role.yaml b/deploy/charts/command-cert-manager-issuer/templates/role.yaml
@@ -23,4 +23,20 @@ rules:
       - events
     verbs:
       - create
-      - patch
+      - patch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRole{{ else }}Role{{ end }}
+metadata:
+  labels:
+    {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
+  name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-role
+rules:
+  - apiGroups:
+      - ""
+    resources:
+      - secrets
+    verbs:
+      - get
+      - list
+      - watch
diff --git a/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml b/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml
@@ -11,4 +11,19 @@ roleRef:
 subjects:
   - kind: ServiceAccount
     name: {{ include "command-cert-manager-issuer.serviceAccountName" . }}
-    namespace: {{ .Release.Namespace }}
+    namespace: {{ .Release.Namespace }}
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRoleBinding{{ else }}RoleBinding{{ end }}
+metadata:
+  labels:
+    {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
+  name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRole{{ else }}Role{{ end }}
+  name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-role
+subjects:
+  - kind: ServiceAccount
+    name: {{ include "command-cert-manager-issuer.serviceAccountName" . }}
+    namespace: {{ .Release.Namespace }}
diff --git a/deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml b/deploy/charts/command-cert-manager-issuer/templates/secretrole.yaml
diff --git a/deploy/charts/command-cert-manager-issuer/templates/service.yaml b/deploy/charts/command-cert-manager-issuer/templates/service.yaml
diff --git a/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml b/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml
@@ -5,6 +5,9 @@ metadata:
   name: {{ include "command-cert-manager-issuer.serviceAccountName" . }}
   labels:
     {{- include "command-cert-manager-issuer.labels" . | nindent 4 }}
+    {{- if .Values.serviceAccount.labels }}
+    {{- toYaml .Values.serviceAccount.labels | nindent 4 }}
+    {{- end }}
   {{- with .Values.serviceAccount.annotations }}
   annotations:
     {{- toYaml . | nindent 4 }}
diff --git a/deploy/charts/command-cert-manager-issuer/values.yaml b/deploy/charts/command-cert-manager-issuer/values.yaml
@@ -13,11 +13,6 @@ imagePullSecrets: []
 nameOverride: ""
 fullnameOverride: ""
 
-# Whether to enable and configure the kube-rbac-proxy sidecar for authorized and authenticated
-# use of the /metrics endpoint by Prometheus.
-secureMetrics:
-  enabled: false
-
 secretConfig:
   # If true, when using Issuer resources, the credential secret must be created in the same namespace as the
   # Issuer resource. This access is facilitated by granting the ServiceAccount [get, list, watch] for the secret
@@ -37,12 +32,16 @@ crd:
 serviceAccount:
   # Specifies whether a service account should be created
   create: true
+  # Labels to add to the service account
+  labels: {}
   # Annotations to add to the service account
   annotations: {}
   # The name of the service account to use.
   # If not set and create is true, a name is generated using the fullname template
   name: ""
 
+podLabels: {}
+
 podAnnotations: {}
 
 podSecurityContext:
diff --git a/internal/command/command.go b/internal/command/command.go
@@ -237,7 +237,7 @@ func newServerConfig(ctx context.Context, config *Config) (*auth_providers.Serve
 		}
 	}
 
-	log.Info("configuration was valid - successfully generated server config", "authMethod", server.AuthType, "hostname", server.Host, "apiPath", server.APIPath)
+	log.Info("Configuration was valid - Successfully generated server config", "authMethod", server.AuthType, "hostname", server.Host, "apiPath", server.APIPath)
 	return server, nil
 }
 
@@ -287,7 +287,7 @@ func newInternalSigner(ctx context.Context, config *Config, newClientFunc newCom
 		testConnection:       client.AuthClient.Authenticate,
 	}
 
-	log.Info("successfully generated Command client")
+	log.Info("Successfully generated Command client")
 	s.client = adapter
 
 	return s, nil
diff --git a/internal/controller/certificaterequest_controller.go b/internal/controller/certificaterequest_controller.go
@@ -109,6 +109,8 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, nil
 	}
 
+	log.Info("Starting CertificateRequest reconciliation run")
+
 	// We now have a CertificateRequest that belongs to us so we are responsible
 	// for updating its Ready condition.
 
@@ -164,7 +166,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 	}
 	issuer, ok := issuerRO.(commandissuer.IssuerLike)
 	if !ok {
-		err := fmt.Errorf("unexpected type for issuer object: %T", issuerRO)
+		err := fmt.Errorf("%w: unexpected type for issuer object: %T", errIssuerRef, issuerRO)
 		log.Error(err, "Failed to cast to commandissuer.IssuerLike")
 		setCertificateRequestReadyCondition(&certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, err.Error())
 		return ctrl.Result{}, nil
diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go
@@ -75,17 +75,19 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 
 	issuer, err := r.newIssuer()
 	if err != nil {
-		log.Error(err, "unrecognized issuer type")
+		log.Error(err, "Unrecognized issuer type")
 		return ctrl.Result{}, nil
 	}
 	if err := r.Get(ctx, req.NamespacedName, issuer); err != nil {
 		if err := client.IgnoreNotFound(err); err != nil {
 			return ctrl.Result{}, fmt.Errorf("unexpected get error: %v", err)
 		}
-		log.Info("Issuer not found. ignoring.")
+		log.Info(fmt.Sprintf("%s not found. Ignoring.", issuer.GetObjectKind().GroupVersionKind().Kind))
 		return ctrl.Result{}, nil
 	}
 
+	log.Info(fmt.Sprintf("Starting %s reconciliation run", issuer.GetObjectKind().GroupVersionKind().Kind))
+
 	// Always attempt to update the Ready condition
 	defer func() {
 		if err != nil {
@@ -103,7 +105,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 	case issuer.IsClusterScoped():
 		secretNamespace = r.ClusterResourceNamespace
 
-	case !issuer.IsClusterScoped():
+	default:
 		secretNamespace = req.Namespace
 	}
 
@@ -152,6 +154,7 @@ func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer comman
 	// The SecretName is optional since the user may elect to use ambient credentials for scenarios like Workload Identity.
 	if issuer.GetSpec().SecretName != "" {
 		var authSecret corev1.Secret
+		log.Info("Fetching commandSecret from ns", "name", issuer.GetSpec().SecretName, "namespace", secretNamespace)
 		err := c.Get(ctx, types.NamespacedName{
 			Name:      issuer.GetSpec().SecretName,
 			Namespace: secretNamespace,
@@ -188,7 +191,7 @@ func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer comman
 			if ok {
 				oauth.Audience = string(audience)
 			}
-			log.Info("found oauth client credentials in secret", "commandSecretName", issuer.GetSpec().SecretName, "type", authSecret.Type)
+			log.Info("Found oauth client credentials in secret", "commandSecretName", issuer.GetSpec().SecretName, "type", authSecret.Type)
 
 		case authSecret.Type == corev1.SecretTypeBasicAuth:
 			username, ok := authSecret.Data[corev1.BasicAuthUsernameKey]
@@ -204,12 +207,11 @@ func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer comman
 				Username: string(username),
 				Password: string(password),
 			}
-			log.Info("found basic auth credentials in secret", "commandSecretName", issuer.GetSpec().SecretName, "type", authSecret.Type)
+			log.Info("Found basic auth credentials in secret", "commandSecretName", issuer.GetSpec().SecretName, "type", authSecret.Type)
 
 		default:
 			return nil, fmt.Errorf("%w: %s", errGetAuthSecret, "found secret with unsupported type")
 		}
-
 	}
 
 	var caSecret corev1.Secret
