Skip to content

Commit f8d8646

Browse files
committed
metrics: Respect central TLS profile using controller-runtime-common
1 parent e72fa8a commit f8d8646

3 files changed

Lines changed: 101 additions & 2 deletions

File tree

pkg/cvo/cvo.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ type Operator struct {
122122
cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister
123123
proxyLister configlistersv1.ProxyLister
124124
featureGateLister configlistersv1.FeatureGateLister
125+
apiServerLister configlistersv1.APIServerLister
125126
cacheSynced []cache.InformerSynced
126127

127128
// queue tracks applying updates to a cluster.
@@ -223,6 +224,7 @@ func New(
223224
proxyInformer configinformersv1.ProxyInformer,
224225
operatorInformerFactory operatorexternalversions.SharedInformerFactory,
225226
featureGateInformer configinformersv1.FeatureGateInformer,
227+
apiServerInformer configinformersv1.APIServerInformer,
226228
client clientset.Interface,
227229
kubeClient kubernetes.Interface,
228230
operatorClient operatorclientset.Interface,
@@ -305,6 +307,8 @@ func New(
305307
optr.featureGateLister = featureGateInformer.Lister()
306308
optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced)
307309

310+
optr.apiServerLister = apiServerInformer.Lister()
311+
308312
// make sure this is initialized after all the listers are initialized
309313
optr.upgradeableChecks = optr.defaultUpgradeableChecks()
310314

@@ -1184,3 +1188,8 @@ func (optr *Operator) shouldReconcileAcceptRisks() bool {
11841188
// HyperShift will be supported later if needed
11851189
return optr.enabledCVOFeatureGates.AcceptRisks() && !optr.hypershift
11861190
}
1191+
1192+
// APIServerLister returns the APIServer lister for accessing cluster TLS configuration
1193+
func (optr *Operator) APIServerLister() configlistersv1.APIServerLister {
1194+
return optr.apiServerLister
1195+
}

pkg/cvo/metrics.go

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"net"
1010
"net/http"
11+
"slices"
1112
"time"
1213

1314
"github.com/prometheus/client_golang/prometheus"
@@ -27,6 +28,8 @@ import (
2728
"k8s.io/klog/v2"
2829

2930
configv1 "github.com/openshift/api/config/v1"
31+
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
32+
tlsprofile "github.com/openshift/controller-runtime-common/pkg/tls"
3033
"github.com/openshift/library-go/pkg/crypto"
3134

3235
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
@@ -134,6 +137,77 @@ type asyncResult struct {
134137
error error
135138
}
136139

140+
type cachedTLSProfile struct {
141+
spec configv1.TLSProfileSpec
142+
apply func(*tls.Config)
143+
144+
// generation is used to detect when the APIServer resource has been updated for caching
145+
generation int64
146+
}
147+
148+
// getAPIServerTLSProfile fetches the cluster TLS profile from APIServer resource.
149+
// On error, returns cached profile or fails if no cache exists.
150+
func getAPIServerTLSProfile(apiServerLister configlistersv1.APIServerLister, lastValidProfile *cachedTLSProfile) (*cachedTLSProfile, error) {
151+
apiServer, err := apiServerLister.Get(tlsprofile.APIServerName)
152+
if err != nil {
153+
klog.Errorf("Failed to get APIServer resource: %v", err)
154+
return fallbackToCached(lastValidProfile)
155+
}
156+
157+
// Check if the cached profile is still valid based on generation
158+
if lastValidProfile != nil && lastValidProfile.generation == apiServer.Generation {
159+
klog.V(4).Info("Using cached TLS profile (generation unchanged)")
160+
return lastValidProfile, nil
161+
}
162+
163+
profile, err := tlsprofile.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile)
164+
if err != nil {
165+
klog.Errorf("Failed to resolve TLS profile from APIServer: %v", err)
166+
return fallbackToCached(lastValidProfile)
167+
}
168+
169+
if lastValidProfile != nil && lastValidProfile.isEqual(&profile) {
170+
klog.V(4).Info("TLS profile spec unchanged despite generation bump, updating generation")
171+
return &cachedTLSProfile{
172+
spec: profile,
173+
apply: lastValidProfile.apply,
174+
generation: apiServer.Generation,
175+
}, nil
176+
}
177+
178+
applyTLSProfile, unsupportedCiphers := tlsprofile.NewTLSConfigFromProfile(profile)
179+
if len(unsupportedCiphers) > 0 {
180+
klog.Warningf("TLS profile contains unsupported ciphers (will be ignored): %v", unsupportedCiphers)
181+
}
182+
klog.Infof("TLS profile changed to: MinTLSVersion=%s, Ciphers=%v", profile.MinTLSVersion, profile.Ciphers)
183+
return &cachedTLSProfile{
184+
spec: profile,
185+
apply: applyTLSProfile,
186+
generation: apiServer.Generation,
187+
}, nil
188+
}
189+
190+
// fallbackToCached returns the cached profile if available, otherwise returns an error.
191+
func fallbackToCached(lastValidProfile *cachedTLSProfile) (*cachedTLSProfile, error) {
192+
if lastValidProfile != nil {
193+
klog.Warningf("Using last valid TLS profile")
194+
return lastValidProfile, nil
195+
}
196+
return nil, fmt.Errorf("no valid TLS profile available")
197+
}
198+
199+
// isEqual checks if the cached profile matches the given profile spec.
200+
func (c *cachedTLSProfile) isEqual(profile *configv1.TLSProfileSpec) bool {
201+
if c == nil && profile == nil {
202+
return true
203+
}
204+
if c == nil || profile == nil {
205+
return false
206+
}
207+
return c.spec.MinTLSVersion == profile.MinTLSVersion &&
208+
slices.Equal(c.spec.Ciphers, profile.Ciphers)
209+
}
210+
137211
func createHttpServer(options MetricsOptions, clientCA dynamiccertificates.CAContentProvider) *http.Server {
138212
if options.DisableAuthentication && options.DisableAuthorization {
139213
handler := http.NewServeMux()
@@ -276,7 +350,7 @@ type MetricsOptions struct {
276350
// Continues serving until runContext.Done() and then attempts a clean
277351
// shutdown limited by shutdownContext.Done(). Assumes runContext.Done()
278352
// occurs before or simultaneously with shutdownContext.Done().
279-
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, options MetricsOptions) error {
353+
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, apiServerLister configlistersv1.APIServerLister, options MetricsOptions) error {
280354
if options.ListenAddress == "" {
281355
return errors.New("listen address is required to serve metrics")
282356
}
@@ -361,6 +435,7 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
361435
// baseTlSConfig is a template passed to servingCertController,
362436
// which generates updated configs via GetConfigForClient callback on each TLS handshake.
363437
// This enables automatic certificate rotation without server restarts.
438+
// The cluster TLS profile will be applied dynamically in GetConfigForClient.
364439
baseTlSConfig := crypto.SecureTLSConfig(&tls.Config{ClientAuth: clientAuth})
365440
servingCertController := dynamiccertificates.NewDynamicServingCertificateController(
366441
baseTlSConfig,
@@ -388,6 +463,12 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
388463
}()
389464

390465
server := createHttpServer(options, clientCA)
466+
467+
// lastValidProfile caches the last successfully fetched TLS profile and its apply function.
468+
// On errors, we use this cached value to maintain stability rather than
469+
// constantly switching between profiles on transient errors.
470+
var lastValidProfile *cachedTLSProfile
471+
391472
tlsConfig := crypto.SecureTLSConfig(&tls.Config{
392473
GetConfigForClient: func(clientHello *tls.ClientHelloInfo) (*tls.Config, error) {
393474
config, err := servingCertController.GetConfigForClient(clientHello)
@@ -399,6 +480,14 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
399480
err := fmt.Errorf("serving certificate controller returned nil TLS configuration")
400481
return nil, err
401482
}
483+
484+
profile, err := getAPIServerTLSProfile(apiServerLister, lastValidProfile)
485+
if err != nil {
486+
return nil, fmt.Errorf("failed to get TLS profile for metrics server: %w", err)
487+
}
488+
lastValidProfile = profile
489+
profile.apply(config)
490+
402491
return config, nil
403492
},
404493
})

pkg/start/start.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
356356
resultChannelCount++
357357
go func() {
358358
defer utilruntime.HandleCrash()
359-
err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, o.MetricsOptions)
359+
err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, controllerCtx.CVO.APIServerLister(), o.MetricsOptions)
360360
resultChannel <- asyncResult{name: "metrics server", error: err}
361361
}()
362362
}
@@ -615,6 +615,7 @@ func (o *Options) NewControllerContext(
615615
configInformerFactory.Config().V1().Proxies(),
616616
operatorInformerFactory,
617617
configInformerFactory.Config().V1().FeatureGates(),
618+
configInformerFactory.Config().V1().APIServers(),
618619
cb.ClientOrDie(o.Namespace),
619620
cvoKubeClient,
620621
operatorClient,

0 commit comments

Comments
 (0)