From b61432767c7df9297fcb8e62cef2d30cd2974b7b Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Thu, 23 Apr 2026 14:52:00 +0200 Subject: [PATCH 1/3] OCPBUGS-78774 - Central TLS configuration WIP: Hard-code kube-rbac-proxy for testing Signed-off-by: Jeff Mesnil --- manifests/10-insights-runtime-extractor.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/manifests/10-insights-runtime-extractor.yaml b/manifests/10-insights-runtime-extractor.yaml index e6c07fdd72..06bac7e20b 100644 --- a/manifests/10-insights-runtime-extractor.yaml +++ b/manifests/10-insights-runtime-extractor.yaml @@ -44,6 +44,7 @@ spec: - '--config-file=/etc/kube-rbac-proxy/config.yaml' - '--tls-cert-file=/etc/tls/private/tls.crt' - '--tls-private-key-file=/etc/tls/private/tls.key' + - '--tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256' terminationMessagePolicy: FallbackToLogsOnError volumeMounts: - mountPath: /etc/tls/private From b9db0dec271dba43eb53bf50e558b420319eb7ef Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Fri, 24 Apr 2026 09:04:46 +0200 Subject: [PATCH 2/3] OCPBUGS-78774 - Central TLS configuration Add a PRD to make the Insights Runtime Extractor DaemonSet a dynamic resource so that its TLS configuration can honor OpenShift central configuration. This fixes OCPBUGS-78774 Signed-off-by: Jeff Mesnil --- docs/prd/prd-dynamic-tls-kube-rbac-proxy.md | 89 +++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 docs/prd/prd-dynamic-tls-kube-rbac-proxy.md diff --git a/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md b/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md new file mode 100644 index 0000000000..6fb16bdc09 --- /dev/null +++ b/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md @@ -0,0 +1,89 @@ +# PRD: Dynamic TLS Configuration for kube-rbac-proxy + +**Bug**: OCPBUGS-78774 +**Date**: 2026-04-23 + +## Problem Statement + +The `kube-rbac-proxy` container in the `insights-runtime-extractor` DaemonSet (`manifests/10-insights-runtime-extractor.yaml`) uses default values for the TLS Cipher suites and the minimum version. + OpenShift clusters allow administrators to configure a cluster-wide TLS security profile via `apiservers.config.openshift.io/cluster`. The kube-rbac-proxy must honor this central configuration by specifying its `--tls-cipher-suites` and `--tls-min-version` arguments instead of relying on default values. + +## Current State + +1. **Static manifest**: The DaemonSet is defined in `manifests/10-insights-runtime-extractor.yaml` and deployed as part of the release payload. The operator does not manage it programmatically. + +2. **Default TLS seetings**: `--tls-cipher-suites` & `--tls-min-version` are not set and do not honor OpenShift central configuration. + +3. **Existing TLS helpers**: `pkg/insights/insightsclient/apiserver_config.go` already fetches the TLS profile from `apiservers.config.openshift.io/cluster` and converts it to a `crypto/tls.Config` using `library-go/pkg/crypto`. This is used for the operator's own outbound HTTP connections. + +4. **RBAC**: The operator's ClusterRole already has `get`, `list`, `watch` on `apiservers` in the `config.openshift.io` API group. + +## Proposed Solution + +Follow the pattern established by [cluster-dns-operator PR #466](https://github.com/openshift/cluster-dns-operator/pull/466/): + +### 1. Extract kube-rbac-proxy TLS arg builder + +Create a function (e.g., in `pkg/insights/insightsclient/apiserver_config.go` or a new dedicated file) that converts the `apiservers.config.openshift.io/cluster` TLS profile into kube-rbac-proxy CLI arguments: + +```go +func KubeRBACProxyTLSArgs(configClient configclientset.Interface) ([]string, error) +``` + +This function must: +- Fetch `apiservers.config.openshift.io/cluster` and read `Spec.TLSSecurityProfile`. +- Fall back to `TLSProfileIntermediateType` if unset. +- Resolve the profile type (Old/Intermediate/Modern/Custom) to a concrete `TLSProfileSpec` using `configv1.TLSProfiles`. +- Convert OpenSSL cipher names to IANA names using `crypto.OpenSSLToIANACipherSuites()` from `github.com/openshift/library-go/pkg/crypto`. +- If all cipher names are unrecognized, fall back to the Intermediate profile. +- Return args: `--tls-cipher-suites=` and `--tls-min-version=`. + +### 2. Operator manages the DaemonSet dynamically + +The operator must take ownership of the `insights-runtime-extractor` DaemonSet lifecycle instead of relying solely on the static manifest: + +- **Embed the base manifest** in the operator binary (using `//go:embed`) as a template. +- **On startup and on TLS config changes**, the operator must: + 1. Read the base DaemonSet manifest. + 2. Fetch the current TLS profile from the API server. + 3. Build the kube-rbac-proxy args with the resolved cipher suites and min TLS version. + 4. Patch/replace the kube-rbac-proxy container args in the DaemonSet spec. + 5. Apply the DaemonSet (create or update). + +### 3. Watch for TLS configuration changes + +The operator must watch `apiservers.config.openshift.io/cluster` for changes and reconcile the DaemonSet when the TLS profile is updated. This ensures the kube-rbac-proxy picks up new cipher suites or TLS version without requiring a cluster upgrade or manual intervention. + +## Cipher Suite Mapping + +The mapping between OpenShift TLS profiles and kube-rbac-proxy arguments uses `github.com/openshift/library-go/pkg/crypto`: + +| Step | Function | Purpose | +|------|----------|---------| +| 1 | `configv1.TLSProfiles[profileType]` | Resolve profile type to `TLSProfileSpec` (ciphers + min version) | +| 2 | `crypto.OpenSSLToIANACipherSuites(spec.Ciphers)` | Convert OpenSSL names (e.g., `ECDHE-RSA-AES128-GCM-SHA256`) to IANA names (e.g., `TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256`) | +| 3 | Format as `--tls-cipher-suites=` | Pass to kube-rbac-proxy CLI | +| 4 | `spec.MinTLSVersion` (e.g., `VersionTLS12`) | Pass as `--tls-min-version=VersionTLS12` | + +## Acceptance Criteria + +- [ ] kube-rbac-proxy parameters (`--tls-cipher-suites` and `--tls-min-version`) are based on `apiservers.config.openshift.io/cluster` TLS security profile. +- [ ] There is a mapping between the cipher suites returned by OpenShift and the arguments passed to kube-rbac-proxy using `github.com/openshift/library-go/pkg/crypto`. +- [ ] When the TLS security profile is not configured, the Intermediate profile is used as the default. +- [ ] When the TLS security profile changes on the cluster, the DaemonSet is reconciled with the updated args. +- [ ] Unit tests cover the TLS arg builder, including all profile types (Old, Intermediate, Modern, Custom) and edge cases (nil profile, unrecognized ciphers). + +## Key Files + +| File | Role | +|------|------| +| `manifests/10-insights-runtime-extractor.yaml` | Base DaemonSet manifest (to be embedded and used as template) | +| `pkg/insights/insightsclient/apiserver_config.go` | Existing TLS profile fetching and conversion (to be extended) | +| `pkg/controller/operator.go` | Main operator controller (DaemonSet reconciliation to be added here) | +| `manifests/03-clusterrole.yaml` | RBAC (already has apiserver read permissions) | + +## Reference + +- [cluster-dns-operator PR #466](https://github.com/openshift/cluster-dns-operator/pull/466/) - Reference implementation of the same pattern. +- `configv1.TLSProfiles` - Profile type to spec mapping from `github.com/openshift/api/config/v1`. +- `crypto.OpenSSLToIANACipherSuites` - Cipher name conversion from `github.com/openshift/library-go/pkg/crypto`. From d540daba77d9ba55c67e7e1e292e9c25e30be8d4 Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Fri, 24 Apr 2026 12:41:02 +0200 Subject: [PATCH 3/3] OCPBUGS-78774 - Central TLS configuration Dynamically configure kube-rbac-proxy TLS args (--tls-cipher-suites, --tls-min-version) from apiservers.config.openshift.io/cluster instead of relying on static defaults. The operator now embeds the DaemonSet manifest, patches it with the resolved TLS profile, and reconciles on APIServer config changes via an informer watch. Co-Authored-By: Claude Opus 4.6 --- docs/prd/prd-dynamic-tls-kube-rbac-proxy.md | 62 ++-- .../10-insights-runtime-extractor.yaml | 133 ++++++++ pkg/controller/operator.go | 8 + pkg/controller/runtime_extractor.go | 165 ++++++++++ pkg/controller/runtime_extractor_test.go | 303 ++++++++++++++++++ .../insightsclient/apiserver_config.go | 20 +- .../insightsclient/apiserver_config_test.go | 4 +- 7 files changed, 660 insertions(+), 35 deletions(-) create mode 100644 pkg/controller/manifests/10-insights-runtime-extractor.yaml create mode 100644 pkg/controller/runtime_extractor.go create mode 100644 pkg/controller/runtime_extractor_test.go diff --git a/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md b/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md index 6fb16bdc09..7796c6081d 100644 --- a/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md +++ b/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md @@ -18,41 +18,44 @@ The `kube-rbac-proxy` container in the `insights-runtime-extractor` DaemonSet (` 4. **RBAC**: The operator's ClusterRole already has `get`, `list`, `watch` on `apiservers` in the `config.openshift.io` API group. -## Proposed Solution +## Implementation -Follow the pattern established by [cluster-dns-operator PR #466](https://github.com/openshift/cluster-dns-operator/pull/466/): +Follows the pattern established by [cluster-dns-operator PR #466](https://github.com/openshift/cluster-dns-operator/pull/466/). -### 1. Extract kube-rbac-proxy TLS arg builder +### 1. Exported TLS helpers (`pkg/insights/insightsclient/apiserver_config.go`) -Create a function (e.g., in `pkg/insights/insightsclient/apiserver_config.go` or a new dedicated file) that converts the `apiservers.config.openshift.io/cluster` TLS profile into kube-rbac-proxy CLI arguments: +Two existing helpers were exported for reuse: -```go -func KubeRBACProxyTLSArgs(configClient configclientset.Interface) ([]string, error) -``` +- `GetTLSSecurityProfile(configClient)` — fetches `apiservers.config.openshift.io/cluster` and reads `Spec.TLSSecurityProfile`, falling back to `TLSProfileIntermediateType` if unset. +- `GetTLSProfileSpec(profile)` — resolves a profile type (Old/Intermediate/Modern/Custom) to a concrete `TLSProfileSpec` using `configv1.TLSProfiles`. -This function must: -- Fetch `apiservers.config.openshift.io/cluster` and read `Spec.TLSSecurityProfile`. -- Fall back to `TLSProfileIntermediateType` if unset. -- Resolve the profile type (Old/Intermediate/Modern/Custom) to a concrete `TLSProfileSpec` using `configv1.TLSProfiles`. -- Convert OpenSSL cipher names to IANA names using `crypto.OpenSSLToIANACipherSuites()` from `github.com/openshift/library-go/pkg/crypto`. -- If all cipher names are unrecognized, fall back to the Intermediate profile. -- Return args: `--tls-cipher-suites=` and `--tls-min-version=`. +These are also used by the existing `GetTLSConfigFromAPIServer` for the operator's own outbound HTTP connections. -### 2. Operator manages the DaemonSet dynamically +### 2. kube-rbac-proxy TLS arg builder (`pkg/controller/runtime_extractor.go`) -The operator must take ownership of the `insights-runtime-extractor` DaemonSet lifecycle instead of relying solely on the static manifest: +`buildKubeRBACProxyArgs(profile)` converts a `TLSSecurityProfile` into kube-rbac-proxy CLI arguments: -- **Embed the base manifest** in the operator binary (using `//go:embed`) as a template. -- **On startup and on TLS config changes**, the operator must: - 1. Read the base DaemonSet manifest. - 2. Fetch the current TLS profile from the API server. - 3. Build the kube-rbac-proxy args with the resolved cipher suites and min TLS version. - 4. Patch/replace the kube-rbac-proxy container args in the DaemonSet spec. - 5. Apply the DaemonSet (create or update). +- Resolves the profile to a `TLSProfileSpec` via `insightsclient.GetTLSProfileSpec`. +- For TLS 1.3 (Modern profile): only emits `--tls-min-version` since TLS 1.3 cipher suites are fixed by the protocol and not configurable by kube-rbac-proxy. +- For other profiles: converts OpenSSL cipher names to IANA names using `crypto.OpenSSLToIANACipherSuites()`. If all cipher names are unrecognized, falls back to the Intermediate profile. +- Returns args: `--tls-cipher-suites=` and `--tls-min-version=`. -### 3. Watch for TLS configuration changes +### 3. DaemonSet reconciliation (`pkg/controller/runtime_extractor.go`) -The operator must watch `apiservers.config.openshift.io/cluster` for changes and reconcile the DaemonSet when the TLS profile is updated. This ensures the kube-rbac-proxy picks up new cipher suites or TLS version without requiring a cluster upgrade or manual intervention. +The operator manages the `insights-runtime-extractor` DaemonSet lifecycle instead of relying solely on the static manifest: + +- **Embeds the base manifest** (`manifests/10-insights-runtime-extractor.yaml`) in the operator binary using `//go:embed` (copied to `pkg/controller/manifests/`). +- `reconcileRuntimeExtractorDaemonSet` performs create-or-update reconciliation: + 1. Parses the embedded DaemonSet manifest. + 2. Fetches the current TLS profile via `insightsclient.GetTLSSecurityProfile`. + 3. Builds kube-rbac-proxy args via `buildKubeRBACProxyArgs`. + 4. Patches the kube-rbac-proxy container args (strips existing `--tls-cipher-suites` / `--tls-min-version` and appends new ones). + 5. Creates the DaemonSet if not found, or updates it if the spec has changed (skips update if unchanged). + +### 4. Watch for TLS configuration changes (`pkg/controller/operator.go`) + +- On startup, performs an initial DaemonSet reconciliation. +- Registers a `tlsReconcileHandler` on the `configInformers.Config().V1().APIServers()` informer, which triggers reconciliation on add/update events for `apiservers.config.openshift.io/cluster`. ## Cipher Suite Mapping @@ -77,9 +80,12 @@ The mapping between OpenShift TLS profiles and kube-rbac-proxy arguments uses `g | File | Role | |------|------| -| `manifests/10-insights-runtime-extractor.yaml` | Base DaemonSet manifest (to be embedded and used as template) | -| `pkg/insights/insightsclient/apiserver_config.go` | Existing TLS profile fetching and conversion (to be extended) | -| `pkg/controller/operator.go` | Main operator controller (DaemonSet reconciliation to be added here) | +| `manifests/10-insights-runtime-extractor.yaml` | Base DaemonSet manifest (source of truth, copied into `pkg/controller/manifests/` for embedding) | +| `pkg/controller/manifests/10-insights-runtime-extractor.yaml` | Embedded copy of the base manifest used at runtime | +| `pkg/controller/runtime_extractor.go` | DaemonSet reconciliation, TLS arg builder, manifest parsing, informer event handler | +| `pkg/controller/runtime_extractor_test.go` | Unit tests for arg builder, patching, reconciliation | +| `pkg/insights/insightsclient/apiserver_config.go` | Exported `GetTLSSecurityProfile` and `GetTLSProfileSpec` helpers | +| `pkg/controller/operator.go` | Initial reconciliation call and APIServer informer watch registration | | `manifests/03-clusterrole.yaml` | RBAC (already has apiserver read permissions) | ## Reference diff --git a/pkg/controller/manifests/10-insights-runtime-extractor.yaml b/pkg/controller/manifests/10-insights-runtime-extractor.yaml new file mode 100644 index 0000000000..5842bd9362 --- /dev/null +++ b/pkg/controller/manifests/10-insights-runtime-extractor.yaml @@ -0,0 +1,133 @@ +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: insights-runtime-extractor + namespace: openshift-insights + annotations: + include.release.openshift.io/hypershift: "true" + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + capability.openshift.io/name: Insights + labels: + app.kubernetes.io/name: insights-runtime-extractor +spec: + selector: + matchLabels: + app.kubernetes.io/name: insights-runtime-extractor + updateStrategy: + rollingUpdate: + maxSurge: 0 + maxUnavailable: 33% + type: RollingUpdate + template: + metadata: + labels: + app.kubernetes.io/name: insights-runtime-extractor + annotations: + openshift.io/required-scc: insights-runtime-extractor-scc + target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' + kubectl.kubernetes.io/default-container: extractor + spec: + serviceAccountName: insights-runtime-extractor-sa + hostPID: true + # Deploy the insights-runtime-extractor only on Linux worker nodes + nodeSelector: + kubernetes.io/os: linux + priorityClassName: openshift-user-critical + containers: + + - name: kube-rbac-proxy + image: quay.io/openshift/origin-kube-rbac-proxy:latest + # tls-cipher-suites & tls-min-version arguments are set at runtime by the operator based on the + # centralized TLS security profile from apiservers.config.openshift.io/cluster + args: + - '--secure-listen-address=:8443' + - '--upstream=http://127.0.0.1:8000' + - '--config-file=/etc/kube-rbac-proxy/config.yaml' + - '--tls-cert-file=/etc/tls/private/tls.crt' + - '--tls-private-key-file=/etc/tls/private/tls.key' + terminationMessagePolicy: FallbackToLogsOnError + volumeMounts: + - mountPath: /etc/tls/private + name: insights-runtime-extractor-tls + - mountPath: /etc/kube-rbac-proxy + name: kube-rbac-proxy-cm + securityContext: + privileged: false + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL + runAsNonRoot: true + ports: + - name: https + containerPort: 8443 + protocol: TCP + resources: + requests: + cpu: 10m + memory: 100Mi + - name: exporter + image: quay.io/openshift/origin-insights-runtime-exporter:latest + imagePullPolicy: Always + volumeMounts: + - mountPath: /data + name: data-volume + securityContext: + privileged: false + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL + resources: + requests: + cpu: 10m + memory: 200Mi + - name: extractor + image: quay.io/openshift/origin-insights-runtime-extractor:latest + imagePullPolicy: Always + env: + - name: CONTAINER_RUNTIME_ENDPOINT + value: unix:///crio.sock + livenessProbe: + exec: + command: + - crictl + - --timeout + - 10s + - info + periodSeconds: 30 + timeoutSeconds: 10 + resources: + requests: + cpu: 10m + memory: 200Mi + securityContext: + privileged: true + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL + add: + - CAP_SYS_ADMIN + volumeMounts: + - mountPath: /crio.sock + name: crio-socket + - mountPath: /data + name: data-volume + volumes: + - name: crio-socket + hostPath: + path: /run/crio/crio.sock + type: Socket + - name: data-volume + emptyDir: {} + - name: kube-rbac-proxy-cm + configMap: + name: kube-rbac-proxy + - name: insights-runtime-extractor-tls + secret: + secretName: insights-runtime-extractor-tls \ No newline at end of file diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go index b090d57b2d..c97835f342 100644 --- a/pkg/controller/operator.go +++ b/pkg/controller/operator.go @@ -320,6 +320,14 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller promRulesController := insights.NewPrometheusRulesController(configAggregator, controller.KubeConfig) go promRulesController.Start(ctx) + // reconcile the insights-runtime-extractor DaemonSet with TLS configuration + // from apiservers.config.openshift.io/cluster + if err := reconcileRuntimeExtractorDaemonSet(ctx, kubeClient, configClient); err != nil { + klog.Errorf("Failed initial runtime extractor DaemonSet reconciliation: %v", err) + } + apiserverInformer := configInformers.Config().V1().APIServers().Informer() + apiserverInformer.AddEventHandler(newTLSReconcileHandler(ctx, kubeClient, configClient)) //nolint:errcheck + // support logLevelController logLevelController := loglevel.NewClusterOperatorLoggingController(opClient, controller.EventRecorder) operatorConfigInformers.Start(ctx.Done()) diff --git a/pkg/controller/runtime_extractor.go b/pkg/controller/runtime_extractor.go new file mode 100644 index 0000000000..caee866463 --- /dev/null +++ b/pkg/controller/runtime_extractor.go @@ -0,0 +1,165 @@ +package controller + +import ( + "bytes" + "context" + _ "embed" + "fmt" + "strings" + + configv1 "github.com/openshift/api/config/v1" + configclientset "github.com/openshift/client-go/config/clientset/versioned" + "github.com/openshift/library-go/pkg/crypto" + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + + "github.com/openshift/insights-operator/pkg/insights/insightsclient" +) + +//go:embed manifests/10-insights-runtime-extractor.yaml +var runtimeExtractorManifest []byte + +const kubeRBACProxyContainerName = "kube-rbac-proxy" + +func reconcileRuntimeExtractorDaemonSet( + ctx context.Context, kubeClient kubernetes.Interface, configClient configclientset.Interface, +) error { + desired, err := buildDesiredDaemonSet(configClient) + if err != nil { + return fmt.Errorf("failed to build desired DaemonSet: %w", err) + } + + existing, err := kubeClient.AppsV1().DaemonSets(desired.Namespace).Get(ctx, desired.Name, metav1.GetOptions{}) + if errors.IsNotFound(err) { + klog.Infof("Creating DaemonSet %s/%s", desired.Namespace, desired.Name) + _, err = kubeClient.AppsV1().DaemonSets(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) + return err + } + if err != nil { + return fmt.Errorf("failed to get DaemonSet: %w", err) + } + + if equality.Semantic.DeepEqual(existing.Spec, desired.Spec) { + return nil + } + + klog.Infof("Updating DaemonSet %s/%s with new TLS configuration", desired.Namespace, desired.Name) + existing.Spec = desired.Spec + _, err = kubeClient.AppsV1().DaemonSets(desired.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) + return err +} + +func buildDesiredDaemonSet(configClient configclientset.Interface) (*appsv1.DaemonSet, error) { + ds, err := parseDaemonSetManifest(runtimeExtractorManifest) + if err != nil { + return nil, err + } + + profile, err := insightsclient.GetTLSSecurityProfile(configClient) + if err != nil { + return nil, fmt.Errorf("failed to get TLS security profile: %w", err) + } + + tlsArgs, err := buildKubeRBACProxyArgs(profile) + if err != nil { + return nil, fmt.Errorf("failed to build TLS args: %w", err) + } + + if err := patchKubeRBACProxyArgs(ds, tlsArgs); err != nil { + return nil, err + } + + return ds, nil +} + +func buildKubeRBACProxyArgs(profile *configv1.TLSSecurityProfile) ([]string, error) { + profileSpec, err := insightsclient.GetTLSProfileSpec(profile) + if err != nil { + return nil, fmt.Errorf("failed to get TLS profile spec: %w", err) + } + + var args []string + + // TLS 1.3 cipher suites are fixed by the protocol and not configurable + if profileSpec.MinTLSVersion != configv1.VersionTLS13 { + cipherNames := crypto.OpenSSLToIANACipherSuites(profileSpec.Ciphers) + if len(cipherNames) == 0 { + intermediateSpec := configv1.TLSProfiles[configv1.TLSProfileIntermediateType] + cipherNames = crypto.OpenSSLToIANACipherSuites(intermediateSpec.Ciphers) + profileSpec = intermediateSpec + } + args = append(args, fmt.Sprintf("--tls-cipher-suites=%s", + strings.Join(cipherNames, ","))) + } + + args = append(args, fmt.Sprintf("--tls-min-version=%s", profileSpec.MinTLSVersion)) + + return args, nil +} + +func parseDaemonSetManifest(data []byte) (*appsv1.DaemonSet, error) { + ds := &appsv1.DaemonSet{} + if err := yaml.NewYAMLOrJSONDecoder( + bytes.NewReader(data), 4096, + ).Decode(ds); err != nil { + return nil, fmt.Errorf("failed to decode DaemonSet manifest: %w", err) + } + return ds, nil +} + +func patchKubeRBACProxyArgs(ds *appsv1.DaemonSet, tlsArgs []string) error { + containers := ds.Spec.Template.Spec.Containers + for i := range containers { + if containers[i].Name != kubeRBACProxyContainerName { + continue + } + + filteredArgs := make([]string, 0, len(containers[i].Args)+len(tlsArgs)) + for _, arg := range containers[i].Args { + if !isTLSArg(arg) { + filteredArgs = append(filteredArgs, arg) + } + } + filteredArgs = append(filteredArgs, tlsArgs...) + containers[i].Args = filteredArgs + return nil + } + return fmt.Errorf("container %q not found in DaemonSet", kubeRBACProxyContainerName) +} + +func isTLSArg(arg string) bool { + return strings.HasPrefix(arg, "--tls-cipher-suites=") || strings.HasPrefix(arg, "--tls-min-version=") +} + +type tlsReconcileHandler struct { + ctx context.Context + kubeClient kubernetes.Interface + configClient configclientset.Interface +} + +func newTLSReconcileHandler( + ctx context.Context, kubeClient kubernetes.Interface, configClient configclientset.Interface, +) *tlsReconcileHandler { + return &tlsReconcileHandler{ctx: ctx, kubeClient: kubeClient, configClient: configClient} +} + +func (h *tlsReconcileHandler) OnAdd(_ interface{}, _ bool) { + h.reconcile() +} + +func (h *tlsReconcileHandler) OnUpdate(_, _ interface{}) { + h.reconcile() +} + +func (h *tlsReconcileHandler) OnDelete(_ interface{}) {} + +func (h *tlsReconcileHandler) reconcile() { + if err := reconcileRuntimeExtractorDaemonSet(h.ctx, h.kubeClient, h.configClient); err != nil { + klog.Errorf("Failed to reconcile runtime extractor DaemonSet on TLS config change: %v", err) + } +} diff --git a/pkg/controller/runtime_extractor_test.go b/pkg/controller/runtime_extractor_test.go new file mode 100644 index 0000000000..b68867ad01 --- /dev/null +++ b/pkg/controller/runtime_extractor_test.go @@ -0,0 +1,303 @@ +package controller + +import ( + "context" + "strings" + "testing" + + configv1 "github.com/openshift/api/config/v1" + fakeconfigclient "github.com/openshift/client-go/config/clientset/versioned/fake" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func newFakeConfigClient(profile *configv1.TLSSecurityProfile) *fakeconfigclient.Clientset { + apiserver := &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: profile, + }, + } + return fakeconfigclient.NewSimpleClientset(apiserver) +} + +func Test_buildKubeRBACProxyArgs(t *testing.T) { + tests := []struct { + name string + profile *configv1.TLSSecurityProfile + expectError bool + expectCipherSuites bool + expectMinVersion string + expectCipherContain string + }{ + { + name: "Intermediate profile", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileIntermediateType, + }, + expectCipherSuites: true, + expectMinVersion: string(configv1.TLSProfiles[configv1.TLSProfileIntermediateType].MinTLSVersion), + }, + { + name: "Old profile", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileOldType, + }, + expectCipherSuites: true, + expectMinVersion: string(configv1.TLSProfiles[configv1.TLSProfileOldType].MinTLSVersion), + }, + { + name: "Modern profile (TLS 1.3 only, no cipher suites)", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + expectCipherSuites: false, + expectMinVersion: string(configv1.TLSProfiles[configv1.TLSProfileModernType].MinTLSVersion), + }, + { + name: "Custom profile with valid ciphers", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{ + "ECDHE-RSA-AES128-GCM-SHA256", + "ECDHE-RSA-AES256-GCM-SHA384", + }, + }, + }, + }, + expectCipherSuites: true, + expectMinVersion: "VersionTLS12", + expectCipherContain: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + }, + { + name: "Custom profile with all unrecognized ciphers falls back to Intermediate", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"UNKNOWN-CIPHER-1", "UNKNOWN-CIPHER-2"}, + }, + }, + }, + expectCipherSuites: true, + expectMinVersion: string(configv1.TLSProfiles[configv1.TLSProfileIntermediateType].MinTLSVersion), + }, + { + name: "Custom profile with nil Custom field", + profile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: nil, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args, err := buildKubeRBACProxyArgs(tt.profile) + if tt.expectError { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotEmpty(t, args) + + var foundMinVersion bool + for _, arg := range args { + if strings.HasPrefix(arg, "--tls-min-version=") { + foundMinVersion = true + assert.Equal(t, "--tls-min-version="+tt.expectMinVersion, arg) + } + } + assert.True(t, foundMinVersion, "expected --tls-min-version arg") + + var foundCipherSuites bool + for _, arg := range args { + if strings.HasPrefix(arg, "--tls-cipher-suites=") { + foundCipherSuites = true + if tt.expectCipherContain != "" { + assert.Contains(t, arg, tt.expectCipherContain) + } + } + } + assert.Equal(t, tt.expectCipherSuites, foundCipherSuites, + "expected cipher suites presence to be %v", tt.expectCipherSuites) + }) + } +} + +func Test_parseDaemonSetManifest(t *testing.T) { + ds, err := parseDaemonSetManifest(runtimeExtractorManifest) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, "insights-runtime-extractor", ds.Name) + assert.Equal(t, "openshift-insights", ds.Namespace) + + var found bool + for _, c := range ds.Spec.Template.Spec.Containers { + if c.Name == kubeRBACProxyContainerName { + found = true + break + } + } + assert.True(t, found, "kube-rbac-proxy container should be present") +} + +func Test_patchKubeRBACProxyArgs(t *testing.T) { + ds, err := parseDaemonSetManifest(runtimeExtractorManifest) + if !assert.NoError(t, err) { + return + } + + tlsArgs := []string{ + "--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "--tls-min-version=VersionTLS12", + } + + err = patchKubeRBACProxyArgs(ds, tlsArgs) + if !assert.NoError(t, err) { + return + } + + var args []string + for _, c := range ds.Spec.Template.Spec.Containers { + if c.Name == kubeRBACProxyContainerName { + args = c.Args + break + } + } + + assert.Contains(t, args, "--secure-listen-address=:8443") + assert.Contains(t, args, "--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + assert.Contains(t, args, "--tls-min-version=VersionTLS12") + + var tlsCipherCount, tlsVersionCount int + for _, arg := range args { + if strings.HasPrefix(arg, "--tls-cipher-suites=") { + tlsCipherCount++ + } + if strings.HasPrefix(arg, "--tls-min-version=") { + tlsVersionCount++ + } + } + assert.Equal(t, 1, tlsCipherCount, "should have exactly one --tls-cipher-suites arg") + assert.Equal(t, 1, tlsVersionCount, "should have exactly one --tls-min-version arg") +} + +func Test_patchKubeRBACProxyArgs_missingContainer(t *testing.T) { + ds := &appsv1.DaemonSet{ + Spec: appsv1.DaemonSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "other"}, + }, + }, + }, + }, + } + err := patchKubeRBACProxyArgs(ds, []string{"--tls-min-version=VersionTLS12"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "not found") +} + +func Test_reconcileRuntimeExtractorDaemonSet_create(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + configClient := newFakeConfigClient(nil) + + err := reconcileRuntimeExtractorDaemonSet(context.Background(), kubeClient, configClient) + if !assert.NoError(t, err) { + return + } + + ds, err := kubeClient.AppsV1().DaemonSets("openshift-insights").Get( + context.Background(), "insights-runtime-extractor", metav1.GetOptions{}) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, "insights-runtime-extractor", ds.Name) + + var args []string + for _, c := range ds.Spec.Template.Spec.Containers { + if c.Name == kubeRBACProxyContainerName { + args = c.Args + } + } + var hasCipherSuites, hasMinVersion bool + for _, arg := range args { + if strings.HasPrefix(arg, "--tls-cipher-suites=") { + hasCipherSuites = true + } + if strings.HasPrefix(arg, "--tls-min-version=") { + hasMinVersion = true + } + } + assert.True(t, hasCipherSuites, "should have --tls-cipher-suites") + assert.True(t, hasMinVersion, "should have --tls-min-version") +} + +func Test_reconcileRuntimeExtractorDaemonSet_update(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + configClient := newFakeConfigClient(nil) + + err := reconcileRuntimeExtractorDaemonSet(context.Background(), kubeClient, configClient) + if !assert.NoError(t, err) { + return + } + + configClient = newFakeConfigClient(&configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileOldType, + }) + + err = reconcileRuntimeExtractorDaemonSet(context.Background(), kubeClient, configClient) + if !assert.NoError(t, err) { + return + } + + ds, err := kubeClient.AppsV1().DaemonSets("openshift-insights").Get( + context.Background(), "insights-runtime-extractor", metav1.GetOptions{}) + if !assert.NoError(t, err) { + return + } + + for _, c := range ds.Spec.Template.Spec.Containers { + if c.Name == kubeRBACProxyContainerName { + for _, arg := range c.Args { + if strings.HasPrefix(arg, "--tls-min-version=") { + expectedVersion := string(configv1.TLSProfiles[configv1.TLSProfileOldType].MinTLSVersion) + assert.Equal(t, "--tls-min-version="+expectedVersion, arg) + } + } + } + } +} + +func Test_reconcileRuntimeExtractorDaemonSet_noUpdateWhenUnchanged(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + configClient := newFakeConfigClient(nil) + + err := reconcileRuntimeExtractorDaemonSet(context.Background(), kubeClient, configClient) + if !assert.NoError(t, err) { + return + } + + err = reconcileRuntimeExtractorDaemonSet(context.Background(), kubeClient, configClient) + if !assert.NoError(t, err) { + return + } +} + +func Test_isTLSArg(t *testing.T) { + assert.True(t, isTLSArg("--tls-cipher-suites=foo")) + assert.True(t, isTLSArg("--tls-min-version=VersionTLS12")) + assert.False(t, isTLSArg("--tls-cert-file=/path")) + assert.False(t, isTLSArg("--secure-listen-address=:8443")) +} diff --git a/pkg/insights/insightsclient/apiserver_config.go b/pkg/insights/insightsclient/apiserver_config.go index fd7236379b..af0b4769de 100644 --- a/pkg/insights/insightsclient/apiserver_config.go +++ b/pkg/insights/insightsclient/apiserver_config.go @@ -11,9 +11,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// GetTLSConfigFromAPIServer fetches the TLS profile from the API Server configuration. -// This is the default source for most components. -func GetTLSConfigFromAPIServer(configClient configclientset.Interface) (*tls.Config, error) { +// GetTLSSecurityProfile fetches the TLS security profile from apiservers.config.openshift.io/cluster. +// Falls back to the Intermediate profile if none is configured. +func GetTLSSecurityProfile(configClient configclientset.Interface) (*configv1.TLSSecurityProfile, error) { apiserver, err := configClient.ConfigV1().APIServers().Get(context.Background(), "cluster", metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("failed to get APIServer config: %w", err) @@ -25,12 +25,21 @@ func GetTLSConfigFromAPIServer(configClient configclientset.Interface) (*tls.Con Type: configv1.TLSProfileIntermediateType, } } + return profile, nil +} +// GetTLSConfigFromAPIServer fetches the TLS profile from the API Server configuration. +// This is the default source for most components. +func GetTLSConfigFromAPIServer(configClient configclientset.Interface) (*tls.Config, error) { + profile, err := GetTLSSecurityProfile(configClient) + if err != nil { + return nil, err + } return buildTLSConfigFromProfile(profile) } func buildTLSConfigFromProfile(profile *configv1.TLSSecurityProfile) (*tls.Config, error) { - profileSpec, err := getTLSProfileSpec(profile) + profileSpec, err := GetTLSProfileSpec(profile) if err != nil { return nil, fmt.Errorf("failed to getTLSProfileSpec: %v", err) } @@ -69,7 +78,8 @@ func buildTLSConfigFromProfile(profile *configv1.TLSSecurityProfile) (*tls.Confi return config, nil } -func getTLSProfileSpec(profile *configv1.TLSSecurityProfile) (*configv1.TLSProfileSpec, error) { +// GetTLSProfileSpec resolves a TLS security profile type to a concrete TLSProfileSpec. +func GetTLSProfileSpec(profile *configv1.TLSSecurityProfile) (*configv1.TLSProfileSpec, error) { switch profile.Type { case configv1.TLSProfileOldType, configv1.TLSProfileIntermediateType, diff --git a/pkg/insights/insightsclient/apiserver_config_test.go b/pkg/insights/insightsclient/apiserver_config_test.go index 732e8ed6d5..f24c9f6361 100644 --- a/pkg/insights/insightsclient/apiserver_config_test.go +++ b/pkg/insights/insightsclient/apiserver_config_test.go @@ -54,7 +54,7 @@ func Test_parseTLSVersion(t *testing.T) { } } -func Test_getTLSProfileSpec(t *testing.T) { +func Test_GetTLSProfileSpec(t *testing.T) { tests := []struct { name string profile *configv1.TLSSecurityProfile @@ -99,7 +99,7 @@ func Test_getTLSProfileSpec(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := getTLSProfileSpec(tt.profile) + result, err := GetTLSProfileSpec(tt.profile) if tt.expectError { assert.Error(t, err) assert.Nil(t, result)