diff --git a/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml b/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml index eadb8fb89..ccf375bb5 100644 --- a/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml +++ b/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cainjector diff --git a/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml b/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml index a45746a57..2ef959647 100644 --- a/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml +++ b/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cert-manager diff --git a/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml new file mode 100644 index 000000000..698759898 --- /dev/null +++ b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml @@ -0,0 +1,25 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: cert-manager-metrics-dynamic-serving +subjects: + - kind: ServiceAccount + name: cert-manager + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-webhook + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-cainjector + namespace: cert-manager diff --git a/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml new file mode 100644 index 000000000..e16013379 --- /dev/null +++ b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml @@ -0,0 +1,29 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +rules: + - apiGroups: + - "" + resourceNames: + - cert-manager-metrics-ca + resources: + - secrets + verbs: + - get + - list + - watch + - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - create diff --git a/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml b/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml index 127e5ec08..82711bbec 100644 --- a/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml +++ b/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: webhook diff --git a/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml b/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml index e76b8e454..ce9b34508 100644 --- a/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml +++ b/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml @@ -1,6 +1,8 @@ apiVersion: v1 kind: Service metadata: + annotations: + service.beta.openshift.io/serving-cert-secret-name: cert-manager-operator-serving-cert creationTimestamp: null labels: app.kubernetes.io/created-by: cert-manager-operator diff --git a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml index aeb2b2fff..54b469184 100644 --- a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml @@ -284,7 +284,7 @@ metadata: features.operators.openshift.io/disconnected: "true" features.operators.openshift.io/fips-compliant: "true" features.operators.openshift.io/proxy-aware: "true" - features.operators.openshift.io/tls-profiles: "false" + features.operators.openshift.io/tls-profiles: "true" features.operators.openshift.io/token-auth-aws: "true" features.operators.openshift.io/token-auth-azure: "true" features.operators.openshift.io/token-auth-gcp: "true" @@ -848,6 +848,9 @@ spec: volumeMounts: - mountPath: /tmp name: tmp + - mountPath: /var/run/secrets/serving-cert + name: serving-cert + readOnly: true securityContext: runAsNonRoot: true seccompProfile: @@ -857,6 +860,10 @@ spec: volumes: - emptyDir: {} name: tmp + - name: serving-cert + secret: + optional: true + secretName: cert-manager-operator-serving-cert permissions: - rules: - apiGroups: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index af071a2b7..411d243cd 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -123,8 +123,15 @@ spec: volumeMounts: - name: tmp mountPath: /tmp + - name: serving-cert + mountPath: /var/run/secrets/serving-cert + readOnly: true serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: - name: tmp emptyDir: {} + - name: serving-cert + secret: + secretName: cert-manager-operator-serving-cert + optional: true diff --git a/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml b/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml index 9f3841f14..efe298419 100644 --- a/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml @@ -14,7 +14,7 @@ metadata: features.operators.openshift.io/disconnected: "true" features.operators.openshift.io/fips-compliant: "true" features.operators.openshift.io/proxy-aware: "true" - features.operators.openshift.io/tls-profiles: "false" + features.operators.openshift.io/tls-profiles: "true" features.operators.openshift.io/token-auth-aws: "true" features.operators.openshift.io/token-auth-azure: "true" features.operators.openshift.io/token-auth-gcp: "true" diff --git a/config/rbac/auth_proxy_service.yaml b/config/rbac/auth_proxy_service.yaml index 3afdfb7d9..5057d176b 100644 --- a/config/rbac/auth_proxy_service.yaml +++ b/config/rbac/auth_proxy_service.yaml @@ -1,6 +1,8 @@ apiVersion: v1 kind: Service metadata: + annotations: + service.beta.openshift.io/serving-cert-secret-name: cert-manager-operator-serving-cert labels: control-plane: controller-manager app.kubernetes.io/name: service diff --git a/go.mod b/go.mod index b3dc04280..1744b345f 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( k8s.io/api v0.35.2 k8s.io/apiextensions-apiserver v0.35.2 k8s.io/apimachinery v0.35.2 + k8s.io/apiserver v0.35.2 k8s.io/client-go v0.35.2 k8s.io/component-base v0.35.2 k8s.io/klog/v2 v2.140.0 @@ -123,7 +124,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/apiserver v0.35.2 // indirect k8s.io/component-helpers v0.35.2 // indirect k8s.io/controller-manager v0.35.2 // indirect k8s.io/kms v0.35.2 // indirect diff --git a/pkg/cmd/operator/cmd.go b/pkg/cmd/operator/cmd.go index 3321b121c..422668486 100644 --- a/pkg/cmd/operator/cmd.go +++ b/pkg/cmd/operator/cmd.go @@ -2,23 +2,89 @@ package operator import ( "context" + "math/rand" + "os" + "time" + + "github.com/spf13/cobra" + "k8s.io/apiserver/pkg/server" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" + "k8s.io/utils/clock" "github.com/openshift/cert-manager-operator/pkg/operator" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" "github.com/openshift/cert-manager-operator/pkg/version" "github.com/openshift/library-go/pkg/controller/controllercmd" - "github.com/spf13/cobra" - "k8s.io/utils/clock" + "github.com/openshift/library-go/pkg/controller/fileobserver" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/serviceability" ) func NewOperator() *cobra.Command { - cmd := controllercmd.NewControllerCommandConfig( + cc := controllercmd.NewControllerCommandConfig( "cert-manager-operator", version.Get(), operator.RunOperator, clock.RealClock{}, - ).NewCommandWithContext(context.TODO()) + ) + + cmd := cc.NewCommandWithContext(context.TODO()) cmd.Use = "start" cmd.Short = "Start the cert-manager Operator" + + // Replace the default Run so we can apply the cluster TLS profile to the + // metrics serving config before the HTTPS listener is created. + cmd.Run = func(cmd *cobra.Command, args []string) { + rand.Seed(time.Now().UTC().UnixNano()) + logs.InitLogs() + defer logs.FlushLogs() + defer serviceability.BehaviorOnPanic(os.Getenv("OPENSHIFT_ON_PANIC"), version.Get())() + defer serviceability.Profile(os.Getenv("OPENSHIFT_PROFILE")).Stop() + serviceability.StartProfiler() + + shutdownCtx, cancel := context.WithCancel(context.Background()) + shutdownHandler := server.SetupSignalHandler() + go func() { + defer cancel() + <-shutdownHandler + klog.Infof("Received SIGTERM or SIGINT signal, shutting down controller.") + }() + + ctx, terminate := context.WithCancel(shutdownCtx) + defer terminate() + + terminateOnFiles, err := cmd.Flags().GetStringArray("terminate-on-files") + if err != nil { + klog.Fatal(err) + } + if len(terminateOnFiles) > 0 { + obs, err := fileobserver.NewObserver(10 * time.Second) + if err != nil { + klog.Fatal(err) + } + files := map[string][]byte{} + for _, fn := range terminateOnFiles { + fileBytes, err := os.ReadFile(fn) + if err != nil { + klog.Warningf("Unable to read initial content of %q: %v", fn, err) + continue + } + files[fn] = fileBytes + } + obs.AddReactor(func(filename string, action fileobserver.ActionType) error { + klog.Infof("exiting because %q changed", filename) + terminate() + return nil + }, files, terminateOnFiles...) + go obs.Run(shutdownHandler) + } + + if err := startControllerWithClusterTLS(ctx, cc, cmd); err != nil { + klog.Fatal(err) + } + } + cmd.Flags().StringVar(&operator.TrustedCAConfigMapName, "trusted-ca-configmap", "", "The name of the config map containing TLS CA(s) which should be trusted by the controller's containers. PEM encoded file under \"ca-bundle.crt\" key is expected.") cmd.Flags().StringVar(&operator.CloudCredentialSecret, "cloud-credentials-secret", "", "The name of the secret containing cloud credentials for authenticating using cert-manager ambient credentials mode.") @@ -34,3 +100,86 @@ These features provide early access to upcoming product features, enabling customers to test functionality and provide feedback during the development process.`) return cmd } + +func startControllerWithClusterTLS(ctx context.Context, c *controllercmd.ControllerCommandConfig, cmd *cobra.Command) error { + unstructuredConfig, config, configContent, err := c.Config() + if err != nil { + return err + } + + startingFileContent, observedFiles, err := c.AddDefaultRotationToConfig(config, configContent) + if err != nil { + return err + } + + listen, err := cmd.Flags().GetString("listen") + if err != nil { + return err + } + if len(listen) != 0 { + config.ServingInfo.BindAddress = listen + } + + kubeConfigFile, err := cmd.Flags().GetString("kubeconfig") + if err != nil { + return err + } + namespace, err := cmd.Flags().GetString("namespace") + if err != nil { + return err + } + + if !c.DisableServing { + restConfig, err := tlsprofile.RESTConfigFromKubeConfig(kubeConfigFile) + if err != nil { + klog.Warningf("unable to build rest config for cluster TLS profile lookup; using Controllercmd default TLS settings: %v", err) + } else { + lookupCtx, cancelLookup := context.WithTimeout(ctx, 30*time.Second) + err := tlsprofile.ApplyClusterProfileToHTTPServingInfo(lookupCtx, restConfig, &config.ServingInfo) + cancelLookup() + if err != nil { + return err + } + } + } + + exitOnChangeReactorCh := make(chan struct{}) + controllerCtx, cancel := context.WithCancel(ctx) + go func() { + select { + case <-exitOnChangeReactorCh: + cancel() + case <-ctx.Done(): + cancel() + } + }() + + config.LeaderElection.Disable = c.DisableLeaderElection + config.LeaderElection.LeaseDuration = c.LeaseDuration + config.LeaderElection.RenewDeadline = c.RenewDeadline + config.LeaderElection.RetryPeriod = c.RetryPeriod + + builder := controllercmd.NewController("cert-manager-operator", operator.RunOperator, clock.RealClock{}). + WithKubeConfigFile(kubeConfigFile, nil). + WithComponentNamespace(namespace). + WithLeaderElection(config.LeaderElection, namespace, "cert-manager-operator-lock"). + WithVersion(version.Get()). + WithEventRecorderOptions(events.RecommendedClusterSingletonCorrelatorOptions()). + WithRestartOnChange(exitOnChangeReactorCh, startingFileContent, observedFiles...) + + if !c.DisableServing { + builder = builder.WithServer(config.ServingInfo, config.Authentication, config.Authorization) + if c.EnableHTTP2 { + builder = builder.WithHTTP2() + } + if c.SkipInClusterAuthenticationLookup { + builder = builder.WithSkipInClusterAuthenticationLookup() + } + } + + if c.TopologyDetector != nil { + builder = builder.WithTopologyDetector(c.TopologyDetector) + } + + return builder.Run(controllerCtx, unstructuredConfig) +} diff --git a/pkg/controller/certmanager/cert_manager_controller_deployment.go b/pkg/controller/certmanager/cert_manager_controller_deployment.go index 4042f4490..0e77a423b 100644 --- a/pkg/controller/certmanager/cert_manager_controller_deployment.go +++ b/pkg/controller/certmanager/cert_manager_controller_deployment.go @@ -47,6 +47,8 @@ var ( "cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml", "cert-manager-deployment/controller/cert-manager-tokenrequest-role.yaml", "cert-manager-deployment/controller/cert-manager-view-cr.yaml", + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml", + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-approve-cert-manager-io-cr.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-approve-cert-manager-io-crb.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-certificatesigningrequests-cr.yaml", diff --git a/pkg/controller/certmanager/deployment_metrics_tls.go b/pkg/controller/certmanager/deployment_metrics_tls.go new file mode 100644 index 000000000..8c9f2a87c --- /dev/null +++ b/pkg/controller/certmanager/deployment_metrics_tls.go @@ -0,0 +1,59 @@ +package certmanager + +import ( + "fmt" + + appsv1 "k8s.io/api/apps/v1" + + operatorv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/cert-manager-operator/pkg/controller/common" +) + +const ( + metricsDynamicServingCASecretName = "cert-manager-metrics-ca" +) + +// withOperandMetricsTLS enables HTTPS on the cert-manager operand metrics +// listeners (port 9402) using cert-manager's dynamic metrics serving CA. +// Cipher/min-version flags continue to come from WithClusterTLSProfileFromAPIServer. +func withOperandMetricsTLS(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + if len(deployment.Spec.Template.Spec.Containers) == 0 { + return fmt.Errorf("deployment %s/%s has no containers", deployment.Namespace, deployment.Name) + } + + extra, ok := operandMetricsTLSArgs(deployment.Name) + if !ok { + return nil + } + + container := &deployment.Spec.Template.Spec.Containers[0] + container.Args = common.MergeContainerArgs(container.Args, extra) + + if deployment.Spec.Template.Annotations == nil { + deployment.Spec.Template.Annotations = map[string]string{} + } + deployment.Spec.Template.Annotations["prometheus.io/scheme"] = "https" + + return nil +} + +func operandMetricsTLSArgs(deploymentName string) ([]string, bool) { + var dnsNames string + switch deploymentName { + case certmanagerControllerDeployment: + dnsNames = "cert-manager,cert-manager.$(POD_NAMESPACE),cert-manager.$(POD_NAMESPACE).svc" + case certmanagerWebhookDeployment: + dnsNames = "cert-manager-webhook,cert-manager-webhook.$(POD_NAMESPACE),cert-manager-webhook.$(POD_NAMESPACE).svc" + case certmanagerCAinjectorDeployment: + dnsNames = "cert-manager-cainjector,cert-manager-cainjector.$(POD_NAMESPACE),cert-manager-cainjector.$(POD_NAMESPACE).svc" + default: + return nil, false + } + + return []string{ + "--metrics-dynamic-serving-ca-secret-namespace=$(POD_NAMESPACE)", + "--metrics-dynamic-serving-ca-secret-name=" + metricsDynamicServingCASecretName, + "--metrics-dynamic-serving-dns-names=" + dnsNames, + }, true +} diff --git a/pkg/controller/certmanager/deployment_metrics_tls_test.go b/pkg/controller/certmanager/deployment_metrics_tls_test.go new file mode 100644 index 000000000..ac8bbacad --- /dev/null +++ b/pkg/controller/certmanager/deployment_metrics_tls_test.go @@ -0,0 +1,90 @@ +package certmanager + +import ( + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestWithOperandMetricsTLS(t *testing.T) { + tests := []struct { + name string + deploymentName string + wantArgs []string + wantScheme string + }{ + { + name: "controller", + deploymentName: certmanagerControllerDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-namespace=$(POD_NAMESPACE)", + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager,cert-manager.$(POD_NAMESPACE),cert-manager.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + { + name: "webhook", + deploymentName: certmanagerWebhookDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager-webhook,cert-manager-webhook.$(POD_NAMESPACE),cert-manager-webhook.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + { + name: "cainjector", + deploymentName: certmanagerCAinjectorDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager-cainjector,cert-manager-cainjector.$(POD_NAMESPACE),cert-manager-cainjector.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: tt.deploymentName, Namespace: "cert-manager"}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "prometheus.io/port": "9402", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: tt.deploymentName, + Args: []string{"--v=2"}, + }}, + }, + }, + }, + } + if err := withOperandMetricsTLS(nil, dep); err != nil { + t.Fatalf("unexpected error: %v", err) + } + argMap := map[string]string{} + for _, a := range dep.Spec.Template.Spec.Containers[0].Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } + } + for _, want := range tt.wantArgs { + parts := strings.SplitN(want, "=", 2) + if argMap[parts[0]] != parts[1] { + t.Fatalf("arg %s: got %q want %q (all=%#v)", parts[0], argMap[parts[0]], parts[1], argMap) + } + } + if dep.Spec.Template.Annotations["prometheus.io/scheme"] != tt.wantScheme { + t.Fatalf("scheme annotation: %q", dep.Spec.Template.Annotations["prometheus.io/scheme"]) + } + }) + } +} diff --git a/pkg/controller/certmanager/generic_deployment_controller.go b/pkg/controller/certmanager/generic_deployment_controller.go index f24fefeff..1ba72b949 100644 --- a/pkg/controller/certmanager/generic_deployment_controller.go +++ b/pkg/controller/certmanager/generic_deployment_controller.go @@ -72,6 +72,9 @@ func newGenericDeploymentController( informers = append(informers, infraInformerFactory.Config().V1().APIServers().Informer()) } + // Enable HTTPS metrics before unsupported overrides so break-glass can still win. + hooks = append(hooks, withOperandMetricsTLS) + // unsupportedConfigOverrides must run after cluster TLS so break-glass operand args win. hooks = append(hooks, withUnsupportedArgsOverrideHook) diff --git a/pkg/controller/trustmanager/controller.go b/pkg/controller/trustmanager/controller.go index 9445542f9..78541680d 100644 --- a/pkg/controller/trustmanager/controller.go +++ b/pkg/controller/trustmanager/controller.go @@ -25,8 +25,11 @@ import ( certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + configv1 "github.com/openshift/api/config/v1" + v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" ) // RequestEnqueueLabelValue is the label value used for filtering reconcile @@ -59,6 +62,7 @@ type Reconciler struct { // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles,verbs=get;list;watch // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles/finalizers,verbs=update // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles/status,verbs=patch +// +kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch // New returns a new Reconciler instance. func New(mgr ctrl.Manager) (*Reconciler, error) { @@ -130,6 +134,11 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { object.GetName() == common.TrustedCABundleConfigMapName }) + // Reconcile when the cluster APIServer TLS profile or adherence changes. + clusterAPIServerPredicate := predicate.NewPredicateFuncs(func(object client.Object) bool { + return object.GetName() == tlsprofile.APIServerClusterName + }) + return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.TrustManager{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Named(ControllerName). @@ -145,6 +154,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&certmanagerv1.Certificate{}, handler.EnqueueRequestsFromMapFunc(mapFunc), withIgnoreStatusUpdatePredicates). Watches(&certmanagerv1.Issuer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), withIgnoreStatusUpdatePredicates). Watches(&admissionregistrationv1.ValidatingWebhookConfiguration{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerManagedResourcePredicates). + Watches(&configv1.APIServer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), builder.WithPredicates(clusterAPIServerPredicate)). Complete(r) } diff --git a/pkg/controller/trustmanager/deployment_tls.go b/pkg/controller/trustmanager/deployment_tls.go new file mode 100644 index 000000000..59657e675 --- /dev/null +++ b/pkg/controller/trustmanager/deployment_tls.go @@ -0,0 +1,59 @@ +package trustmanager + +import ( + "fmt" + + appsv1 "k8s.io/api/apps/v1" + + configv1 "github.com/openshift/api/config/v1" + + "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" +) + +// applyClusterTLSProfile merges cluster TLS security profile flags onto the +// trust-manager webhook container when apiserver tlsAdherence requires it. +// When the APIServer resource is missing (non-OpenShift) or adherence does not +// require enforcement, this is a no-op. +func (r *Reconciler) applyClusterTLSProfile(deployment *appsv1.Deployment) error { + if r.CtrlClient == nil { + return nil + } + + effective, err := tlsprofile.ResolveHonoredTLSProfile( + r.ctx, + tlsprofile.NewClientReaderAPIServerFetch(r.CtrlClient), + "trust-manager", + tlsprofile.FetchErrorPropagateExceptNotFound, + ) + if err != nil { + return err + } + if effective == nil { + return nil + } + + return applyTrustManagerWebhookTLSArgs(deployment, effective) +} + +// applyTrustManagerWebhookTLSArgs merges profile-derived webhook TLS flags onto +// the trust-manager container. Exported for unit tests via package-level use. +func applyTrustManagerWebhookTLSArgs(deployment *appsv1.Deployment, spec *configv1.TLSProfileSpec) error { + extra := tlsprofile.TrustManagerWebhookTLSArgs(spec) + if len(extra) == 0 { + return nil + } + + for i := range deployment.Spec.Template.Spec.Containers { + if deployment.Spec.Template.Spec.Containers[i].Name != trustManagerContainerName { + continue + } + sourceArgs := deployment.Spec.Template.Spec.Containers[i].Args + if spec != nil && spec.MinTLSVersion == configv1.VersionTLS13 { + sourceArgs = common.StripArgsByKeys(sourceArgs, common.ArgKeysSet(tlsprofile.TrustManagerCipherSuiteArgKeys)) + } + deployment.Spec.Template.Spec.Containers[i].Args = common.MergeContainerArgs(sourceArgs, extra) + return nil + } + return fmt.Errorf("deployment %s/%s missing container %q", deployment.Namespace, deployment.Name, trustManagerContainerName) +} diff --git a/pkg/controller/trustmanager/deployment_tls_test.go b/pkg/controller/trustmanager/deployment_tls_test.go new file mode 100644 index 000000000..eb14c77a8 --- /dev/null +++ b/pkg/controller/trustmanager/deployment_tls_test.go @@ -0,0 +1,340 @@ +package trustmanager + +import ( + "context" + "errors" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + + "sigs.k8s.io/controller-runtime/pkg/client" + + configv1 "github.com/openshift/api/config/v1" + + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" +) + +func TestApplyTrustManagerWebhookTLSArgs(t *testing.T) { + tests := []struct { + name string + spec *configv1.TLSProfileSpec + wantKeys []string + wantAbsent []string + }{ + { + name: "intermediate sets min version and ciphers", + spec: &configv1.TLSProfileSpec{ + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256"}, + MinTLSVersion: configv1.VersionTLS12, + }, + wantKeys: []string{"--tls-min-version", "--tls-cipher-suites"}, + }, + { + name: "modern tls13 omits cipher suites", + spec: &configv1.TLSProfileSpec{ + Ciphers: []string{"TLS_AES_128_GCM_SHA256"}, + MinTLSVersion: configv1.VersionTLS13, + }, + wantKeys: []string{"--tls-min-version"}, + wantAbsent: []string{"--tls-cipher-suites"}, + }, + { + name: "nil spec is no-op", + spec: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: trustManagerContainerName, + Args: []string{"--webhook-port=6443", "--tls-cipher-suites=STALE"}, + }}, + }, + }, + }, + } + if err := applyTrustManagerWebhookTLSArgs(dep, tt.spec); err != nil { + t.Fatalf("unexpected error: %v", err) + } + argMap := map[string]string{} + for _, a := range dep.Spec.Template.Spec.Containers[0].Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } else { + argMap[parts[0]] = "" + } + } + for _, key := range tt.wantKeys { + if _, ok := argMap[key]; !ok { + t.Fatalf("expected arg key %q, got %#v", key, argMap) + } + } + for _, key := range tt.wantAbsent { + if _, ok := argMap[key]; ok { + t.Fatalf("did not expect arg key %q, got %#v", key, argMap) + } + } + if tt.spec != nil && tt.spec.MinTLSVersion == configv1.VersionTLS13 { + if argMap["--tls-min-version"] != "VersionTLS13" { + t.Fatalf("got min version %q", argMap["--tls-min-version"]) + } + } + }) + } +} + +func TestApplyClusterTLSProfile_adherence(t *testing.T) { + tests := []struct { + name string + apiServer *configv1.APIServer + wantTLSArgs bool + wantMinVer string + wantCipherKey bool + }{ + { + name: "strict modern injects tls13 min version without ciphers", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: true, + wantMinVer: "VersionTLS13", + wantCipherKey: false, + }, + { + name: "unknown adherence treated as strict", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicy("FutureStrictMode"), + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: true, + wantMinVer: "VersionTLS13", + wantCipherKey: false, + }, + { + name: "empty adherence skips injection", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: false, + }, + { + name: "legacy adherence skips injection", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: false, + }, + { + name: "missing apiserver skips injection", + apiServer: nil, + wantTLSArgs: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + r := testReconciler(t) + r.CtrlClient = fakeCtrlClientWithAPIServer(tt.apiServer) + + tm := testTrustManager().Build() + dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + argMap := containerArgMap(dep) + _, hasMin := argMap["--tls-min-version"] + _, hasCipher := argMap["--tls-cipher-suites"] + if tt.wantTLSArgs != hasMin { + t.Fatalf("wantTLSArgs=%v hasMin=%v args=%#v", tt.wantTLSArgs, hasMin, argMap) + } + if tt.wantTLSArgs && argMap["--tls-min-version"] != tt.wantMinVer { + t.Fatalf("min version got %q want %q", argMap["--tls-min-version"], tt.wantMinVer) + } + if hasCipher != tt.wantCipherKey { + t.Fatalf("wantCipherKey=%v hasCipher=%v", tt.wantCipherKey, hasCipher) + } + }) + } +} + +func TestApplyClusterTLSProfile_forbiddenPropagates(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(_ context.Context, key client.ObjectKey, _ client.Object) error { + return apierrors.NewForbidden(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name, errors.New("denied")) + }) + r.CtrlClient = mock + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: trustManagerContainerName}}, + }, + }, + }, + } + if err := r.applyClusterTLSProfile(dep); err == nil { + t.Fatal("expected Forbidden to propagate") + } +} + +func TestApplyClusterTLSProfile_nilClientIsNoop(t *testing.T) { + r := testReconciler(t) + r.CtrlClient = nil + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: trustManagerContainerName, + Args: []string{"--webhook-port=6443"}, + }}, + }, + }, + }, + } + if err := r.applyClusterTLSProfile(dep); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(dep.Spec.Template.Spec.Containers[0].Args) != 1 { + t.Fatalf("expected args unchanged, got %#v", dep.Spec.Template.Spec.Containers[0].Args) + } +} + +func TestApplyTrustManagerWebhookTLSArgs_missingContainer(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "not-trust-manager"}}, + }, + }, + }, + } + err := applyTrustManagerWebhookTLSArgs(dep, &configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256"}, + }) + if err == nil { + t.Fatal("expected error for missing container") + } +} + +func TestApplyClusterTLSProfile_intermediateCiphers(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + apiServer := &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileIntermediateType, + }, + }, + } + r := testReconciler(t) + r.CtrlClient = fakeCtrlClientWithAPIServer(apiServer) + + tm := testTrustManager().Build() + dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected, err := tlsprofile.EffectiveSpec(apiServer.Spec.TLSSecurityProfile) + if err != nil { + t.Fatal(err) + } + want := tlsprofile.TrustManagerWebhookTLSArgs(expected) + gotArgs := dep.Spec.Template.Spec.Containers[0].Args + for _, w := range want { + found := false + for _, g := range gotArgs { + if g == w { + found = true + break + } + } + if !found { + t.Fatalf("expected arg %q in %#v", w, gotArgs) + } + } +} + +func fakeCtrlClientWithAPIServer(apiServer *configv1.APIServer) *fakes.FakeCtrlClient { + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(_ context.Context, key client.ObjectKey, obj client.Object) error { + if key.Name != tlsprofile.APIServerClusterName { + return apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name) + } + if apiServer == nil { + return apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name) + } + dst, ok := obj.(*configv1.APIServer) + if !ok { + return apierrors.NewBadRequest("unexpected object type") + } + apiServer.DeepCopyInto(dst) + return nil + }) + return mock +} + +func containerArgMap(dep *appsv1.Deployment) map[string]string { + argMap := map[string]string{} + for _, c := range dep.Spec.Template.Spec.Containers { + if c.Name != trustManagerContainerName { + continue + } + for _, a := range c.Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } else { + argMap[parts[0]] = "" + } + } + } + return argMap +} diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index da96689d4..a6e6d372c 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -58,6 +58,9 @@ func (r *Reconciler) getDeploymentObject(trustManager *v1alpha1.TrustManager, re updateResourceAnnotations(deployment, resourceAnnotations) updatePodTemplateLabels(deployment, resourceLabels) updateDeploymentArgs(deployment, trustManager) + if err := r.applyClusterTLSProfile(deployment); err != nil { + return nil, err + } updateServiceAccountName(deployment) updateTLSSecretVolume(deployment) diff --git a/pkg/operator/assets/bindata.go b/pkg/operator/assets/bindata.go index 56e814627..ad90cf737 100644 --- a/pkg/operator/assets/bindata.go +++ b/pkg/operator/assets/bindata.go @@ -29,6 +29,8 @@ // bindata/cert-manager-deployment/controller/cert-manager-edit-cr.yaml // bindata/cert-manager-deployment/controller/cert-manager-leaderelection-rb.yaml // bindata/cert-manager-deployment/controller/cert-manager-leaderelection-role.yaml +// bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml +// bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml // bindata/cert-manager-deployment/controller/cert-manager-sa.yaml // bindata/cert-manager-deployment/controller/cert-manager-svc.yaml // bindata/cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml @@ -275,6 +277,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cainjector @@ -1446,6 +1449,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cert-manager @@ -1665,6 +1669,94 @@ func certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml() (*asset, return a, nil } +var _certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: cert-manager-metrics-dynamic-serving +subjects: + - kind: ServiceAccount + name: cert-manager + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-webhook + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-cainjector + namespace: cert-manager +`) + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYamlBytes() ([]byte, error) { + return _certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, nil +} + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml() (*asset, error) { + bytes, err := certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +rules: + - apiGroups: + - "" + resourceNames: + - cert-manager-metrics-ca + resources: + - secrets + verbs: + - get + - list + - watch + - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - create +`) + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYamlBytes() ([]byte, error) { + return _certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, nil +} + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml() (*asset, error) { + bytes, err := certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _certManagerDeploymentControllerCertManagerSaYaml = []byte(`apiVersion: v1 automountServiceAccountToken: true kind: ServiceAccount @@ -1880,6 +1972,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: webhook @@ -4094,6 +4187,8 @@ var _bindata = map[string]func() (*asset, error){ "cert-manager-deployment/controller/cert-manager-edit-cr.yaml": certManagerDeploymentControllerCertManagerEditCrYaml, "cert-manager-deployment/controller/cert-manager-leaderelection-rb.yaml": certManagerDeploymentControllerCertManagerLeaderelectionRbYaml, "cert-manager-deployment/controller/cert-manager-leaderelection-role.yaml": certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml, + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml": certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml": certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, "cert-manager-deployment/controller/cert-manager-sa.yaml": certManagerDeploymentControllerCertManagerSaYaml, "cert-manager-deployment/controller/cert-manager-svc.yaml": certManagerDeploymentControllerCertManagerSvcYaml, "cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml": certManagerDeploymentControllerCertManagerTokenrequestRbYaml, @@ -4225,6 +4320,8 @@ var _bintree = &bintree{nil, map[string]*bintree{ "cert-manager-edit-cr.yaml": {certManagerDeploymentControllerCertManagerEditCrYaml, map[string]*bintree{}}, "cert-manager-leaderelection-rb.yaml": {certManagerDeploymentControllerCertManagerLeaderelectionRbYaml, map[string]*bintree{}}, "cert-manager-leaderelection-role.yaml": {certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml, map[string]*bintree{}}, + "cert-manager-metrics-dynamic-serving-rb.yaml": {certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, map[string]*bintree{}}, + "cert-manager-metrics-dynamic-serving-role.yaml": {certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, map[string]*bintree{}}, "cert-manager-sa.yaml": {certManagerDeploymentControllerCertManagerSaYaml, map[string]*bintree{}}, "cert-manager-svc.yaml": {certManagerDeploymentControllerCertManagerSvcYaml, map[string]*bintree{}}, "cert-manager-tokenrequest-rb.yaml": {certManagerDeploymentControllerCertManagerTokenrequestRbYaml, map[string]*bintree{}}, diff --git a/pkg/tlsprofile/cluster.go b/pkg/tlsprofile/cluster.go new file mode 100644 index 000000000..ccbebebd8 --- /dev/null +++ b/pkg/tlsprofile/cluster.go @@ -0,0 +1,98 @@ +package tlsprofile + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + + configv1 "github.com/openshift/api/config/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned" + libgocrypto "github.com/openshift/library-go/pkg/crypto" +) + +// APIServerClusterName is the singleton apiserver.config.openshift.io object. +const APIServerClusterName = "cluster" + +// FetchAPIServerFunc retrieves apiserver.config.openshift.io/cluster. +type FetchAPIServerFunc func(ctx context.Context) (*configv1.APIServer, error) + +// FetchErrorMode controls how ResolveHonoredTLSProfile treats fetch failures. +type FetchErrorMode int + +const ( + // FetchErrorPropagateExceptNotFound returns NotFound as a soft skip and + // propagates all other fetch errors. + FetchErrorPropagateExceptNotFound FetchErrorMode = iota +) + +// ObjectGetter is the Get subset used to fetch APIServer. It matches +// common.CtrlClient's Get signature (no GetOption variadic), which is what +// production callers pass via NewClientReaderAPIServerFetch. +type ObjectGetter interface { + Get(ctx context.Context, key client.ObjectKey, obj client.Object) error +} + +// NewRESTConfigAPIServerFetch returns a client-go fetcher for the cluster APIServer. +func NewRESTConfigAPIServerFetch(restConfig *rest.Config) (FetchAPIServerFunc, error) { + if restConfig == nil { + return nil, fmt.Errorf("rest config is nil") + } + configClient, err := configv1client.NewForConfig(restConfig) + if err != nil { + return nil, fmt.Errorf("failed to create config client: %w", err) + } + return func(ctx context.Context) (*configv1.APIServer, error) { + return configClient.ConfigV1().APIServers().Get(ctx, APIServerClusterName, metav1.GetOptions{}) + }, nil +} + +// NewClientReaderAPIServerFetch returns a controller-runtime fetcher for the cluster APIServer. +func NewClientReaderAPIServerFetch(r ObjectGetter) FetchAPIServerFunc { + return func(ctx context.Context) (*configv1.APIServer, error) { + apiServer := &configv1.APIServer{} + if err := r.Get(ctx, types.NamespacedName{Name: APIServerClusterName}, apiServer); err != nil { + return nil, err + } + return apiServer, nil + } +} + +// ResolveHonoredTLSProfile fetches the cluster APIServer via fetch and, when +// tlsAdherence requires enforcement, returns EffectiveSpec. A nil profile with +// a nil error means the caller should leave existing TLS settings unchanged. +func ResolveHonoredTLSProfile(ctx context.Context, fetch FetchAPIServerFunc, component string, mode FetchErrorMode) (*configv1.TLSProfileSpec, error) { + if fetch == nil { + return nil, fmt.Errorf("APIServer fetch function is nil") + } + + apiServer, err := fetch(ctx) + if err != nil { + switch mode { + case FetchErrorPropagateExceptNotFound: + if apierrors.IsNotFound(err) { + klog.V(4).Infof("skipping cluster TLS profile for %s: apiserver.config.openshift.io/cluster not found", component) + return nil, nil + } + return nil, fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) + default: + return nil, fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) + } + } + + adherence := apiServer.Spec.TLSAdherence + if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { + klog.V(4).Infof("skipping cluster TLS profile for %s: apiserver tlsAdherence=%q", component, adherence) + return nil, nil + } + if adherence != configv1.TLSAdherencePolicyStrictAllComponents { + klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for %s", adherence, component) + } + + return EffectiveSpec(apiServer.Spec.TLSSecurityProfile) +} diff --git a/pkg/tlsprofile/cluster_test.go b/pkg/tlsprofile/cluster_test.go new file mode 100644 index 000000000..7a8707105 --- /dev/null +++ b/pkg/tlsprofile/cluster_test.go @@ -0,0 +1,156 @@ +package tlsprofile + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestResolveHonoredTLSProfile_fetchErrors(t *testing.T) { + notFound := apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, APIServerClusterName) + forbidden := apierrors.NewForbidden(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, APIServerClusterName, errors.New("denied")) + + t.Run("NotFound is soft skip", func(t *testing.T) { + spec, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return nil, notFound + }, "test", FetchErrorPropagateExceptNotFound) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if spec != nil { + t.Fatalf("expected nil spec on NotFound, got %#v", spec) + } + }) + + t.Run("Forbidden propagates", func(t *testing.T) { + spec, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return nil, forbidden + }, "test", FetchErrorPropagateExceptNotFound) + if err == nil { + t.Fatal("expected error") + } + if spec != nil { + t.Fatalf("expected nil spec on Forbidden, got %#v", spec) + } + }) + + t.Run("nil fetch function errors", func(t *testing.T) { + _, err := ResolveHonoredTLSProfile(context.Background(), nil, "test", FetchErrorPropagateExceptNotFound) + if err == nil { + t.Fatal("expected error for nil fetch") + } + }) +} + +func TestResolveHonoredTLSProfile_adherence(t *testing.T) { + wantModern, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) + if err != nil { + t.Fatal(err) + } + wantIntermediate, err := EffectiveSpec(nil) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + apiServer *configv1.APIServer + wantSpec *configv1.TLSProfileSpec + wantErr string + }{ + { + name: "empty adherence skips", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + }, + { + name: "legacy adherence skips", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + }, + { + name: "strict modern returns effective spec", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + wantSpec: wantModern, + }, + { + name: "unknown adherence treated as strict with nil profile falls back to Intermediate", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicy("FutureStrictMode"), + }, + }, + wantSpec: wantIntermediate, + }, + { + name: "strict with invalid custom profile propagates error", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + }, + }, + }, + wantErr: "custom TLS profile is missing custom settings", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return tc.apiServer, nil + }, "test", FetchErrorPropagateExceptNotFound) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error = %v, want containing %q", err, tc.wantErr) + } + if got != nil { + t.Fatalf("expected nil spec on error, got %#v", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tc.wantSpec == nil { + if got != nil { + t.Fatalf("expected nil spec, got %#v", got) + } + return + } + if got.MinTLSVersion != tc.wantSpec.MinTLSVersion { + t.Fatalf("MinTLSVersion = %q, want %q", got.MinTLSVersion, tc.wantSpec.MinTLSVersion) + } + if !reflect.DeepEqual(got.Ciphers, tc.wantSpec.Ciphers) { + t.Fatalf("Ciphers = %#v, want %#v", got.Ciphers, tc.wantSpec.Ciphers) + } + }) + } +} + +func TestNewRESTConfigAPIServerFetch_nilConfig(t *testing.T) { + _, err := NewRESTConfigAPIServerFetch(nil) + if err == nil { + t.Fatal("expected error for nil rest config") + } +} diff --git a/pkg/tlsprofile/serving.go b/pkg/tlsprofile/serving.go new file mode 100644 index 000000000..c034d0699 --- /dev/null +++ b/pkg/tlsprofile/serving.go @@ -0,0 +1,50 @@ +package tlsprofile + +import ( + "context" + "fmt" + + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog/v2" + + configv1 "github.com/openshift/api/config/v1" +) + +// ApplyClusterProfileToHTTPServingInfo reads apiserver.config.openshift.io/cluster +// and, when tlsAdherence requires enforcement, applies the effective TLS profile +// to serving. A missing APIServer (NotFound) or non-enforcing adherence leaves +// serving unchanged; other APIServer lookup failures are returned. +func ApplyClusterProfileToHTTPServingInfo(ctx context.Context, restConfig *rest.Config, serving *configv1.HTTPServingInfo) error { + if serving == nil { + return fmt.Errorf("HTTPServingInfo is nil") + } + + fetch, err := NewRESTConfigAPIServerFetch(restConfig) + if err != nil { + return err + } + + effective, err := ResolveHonoredTLSProfile(ctx, fetch, "operator serving", FetchErrorPropagateExceptNotFound) + if err != nil { + return err + } + if effective == nil { + return nil + } + + if err := ApplyToHTTPServingInfo(serving, effective); err != nil { + return err + } + klog.V(2).Infof("applied cluster TLS profile to operator serving: minTLSVersion=%s ciphers=%d", serving.MinTLSVersion, len(serving.CipherSuites)) + return nil +} + +// RESTConfigFromKubeConfig returns an in-cluster rest.Config when kubeConfigFile +// is empty, otherwise loads the given kubeconfig path. +func RESTConfigFromKubeConfig(kubeConfigFile string) (*rest.Config, error) { + if len(kubeConfigFile) == 0 { + return rest.InClusterConfig() + } + return clientcmd.BuildConfigFromFlags("", kubeConfigFile) +} diff --git a/pkg/tlsprofile/serving_test.go b/pkg/tlsprofile/serving_test.go new file mode 100644 index 000000000..22dcb7595 --- /dev/null +++ b/pkg/tlsprofile/serving_test.go @@ -0,0 +1,71 @@ +package tlsprofile + +import ( + "context" + "testing" + + "k8s.io/client-go/rest" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestApplyClusterProfileToHTTPServingInfo_guards(t *testing.T) { + t.Run("nil serving", func(t *testing.T) { + err := ApplyClusterProfileToHTTPServingInfo(context.Background(), &rest.Config{}, nil) + if err == nil { + t.Fatal("expected error") + } + }) + t.Run("nil rest config", func(t *testing.T) { + serving := &configv1.HTTPServingInfo{} + err := ApplyClusterProfileToHTTPServingInfo(context.Background(), nil, serving) + if err == nil { + t.Fatal("expected error") + } + if serving.MinTLSVersion != "" { + t.Fatalf("serving should remain unchanged on error, got min=%q", serving.MinTLSVersion) + } + }) +} + +func TestApplyToHTTPServingInfo(t *testing.T) { + t.Run("intermediate", func(t *testing.T) { + spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType}) + if err != nil { + t.Fatal(err) + } + serving := &configv1.HTTPServingInfo{} + if err := ApplyToHTTPServingInfo(serving, spec); err != nil { + t.Fatal(err) + } + if serving.MinTLSVersion != string(configv1.VersionTLS12) { + t.Fatalf("min version: %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) == 0 { + t.Fatal("expected ciphers") + } + }) + + t.Run("modern tls13 keeps non-empty ciphers to block defaults", func(t *testing.T) { + spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) + if err != nil { + t.Fatal(err) + } + serving := &configv1.HTTPServingInfo{} + if err := ApplyToHTTPServingInfo(serving, spec); err != nil { + t.Fatal(err) + } + if serving.MinTLSVersion != string(configv1.VersionTLS13) { + t.Fatalf("min version: %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) == 0 { + t.Fatal("expected non-empty cipher list so library-go defaults are not reapplied") + } + }) + + t.Run("nil serving", func(t *testing.T) { + if err := ApplyToHTTPServingInfo(nil, &configv1.TLSProfileSpec{MinTLSVersion: configv1.VersionTLS12}); err == nil { + t.Fatal("expected error") + } + }) +} diff --git a/pkg/tlsprofile/tlsprofile.go b/pkg/tlsprofile/tlsprofile.go index 5be7aca56..3569709cc 100644 --- a/pkg/tlsprofile/tlsprofile.go +++ b/pkg/tlsprofile/tlsprofile.go @@ -49,6 +49,12 @@ var CertManagerCipherSuiteArgKeys = []string{ "--metrics-tls-cipher-suites", } +// TrustManagerCipherSuiteArgKeys are trust-manager webhook flags that must not be +// set when the effective minimum TLS version is 1.3. +var TrustManagerCipherSuiteArgKeys = []string{ + "--tls-cipher-suites", +} + // CertManagerWebhookTLSArgs returns cert-manager-webhook flags for the main HTTPS // listener and the metrics TLS listener when TLS is enabled for metrics. func CertManagerWebhookTLSArgs(spec *configv1.TLSProfileSpec) []string { @@ -90,7 +96,62 @@ func CertManagerOperandMetricsTLSArgs(spec *configv1.TLSProfileSpec) []string { } } +// TrustManagerWebhookTLSArgs returns trust-manager webhook TLS flags for the +// cluster TLS security profile. Metrics remain plain HTTP upstream and are out +// of scope. +func TrustManagerWebhookTLSArgs(spec *configv1.TLSProfileSpec) []string { + if spec == nil { + return []string{} + } + minVersion := string(spec.MinTLSVersion) + if spec.MinTLSVersion == configv1.VersionTLS13 { + return []string{ + "--tls-min-version=" + minVersion, + } + } + ciphers := joinIANACiphers(spec.Ciphers) + return []string{ + "--tls-min-version=" + minVersion, + "--tls-cipher-suites=" + ciphers, + } +} + func joinIANACiphers(openSSLNames []string) string { iana := libgocrypto.OpenSSLToIANACipherSuites(openSSLNames) return strings.Join(iana, ",") } + +// ApplyToHTTPServingInfo sets MinTLSVersion and CipherSuites on serving info from +// a resolved cluster TLS profile. For TLS 1.3, cipher suites are cleared so +// library-go defaults are not re-applied over an intentional empty list when +// callers set MinTLSVersion first; callers should set both fields together and +// rely on WithServer's SetRecommended* only filling empty values. +func ApplyToHTTPServingInfo(serving *configv1.HTTPServingInfo, spec *configv1.TLSProfileSpec) error { + if serving == nil { + return fmt.Errorf("HTTPServingInfo is nil") + } + if spec == nil { + return fmt.Errorf("TLS profile spec is nil") + } + serving.MinTLSVersion = string(spec.MinTLSVersion) + if spec.MinTLSVersion == configv1.VersionTLS13 { + // TLS 1.3 ignores CipherSuites in Go; leave empty so defaults are not + // forced to Intermediate TLS 1.2 suites after MinTLSVersion is set. + // Set a single TLS 1.3 suite name placeholder? No - empty means + // SetRecommended will fill Intermediate ciphers. To prevent that, + // set Modern profile's TLS 1.3 cipher names explicitly when available. + serving.CipherSuites = append([]string(nil), libgocrypto.OpenSSLToIANACipherSuites(spec.Ciphers)...) + if len(serving.CipherSuites) == 0 { + // Keep a non-empty list so SetRecommendedHTTPServingInfoDefaults does + // not overwrite with Intermediate defaults; TLS 1.3 ignores these. + serving.CipherSuites = []string{"TLS_AES_128_GCM_SHA256"} + } + return nil + } + iana := libgocrypto.OpenSSLToIANACipherSuites(spec.Ciphers) + if len(spec.Ciphers) > 0 && len(iana) == 0 { + return fmt.Errorf("no cipher suites after OpenSSL→IANA mapping") + } + serving.CipherSuites = iana + return nil +} diff --git a/pkg/tlsprofile/tlsprofile_test.go b/pkg/tlsprofile/tlsprofile_test.go index 5778609c2..65e982ac2 100644 --- a/pkg/tlsprofile/tlsprofile_test.go +++ b/pkg/tlsprofile/tlsprofile_test.go @@ -158,3 +158,47 @@ func TestCertManagerOperandMetricsTLSArgs_tls13OmitsCipherFlags(t *testing.T) { t.Fatalf("unexpected args: %#v", args) } } + +func TestTrustManagerWebhookTLSArgs_nilSpecReturnsEmpty(t *testing.T) { + args := TrustManagerWebhookTLSArgs(nil) + if len(args) != 0 { + t.Fatalf("expected empty args, got %#v", args) + } +} + +func TestTrustManagerWebhookTLSArgs_joinsCiphers(t *testing.T) { + spec := &configv1.TLSProfileSpec{ + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256", "TLS_AES_128_GCM_SHA256"}, + MinTLSVersion: configv1.VersionTLS12, + } + args := TrustManagerWebhookTLSArgs(spec) + argMap := map[string]string{} + for _, a := range args { + parts := strings.SplitN(a, "=", 2) + if len(parts) != 2 { + t.Fatalf("bad arg %q", a) + } + argMap[parts[0]] = parts[1] + } + if argMap["--tls-min-version"] != "VersionTLS12" { + t.Fatalf("unexpected min version: %q", argMap["--tls-min-version"]) + } + if !strings.Contains(argMap["--tls-cipher-suites"], "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") { + t.Fatalf("unexpected tls ciphers: %q", argMap["--tls-cipher-suites"]) + } +} + +func TestTrustManagerWebhookTLSArgs_tls13OmitsCipherFlags(t *testing.T) { + spec := &configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256", + }, + MinTLSVersion: configv1.VersionTLS13, + } + args := TrustManagerWebhookTLSArgs(spec) + if len(args) != 1 || args[0] != "--tls-min-version=VersionTLS13" { + t.Fatalf("unexpected args: %#v", args) + } +} diff --git a/test/e2e/tls_profile_test.go b/test/e2e/tls_profile_test.go index 46c612cc6..e95b89bb1 100644 --- a/test/e2e/tls_profile_test.go +++ b/test/e2e/tls_profile_test.go @@ -11,6 +11,7 @@ import ( "github.com/openshift/cert-manager-operator/pkg/tlsprofile" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -67,5 +68,14 @@ var _ = Describe("Cluster TLS security profile", Label("Platform:Generic", "Feat err := verifyOperandTLSArgsMatchClusterProfile(name, expectedSpec) Expect(err).NotTo(HaveOccurred(), "deployment %s", name) } + + By("verifying trust-manager webhook TLS flags when the deployment is present") + _, tmErr := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(ctx, "trust-manager", metav1.GetOptions{}) + if apierrors.IsNotFound(tmErr) { + Skip("trust-manager deployment not present; skipping trust-manager TLS profile verification") + } + Expect(tmErr).NotTo(HaveOccurred(), "failed to get trust-manager deployment") + err = verifyOperandTLSArgsMatchClusterProfile("trust-manager", expectedSpec) + Expect(err).NotTo(HaveOccurred(), "deployment trust-manager") }) }) diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 6115bec6b..1bff8322c 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -1824,6 +1824,8 @@ func expectedOperandTLSArgs(deploymentName string, spec *configapiv1.TLSProfileS return tlsprofile.CertManagerWebhookTLSArgs(spec) case certmanagerControllerDeployment, certmanagerCAinjectorDeployment: return tlsprofile.CertManagerOperandMetricsTLSArgs(spec) + case "trust-manager": + return tlsprofile.TrustManagerWebhookTLSArgs(spec) default: return nil } @@ -1857,8 +1859,12 @@ func verifyOperandTLSArgsMatchClusterProfile(deploymentName string, spec *config return false, fmt.Errorf("deployment %q has no containers", deploymentName) } + cipherKeys := tlsprofile.CertManagerCipherSuiteArgKeys + if deploymentName == "trust-manager" { + cipherKeys = tlsprofile.TrustManagerCipherSuiteArgKeys + } for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { - for _, key := range tlsprofile.CertManagerCipherSuiteArgKeys { + for _, key := range cipherKeys { if strings.HasPrefix(arg, key+"=") { return false, nil }