-
Notifications
You must be signed in to change notification settings - Fork 354
feat: Configuring TLS options from Central TLS Profile #1184
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: master
Are you sure you want to change the base?
Changes from all commits
f227e75
8ac93d3
a5aa76e
d15c5a1
22f5319
44ed6b4
4c0b6a7
d80dd4b
82cf637
c9ee8b3
23d0911
e7fc0a0
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 |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ limitations under the License. | |
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "flag" | ||
| "fmt" | ||
|
|
@@ -64,6 +65,7 @@ import ( | |
| "sigs.k8s.io/controller-runtime/pkg/manager" | ||
| "sigs.k8s.io/controller-runtime/pkg/webhook" | ||
|
|
||
| tlspkg "github.com/openshift/controller-runtime-common/pkg/tls" | ||
| pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1" | ||
| "github.com/redhat-developer/gitops-operator/common" | ||
| "github.com/redhat-developer/gitops-operator/controllers" | ||
|
|
@@ -94,6 +96,7 @@ func main() { | |
|
|
||
| var enableHTTP2 = false | ||
| var skipControllerNameValidation = true | ||
| var disableClusterTLSProfile = false | ||
|
|
||
| var labelSelectorFlag string | ||
| flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") | ||
|
|
@@ -103,7 +106,7 @@ func main() { | |
| "Enable leader election for controller manager. "+ | ||
| "Enabling this will ensure there is only one active controller manager.") | ||
| flag.BoolVar(&enableHTTP2, "enable-http2", enableHTTP2, "If HTTP/2 should be enabled for the metrics and webhook servers.") | ||
|
|
||
| flag.BoolVar(&disableClusterTLSProfile, "disable-cluster-tls-profile", false, "Disable use of the cluster TLS security profile") | ||
| //Configure log level | ||
| logLevelStr := strings.ToLower(os.Getenv("LOG_LEVEL")) | ||
| logLevel := zapcore.InfoLevel | ||
|
|
@@ -129,9 +132,12 @@ func main() { | |
| } | ||
| opts.BindFlags(flag.CommandLine) | ||
| flag.Parse() | ||
|
|
||
| if strings.EqualFold(os.Getenv("DISABLE_CLUSTER_TLS_PROFILE"), "true") { | ||
| disableClusterTLSProfile = true | ||
| } | ||
| ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) | ||
|
|
||
| ctx, cancel := context.WithCancel(ctrl.SetupSignalHandler()) | ||
| defer cancel() | ||
| if err := util.InspectCluster(); err != nil { | ||
| setupLog.Error(err, "unable to inspect cluster") | ||
| } | ||
|
|
@@ -142,15 +148,40 @@ func main() { | |
| } | ||
| c.NextProtos = []string{"http/1.1"} | ||
| } | ||
| var profile configv1.TLSProfileSpec | ||
| var err error | ||
| tlsOpts := []func(*tls.Config){disableHTTP2} | ||
| if util.IsConfigAPIFound() && !disableClusterTLSProfile { | ||
| utilruntime.Must(configv1.Install(scheme)) | ||
| bootstrapClient, err := crclient.New(ctrl.GetConfigOrDie(), crclient.Options{ | ||
| Scheme: scheme, | ||
| }) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to create bootstrap client") | ||
| os.Exit(1) | ||
| } | ||
| profile, err = tlspkg.FetchAPIServerTLSProfile(ctx, bootstrapClient) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to fetch cluster TLS profile") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| tlsConfigFn, unsupported := tlspkg.NewTLSConfigFromProfile(profile) | ||
| if len(unsupported) > 0 { | ||
| setupLog.Info("TLS profile contains unsupported Go cipher suites", "ciphers", unsupported) | ||
| } | ||
|
|
||
| tlsOpts = append(tlsOpts, tlsConfigFn) | ||
| } | ||
| webhookServerOptions := webhook.Options{ | ||
| TLSOpts: []func(config *tls.Config){disableHTTP2}, | ||
| TLSOpts: tlsOpts, | ||
| Port: 9443, | ||
| } | ||
| webhookServer := webhook.NewServer(webhookServerOptions) | ||
|
|
||
| metricsServerOptions := metricsserver.Options{ | ||
| BindAddress: metricsAddr, | ||
| TLSOpts: []func(*tls.Config){disableHTTP2}, | ||
| TLSOpts: tlsOpts, | ||
| FilterProvider: filters.WithAuthenticationAndAuthorization, | ||
| } | ||
|
|
||
|
|
@@ -198,6 +229,27 @@ func main() { | |
| client = mgr.GetClient() | ||
| } | ||
|
|
||
| if util.IsConfigAPIFound() && !disableClusterTLSProfile { | ||
| watcher := &tlspkg.SecurityProfileWatcher{ | ||
| Client: mgr.GetClient(), | ||
| InitialTLSProfileSpec: profile, | ||
| OnProfileChange: func(_ context.Context, oldProfile, newProfile configv1.TLSProfileSpec) { | ||
| if reflect.DeepEqual(oldProfile, newProfile) { | ||
| return | ||
| } | ||
| setupLog.Info( | ||
| "cluster TLS profile changed, restarting operator", | ||
| "oldProfileMinVersion", oldProfile.MinTLSVersion, | ||
| "newProfileMinVersion", newProfile.MinTLSVersion, | ||
| ) | ||
| cancel() | ||
| }, | ||
| } | ||
| if err := watcher.SetupWithManager(mgr); err != nil { | ||
| setupLog.Error(err, "unable to setup TLS security profile watcher") | ||
| os.Exit(1) | ||
| } | ||
| } | ||
| // Setup Scheme for OpenShift Console if available (verified by InspectCluster) | ||
| if util.IsConsoleAPIFound() { | ||
| registerComponentOrExit(mgr, console.AddToScheme) | ||
|
|
@@ -314,6 +366,11 @@ func main() { | |
| K8sClient: k8sClient, | ||
| LocalUsers: argocdprovisioner.NewLocalUsersInfo(), | ||
| FipsConfigChecker: argoutil.NewLinuxFipsConfigChecker(), | ||
| CentralTLSConfigProfile: argocdprovisioner.TLSConfigProfile{ | ||
| DisableClusterTLSProfile: disableClusterTLSProfile, | ||
| MinVersion: profile.MinTLSVersion, | ||
| Ciphers: profile.Ciphers, | ||
| }, | ||
| }).SetupWithManager(mgr); err != nil { | ||
| setupLog.Error(err, "unable to create controller", "controller", "Argo CD") | ||
| os.Exit(1) | ||
|
|
@@ -362,7 +419,7 @@ func main() { | |
| } | ||
|
|
||
| setupLog.Info("starting manager") | ||
| if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { | ||
| if err := mgr.Start(ctx); err != nil { | ||
|
Member
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. Why this change is required?
Member
Author
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. this change is required for controlling the controller runtime. example reference from https://docs.google.com/document/d/1cMc9E8psHfnoK06ntR8kHSWB8d3rMtmldhnmM4nImjs/edit?tab=t.4cxmujrb3zyn#heading=h.111788dqn5nc In main: get initial profile, create manager (with TLSOpts if you have a TLS metrics server), create context with cancel, call setupTLSProfileWatcher(ctx, mgr, cancel), then mgr.Start(ctx). When the cluster profile changes, OnProfileChange runs; typically you cancel the context so the process exits and restarts with the new profile (e.g. under a pod restart). |
||
| setupLog.Error(err, "problem running manager") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,7 +143,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { | |
|
|
||
| By("expecting redis-server to have desired container process command/arguments") | ||
|
|
||
| expectedString := "--save \"\" --appendonly no --aclfile /app/config/redis-auth/users.acl --tls-port 6379 --port 0 --tls-cert-file /app/config/redis/tls/tls.crt --tls-key-file /app/config/redis/tls/tls.key --tls-auth-clients no" | ||
| expectedString := "--save \"\" --appendonly no --aclfile /app/config/redis-auth/users.acl --tls-protocols TLSv1.2 --tls-ciphers TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256:TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384:TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256:TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 --tls-ciphersuites TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256:TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384:TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256:TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 --tls-port 6379 --port 0 --tls-cert-file /app/config/redis/tls/tls.crt --tls-key-file /app/config/redis/tls/tls.key --tls-auth-clients no" | ||
|
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. Make the TLS expectation profile-aware, not hard-coded. Line 146 locks the test to one cipher/protocol set, so it can fail on clusters with a different APIServer TLS profile even when operator behavior is correct. That also weakens validation of the PR goal (inherit cluster profile dynamically). Build expected Redis TLS flags from the live cluster TLSProfileSpec in-test (or via a shared helper) before asserting deployment args. 🤖 Prompt for AI Agents |
||
|
|
||
| if !fixture.IsUpstreamOperatorTests() { | ||
| // Downstream operator adds these arguments | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.