Skip to content

Commit c2493e5

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

3 files changed

Lines changed: 111 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: 100 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,87 @@ type asyncResult struct {
134137
error error
135138
}
136139

140+
type cachedTLSProfile struct {
141+
// spec is the TLS profile specification, or nil if no profile should be applied
142+
spec *configv1.TLSProfileSpec
143+
apply func(*tls.Config)
144+
145+
// generation is used to detect when the APIServer resource has been updated for caching
146+
generation int64
147+
}
148+
149+
// getAPIServerTLSProfile fetches the cluster TLS profile from APIServer resource.
150+
// On error, returns cached profile or fails if no cache exists.
151+
func getAPIServerTLSProfile(apiServerLister configlistersv1.APIServerLister, lastValidProfile *cachedTLSProfile) (*cachedTLSProfile, error) {
152+
apiServer, err := apiServerLister.Get(tlsprofile.APIServerName)
153+
if err != nil {
154+
klog.Errorf("Failed to get APIServer resource: %v", err)
155+
return fallbackToCached(lastValidProfile)
156+
}
157+
158+
// Check if the cached profile is still valid based on generation
159+
if lastValidProfile != nil && lastValidProfile.generation == apiServer.Generation {
160+
klog.V(4).Info("Using cached TLS profile (generation unchanged)")
161+
return lastValidProfile, nil
162+
}
163+
164+
if false { // TODO: Add TLS adherence logic when API changes merge
165+
return &cachedTLSProfile{
166+
spec: nil,
167+
apply: func(config *tls.Config) {}, // do nothing
168+
generation: apiServer.Generation,
169+
}, nil
170+
}
171+
172+
profile, err := tlsprofile.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile)
173+
if err != nil {
174+
klog.Errorf("Failed to resolve TLS profile from APIServer: %v", err)
175+
return fallbackToCached(lastValidProfile)
176+
}
177+
178+
if lastValidProfile != nil && lastValidProfile.isEqual(&profile) {
179+
klog.V(4).Info("TLS profile spec unchanged despite generation bump, updating generation")
180+
return &cachedTLSProfile{
181+
spec: &profile,
182+
apply: lastValidProfile.apply,
183+
generation: apiServer.Generation,
184+
}, nil
185+
}
186+
187+
applyTLSProfile, unsupportedCiphers := tlsprofile.NewTLSConfigFromProfile(profile)
188+
if len(unsupportedCiphers) > 0 {
189+
klog.Warningf("TLS profile contains unsupported ciphers (will be ignored): %v", unsupportedCiphers)
190+
}
191+
klog.Infof("TLS profile changed to: MinTLSVersion=%s, Ciphers=%v", profile.MinTLSVersion, profile.Ciphers)
192+
return &cachedTLSProfile{
193+
spec: &profile,
194+
apply: applyTLSProfile,
195+
generation: apiServer.Generation,
196+
}, nil
197+
}
198+
199+
// fallbackToCached returns the cached profile if available, otherwise returns an error.
200+
func fallbackToCached(lastValidProfile *cachedTLSProfile) (*cachedTLSProfile, error) {
201+
if lastValidProfile != nil {
202+
klog.Warningf("Using last valid TLS profile")
203+
return lastValidProfile, nil
204+
}
205+
return nil, fmt.Errorf("no valid TLS profile available")
206+
}
207+
208+
// isEqual checks if the cached profile matches the given profile spec.
209+
func (c *cachedTLSProfile) isEqual(profile *configv1.TLSProfileSpec) bool {
210+
switch {
211+
case c == nil || c.spec == nil:
212+
return profile == nil
213+
case profile == nil:
214+
return false
215+
default:
216+
return c.spec.MinTLSVersion == profile.MinTLSVersion &&
217+
slices.Equal(c.spec.Ciphers, profile.Ciphers)
218+
}
219+
}
220+
137221
func createHttpServer(options MetricsOptions, clientCA dynamiccertificates.CAContentProvider) *http.Server {
138222
if options.DisableAuthentication && options.DisableAuthorization {
139223
handler := http.NewServeMux()
@@ -276,7 +360,7 @@ type MetricsOptions struct {
276360
// Continues serving until runContext.Done() and then attempts a clean
277361
// shutdown limited by shutdownContext.Done(). Assumes runContext.Done()
278362
// occurs before or simultaneously with shutdownContext.Done().
279-
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, options MetricsOptions) error {
363+
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, apiServerLister configlistersv1.APIServerLister, options MetricsOptions) error {
280364
if options.ListenAddress == "" {
281365
return errors.New("listen address is required to serve metrics")
282366
}
@@ -361,6 +445,7 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
361445
// baseTlSConfig is a template passed to servingCertController,
362446
// which generates updated configs via GetConfigForClient callback on each TLS handshake.
363447
// This enables automatic certificate rotation without server restarts.
448+
// The cluster TLS profile will be applied dynamically in GetConfigForClient.
364449
baseTlSConfig := crypto.SecureTLSConfig(&tls.Config{ClientAuth: clientAuth})
365450
servingCertController := dynamiccertificates.NewDynamicServingCertificateController(
366451
baseTlSConfig,
@@ -388,6 +473,12 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
388473
}()
389474

390475
server := createHttpServer(options, clientCA)
476+
477+
// lastValidProfile caches the last successfully fetched TLS profile and its apply function.
478+
// On errors, we use this cached value to maintain stability rather than
479+
// constantly switching between profiles on transient errors.
480+
var lastValidProfile *cachedTLSProfile
481+
391482
tlsConfig := crypto.SecureTLSConfig(&tls.Config{
392483
GetConfigForClient: func(clientHello *tls.ClientHelloInfo) (*tls.Config, error) {
393484
config, err := servingCertController.GetConfigForClient(clientHello)
@@ -399,6 +490,14 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
399490
err := fmt.Errorf("serving certificate controller returned nil TLS configuration")
400491
return nil, err
401492
}
493+
494+
profile, err := getAPIServerTLSProfile(apiServerLister, lastValidProfile)
495+
if err != nil {
496+
return nil, fmt.Errorf("failed to get TLS profile for metrics server: %w", err)
497+
}
498+
lastValidProfile = profile
499+
profile.apply(config)
500+
402501
return config, nil
403502
},
404503
})

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)