Skip to content

CNTRLPLANE-3426: restart operator upon tls config change #1389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ricardomaraschini wants to merge 1 commit into
base: main
Choose a base branch
from
+234 −0
Open

CNTRLPLANE-3426: restart operator upon tls config change #1389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions pkg/cvo/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
128 changes: 128 additions & 0 deletions pkg/cvo/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
})
}
}
33 changes: 33 additions & 0 deletions pkg/start/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/url"
"os"
"os/signal"
"slices"
"syscall"
"time"

Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand Down