-
Notifications
You must be signed in to change notification settings - Fork 236
OCPCLOUD-3346: tls: use centralized TLS #1456
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
d66f0a4
664ba59
01cd096
c5538af
c0164fe
069efdc
8c0632b
7ff42a6
430a883
a89f6ef
fe1ce87
77d0b59
1659685
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,38 +2,50 @@ package main | |
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "errors" | ||
| "flag" | ||
| "fmt" | ||
| "net/http" | ||
| "os" | ||
| "reflect" | ||
| "strconv" | ||
| "sync" | ||
| "sync/atomic" | ||
|
|
||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/client_golang/prometheus/promhttp" | ||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/pflag" | ||
| v1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "k8s.io/client-go/kubernetes" | ||
| 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/leaderelection" | ||
| "k8s.io/client-go/tools/record" | ||
| "k8s.io/klog/v2" | ||
| "k8s.io/utils/clock" | ||
|
|
||
| osconfigv1 "github.com/openshift/api/config/v1" | ||
| osclientset "github.com/openshift/client-go/config/clientset/versioned" | ||
| utiltls "github.com/openshift/controller-runtime-common/pkg/tls" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
| "github.com/openshift/machine-api-operator/pkg/metrics" | ||
| maometrics "github.com/openshift/machine-api-operator/pkg/metrics" | ||
| "github.com/openshift/machine-api-operator/pkg/operator" | ||
| "github.com/openshift/machine-api-operator/pkg/util" | ||
| "github.com/openshift/machine-api-operator/pkg/version" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" | ||
| "sigs.k8s.io/controller-runtime/pkg/metrics/filters" | ||
| metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" | ||
| ) | ||
|
|
||
| const ( | ||
| // defaultMetricsPort is the default port to expose metrics. | ||
| defaultMetricsPort = 8080 | ||
| defaultMetricsPort = 8443 | ||
| metricsCertDir = "/etc/tls/private" | ||
| metricsCertFile = "tls.crt" | ||
| metricsKeyFile = "tls.key" | ||
| ) | ||
|
|
||
| var ( | ||
|
|
@@ -82,33 +94,50 @@ func runStartCmd(cmd *cobra.Command, args []string) error { | |
| return fmt.Errorf("error creating clients: %v", err) | ||
| } | ||
| stopCh := make(chan struct{}) | ||
| leaderElectionCtx, leaderElectionCancel := context.WithCancel(context.Background()) | ||
| var shutdownOnce sync.Once | ||
| var shuttingDown atomic.Bool | ||
| shutdown := func() { | ||
| shutdownOnce.Do(func() { | ||
| shuttingDown.Store(true) | ||
| close(stopCh) | ||
| leaderElectionCancel() | ||
| }) | ||
| } | ||
|
|
||
| le := util.GetLeaderElectionConfig(cb.config, osconfigv1.LeaderElection{}) | ||
|
|
||
| leaderelection.RunOrDie(context.TODO(), leaderelection.LeaderElectionConfig{ | ||
| leaderelection.RunOrDie(leaderElectionCtx, leaderelection.LeaderElectionConfig{ | ||
| Lock: CreateResourceLock(cb, componentNamespace, componentName), | ||
| RenewDeadline: le.RenewDeadline.Duration, | ||
| RetryPeriod: le.RetryPeriod.Duration, | ||
| LeaseDuration: le.LeaseDuration.Duration, | ||
| Callbacks: leaderelection.LeaderCallbacks{ | ||
| OnStartedLeading: func(ctx context.Context) { | ||
| ctrlCtx := CreateControllerContext(cb, stopCh, componentNamespace) | ||
| if err := setupTLSProfileWatcher(ctrlCtx, shutdown); err != nil { | ||
| klog.Fatalf("Unable to set up TLS profile watcher: %v", err) | ||
| } | ||
| startControllersOrDie(ctrlCtx) | ||
| ctrlCtx.KubeNamespacedInformerFactory.Start(ctrlCtx.Stop) | ||
| ctrlCtx.ConfigInformerFactory.Start(ctrlCtx.Stop) | ||
| initMachineAPIInformers(ctrlCtx) | ||
| startMetricsCollectionAndServer(ctrlCtx) | ||
| close(ctrlCtx.InformersStarted) | ||
|
|
||
| select {} | ||
| <-stopCh | ||
| }, | ||
| OnStoppedLeading: func() { | ||
| if shuttingDown.Load() { | ||
| klog.Info("Leader election stopped due to shutdown") | ||
| return | ||
| } | ||
| klog.Fatalf("Leader election lost") | ||
| }, | ||
| }, | ||
| ReleaseOnCancel: true, | ||
| }) | ||
| panic("unreachable") | ||
| return nil | ||
| } | ||
|
|
||
| func initMachineAPIInformers(ctx *ControllerContext) { | ||
|
|
@@ -182,11 +211,11 @@ func startControllersOrDie(ctx *ControllerContext) { | |
| func startMetricsCollectionAndServer(ctx *ControllerContext) { | ||
| machineInformer := ctx.MachineInformerFactory.Machine().V1beta1().Machines() | ||
| machinesetInformer := ctx.MachineInformerFactory.Machine().V1beta1().MachineSets() | ||
| machineMetricsCollector := metrics.NewMachineCollector( | ||
| machineMetricsCollector := maometrics.NewMachineCollector( | ||
| machineInformer, | ||
| machinesetInformer, | ||
| componentNamespace) | ||
| prometheus.MustRegister(machineMetricsCollector) | ||
| ctrlmetrics.Registry.MustRegister(machineMetricsCollector) | ||
| metricsPort := defaultMetricsPort | ||
| if port, ok := os.LookupEnv("METRICS_PORT"); ok { | ||
| v, err := strconv.Atoi(port) | ||
|
|
@@ -195,17 +224,147 @@ func startMetricsCollectionAndServer(ctx *ControllerContext) { | |
| } | ||
| metricsPort = v | ||
| } | ||
| klog.V(4).Info("Starting server to serve prometheus metrics") | ||
| go startHTTPMetricServer(fmt.Sprintf("localhost:%d", metricsPort)) | ||
| klog.V(4).Info("Starting secure metrics server") | ||
| tlsOpts, err := metricsTLSOptions(ctx) | ||
| if err != nil { | ||
| klog.Fatalf("Unable to configure metrics TLS: %v", err) | ||
| } | ||
| metricsServer, err := newSecureMetricsServer( | ||
| ctx, | ||
| fmt.Sprintf(":%d", metricsPort), | ||
| tlsOpts, | ||
| ) | ||
| if err != nil { | ||
| klog.Fatalf("Unable to initialize secure metrics server: %v", err) | ||
| } | ||
|
|
||
| metricsServerCtx, cancel := context.WithCancel(context.Background()) | ||
| go func() { | ||
| <-ctx.Stop | ||
| cancel() | ||
| }() | ||
|
|
||
| go func() { | ||
| if err := metricsServer.Start(metricsServerCtx); err != nil { | ||
| if errors.Is(err, context.Canceled) { | ||
| klog.V(2).Info("Secure metrics server shutdown complete") | ||
| return | ||
| } | ||
| klog.Fatalf("Unable to start secure metrics server: %v", err) | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| func metricsTLSOptions(ctx *ControllerContext) ([]func(*tls.Config), error) { | ||
| scheme := runtime.NewScheme() | ||
| if err := osconfigv1.Install(scheme); err != nil { | ||
| return nil, fmt.Errorf("unable to add config.openshift.io scheme: %w", err) | ||
| } | ||
|
|
||
| k8sClient, err := client.New(ctx.ClientBuilder.config, client.Options{Scheme: scheme}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create Kubernetes client: %w", err) | ||
| } | ||
|
|
||
| tlsSecurityProfileSpec, err := utiltls.FetchAPIServerTLSProfile(context.Background(), k8sClient) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to get TLS profile from API server: %w", err) | ||
| } | ||
|
|
||
| tlsConfigFn, unsupportedCiphers := utiltls.NewTLSConfigFromProfile(tlsSecurityProfileSpec) | ||
| if len(unsupportedCiphers) > 0 { | ||
| klog.Infof("TLS configuration contains unsupported ciphers that will be ignored: %v", unsupportedCiphers) | ||
| } | ||
|
|
||
| return []func(*tls.Config){tlsConfigFn}, nil | ||
| } | ||
|
|
||
| func startHTTPMetricServer(metricsPort string) { | ||
| mux := http.NewServeMux() | ||
| mux.Handle("/metrics", promhttp.Handler()) | ||
| func newSecureMetricsServer(ctx *ControllerContext, metricsAddr string, tlsOpts []func(*tls.Config)) (metricsserver.Server, error) { | ||
| httpClient, err := rest.HTTPClientFor(ctx.ClientBuilder.config) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to create HTTP client for metrics authn/authz: %w", err) | ||
| } | ||
|
|
||
| return metricsserver.NewServer(metricsserver.Options{ | ||
| BindAddress: metricsAddr, | ||
| SecureServing: true, | ||
| FilterProvider: filters.WithAuthenticationAndAuthorization, | ||
damdo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| CertDir: metricsCertDir, | ||
| CertName: metricsCertFile, | ||
| KeyName: metricsKeyFile, | ||
| TLSOpts: tlsOpts, | ||
| }, ctx.ClientBuilder.config, httpClient) | ||
| } | ||
|
|
||
| server := &http.Server{ | ||
| Addr: metricsPort, | ||
| Handler: mux, | ||
| func setupTLSProfileWatcher(ctx *ControllerContext, shutdown func()) error { | ||
| configClient := ctx.ClientBuilder.OpenshiftClientOrDie("tls-profile-watcher") | ||
| initialProfile, err := fetchAPIServerTLSProfileSpec(context.Background(), configClient) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| klog.Fatal(server.ListenAndServe()) | ||
|
|
||
| apiServerInformer := ctx.ConfigInformerFactory.Config().V1().APIServers().Informer() | ||
| _, err = apiServerInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ | ||
| AddFunc: func(obj interface{}) { | ||
| handleTLSProfileEvent(obj, &initialProfile, shutdown) | ||
| }, | ||
| UpdateFunc: func(_, newObj interface{}) { | ||
| handleTLSProfileEvent(newObj, &initialProfile, shutdown) | ||
| }, | ||
| DeleteFunc: func(obj interface{}) { | ||
| handleTLSProfileEvent(obj, &initialProfile, shutdown) | ||
| }, | ||
RadekManak marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+299
to
+316
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify control flow: read the key sections mentioned in the review
echo "=== Section 1: Lines 96-140 (startup context) ==="
sed -n '96,140p' cmd/machine-api-operator/start.go
echo -e "\n=== Section 2: Lines 143-153 (cache sync with fatal) ==="
sed -n '143,153p' cmd/machine-api-operator/start.go
echo -e "\n=== Section 3: Lines 299-370 (setupTLSProfileWatcher) ==="
sed -n '299,370p' cmd/machine-api-operator/start.goRepository: openshift/machine-api-operator Length of output: 4655 Prevent fatal crash when TLS watcher triggers shutdown before cache sync completes.
Suggested hardeningfunc initMachineAPIInformers(ctx *ControllerContext) {
mInformer := ctx.MachineInformerFactory.Machine().V1beta1().Machines().Informer()
msInformer := ctx.MachineInformerFactory.Machine().V1beta1().MachineSets().Informer()
ctx.MachineInformerFactory.Start(ctx.Stop)
if !cache.WaitForCacheSync(ctx.Stop,
mInformer.HasSynced,
msInformer.HasSynced) {
+ select {
+ case <-ctx.Stop:
+ klog.V(2).Info("Skipping Machine API informer sync due to shutdown")
+ return
+ default:
+ klog.Fatal("Failed to sync caches for Machine api informers")
+ }
}
klog.Info("Synced up machine api informer caches")
}🤖 Prompt for AI Agents |
||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to add APIServer event handler: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func fetchAPIServerTLSProfileSpec(ctx context.Context, configClient osclientset.Interface) (osconfigv1.TLSProfileSpec, error) { | ||
| apiServer, err := configClient.ConfigV1().APIServers().Get(ctx, utiltls.APIServerName, metav1.GetOptions{}) | ||
| if err != nil { | ||
| return osconfigv1.TLSProfileSpec{}, fmt.Errorf("failed to get APIServer %q: %w", utiltls.APIServerName, err) | ||
| } | ||
|
|
||
| profile, err := utiltls.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile) | ||
| if err != nil { | ||
| return osconfigv1.TLSProfileSpec{}, fmt.Errorf("failed to get TLS profile from APIServer %q: %w", utiltls.APIServerName, err) | ||
| } | ||
|
|
||
| return profile, nil | ||
| } | ||
|
|
||
| func handleTLSProfileEvent(obj interface{}, initialProfile *osconfigv1.TLSProfileSpec, shutdown func()) { | ||
| apiServer, ok := obj.(*osconfigv1.APIServer) | ||
| if !ok { | ||
| return | ||
| } | ||
| if apiServer.Name != utiltls.APIServerName { | ||
| return | ||
| } | ||
|
|
||
| currentProfile, err := utiltls.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile) | ||
| if err != nil { | ||
| klog.Errorf("Failed to get TLS profile from APIServer %q: %v", apiServer.Name, err) | ||
| return | ||
| } | ||
|
|
||
| if reflect.DeepEqual(*initialProfile, currentProfile) { | ||
| klog.V(2).Info("TLS security profile unchanged") | ||
| return | ||
| } | ||
|
|
||
damdo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| klog.Infof("TLS security profile has changed, initiating a shutdown to pick up the new configuration: initialMinTLSVersion=%s currentMinTLSVersion=%s initialCiphers=%v currentCiphers=%v", | ||
| initialProfile.MinTLSVersion, | ||
| currentProfile.MinTLSVersion, | ||
| initialProfile.Ciphers, | ||
| currentProfile.Ciphers, | ||
| ) | ||
|
|
||
| // Persist the new profile for future change detection. | ||
| *initialProfile = currentProfile | ||
|
|
||
| shutdown() | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.