From ab62b8bb2cbc1351b4aeccca8d9adca05c85294e Mon Sep 17 00:00:00 2001 From: Ricardo Maraschini Date: Mon, 18 May 2026 13:16:55 +0200 Subject: [PATCH] CNTRLPLANE-3426: restart operator upon tls config change when the cluster wide tls config changes we now restart the operator by cancelling the run context. by doing so we ensure that the metrics end point will always use the correct tls version and ciphers. --- pkg/cvo/metrics.go | 73 +++++++++++++++++++++++ pkg/cvo/metrics_test.go | 128 ++++++++++++++++++++++++++++++++++++++++ pkg/start/start.go | 33 +++++++++++ 3 files changed, 234 insertions(+) diff --git a/pkg/cvo/metrics.go b/pkg/cvo/metrics.go index 30d04302f3..af9cdffab6 100644 --- a/pkg/cvo/metrics.go +++ b/pkg/cvo/metrics.go @@ -27,6 +27,7 @@ import ( "k8s.io/klog/v2" configv1 "github.com/openshift/api/config/v1" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/cluster-version-operator/lib/resourcemerge" @@ -267,6 +268,73 @@ type MetricsOptions struct { DisableAuthentication bool DisableAuthorization bool + + // TLS configuration observed from APIServer cluster object + MinTLSVersion uint16 + CipherSuites []uint16 +} + +// ObserveAPIServerTLSConfig extracts and converts TLS configuration from the +// APIServer object. Always returns valid defaults even if APIServer object is +// missing, has no TLS profile configured, or contains invalid values. Never +// returns an error, any issues result in default values being returned. +func ObserveAPIServerTLSConfig(lister configlistersv1.APIServerLister) (uint16, []uint16) { + defaultProfileConfig := configv1.TLSProfiles[crypto.DefaultTLSProfileType] + + // capture the defaults. if we fail at any stage on this function we + // log and return the default values. + defaultMinTLS := crypto.TLSVersionOrDie(string(defaultProfileConfig.MinTLSVersion)) + defaultCipherSuites := crypto.CipherSuitesOrDie(crypto.OpenSSLToIANACipherSuites(defaultProfileConfig.Ciphers)) + + apiServer, err := lister.Get("cluster") + if err != nil { + klog.Warningf("Unable to read APIServer object, using default TLS profile: %v", err) + return defaultMinTLS, defaultCipherSuites + } + + profile := apiServer.Spec.TLSSecurityProfile + if profile == nil { + return defaultMinTLS, defaultCipherSuites + } + + profileConfig := configv1.TLSProfiles[profile.Type] + if profile.Type == configv1.TLSProfileCustomType { + if profile.Custom == nil { + klog.Warningf("APIServer TLS profile type is Custom but no custom spec provided, using default TLS profile") + return defaultMinTLS, defaultCipherSuites + } + profileConfig = &profile.Custom.TLSProfileSpec + } + + if profileConfig == nil { + klog.Warningf("TLS config for profile %q not found, using default TLS profile", profile.Type) + return defaultMinTLS, defaultCipherSuites + } + + minTLSVersion, err := crypto.TLSVersion(string(profileConfig.MinTLSVersion)) + if err != nil { + klog.Warningf("Invalid minTLSVersion %q in APIServer TLS profile, using default TLS profile: %v", profileConfig.MinTLSVersion, err) + return defaultMinTLS, defaultCipherSuites + } + + // convert OpenSSL cipher names to IANA names. + ianaCiphers := crypto.OpenSSLToIANACipherSuites(profileConfig.Ciphers) + if len(ianaCiphers) == 0 { + klog.Warningf("Failed to convert APIServer cipher suites %v to IANA format, using default TLS profile", profileConfig.Ciphers) + return defaultMinTLS, defaultCipherSuites + } + + cipherSuites := make([]uint16, 0, len(ianaCiphers)) + for _, cipherName := range ianaCiphers { + cipher, err := crypto.CipherSuite(cipherName) + if err != nil { + klog.Warningf("Invalid cipher suite %q in APIServer TLS profile, using default TLS profile: %v", cipherName, err) + return defaultMinTLS, defaultCipherSuites + } + cipherSuites = append(cipherSuites, cipher) + } + + return minTLSVersion, cipherSuites } // RunMetrics launches an HTTPS server bound to listenAddress serving @@ -362,6 +430,11 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res // which generates updated configs via GetConfigForClient callback on each TLS handshake. // This enables automatic certificate rotation without server restarts. baseTlSConfig := crypto.SecureTLSConfig(&tls.Config{ClientAuth: clientAuth}) + + // Apply APIServer TLS configuration from options + baseTlSConfig.MinVersion = options.MinTLSVersion + baseTlSConfig.CipherSuites = options.CipherSuites + servingCertController := dynamiccertificates.NewDynamicServingCertificateController( baseTlSConfig, clientCA, diff --git a/pkg/cvo/metrics_test.go b/pkg/cvo/metrics_test.go index f12852be31..feef4a6d2b 100644 --- a/pkg/cvo/metrics_test.go +++ b/pkg/cvo/metrics_test.go @@ -18,11 +18,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/client-go/tools/record" configv1 "github.com/openshift/api/config/v1" + fakeconfigclientv1 "github.com/openshift/client-go/config/clientset/versioned/fake" + configinformers "github.com/openshift/client-go/config/informers/externalversions" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/cluster-version-operator/pkg/featuregates" @@ -1428,3 +1431,128 @@ func (m *mockCAProvider) VerifyOptions() (x509.VerifyOptions, bool) { func (m *mockCAProvider) AddListener(_ dynamiccertificates.Listener) { } + +func TestObserveAPIServerTLSConfig(t *testing.T) { + tests := []struct { + name string + apiServer *configv1.APIServer + expectMinVersion uint16 + checkCiphers bool + }{ + { + name: "custom TLS profile", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS13, + Ciphers: []string{ + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + }, + }, + }, + }, + }, + }, + expectMinVersion: tls.VersionTLS13, + checkCiphers: true, + }, + { + name: "intermediate TLS profile", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileIntermediateType, + }, + }, + }, + expectMinVersion: tls.VersionTLS12, + checkCiphers: true, + }, + { + name: "no APIServer object (uses defaults)", + apiServer: nil, + expectMinVersion: tls.VersionTLS12, // default is intermediate + checkCiphers: true, + }, + { + name: "nil TLS profile (uses defaults)", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{}, + }, + expectMinVersion: tls.VersionTLS12, + checkCiphers: true, + }, + { + name: "custom TLS profile type but no custom spec (uses defaults)", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + }, + }, + }, + expectMinVersion: tls.VersionTLS12, + checkCiphers: true, + }, + { + name: "invalid TLS version and cipher suites (uses defaults)", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: &configv1.CustomTLSProfile{ + TLSProfileSpec: configv1.TLSProfileSpec{ + MinTLSVersion: "InvalidTLSVersion", + Ciphers: []string{ + "RANDOM_CIPHER_123", + "INVALID_CIPHER_XYZ", + "NOT_A_REAL_CIPHER", + }, + }, + }, + }, + }, + }, + expectMinVersion: tls.VersionTLS12, + checkCiphers: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create fake client and informer + var objects []runtime.Object + if tt.apiServer != nil { + objects = append(objects, tt.apiServer) + } + fakeClient := fakeconfigclientv1.NewClientset(objects...) + informerFactory := configinformers.NewSharedInformerFactory(fakeClient, 0) + lister := informerFactory.Config().V1().APIServers().Lister() + + // Start informers and wait for cache sync + stopCh := make(chan struct{}) + defer close(stopCh) + informerFactory.Start(stopCh) + informerFactory.WaitForCacheSync(stopCh) + + minVer, ciphers := ObserveAPIServerTLSConfig(lister) + + if minVer != tt.expectMinVersion { + t.Errorf("expected minTLSVersion 0x%04x, got 0x%04x", tt.expectMinVersion, minVer) + } + + if tt.checkCiphers && len(ciphers) == 0 { + t.Errorf("expected cipher suites to be non-empty") + } + }) + } +} diff --git a/pkg/start/start.go b/pkg/start/start.go index 0470d70cf2..0beac137aa 100644 --- a/pkg/start/start.go +++ b/pkg/start/start.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "os/signal" + "slices" "syscall" "time" @@ -26,6 +27,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" coreclientsetv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" @@ -326,6 +328,9 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource resultChannel := make(chan asyncResult, 1) resultChannelCount := 0 + apiServerLister := controllerCtx.ConfigInformerFactory.Config().V1().APIServers().Lister() + apiServerSharedIndexInformer := controllerCtx.ConfigInformerFactory.Config().V1().APIServers().Informer() + informersDone := postMainContext.Done() // FIXME: would be nice if there was a way to collect these. controllerCtx.ClusterVersionInformerFactory.Start(informersDone) @@ -341,6 +346,13 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource } } + allSynced = controllerCtx.ConfigInformerFactory.WaitForCacheSync(informersDone) + for _, synced := range allSynced { + if !synced { + klog.Fatalf("Caches never synchronized: %v", postMainContext.Err()) + } + } + resultChannelCount++ go func() { defer utilruntime.HandleCrash() @@ -355,6 +367,27 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource OnStartedLeading: func(_ context.Context) { // no need for this passed-through postMainContext, because goroutines we launch inside will use runContext launchedMain = true if o.MetricsOptions.ListenAddress != "" { + startMinTLSVersion, startCipherSuites := cvo.ObserveAPIServerTLSConfig(apiServerLister) + apiServerHandler := func() { + currentMinTLS, currentCiphers := cvo.ObserveAPIServerTLSConfig(apiServerLister) + if currentMinTLS == startMinTLSVersion && slices.Equal(currentCiphers, startCipherSuites) { + return + } + klog.Infof("APIServer TLS configuration changed; requesting shutdown") + runCancel() + } + + if _, err := apiServerSharedIndexInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + UpdateFunc: func(_, _ any) { apiServerHandler() }, + AddFunc: func(_ any) { apiServerHandler() }, + DeleteFunc: func(_ any) { apiServerHandler() }, + }); err != nil { + klog.Warningf("Failed to add watcher for APIServer Config: %v", err) + } + + o.MetricsOptions.MinTLSVersion = startMinTLSVersion + o.MetricsOptions.CipherSuites = startCipherSuites + resultChannelCount++ go func() { defer utilruntime.HandleCrash()