Skip to content

Commit fc32b95

Browse files
ricardomaraschiniricardomaraschini
committed
NO-JIRA: 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.
1 parent e75b70a commit fc32b95

3 files changed

Lines changed: 241 additions & 0 deletions

File tree

‎pkg/cvo/metrics.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ 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"
3031
"github.com/openshift/library-go/pkg/crypto"
3132

3233
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
@@ -267,6 +268,74 @@ type MetricsOptions struct {
267268

268269
DisableAuthentication bool
269270
DisableAuthorization bool
271+
272+
// TLS configuration observed from APIServer cluster object
273+
MinTLSVersion uint16
274+
CipherSuites []uint16
275+
}
276+
277+
// ObserveAPIServerTLSConfig extracts and converts TLS configuration from the
278+
// APIServer object. Always returns valid defaults even if APIServer object is
279+
// missing, has no TLS profile configured, or contains invalid values. Never
280+
// returns an error, any issues result in default values being returned.
281+
func ObserveAPIServerTLSConfig(ctx context.Context, lister configlistersv1.APIServerLister) (uint16, []uint16) {
282+
defaultProfileConfig := configv1.TLSProfiles[crypto.DefaultTLSProfileType]
283+
284+
// capture the defaults. if we fail at any stage on this function we
285+
// log and return the default values.
286+
defaultMinTLS := crypto.TLSVersionOrDie(string(defaultProfileConfig.MinTLSVersion))
287+
defaultCipherSuites := crypto.CipherSuitesOrDie(crypto.OpenSSLToIANACipherSuites(defaultProfileConfig.Ciphers))
288+
289+
apiServer, err := lister.Get("cluster")
290+
if err != nil {
291+
klog.V(4).Infof("Unable to read APIServer object, using default TLS profile: %v", err)
292+
return defaultMinTLS, defaultCipherSuites
293+
}
294+
295+
if apiServer.Spec.TLSSecurityProfile == nil {
296+
return defaultMinTLS, defaultCipherSuites
297+
}
298+
299+
profile := apiServer.Spec.TLSSecurityProfile
300+
301+
profileConfig := configv1.TLSProfiles[profile.Type]
302+
if profile.Type == configv1.TLSProfileCustomType {
303+
if profile.Custom == nil {
304+
klog.Warningf("APIServer TLS profile type is Custom but no custom spec provided, using default TLS profile")
305+
return defaultMinTLS, defaultCipherSuites
306+
}
307+
profileConfig = &profile.Custom.TLSProfileSpec
308+
}
309+
310+
if profileConfig == nil {
311+
klog.Warningf("TLS config for profile %q not found, using default TLS profile", profile.Type)
312+
return defaultMinTLS, defaultCipherSuites
313+
}
314+
315+
minTLSVersion, err := crypto.TLSVersion(string(profileConfig.MinTLSVersion))
316+
if err != nil {
317+
klog.Warningf("Invalid minTLSVersion %q in APIServer TLS profile, using default: %v", profileConfig.MinTLSVersion, err)
318+
return defaultMinTLS, defaultCipherSuites
319+
}
320+
321+
// convert OpenSSL cipher names to IANA names.
322+
ianaCiphers := crypto.OpenSSLToIANACipherSuites(profileConfig.Ciphers)
323+
if len(ianaCiphers) == 0 {
324+
klog.Warningf("Failed to convert APIServer cipher suites %v to IANA format, using defaults", profileConfig.Ciphers)
325+
return defaultMinTLS, defaultCipherSuites
326+
}
327+
328+
cipherSuites := make([]uint16, 0, len(ianaCiphers))
329+
for _, cipherName := range ianaCiphers {
330+
cipher, err := crypto.CipherSuite(cipherName)
331+
if err != nil {
332+
klog.Warningf("Invalid cipher suite %q in APIServer TLS profile, using default TLS profile: %v", cipherName, err)
333+
return defaultMinTLS, defaultCipherSuites
334+
}
335+
cipherSuites = append(cipherSuites, cipher)
336+
}
337+
338+
return minTLSVersion, cipherSuites
270339
}
271340

