Skip to content

Commit 42ab61b

Browse files
committed
metrics: Respect central TLS profile using controller-runtime-common
At the moment, do not introduce sophisticated caching. Utilize the generation and keep the code simple. We can iterate in the future in case the optimization in this area is worth the effort.
1 parent 571b5da commit 42ab61b

3 files changed

Lines changed: 92 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: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"k8s.io/klog/v2"
2828

2929
configv1 "github.com/openshift/api/config/v1"
30+
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
31+
tlsprofile "github.com/openshift/controller-runtime-common/pkg/tls"
3032
"github.com/openshift/library-go/pkg/crypto"
3133

3234
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
@@ -259,6 +261,63 @@ func handleServerResult(result asyncResult, lastLoopError error) error {
259261
return lastError
260262
}
261263

264+
type centralTLSProfileApplier struct {
265+
// applyTLSProfile is the function to apply the central TLS profile to a config
266+
applyTLSProfile func(*tls.Config)
267+
268+
// generation is used to detect when the APIServer resource has been updated for caching
269+
generation int64
270+
}
271+
272+
// getCentralTLSProfileApplier fetches the central TLS profile from the APIServer resource and returns an applier.
273+
// On error, falls back to the cached applier or fails if no cache exists.
274+
func getCentralTLSProfileApplier(apiServerLister configlistersv1.APIServerLister, lastValidApplier *centralTLSProfileApplier) (*centralTLSProfileApplier, error) {
275+
apiServer, err := apiServerLister.Get(tlsprofile.APIServerName)
276+
if err != nil {
277+
klog.Errorf("Failed to get APIServer resource: %v", err)
278+
return fallbackOrFail(lastValidApplier)
279+
}
280+
281+
// Check if the cached spec is still valid based on generation
282+
if lastValidApplier != nil && lastValidApplier.generation == apiServer.Generation {
283+
klog.V(4).Info("Using cached TLS apply function (generation unchanged)")
284+
return lastValidApplier, nil
285+
}
286+
287+
if !crypto.ShouldHonorClusterTLSProfile(apiServer.Spec.TLSAdherence) {
288+
klog.V(4).Infof("Not honoring cluster TLS profile based on adherence policy")
289+
return &centralTLSProfileApplier{
290+
applyTLSProfile: func(config *tls.Config) {}, // do nothing
291+
generation: apiServer.Generation,
292+
}, nil
293+
}
294+
295+
profile, err := tlsprofile.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile)
296+
if err != nil {
297+
klog.Errorf("Failed to resolve TLS profile from APIServer: %v", err)
298+
return fallbackOrFail(lastValidApplier)
299+
}
300+
301+
applyTLSProfile, unsupportedCiphers := tlsprofile.NewTLSConfigFromProfile(profile)
302+
if len(unsupportedCiphers) > 0 {
303+
klog.V(4).Infof("TLS profile contains unsupported ciphers (will be ignored): %v", unsupportedCiphers)
304+
}
305+
klog.V(4).Infof("Applied TLS configuration: MinTLSVersion=%s, Ciphers=%v", profile.MinTLSVersion, profile.Ciphers)
306+
return &centralTLSProfileApplier{
307+
applyTLSProfile: applyTLSProfile,
308+
generation: apiServer.Generation,
309+
}, nil
310+
}
311+
312+
// fallbackOrFail returns the cached applier if available, otherwise returns an error.
313+
func fallbackOrFail(cached *centralTLSProfileApplier) (*centralTLSProfileApplier, error) {
314+
if cached != nil {
315+
klog.Warningf("Using last valid TLS profile applier")
316+
return cached, nil
317+
}
318+
return nil, fmt.Errorf("no TLS profile applier available")
319+
}
320+
262321
type MetricsOptions struct {
263322
ListenAddress string
264323

@@ -276,7 +335,7 @@ type MetricsOptions struct {
276335
// Continues serving until runContext.Done() and then attempts a clean
277336
// shutdown limited by shutdownContext.Done(). Assumes runContext.Done()
278337
// occurs before or simultaneously with shutdownContext.Done().
279-
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, options MetricsOptions) error {
338+
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, apiServerLister configlistersv1.APIServerLister, options MetricsOptions) error {
280339
if options.ListenAddress == "" {
281340
return errors.New("listen address is required to serve metrics")
282341
}
@@ -361,6 +420,7 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
361420
// baseTlSConfig is a template passed to servingCertController,
362421
// which generates updated configs via GetConfigForClient callback on each TLS handshake.
363422
// This enables automatic certificate rotation without server restarts.
423+
// The cluster TLS profile will be applied dynamically in GetConfigForClient.
364424
baseTlSConfig := crypto.SecureTLSConfig(&tls.Config{ClientAuth: clientAuth})
365425
servingCertController := dynamiccertificates.NewDynamicServingCertificateController(
366426
baseTlSConfig,
@@ -388,6 +448,12 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
388448
}()
389449

390450
server := createHttpServer(options, clientCA)
451+
452+
// lastApplier caches the last successfully fetched APIServer spec and its apply function.
453+
// On errors, we use this cached value to maintain stability rather than
454+
// constantly switching between profiles on transient errors.
455+
var lastApplier *centralTLSProfileApplier
456+
391457
tlsConfig := crypto.SecureTLSConfig(&tls.Config{
392458
GetConfigForClient: func(clientHello *tls.ClientHelloInfo) (*tls.Config, error) {
393459
config, err := servingCertController.GetConfigForClient(clientHello)
@@ -399,6 +465,20 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
399465
err := fmt.Errorf("serving certificate controller returned nil TLS configuration")
400466
return nil, err
401467
}
468+
469+
// First apply central TLS profile
470+
applier, err := getCentralTLSProfileApplier(apiServerLister, lastApplier)
471+
if err != nil {
472+
return nil, fmt.Errorf("failed to get TLS profile for metrics server: %w", err)
473+
}
474+
lastApplier = applier
475+
applier.applyTLSProfile(config)
476+
477+
// Then apply flag-based overrides
478+
if err := options.ApplyTLSOptions(config); err != nil {
479+
return nil, fmt.Errorf("failed to apply TLS options: %w", err)
480+
}
481+
402482
return config, nil
403483
},
404484
})

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)