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 000000000..7796c6081 --- /dev/null +++ b/docs/prd/prd-dynamic-tls-kube-rbac-proxy.md @@ -0,0 +1,95 @@ +# 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. + +## Implementation + +Follows the pattern established by [cluster-dns-operator PR #466](https://github.com/openshift/cluster-dns-operator/pull/466/). + +### 1. Exported TLS helpers (`pkg/insights/insightsclient/apiserver_config.go`) + +Two existing helpers were exported for reuse: + +- `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`. + +These are also used by the existing `GetTLSConfigFromAPIServer` for the operator's own outbound HTTP connections. + +### 2. kube-rbac-proxy TLS arg builder (`pkg/controller/runtime_extractor.go`) + +`buildKubeRBACProxyArgs(profile)` converts a `TLSSecurityProfile` into kube-rbac-proxy CLI arguments: + +- 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. DaemonSet reconciliation (`pkg/controller/runtime_extractor.go`) + +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 + +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 (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 + +- [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`. diff --git a/manifests/10-insights-runtime-extractor.yaml b/manifests/10-insights-runtime-extractor.yaml index e6c07fdd7..06bac7e20 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 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 000000000..5842bd936 --- /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 b090d57b2..c97835f34 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 000000000..caee86646 --- /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 000000000..b68867ad0 --- /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 fd7236379..af0b4769d 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 732e8ed6d..f24c9f636 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)