272341
// RunMetrics launches an HTTPS server bound to listenAddress serving
@@ -362,6 +431,15 @@ func RunMetrics(runContext context.Context, shutdownContext context.Context, res
362431
// which generates updated configs via GetConfigForClient callback on each TLS handshake.
363432
// This enables automatic certificate rotation without server restarts.
364433
baseTlSConfig := crypto.SecureTLSConfig(&tls.Config{ClientAuth: clientAuth})
434+
435+
// Apply APIServer TLS configuration from options
436+
if options.MinTLSVersion != 0 {
437+
baseTlSConfig.MinVersion = options.MinTLSVersion
438+
}
439+
if len(options.CipherSuites) > 0 {
440+
baseTlSConfig.CipherSuites = options.CipherSuites
441+
}
442+
365443
servingCertController := dynamiccertificates.NewDynamicServingCertificateController(
366444
baseTlSConfig,
367445
clientCA,

‎pkg/cvo/metrics_test.go‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ import (
1818

1919
corev1 "k8s.io/api/core/v1"
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
"k8s.io/apimachinery/pkg/runtime"
2122
"k8s.io/apiserver/pkg/authentication/user"
2223
"k8s.io/apiserver/pkg/server/dynamiccertificates"
2324
"k8s.io/client-go/tools/record"
2425

2526
configv1 "github.com/openshift/api/config/v1"
27+
fakeconfigclientv1 "github.com/openshift/client-go/config/clientset/versioned/fake"
28+
configinformers "github.com/openshift/client-go/config/informers/externalversions"
2629
"github.com/openshift/library-go/pkg/crypto"
2730

2831
"github.com/openshift/cluster-version-operator/pkg/featuregates"
@@ -1428,3 +1431,130 @@ func (m *mockCAProvider) VerifyOptions() (x509.VerifyOptions, bool) {
14281431

14291432
func (m *mockCAProvider) AddListener(_ dynamiccertificates.Listener) {
14301433
}
1434+
1435+
func TestObserveAPIServerTLSConfig(t *testing.T) {
1436+
ctx := t.Context()
1437+
1438+
tests := []struct {
1439+
name string
1440+
apiServer *configv1.APIServer
1441+
expectMinVersion uint16
1442+
checkCiphers bool
1443+
}{
1444+
{
1445+
name: "custom TLS profile",
1446+
apiServer: &configv1.APIServer{
1447+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
1448+
Spec: configv1.APIServerSpec{
1449+
TLSSecurityProfile: &configv1.TLSSecurityProfile{
1450+
Type: configv1.TLSProfileCustomType,
1451+
Custom: &configv1.CustomTLSProfile{
1452+
TLSProfileSpec: configv1.TLSProfileSpec{
1453+
MinTLSVersion: configv1.VersionTLS13,
1454+
Ciphers: []string{
1455+
"TLS_AES_128_GCM_SHA256",
1456+
"TLS_AES_256_GCM_SHA384",
1457+
},
1458+
},
1459+
},
1460+
},
1461+
},
1462+
},
1463+
expectMinVersion: tls.VersionTLS13,
1464+
checkCiphers: true,
1465+
},
1466+
{
1467+
name: "intermediate TLS profile",
1468+
apiServer: &configv1.APIServer{
1469+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
1470+
Spec: configv1.APIServerSpec{
1471+
TLSSecurityProfile: &configv1.TLSSecurityProfile{
1472+
Type: configv1.TLSProfileIntermediateType,
1473+
},
1474+
},
1475+
},
1476+
expectMinVersion: tls.VersionTLS12,
1477+
checkCiphers: true,
1478+
},
1479+
{
1480+
name: "no APIServer object (uses defaults)",
1481+
apiServer: nil,
1482+
expectMinVersion: tls.VersionTLS12, // default is intermediate
1483+
checkCiphers: true,
1484+
},
1485+
{
1486+
name: "nil TLS profile (uses defaults)",
1487+
apiServer: &configv1.APIServer{
1488+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
1489+
Spec: configv1.APIServerSpec{},
1490+
},
1491+
expectMinVersion: tls.VersionTLS12,
1492+
checkCiphers: true,
1493+
},
1494+
{
1495+
name: "custom TLS profile type but no custom spec (uses defaults)",
1496+
apiServer: &configv1.APIServer{
1497+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
1498+
Spec: configv1.APIServerSpec{
1499+
TLSSecurityProfile: &configv1.TLSSecurityProfile{
1500+
Type: configv1.TLSProfileCustomType,
1501+
},
1502+
},
1503+
},
1504+
expectMinVersion: tls.VersionTLS12,
1505+
checkCiphers: true,
1506+
},
1507+
{
1508+
name: "invalid TLS version and cipher suites (uses defaults)",
1509+
apiServer: &configv1.APIServer{
1510+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
1511+
Spec: configv1.APIServerSpec{
1512+
TLSSecurityProfile: &configv1.TLSSecurityProfile{
1513+
Type: configv1.TLSProfileCustomType,
1514+
Custom: &configv1.CustomTLSProfile{
1515+
TLSProfileSpec: configv1.TLSProfileSpec{
1516+
MinTLSVersion: "InvalidTLSVersion",
1517+
Ciphers: []string{
1518+
"RANDOM_CIPHER_123",
1519+
"INVALID_CIPHER_XYZ",
1520+
"NOT_A_REAL_CIPHER",
1521+
},
1522+
},
1523+
},
1524+
},
1525+
},
1526+
},
1527+
expectMinVersion: tls.VersionTLS12,
1528+
checkCiphers: true,
1529+
},
1530+
}
1531+
1532+
for _, tt := range tests {
1533+
t.Run(tt.name, func(t *testing.T) {
1534+
// Create fake client and informer
1535+
var objects []runtime.Object
1536+
if tt.apiServer != nil {
1537+
objects = append(objects, tt.apiServer)
1538+
}
1539+
fakeClient := fakeconfigclientv1.NewSimpleClientset(objects...)
1540+
informerFactory := configinformers.NewSharedInformerFactory(fakeClient, 0)
1541+
lister := informerFactory.Config().V1().APIServers().Lister()
1542+
1543+
// Start informers and wait for cache sync
1544+
stopCh := make(chan struct{})
1545+
defer close(stopCh)
1546+
informerFactory.Start(stopCh)
1547+
informerFactory.WaitForCacheSync(stopCh)
1548+
1549+
minVer, ciphers := ObserveAPIServerTLSConfig(ctx, lister)
1550+
1551+
if minVer != tt.expectMinVersion {
1552+
t.Errorf("expected minTLSVersion 0x%04x, got 0x%04x", tt.expectMinVersion, minVer)
1553+
}
1554+
1555+
if tt.checkCiphers && len(ciphers) == 0 {
1556+
t.Errorf("expected cipher suites to be non-empty")
1557+
}
1558+
})
1559+
}
1560+
}

‎pkg/start/start.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/url"
1010
"os"
1111
"os/signal"
12+
"slices"
1213
"syscall"
1314
"time"
1415

@@ -26,6 +27,7 @@ import (
2627
"k8s.io/client-go/kubernetes/scheme"
2728
coreclientsetv1 "k8s.io/client-go/kubernetes/typed/core/v1"
2829
"k8s.io/client-go/rest"
30+
"k8s.io/client-go/tools/cache"
2931
"k8s.io/client-go/tools/clientcmd"
3032
"k8s.io/client-go/tools/leaderelection"
3133
"k8s.io/client-go/tools/leaderelection/resourcelock"
@@ -326,6 +328,31 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
326328
resultChannel := make(chan asyncResult, 1)
327329
resultChannelCount := 0
328330

331+
// add a watcher for APIServer TLS config changes before starting
332+
// informers. this watcher will stop the cvo upon tls configuration
333+
// changes (we need to restart it).
334+
if o.MetricsOptions.ListenAddress != "" {
335+
apiServerInformer := controllerCtx.ConfigInformerFactory.Config().V1().APIServers()
336+
startMinTLS, startCiphers := cvo.ObserveAPIServerTLSConfig(runContext, apiServerInformer.Lister())
337+
338+
handler := func() {
339+
currentMinTLS, currentCiphers := cvo.ObserveAPIServerTLSConfig(runContext, apiServerInformer.Lister())
340+
if currentMinTLS == startMinTLS && slices.Equal(currentCiphers, startCiphers) {
341+
return
342+
}
343+
klog.Infof("APIServer TLS configuration changed; requesting shutdown")
344+
runCancel()
345+
}
346+
347+
if _, err := apiServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
348+
AddFunc: func(_ any) { handler() },
349+
UpdateFunc: func(_, _ any) { handler() },
350+
DeleteFunc: func(_ any) { handler() },
351+
}); err != nil {
352+
klog.Warningf("Failed to add watcher for APIServer TLS Config")
353+
}
354+
}
355+
329356
informersDone := postMainContext.Done()
330357
// FIXME: would be nice if there was a way to collect these.
331358
controllerCtx.ClusterVersionInformerFactory.Start(informersDone)
@@ -355,6 +382,12 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
355382
OnStartedLeading: func(_ context.Context) { // no need for this passed-through postMainContext, because goroutines we launch inside will use runContext
356383
launchedMain = true
357384
if o.MetricsOptions.ListenAddress != "" {
385+
// observe cluster tls config, our metrics server should use it.
386+
o.MetricsOptions.MinTLSVersion, o.MetricsOptions.CipherSuites = cvo.ObserveAPIServerTLSConfig(
387+
runContext,
388+
controllerCtx.ConfigInformerFactory.Config().V1().APIServers().Lister(),
389+
)
390+
358391
resultChannelCount++
359392
go func() {
360393
defer utilruntime.HandleCrash()

0 commit comments

Comments
 (0)