diff --git a/cmd/cluster-authentication-operator-tests-ext/main.go b/cmd/cluster-authentication-operator-tests-ext/main.go index 98af196201..d5d7145689 100644 --- a/cmd/cluster-authentication-operator-tests-ext/main.go +++ b/cmd/cluster-authentication-operator-tests-ext/main.go @@ -13,6 +13,7 @@ import ( "github.com/openshift/cluster-authentication-operator/pkg/version" _ "github.com/openshift/cluster-authentication-operator/test/e2e" + _ "github.com/openshift/cluster-authentication-operator/test/e2e-component-proxy" _ "github.com/openshift/cluster-authentication-operator/test/e2e-encryption" _ "github.com/openshift/cluster-authentication-operator/test/e2e-encryption-kms" _ "github.com/openshift/cluster-authentication-operator/test/e2e-encryption-perf" @@ -85,6 +86,17 @@ func prepareOperatorTestsRegistry() (*oteextension.Registry, error) { }, }) + // ClusterStability set to Disruptive: component-proxy tests intentionally + // degrade the authentication operator to validate error handling. + extension.AddSuite(oteextension.Suite{ + Name: "openshift/cluster-authentication-operator/component-proxy/disruptive", + Parallelism: 1, + ClusterStability: oteextension.ClusterStabilityDisruptive, + Qualifiers: []string{ + `name.contains("[ComponentProxy]")`, + }, + }) + // The following suite runs tests that are disruptive to the cluster. extension.AddSuite(oteextension.Suite{ Name: "openshift/cluster-authentication-operator/operator/disruptive", diff --git a/pkg/controllers/common/proxy.go b/pkg/controllers/common/proxy.go new file mode 100644 index 0000000000..158be6e437 --- /dev/null +++ b/pkg/controllers/common/proxy.go @@ -0,0 +1,114 @@ +package common + +import ( + "fmt" + "os" + "strings" + + "golang.org/x/net/http/httpproxy" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + + "github.com/openshift/cluster-authentication-operator/pkg/transport" +) + +// ResolvedProxy holds the effective proxy configuration for authentication components. +type ResolvedProxy struct { + *httpproxy.Config + TrustedCAName string +} + +func (p *ResolvedProxy) IsProxyConfigured() bool { + return len(p.HTTPProxy) != 0 || len(p.HTTPSProxy) != 0 +} + +// ProxyFunc returns a function that resolves the proxy URL for a given request URL. +func (p *ResolvedProxy) ProxyFunc() transport.ProxyFunc { + return transport.ProxyFunc(p.Config.ProxyFunc()) +} + +// ResolveProxy reads the component-scoped proxy from the Authentication +// operator CR (when the feature gate is enabled) and returns the effective proxy +// settings. When no component proxy is configured, it falls back to the process +// environment (which reflects the cluster-wide proxy). +func ResolveProxy( + featureGateAccessor featuregates.FeatureGateAccess, + operatorAuthLister operatorv1listers.AuthenticationLister, +) (*ResolvedProxy, error) { + authProxy, err := getComponentProxyConfig(featureGateAccessor, operatorAuthLister) + if err != nil { + return nil, err + } + + if authProxy != nil { + return &ResolvedProxy{ + TrustedCAName: authProxy.TrustedCA.Name, + Config: &httpproxy.Config{ + HTTPProxy: authProxy.HTTPProxy, + HTTPSProxy: authProxy.HTTPSProxy, + NoProxy: mergeNoProxy(authProxy.NoProxy), + }, + }, nil + } + + return &ResolvedProxy{ + Config: httpproxy.FromEnvironment(), + }, nil +} + +// getComponentProxyConfig returns the component-scoped proxy configuration +// from operator.openshift.io/v1 Authentication if the feature gate is enabled. +// Returns (nil, nil) when the gate is disabled or the resource is not found. +func getComponentProxyConfig( + featureGateAccessor featuregates.FeatureGateAccess, + operatorAuthLister operatorv1listers.AuthenticationLister, +) (*operatorv1.AuthenticationProxyConfig, error) { + featureGates, err := featureGateAccessor.CurrentFeatureGates() + if err != nil { + return nil, fmt.Errorf("failed to get current feature gates: %w", err) + } + if !featureGates.Enabled(features.FeatureGateAuthenticationComponentProxy) { + return nil, nil + } + + authOp, err := operatorAuthLister.Get("cluster") + if err != nil { + if errors.IsNotFound(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to get operator.openshift.io/v1 authentication/cluster: %w", err) + } + + proxy := authOp.Spec.Proxy + if proxy.HTTPProxy == "" && proxy.HTTPSProxy == "" { + return nil, nil + } + return &proxy, nil +} + +// staticNoProxyEntries contains cluster-internal addresses that must bypass the +// proxy. Auth components connect to internal services via DNS names covered by +// .svc and .cluster.local. The kubernetes API service IP (KUBERNETES_SERVICE_HOST) +// is included explicitly because Go's in-cluster client connects to it by raw IP, +// which does not match the hostname-based entries above. +var staticNoProxyEntries = func() []string { + entries := []string{".cluster.local", ".svc", "127.0.0.1", "localhost"} + if host := os.Getenv("KUBERNETES_SERVICE_HOST"); host != "" { + entries = append(entries, host) + } + return entries +}() + +// mergeNoProxy combines user-provided noProxy entries with static cluster-internal +// defaults. The result is deduplicated and sorted for deterministic output. +func mergeNoProxy(userNoProxy []string) string { + entries := sets.New[string](staticNoProxyEntries...) + entries.Insert(userNoProxy...) + return strings.Join(sets.List(entries), ",") +} diff --git a/pkg/controllers/common/proxy_test.go b/pkg/controllers/common/proxy_test.go new file mode 100644 index 0000000000..bfca8a3455 --- /dev/null +++ b/pkg/controllers/common/proxy_test.go @@ -0,0 +1,362 @@ +package common + +import ( + "errors" + "net/url" + "testing" + + "golang.org/x/net/http/httpproxy" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/tools/cache" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + + "github.com/openshift/cluster-authentication-operator/pkg/internal/transporttest" +) + +var ( + enabledGate = featuregates.NewHardcodedFeatureGateAccess( + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + nil, + ) + disabledGate = featuregates.NewHardcodedFeatureGateAccess( + nil, + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + ) +) + +func TestResolveProxy(t *testing.T) { + errorGate := featuregates.NewHardcodedFeatureGateAccessForTesting(nil, nil, make(chan struct{}), errors.New("not yet observed")) + + proxySet := &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://proxy:3128", + TrustedCA: operatorv1.AuthenticationConfigMapReference{Name: "my-ca-bundle"}, + }, + }, + } + proxyEmpty := &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + + proxyConfig := func(http, https, noProxy string) *httpproxy.Config { + return &httpproxy.Config{HTTPProxy: http, HTTPSProxy: https, NoProxy: noProxy} + } + + tests := []struct { + name string + gate featuregates.FeatureGateAccess + lister operatorv1listers.AuthenticationLister + envHTTP string + envHTTPS string + envNoProxy string + want *ResolvedProxy + wantErr string + }{ + { + name: "gate disabled falls back to env", + gate: disabledGate, + lister: newOperatorAuthLister(proxySet), + envHTTP: "http://cluster:3128", + want: &ResolvedProxy{Config: proxyConfig("http://cluster:3128", "", "")}, + }, + { + name: "gate error propagates", + gate: errorGate, + lister: newOperatorAuthLister(proxySet), + wantErr: "failed to get current feature gates: not yet observed", + }, + { + name: "lister error propagates", + gate: enabledGate, + lister: newErrorAuthLister(errors.New("connection refused")), + wantErr: "failed to get operator.openshift.io/v1 authentication/cluster: connection refused", + }, + { + name: "CR not found falls back to env", + gate: enabledGate, + lister: newOperatorAuthLister(nil), + envHTTPS: "http://cluster:3129", + want: &ResolvedProxy{Config: proxyConfig("", "http://cluster:3129", "")}, + }, + { + name: "CR exists but proxy is zero-value falls back to env", + gate: enabledGate, + lister: newOperatorAuthLister(proxyEmpty), + want: &ResolvedProxy{Config: proxyConfig("", "", "")}, + }, + { + name: "component proxy set returns active proxy with trustedCA", + gate: enabledGate, + lister: newOperatorAuthLister(proxySet), + envHTTP: "http://should-be-ignored:3128", + want: &ResolvedProxy{ + + TrustedCAName: "my-ca-bundle", + Config: proxyConfig("", "http://proxy:3128", ".cluster.local,.svc,127.0.0.1,localhost"), + }, + }, + { + name: "component proxy with user noProxy merges with defaults", + gate: enabledGate, + lister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPProxy: "http://component:3128", + NoProxy: []string{"idp.example.com", ".corp.example.com"}, + }, + }, + }), + want: &ResolvedProxy{ + + Config: proxyConfig("http://component:3128", "", ".cluster.local,.corp.example.com,.svc,127.0.0.1,idp.example.com,localhost"), + }, + }, + { + name: "component proxy with user noProxy duplicating defaults deduplicates", + gate: enabledGate, + lister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPProxy: "http://component:3128", + NoProxy: []string{".svc", "idp.example.com", "127.0.0.1"}, + }, + }, + }), + want: &ResolvedProxy{ + + Config: proxyConfig("http://component:3128", "", ".cluster.local,.svc,127.0.0.1,idp.example.com,localhost"), + }, + }, + { + name: "component proxy with only httpsProxy overrides env", + gate: enabledGate, + lister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://component:3129", + }, + }, + }), + envHTTP: "http://cluster:3128", + envHTTPS: "http://cluster:3129", + want: &ResolvedProxy{ + + Config: proxyConfig("", "http://component:3129", ".cluster.local,.svc,127.0.0.1,localhost"), + }, + }, + { + name: "neither configured returns empty", + gate: disabledGate, + lister: newOperatorAuthLister(nil), + want: &ResolvedProxy{Config: proxyConfig("", "", "")}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("HTTP_PROXY", tt.envHTTP) + t.Setenv("HTTPS_PROXY", tt.envHTTPS) + t.Setenv("NO_PROXY", tt.envNoProxy) + + got, err := ResolveProxy(tt.gate, tt.lister) + + var errMsg string + if err != nil { + errMsg = err.Error() + } + if errMsg != tt.wantErr { + t.Fatalf("error = %q, want %q", errMsg, tt.wantErr) + } + if tt.wantErr != "" { + return + } + + if got.HTTPProxy != tt.want.HTTPProxy { + t.Errorf("HTTPProxy = %q, want %q", got.HTTPProxy, tt.want.HTTPProxy) + } + if got.HTTPSProxy != tt.want.HTTPSProxy { + t.Errorf("HTTPSProxy = %q, want %q", got.HTTPSProxy, tt.want.HTTPSProxy) + } + if got.NoProxy != tt.want.NoProxy { + t.Errorf("NoProxy = %q, want %q", got.NoProxy, tt.want.NoProxy) + } + if got.TrustedCAName != tt.want.TrustedCAName { + t.Errorf("TrustedCAName = %q, want %q", got.TrustedCAName, tt.want.TrustedCAName) + } + }) + } +} + +func TestResolvedProxy_IsProxyConfigured(t *testing.T) { + tests := []struct { + name string + proxy *ResolvedProxy + want bool + }{ + { + name: "both empty", + proxy: &ResolvedProxy{Config: &httpproxy.Config{}}, + want: false, + }, + { + name: "http proxy set", + proxy: &ResolvedProxy{Config: &httpproxy.Config{HTTPProxy: "http://proxy:3128"}}, + want: true, + }, + { + name: "https proxy set", + proxy: &ResolvedProxy{Config: &httpproxy.Config{HTTPSProxy: "http://proxy:3128"}}, + want: true, + }, + { + name: "both set", + proxy: &ResolvedProxy{Config: &httpproxy.Config{HTTPProxy: "http://proxy:3128", HTTPSProxy: "http://proxy:3129"}}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.proxy.IsProxyConfigured(); got != tt.want { + t.Errorf("IsProxyConfigured() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestResolvedProxy_ProxyFunc(t *testing.T) { + httpsURL := transporttest.MustParseURL(t, "https://idp.example.com/.well-known/openid-configuration") + httpURL := transporttest.MustParseURL(t, "http://idp.example.com/callback") + + tests := []struct { + name string + featureGate featuregates.FeatureGateAccess + authLister operatorv1listers.AuthenticationLister + reqURL *url.URL + wantProxyURL string + }{ + { + name: "feature gate disabled returns nil proxy (no env)", + featureGate: disabledGate, + authLister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://should-not-be-used:3128", + }, + }, + }), + reqURL: httpsURL, + }, + { + name: "feature gate enabled but no proxy configured returns nil proxy (no env)", + featureGate: enabledGate, + authLister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + }), + reqURL: httpsURL, + }, + { + name: "feature gate enabled with httpsProxy configured returns proxy for https request", + featureGate: enabledGate, + authLister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://proxy.corp.example.com:3128", + }, + }, + }), + reqURL: httpsURL, + wantProxyURL: "http://proxy.corp.example.com:3128", + }, + { + name: "feature gate enabled with httpProxy configured returns proxy for http request", + featureGate: enabledGate, + authLister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPProxy: "http://proxy.corp.example.com:3128", + }, + }, + }), + reqURL: httpURL, + wantProxyURL: "http://proxy.corp.example.com:3128", + }, + { + name: "noProxy match returns nil proxy", + featureGate: enabledGate, + authLister: newOperatorAuthLister(&operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://proxy.corp.example.com:3128", + NoProxy: []string{httpURL.Host}, + }, + }, + }), + reqURL: httpsURL, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("HTTP_PROXY", "") + t.Setenv("HTTPS_PROXY", "") + t.Setenv("NO_PROXY", "") + + proxy, err := ResolveProxy(tt.featureGate, tt.authLister) + if err != nil { + t.Fatalf("unexpected resolve error: %v", err) + } + proxyURL, err := proxy.ProxyFunc()(tt.reqURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var gotProxyURL string + if proxyURL != nil { + gotProxyURL = proxyURL.String() + } + if gotProxyURL != tt.wantProxyURL { + t.Errorf("proxy URL = %q, want %q", gotProxyURL, tt.wantProxyURL) + } + }) + } +} + +func newOperatorAuthLister(auth *operatorv1.Authentication) operatorv1listers.AuthenticationLister { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if auth != nil { + _ = indexer.Add(auth) + } + return operatorv1listers.NewAuthenticationLister(indexer) +} + +type errorAuthLister struct { + err error +} + +func newErrorAuthLister(err error) operatorv1listers.AuthenticationLister { + return &errorAuthLister{err} +} + +func (l *errorAuthLister) List(_ labels.Selector) ([]*operatorv1.Authentication, error) { + return nil, l.err +} + +func (l *errorAuthLister) Get(_ string) (*operatorv1.Authentication, error) { + return nil, l.err +} diff --git a/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go b/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go index 2c8260268f..4fe343300b 100644 --- a/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go +++ b/pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go @@ -5,9 +5,11 @@ import ( "k8s.io/client-go/tools/cache" configinformers "github.com/openshift/client-go/config/informers/externalversions" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/configobserver" "github.com/openshift/library-go/pkg/operator/configobserver/apiserver" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" configobserveroauth "github.com/openshift/library-go/pkg/operator/configobserver/oauth" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" @@ -27,6 +29,8 @@ func NewConfigObserver( resourceSyncer resourcesynccontroller.ResourceSyncer, enabledClusterCapabilities sets.String, eventRecorder events.Recorder, + operatorAuthInformer operatorv1informers.AuthenticationInformer, + featureGateAccessor featuregates.FeatureGateAccess, ) factory.Controller { interestingNamespaces := []string{ "openshift-authentication", @@ -73,6 +77,7 @@ func NewConfigObserver( oauth.ObserveTemplates, oauth.ObserveTokenConfig, oauth.ObserveAudit, + oauth.ObserveComponentProxyTrustedCA, configobserveroauth.ObserveAccessTokenInactivityTimeout, routersecret.ObserveRouterSecret, } { @@ -80,6 +85,9 @@ func NewConfigObserver( configobserver.WithPrefix(o, configobservation.OAuthServerConfigPrefix)) } + preRunCacheSynced = append(preRunCacheSynced, operatorAuthInformer.Informer().HasSynced) + informers = append(informers, operatorAuthInformer.Informer()) + listers := configobservation.Listers{ ConfigMapLister: kubeInformersForNamespaces.ConfigMapLister(), SecretsLister: kubeInformersForNamespaces.SecretLister(), @@ -91,6 +99,9 @@ func NewConfigObserver( OAuthLister_: configInformer.Config().V1().OAuths().Lister(), ResourceSync: resourceSyncer, PreRunCachesSynced: preRunCacheSynced, + + OperatorAuthLister: operatorAuthInformer.Lister(), + FeatureGateAccessor: featureGateAccessor, } // Check if the Console capability is enabled on the cluster and sync and add its informer, lister, and config observer diff --git a/pkg/controllers/configobservation/interfaces.go b/pkg/controllers/configobservation/interfaces.go index 781e96f802..ef6d918b37 100644 --- a/pkg/controllers/configobservation/interfaces.go +++ b/pkg/controllers/configobservation/interfaces.go @@ -5,7 +5,9 @@ import ( "k8s.io/client-go/tools/cache" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" ) @@ -26,6 +28,9 @@ type Listers struct { OAuthLister_ configlistersv1.OAuthLister IngressLister configlistersv1.IngressLister + OperatorAuthLister operatorv1listers.AuthenticationLister + FeatureGateAccessor featuregates.FeatureGateAccess + ResourceSync resourcesynccontroller.ResourceSyncer PreRunCachesSynced []cache.InformerSynced } diff --git a/pkg/controllers/configobservation/oauth/idp_conversions.go b/pkg/controllers/configobservation/oauth/idp_conversions.go index 375a551c07..48d9ad594b 100644 --- a/pkg/controllers/configobservation/oauth/idp_conversions.go +++ b/pkg/controllers/configobservation/oauth/idp_conversions.go @@ -19,9 +19,12 @@ import ( osinv1 "github.com/openshift/api/osin/v1" "github.com/openshift/cluster-authentication-operator/pkg/operator/datasync" - "github.com/openshift/cluster-authentication-operator/pkg/transport" ) +// transportForCABuilderFunc builds an http.RoundTripper for a given CA ConfigMap +// reference. The cmLister and any proxy settings are captured in the closure. +type transportForCABuilderFunc func(caConfigMapName, key string) (http.RoundTripper, error) + // field names are used to uniquely identify a secret or config map reference // within a given identity provider. thus the same IDP cannot use the same field // more than once. ex: if an idp had two CA fields, it would need to use something @@ -54,16 +57,16 @@ type idpData struct { } func convertIdentityProviders( - cmLister corelistersv1.ConfigMapLister, secretsLister corelistersv1.SecretLister, identityProviders []configv1.IdentityProvider, + buildTransport transportForCABuilderFunc, ) ([]interface{}, *datasync.ConfigSyncData, []error) { converted := []osinv1.IdentityProvider{} syncData := datasync.NewConfigSyncData() errs := []error{} for i, idp := range defaultIDPMappingMethods(identityProviders) { - data, err := convertProviderConfigToIDPData(cmLister, secretsLister, &idp.IdentityProviderConfig, syncData, i) + data, err := convertProviderConfigToIDPData(secretsLister, &idp.IdentityProviderConfig, syncData, i, buildTransport) if err != nil { errs = append(errs, fmt.Errorf("failed to apply IDP %s config: %v", idp.Name, err)) continue @@ -111,11 +114,11 @@ func defaultIDPMappingMethods(identityProviders []configv1.IdentityProvider) []c } func convertProviderConfigToIDPData( - cmLister corelistersv1.ConfigMapLister, secretsLister corelistersv1.SecretLister, providerConfig *configv1.IdentityProviderConfig, syncData *datasync.ConfigSyncData, i int, + buildTransport transportForCABuilderFunc, ) (*idpData, error) { const missingProviderFmt string = "type %s was specified, but its configuration is missing" @@ -241,7 +244,7 @@ func convertProviderConfigToIDPData( return nil, fmt.Errorf(missingProviderFmt, providerConfig.Type) } - urls, err := discoverOpenIDURLs(cmLister, openIDConfig.Issuer, corev1.ServiceAccountRootCAKey, openIDConfig.CA) + urls, err := discoverOpenIDURLs(openIDConfig.Issuer, corev1.ServiceAccountRootCAKey, openIDConfig.CA, buildTransport) if err != nil { return nil, err } @@ -272,12 +275,12 @@ func convertProviderConfigToIDPData( // challenge-redirecting IdPs to be configured with OIDC so it is safe // to allow challenge-issuing flow if it's available on the OIDC side challengeFlowsAllowed, err := checkOIDCPasswordGrantFlow( - cmLister, secretsLister, urls.Token, openIDConfig.ClientID, openIDConfig.CA, openIDConfig.ClientSecret, + buildTransport, ) if err != nil { return nil, fmt.Errorf("error attempting password grant flow: %v", err) @@ -312,7 +315,7 @@ func convertProviderConfigToIDPData( // discoverOpenIDURLs retrieves basic information about an OIDC server with hostname // given by the `issuer` argument -func discoverOpenIDURLs(cmLister corelistersv1.ConfigMapLister, issuer, key string, ca configv1.ConfigMapNameReference) (*osinv1.OpenIDURLs, error) { +func discoverOpenIDURLs(issuer, key string, ca configv1.ConfigMapNameReference, buildTransport transportForCABuilderFunc) (*osinv1.OpenIDURLs, error) { issuer = strings.TrimRight(issuer, "/") // TODO make impossible via validation and remove wellKnown := issuer + "/.well-known/openid-configuration" @@ -321,7 +324,7 @@ func discoverOpenIDURLs(cmLister corelistersv1.ConfigMapLister, issuer, key stri return nil, err } - rt, err := transport.TransportForCARef(cmLister, ca.Name, key) + rt, err := buildTransport(ca.Name, key) if err != nil { return nil, err } @@ -371,11 +374,11 @@ func discoverOpenIDURLs(cmLister corelistersv1.ConfigMapLister, issuer, key stri } func checkOIDCPasswordGrantFlow( - cmLister corelistersv1.ConfigMapLister, secretsLister corelistersv1.SecretLister, tokenURL, clientID string, caRererence configv1.ConfigMapNameReference, clientSecretReference configv1.SecretNameReference, + buildTransport transportForCABuilderFunc, ) (bool, error) { secret, err := secretsLister.Secrets("openshift-config").Get(clientSecretReference.Name) if err != nil { @@ -394,7 +397,7 @@ func checkOIDCPasswordGrantFlow( return false, fmt.Errorf("the referenced secret does not contain a value for the 'clientSecret' key") } - transport, err := transport.TransportForCARef(cmLister, caRererence.Name, corev1.ServiceAccountRootCAKey) + rt, err := buildTransport(caRererence.Name, corev1.ServiceAccountRootCAKey) if err != nil { return false, fmt.Errorf("couldn't get a transport for the referenced CA: %v", err) } @@ -417,7 +420,7 @@ func checkOIDCPasswordGrantFlow( // explicitly set Accept to 'application/json' as that's the expected deserializable output req.Header.Set("Accept", "application/json") - client := &http.Client{Transport: transport} + client := &http.Client{Transport: rt} resp, err := client.Do(req) if err != nil { return false, err diff --git a/pkg/controllers/configobservation/oauth/idp_conversions_test.go b/pkg/controllers/configobservation/oauth/idp_conversions_test.go index 487541029c..5ba7fa2cbf 100644 --- a/pkg/controllers/configobservation/oauth/idp_conversions_test.go +++ b/pkg/controllers/configobservation/oauth/idp_conversions_test.go @@ -23,6 +23,7 @@ import ( configv1 "github.com/openshift/api/config/v1" osinv1 "github.com/openshift/api/osin/v1" "github.com/openshift/cluster-authentication-operator/pkg/operator/datasync" + "github.com/openshift/cluster-authentication-operator/pkg/transport" "github.com/openshift/library-go/pkg/crypto" ) @@ -210,7 +211,7 @@ func Test_convertProviderConfigToIDPData(t *testing.T) { tt.providerConfig.OpenID.Issuer = server.URL } - got, err := convertProviderConfigToIDPData(cmLister, secretLister, tt.providerConfig, syncData, 0) + got, err := convertProviderConfigToIDPData(secretLister, tt.providerConfig, syncData, 0, newTestTransportBuilder(cmLister)) if (err != nil) != tt.wantErr { t.Errorf("convertProviderConfigToIDPData() error = %v, wantErr %v", err, tt.wantErr) return @@ -242,6 +243,16 @@ func Test_convertProviderConfigToIDPData(t *testing.T) { } } +func newTestTransportBuilder(cmLister corelistersv1.ConfigMapLister) transportForCABuilderFunc { + return func(caConfigMapName, caConfigMapKey string) (http.RoundTripper, error) { + var caRefs []transport.CAReference + if len(caConfigMapName) > 0 { + caRefs = append(caRefs, transport.CAReference{ConfigMapName: caConfigMapName, ConfigMapKey: caConfigMapKey}) + } + return transport.TransportForCARef(cmLister, caRefs, "", "", "") + } +} + func newTestHTTPSServer(certPEM, keyPEM []byte, content string) (*httptest.Server, error) { // use a byte slice reference to replace with a valid content with replaced // server URLs once the server is started @@ -304,16 +315,17 @@ func TestCheckOIDCPasswordGrantFlowCaching(t *testing.T) { require.NoError(t, indexer.Add(secret)) cmLister := corelistersv1.NewConfigMapLister(indexer) secretLister := corelistersv1.NewSecretLister(indexer) + buildTransport := newTestTransportBuilder(cmLister) t.Run("5xx responses are not cached", func(t *testing.T) { shouldError = true result, err := checkOIDCPasswordGrantFlow( - cmLister, secretLister, server.URL+"/token", "test-client", configv1.ConfigMapNameReference{Name: ""}, configv1.SecretNameReference{Name: "test-secret"}, + buildTransport, ) require.NoError(t, err) @@ -325,12 +337,12 @@ func TestCheckOIDCPasswordGrantFlowCaching(t *testing.T) { responseContent = `{"error": "invalid_grant"}` shouldError = false result, err := checkOIDCPasswordGrantFlow( - cmLister, secretLister, server.URL+"/token", "test-client", configv1.ConfigMapNameReference{Name: ""}, configv1.SecretNameReference{Name: "test-secret"}, + buildTransport, ) require.NoError(t, err) @@ -344,12 +356,12 @@ func TestCheckOIDCPasswordGrantFlowCaching(t *testing.T) { shouldError = true res1, err := checkOIDCPasswordGrantFlow( - cmLister, secretLister, server.URL+"/token", "test-client", configv1.ConfigMapNameReference{Name: ""}, configv1.SecretNameReference{Name: "test-secret"}, + buildTransport, ) require.NoError(t, err) require.False(t, res1) @@ -359,12 +371,12 @@ func TestCheckOIDCPasswordGrantFlowCaching(t *testing.T) { responseContent = `{"error": "invalid_grant"}` shouldError = false res2, err := checkOIDCPasswordGrantFlow( - cmLister, secretLister, server.URL+"/token", "test-client", configv1.ConfigMapNameReference{Name: ""}, configv1.SecretNameReference{Name: "test-secret"}, + buildTransport, ) require.NoError(t, err) require.True(t, res2) diff --git a/pkg/controllers/configobservation/oauth/observe_idps.go b/pkg/controllers/configobservation/oauth/observe_idps.go index 35bac51463..eaaaf374ff 100644 --- a/pkg/controllers/configobservation/oauth/observe_idps.go +++ b/pkg/controllers/configobservation/oauth/observe_idps.go @@ -1,6 +1,8 @@ package oauth import ( + "net/http" + "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/api/equality" @@ -10,8 +12,10 @@ import ( "github.com/openshift/library-go/pkg/operator/configobserver" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation" "github.com/openshift/cluster-authentication-operator/pkg/operator/datasync" + "github.com/openshift/cluster-authentication-operator/pkg/transport" ) var identityProvidersMounts = []string{"volumesToMount", "identityProviders"} @@ -50,9 +54,14 @@ func ObserveIdentityProviders(genericlisters configobserver.Listers, recorder ev return existingConfig, append(errs, err) } + buildTransport, err := buildIDPTransport(listers) + if err != nil { + return existingConfig, append(errs, err) + } + // convert identity providers from config to oauth-configuration API and // extract the CMs and Secrets that need to be synchronized to the target NS - convertedObservedIdentityProviders, observedSyncData, idpErrs := convertIdentityProviders(listers.ConfigMapLister, listers.SecretsLister, oauthConfig.Spec.IdentityProviders) + convertedObservedIdentityProviders, observedSyncData, idpErrs := convertIdentityProviders(listers.SecretsLister, oauthConfig.Spec.IdentityProviders, buildTransport) if len(idpErrs) > 0 { return existingConfig, append(errs, idpErrs...) } @@ -100,3 +109,22 @@ func GetIDPConfigSyncData(observedConfig map[string]interface{}) (*datasync.Conf return datasync.NewConfigSyncDataFromJSON(currentSyncDataBytes) } + +// buildIDPTransport returns a transport builder that uses the resolved proxy settings. +func buildIDPTransport(listers configobservation.Listers) (transportForCABuilderFunc, error) { + proxy, err := common.ResolveProxy(listers.FeatureGateAccessor, listers.OperatorAuthLister) + if err != nil { + return nil, err + } + + return func(caConfigMapName, caConfigMapKey string) (http.RoundTripper, error) { + var caRefs []transport.CAReference + if len(caConfigMapName) > 0 { + caRefs = append(caRefs, transport.CAReference{ConfigMapName: caConfigMapName, ConfigMapKey: caConfigMapKey}) + } + if len(proxy.TrustedCAName) > 0 { + caRefs = append(caRefs, transport.CAReference{ConfigMapName: proxy.TrustedCAName, ConfigMapKey: "ca-bundle.crt"}) + } + return transport.TransportForCARef(listers.ConfigMapLister, caRefs, proxy.HTTPProxy, proxy.HTTPSProxy, proxy.NoProxy) + }, nil +} diff --git a/pkg/controllers/configobservation/oauth/observe_idps_test.go b/pkg/controllers/configobservation/oauth/observe_idps_test.go index 152925aa75..f6243ac5ec 100644 --- a/pkg/controllers/configobservation/oauth/observe_idps_test.go +++ b/pkg/controllers/configobservation/oauth/observe_idps_test.go @@ -2,10 +2,12 @@ package oauth import ( "fmt" + "net/http" "testing" "time" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -15,11 +17,16 @@ import ( clocktesting "k8s.io/utils/clock/testing" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation" + "github.com/openshift/cluster-authentication-operator/pkg/internal/transporttest" ) type mockResourceSyncer struct { @@ -195,11 +202,17 @@ func TestObserveIdentityProviders(t *testing.T) { } syncerData := tt.previousSyncerData + operatorAuthIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) listers := configobservation.Listers{ - ConfigMapLister: corelistersv1.NewConfigMapLister(indexer), - SecretsLister: corelistersv1.NewSecretLister(indexer), - OAuthLister_: configlistersv1.NewOAuthLister(indexer), - ResourceSync: &mockResourceSyncer{t: t, synced: syncerData}, + ConfigMapLister: corelistersv1.NewConfigMapLister(indexer), + SecretsLister: corelistersv1.NewSecretLister(indexer), + OAuthLister_: configlistersv1.NewOAuthLister(indexer), + ResourceSync: &mockResourceSyncer{t: t, synced: syncerData}, + OperatorAuthLister: operatorv1listers.NewAuthenticationLister(operatorAuthIndexer), + FeatureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + nil, + ), } eventsRecorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) @@ -223,6 +236,57 @@ func TestObserveIdentityProviders(t *testing.T) { } } +// TestBuildIDPTransport verifies that when the feature gate is enabled and the +// Authentication CR has a component proxy with a trusted CA, the transport +// builder wires the proxy function and loads the CA into the transport. +func TestBuildIDPTransport(t *testing.T) { + _, caPEM := transporttest.MakeSelfSignedCA(t) + proxyCA := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "my-proxy-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(caPEM)}, + } + authCR := &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://proxy:3128", + TrustedCA: operatorv1.AuthenticationConfigMapReference{Name: "my-proxy-ca"}, + }, + }, + } + + cmIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, cmIndexer.Add(proxyCA)) + + authIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, authIndexer.Add(authCR)) + + listers := configobservation.Listers{ + ConfigMapLister: corelistersv1.NewConfigMapLister(cmIndexer), + OperatorAuthLister: operatorv1listers.NewAuthenticationLister(authIndexer), + FeatureGateAccessor: featuregates.NewHardcodedFeatureGateAccess( + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + nil, + ), + } + + buildTransport, err := buildIDPTransport(listers) + require.NoError(t, err) + + // Call without an IdP CA — should still succeed because the proxy CA is loaded. + rt, err := buildTransport("", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + require.NotNil(t, tr.Proxy, "transport should have a proxy function set") + + // Verify proxy is wired: an HTTPS request should be proxied. + proxyURL, err := tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "https://idp.example.com")}) + require.NoError(t, err) + require.NotNil(t, proxyURL) + require.Equal(t, "http://proxy:3128", proxyURL.String()) +} + func eventsReasonMessage(e []*corev1.Event) []string { reasonMessages := make([]string, 0, len(e)) for _, ev := range e { diff --git a/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca.go b/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca.go new file mode 100644 index 0000000000..2cd76e9a26 --- /dev/null +++ b/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca.go @@ -0,0 +1,54 @@ +package oauth + +import ( + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/events" + + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation" +) + +var proxyTrustedCAPath = []string{"oauthConfig", "proxyTrustedCA"} + +const proxyTrustedCAFile = "/var/config/system/configmaps/v4-0-config-system-auth-proxy-ca/ca-bundle.crt" + +// ObserveComponentProxyTrustedCA sets oauthConfig.proxyTrustedCA to the on-disk path of +// the component-scoped proxy trusted CA bundle when the operator-level +// Authentication CR specifies a proxy trustedCA configmap and the corresponding +// feature gate is enabled. The path points to the configmap volume mount +// managed by the deployment controller (v4-0-config-system-auth-proxy-ca). +func ObserveComponentProxyTrustedCA(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, _ []error) { + defer func() { + ret = configobserver.Pruned(ret, proxyTrustedCAPath) + }() + + listers := genericListers.(configobservation.Listers) + + existingValue, _, err := unstructured.NestedFieldCopy(existingConfig, proxyTrustedCAPath...) + if err != nil { + return existingConfig, []error{err} + } + + proxy, err := common.ResolveProxy(listers.FeatureGateAccessor, listers.OperatorAuthLister) + if err != nil { + return existingConfig, []error{err} + } + + observedConfig := map[string]interface{}{} + var observedValue interface{} + if len(proxy.TrustedCAName) > 0 { + observedValue = proxyTrustedCAFile + if err := unstructured.SetNestedField(observedConfig, proxyTrustedCAFile, proxyTrustedCAPath...); err != nil { + return existingConfig, []error{err} + } + } + + if !equality.Semantic.DeepEqual(existingValue, observedValue) { + recorder.Eventf("ObserveComponentProxyTrustedCA", "proxyTrustedCA changed from %v to %v", existingValue, observedValue) + } + + return observedConfig, nil +} diff --git a/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca_test.go b/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca_test.go new file mode 100644 index 0000000000..d9fa58d857 --- /dev/null +++ b/pkg/controllers/configobservation/oauth/observe_proxy_trusted_ca_test.go @@ -0,0 +1,217 @@ +package oauth_test + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/tools/cache" + clocktesting "k8s.io/utils/clock/testing" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + "github.com/openshift/library-go/pkg/operator/events" + + "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation/oauth" +) + +func TestObserveComponentProxyTrustedCA(t *testing.T) { + enabledGate := featuregates.NewHardcodedFeatureGateAccess( + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + nil, + ) + disabledGate := featuregates.NewHardcodedFeatureGateAccess( + nil, + []configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy}, + ) + + expectedConfig := map[string]interface{}{ + "oauthConfig": map[string]interface{}{ + "proxyTrustedCA": "/var/config/system/configmaps/v4-0-config-system-auth-proxy-ca/ca-bundle.crt", + }, + } + + tests := []struct { + name string + gate featuregates.FeatureGateAccess + auth *operatorv1.Authentication + lister operatorv1listers.AuthenticationLister + existingConfig map[string]interface{} + expected map[string]interface{} + expectErrorContains string + expectEvent bool + }{ + { + name: "feature gate disabled returns empty config", + gate: disabledGate, + auth: nil, + expected: map[string]interface{}{}, + }, + { + name: "no operator authentication resource returns empty config", + gate: enabledGate, + auth: nil, + expected: map[string]interface{}{}, + }, + { + name: "proxy configured without trustedCA returns empty config", + gate: enabledGate, + auth: &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "https://proxy:3128", + }, + }, + }, + expected: map[string]interface{}{}, + }, + { + name: "proxy configured with trustedCA sets path", + gate: enabledGate, + auth: &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "https://proxy:3128", + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: "my-proxy-ca", + }, + }, + }, + }, + expected: expectedConfig, + expectEvent: true, + }, + { + name: "trustedCA removed clears path", + gate: enabledGate, + auth: &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "https://proxy:3128", + }, + }, + }, + existingConfig: expectedConfig, + expected: map[string]interface{}{}, + expectEvent: true, + }, + { + name: "no change emits no event", + gate: enabledGate, + auth: &operatorv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: operatorv1.AuthenticationSpec{ + Proxy: operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "https://proxy:3128", + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: "my-proxy-ca", + }, + }, + }, + }, + existingConfig: expectedConfig, + expected: expectedConfig, + }, + { + name: "feature gate error propagates error and returns existing config", + gate: featuregates.NewHardcodedFeatureGateAccessForTesting( + nil, nil, make(chan struct{}), errors.New("not yet observed"), + ), + auth: nil, + existingConfig: expectedConfig, + expected: expectedConfig, + expectErrorContains: "failed to get current feature gates", + }, + { + name: "lister error propagates error and returns existing config", + gate: enabledGate, + lister: newErrorAuthLister(errors.New("connection refused")), + existingConfig: expectedConfig, + expected: expectedConfig, + expectErrorContains: "failed to get operator.openshift.io/v1 authentication/cluster", + }, + { + name: "malformed existingConfig with non-map oauthConfig returns error", + gate: enabledGate, + auth: nil, + existingConfig: map[string]interface{}{ + "oauthConfig": "not-a-map", + }, + expected: map[string]interface{}{"oauthConfig": "not-a-map"}, + expectErrorContains: "accessor error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authLister := tt.lister + if authLister == nil { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if tt.auth != nil { + require.NoError(t, indexer.Add(tt.auth)) + } + authLister = operatorv1listers.NewAuthenticationLister(indexer) + } + + listers := configobservation.Listers{ + OperatorAuthLister: authLister, + FeatureGateAccessor: tt.gate, + } + + existing := tt.existingConfig + if existing == nil { + existing = map[string]interface{}{} + } + + recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + observed, errs := oauth.ObserveComponentProxyTrustedCA(listers, recorder, existing) + + if tt.expectErrorContains != "" { + require.NotEmpty(t, errs) + require.ErrorContains(t, errs[0], tt.expectErrorContains) + } else { + require.Empty(t, errs) + } + + observedValue, _, _ := unstructured.NestedString(observed, "oauthConfig", "proxyTrustedCA") + expectedValue, _, _ := unstructured.NestedString(tt.expected, "oauthConfig", "proxyTrustedCA") + require.Equal(t, expectedValue, observedValue) + + recordedEvents := recorder.Events() + if tt.expectEvent { + require.Len(t, recordedEvents, 1) + require.Equal(t, "ObserveComponentProxyTrustedCA", recordedEvents[0].Reason) + } else { + require.Empty(t, recordedEvents) + } + }) + } +} + +type errorAuthLister struct { + err error +} + +func newErrorAuthLister(err error) operatorv1listers.AuthenticationLister { + return &errorAuthLister{err} +} + +func (l *errorAuthLister) List(_ labels.Selector) ([]*operatorv1.Authentication, error) { + return nil, l.err +} + +func (l *errorAuthLister) Get(_ string) (*operatorv1.Authentication, error) { + return nil, l.err +} diff --git a/pkg/controllers/customroute/custom_route_conditions.go b/pkg/controllers/customroute/custom_route_conditions.go index 10b0d83b05..7460884338 100644 --- a/pkg/controllers/customroute/custom_route_conditions.go +++ b/pkg/controllers/customroute/custom_route_conditions.go @@ -7,16 +7,20 @@ import ( "encoding/pem" "fmt" "net/http" + "net/url" "reflect" "time" - configv1 "github.com/openshift/api/config/v1" - routev1 "github.com/openshift/api/route/v1" - "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" - "github.com/openshift/library-go/pkg/route/routeapihelpers" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/client-go/applyconfigurations/meta/v1" corev1listers "k8s.io/client-go/listers/core/v1" + + configv1 "github.com/openshift/api/config/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/openshift/library-go/pkg/route/routeapihelpers" + + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" + "github.com/openshift/cluster-authentication-operator/pkg/transport" ) var ( @@ -115,8 +119,8 @@ func degradeIfTimeElapsed(conditions []metav1.Condition, condition *v1.Condition } } -func checkRouteAvailablity(secretLister corev1listers.SecretLister, ingressConfig *configv1.Ingress, route *routev1.Route) []*v1.ConditionApplyConfiguration { - if err := routeAvailablity(secretLister, route.Spec.Host, ingressConfig); err != nil { +func checkRouteAvailability(secretLister corev1listers.SecretLister, configMapLister corev1listers.ConfigMapLister, ingressConfig *configv1.Ingress, route *routev1.Route, proxy *common.ResolvedProxy) []*v1.ConditionApplyConfiguration { + if err := routeAvailability(secretLister, configMapLister, route.Spec.Host, ingressConfig, proxy); err != nil { now := metav1.Now() reason := "ErrorReachingOutToService" message := fmt.Sprintf("unexpected error at %s: %v", route.Spec.Host, err) @@ -137,13 +141,13 @@ func checkRouteAvailablity(secretLister corev1listers.SecretLister, ingressConfi return nil } -func routeAvailablity(secretLister corev1listers.SecretLister, host string, ingress *configv1.Ingress) error { - url := "https://" + host + "/healthz" +func routeAvailability(secretLister corev1listers.SecretLister, configMapLister corev1listers.ConfigMapLister, host string, ingress *configv1.Ingress, proxy *common.ResolvedProxy) error { + healthzURL := "https://" + host + "/healthz" reqCtx, cancel := context.WithTimeout(context.TODO(), 10*time.Second) // avoid waiting forever defer cancel() - req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, healthzURL, nil) if err != nil { return err } @@ -158,10 +162,18 @@ func routeAvailablity(secretLister corev1listers.SecretLister, host string, ingr return err } + if len(proxy.TrustedCAName) > 0 { + caData, err := transport.LoadCAData(configMapLister, proxy.TrustedCAName, "ca-bundle.crt") + if err != nil { + return fmt.Errorf("failed to load proxy CA: %w", err) + } + rootCAs.AppendCertsFromPEM(caData) + } + httpClient := http.Client{ Timeout: 5 * time.Second, Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, + Proxy: func(req *http.Request) (*url.URL, error) { return proxy.ProxyFunc()(req.URL) }, TLSClientConfig: &tls.Config{ RootCAs: rootCAs, }, @@ -176,11 +188,11 @@ func routeAvailablity(secretLister corev1listers.SecretLister, host string, ingr defer resp.Body.Close() if resp.StatusCode != 200 { - return fmt.Errorf("request against %s returned %d instead of 200", url, resp.StatusCode) + return fmt.Errorf("request against %s returned %d instead of 200", healthzURL, resp.StatusCode) } if resp.TLS == nil { - return fmt.Errorf("unable to retrieve TLS information from %s", url) + return fmt.Errorf("unable to retrieve TLS information from %s", healthzURL) } // Compare the certificates served against those defined in the secret diff --git a/pkg/controllers/customroute/custom_route_controller.go b/pkg/controllers/customroute/custom_route_controller.go index c3da4aedc6..5fc2007921 100644 --- a/pkg/controllers/customroute/custom_route_controller.go +++ b/pkg/controllers/customroute/custom_route_controller.go @@ -21,10 +21,13 @@ import ( configsetterv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1lister "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" "github.com/openshift/library-go/pkg/operator/resource/resourceread" @@ -42,16 +45,19 @@ const ( ) type customRouteController struct { - destSecret types.NamespacedName - componentRoute types.NamespacedName - ingressLister configlistersv1.IngressLister - ingressClient configsetterv1.IngressInterface - routeLister routev1lister.RouteLister - routeClient routeclient.RouteInterface - secretLister corev1listers.SecretLister - resourceSyncer resourcesynccontroller.ResourceSyncer - operatorClient v1helpers.OperatorClient - authConfigChecker common.AuthConfigChecker + destSecret types.NamespacedName + componentRoute types.NamespacedName + ingressLister configlistersv1.IngressLister + ingressClient configsetterv1.IngressInterface + routeLister routev1lister.RouteLister + routeClient routeclient.RouteInterface + secretLister corev1listers.SecretLister + resourceSyncer resourcesynccontroller.ResourceSyncer + operatorClient v1helpers.OperatorClient + authConfigChecker common.AuthConfigChecker + configMapLister corev1listers.ConfigMapLister + operatorAuthLister operatorv1listers.AuthenticationLister + featureGateAccessor featuregates.FeatureGateAccess } func NewCustomRouteController( @@ -64,6 +70,8 @@ func NewCustomRouteController( routeInformer routeinformer.RouteInformer, routeClient routeclient.RouteInterface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, + operatorAuthInformer operatorv1informers.AuthenticationInformer, + featureGateAccessor featuregates.FeatureGateAccess, operatorClient v1helpers.OperatorClient, authConfigChecker common.AuthConfigChecker, eventRecorder events.Recorder, @@ -78,14 +86,17 @@ func NewCustomRouteController( Namespace: componentRouteNamespace, Name: componentRouteName, }, - ingressLister: ingressInformer.Lister(), - ingressClient: ingressClient, - routeLister: routeInformer.Lister(), - routeClient: routeClient, - secretLister: kubeInformersForNamespaces.SecretLister(), - operatorClient: operatorClient, - resourceSyncer: resourceSyncer, - authConfigChecker: authConfigChecker, + ingressLister: ingressInformer.Lister(), + ingressClient: ingressClient, + routeLister: routeInformer.Lister(), + routeClient: routeClient, + secretLister: kubeInformersForNamespaces.SecretLister(), + configMapLister: kubeInformersForNamespaces.ConfigMapLister(), + operatorClient: operatorClient, + resourceSyncer: resourceSyncer, + authConfigChecker: authConfigChecker, + operatorAuthLister: operatorAuthInformer.Lister(), + featureGateAccessor: featureGateAccessor, } return factory.New(). @@ -94,6 +105,7 @@ func NewCustomRouteController( routeInformer.Informer(), kubeInformersForNamespaces.InformersFor("openshift-config").Core().V1().Secrets().Informer(), kubeInformersForNamespaces.InformersFor("openshift-authentication").Core().V1().Secrets().Informer(), + operatorAuthInformer.Informer(), ). WithInformers(common.AuthConfigCheckerInformers[factory.Informer](&authConfigChecker)...). WithSyncDegradedOnError(operatorClient). @@ -253,7 +265,11 @@ func (c *customRouteController) updateIngressConfigStatus(ctx context.Context, i if newConditions == nil { newConditions = checkIngressURI(ingressConfig, route) if newConditions == nil { - newConditions = checkRouteAvailablity(c.secretLister, ingressConfig, route) + proxy, err := common.ResolveProxy(c.featureGateAccessor, c.operatorAuthLister) + if err != nil { + return err + } + newConditions = checkRouteAvailability(c.secretLister, c.configMapLister, ingressConfig, route, proxy) } } newConditions = ensureDefaultConditions(newConditions) diff --git a/pkg/controllers/deployment/default_deployment.go b/pkg/controllers/deployment/default_deployment.go index f4eeb2107a..d659faa3e3 100644 --- a/pkg/controllers/deployment/default_deployment.go +++ b/pkg/controllers/deployment/default_deployment.go @@ -14,7 +14,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2" - configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/operator/resource/resourceread" @@ -28,7 +27,7 @@ import ( func getOAuthServerDeployment( operatorSpec *operatorv1.OperatorSpec, - proxyConfig *configv1.Proxy, + httpProxy, httpsProxy, noProxy string, bootstrapUserExists bool, resourceVersions ...string, ) (*appsv1.Deployment, error) { @@ -67,7 +66,7 @@ func getOAuthServerDeployment( } // set proxy env vars - container.Env = append(container.Env, proxyConfigToEnvVars(proxyConfig)...) + container.Env = append(container.Env, proxyEnvVars(httpProxy, httpsProxy, noProxy)...) // set log level container.Args[0] = strings.Replace(container.Args[0], "${LOG_LEVEL}", fmt.Sprintf("%d", getLogLevel(operatorSpec.LogLevel)), -1) @@ -142,12 +141,11 @@ func getLogLevel(logLevel operatorv1.LogLevel) int { } } -// TODO: move to library-go:w -func proxyConfigToEnvVars(proxy *configv1.Proxy) []corev1.EnvVar { +func proxyEnvVars(httpProxy, httpsProxy, noProxy string) []corev1.EnvVar { var envVars []corev1.EnvVar - envVars = appendEnvVar(envVars, "NO_PROXY", proxy.Status.NoProxy) - envVars = appendEnvVar(envVars, "HTTP_PROXY", proxy.Status.HTTPProxy) - envVars = appendEnvVar(envVars, "HTTPS_PROXY", proxy.Status.HTTPSProxy) + envVars = appendEnvVar(envVars, "NO_PROXY", noProxy) + envVars = appendEnvVar(envVars, "HTTP_PROXY", httpProxy) + envVars = appendEnvVar(envVars, "HTTPS_PROXY", httpsProxy) return envVars } diff --git a/pkg/controllers/deployment/deployment_controller.go b/pkg/controllers/deployment/deployment_controller.go index b6b8732ec1..93a5fab64a 100644 --- a/pkg/controllers/deployment/deployment_controller.go +++ b/pkg/controllers/deployment/deployment_controller.go @@ -12,17 +12,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" + corev1ac "k8s.io/client-go/applyconfigurations/core/v1" "k8s.io/client-go/informers" coreinformers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" appsv1listers "k8s.io/client-go/listers/apps/v1" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" + "k8s.io/utils/ptr" - configv1 "github.com/openshift/api/config/v1" configinformer "github.com/openshift/client-go/config/informers/externalversions" - configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformers "github.com/openshift/client-go/route/informers/externalversions" routev1listers "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/cluster-authentication-operator/bindata" @@ -30,6 +33,7 @@ import ( bootstrap "github.com/openshift/library-go/pkg/authentication/bootstrapauthenticator" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/apiserver/controller/workload" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" @@ -39,6 +43,11 @@ import ( "github.com/openshift/library-go/pkg/route/routeapihelpers" ) +const ( + componentProxyCAConfigMapName = "v4-0-config-system-auth-proxy-ca" + componentProxyCAMountPath = "/var/config/system/configmaps/" + componentProxyCAConfigMapName +) + var _ workload.Delegate = &oauthServerDeploymentSyncer{} // nodeCountFunction a function to return count of nodes @@ -49,6 +58,7 @@ type nodeCountFunc func(nodeSelector map[string]string) (*int32, error) type ensureAtMostOnePodPerNodeFunc func(spec *appsv1.DeploymentSpec, componentName string) error type oauthServerDeploymentSyncer struct { + name string operatorClient v1helpers.OperatorClient // countNodes a function to return count of nodes on which the workload will be installed @@ -60,11 +70,15 @@ type oauthServerDeploymentSyncer struct { deployments appsv1client.DeploymentsGetter deploymentsLister appsv1listers.DeploymentLister - configMapLister corev1listers.ConfigMapLister - secretLister corev1listers.SecretLister - podsLister corev1listers.PodLister - proxyLister configv1listers.ProxyLister - routeLister routev1listers.RouteLister + configMaps corev1client.ConfigMapsGetter + configMapLister corev1listers.ConfigMapLister + sourceConfigMapLister corev1listers.ConfigMapLister // openshift-config namespace + secretLister corev1listers.SecretLister + podsLister corev1listers.PodLister + routeLister routev1listers.RouteLister + + operatorAuthLister operatorv1listers.AuthenticationLister + featureGateAccessor featuregates.FeatureGateAccess authConfigChecker common.AuthConfigChecker bootstrapUserDataGetter bootstrap.BootstrapUserDataGetter @@ -83,11 +97,15 @@ func NewOAuthServerWorkloadController( eventsRecorder events.Recorder, versionRecorder status.VersionGetter, kubeInformersForTargetNamespace informers.SharedInformerFactory, + kubeInformersForSourceNamespace informers.SharedInformerFactory, authConfigChecker common.AuthConfigChecker, + operatorAuthInformer operatorv1informers.AuthenticationInformer, + featureGateAccessor featuregates.FeatureGateAccess, ) factory.Controller { targetNS := "openshift-authentication" oauthDeploymentSyncer := &oauthServerDeploymentSyncer{ + name: "OAuthServerDeploymentController", operatorClient: operatorClient, countNodes: countNodes, @@ -96,11 +114,15 @@ func NewOAuthServerWorkloadController( deployments: kubeClient.AppsV1(), deploymentsLister: kubeInformersForTargetNamespace.Apps().V1().Deployments().Lister(), - configMapLister: kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Lister(), - secretLister: kubeInformersForTargetNamespace.Core().V1().Secrets().Lister(), - podsLister: kubeInformersForTargetNamespace.Core().V1().Pods().Lister(), - proxyLister: configInformers.Config().V1().Proxies().Lister(), - routeLister: routeInformersForTargetNamespace.Route().V1().Routes().Lister(), + configMaps: kubeClient.CoreV1(), + configMapLister: kubeInformersForTargetNamespace.Core().V1().ConfigMaps().Lister(), + sourceConfigMapLister: kubeInformersForSourceNamespace.Core().V1().ConfigMaps().Lister(), + secretLister: kubeInformersForTargetNamespace.Core().V1().Secrets().Lister(), + podsLister: kubeInformersForTargetNamespace.Core().V1().Pods().Lister(), + routeLister: routeInformersForTargetNamespace.Route().V1().Routes().Lister(), + + operatorAuthLister: operatorAuthInformer.Lister(), + featureGateAccessor: featureGateAccessor, authConfigChecker: authConfigChecker, bootstrapUserDataGetter: bootstrapUserDataGetter, @@ -115,8 +137,8 @@ func NewOAuthServerWorkloadController( clusterScopedInformers := []factory.Informer{ configInformers.Config().V1().Ingresses().Informer(), - configInformers.Config().V1().Proxies().Informer(), nodeInformer.Informer(), + operatorAuthInformer.Informer(), } clusterScopedInformers = append(clusterScopedInformers, common.AuthConfigCheckerInformers[factory.Informer](&authConfigChecker)...) @@ -138,6 +160,10 @@ func NewOAuthServerWorkloadController( kubeInformersForTargetNamespace.Core().V1().Pods().Informer(), kubeInformersForTargetNamespace.Core().V1().Namespaces().Informer(), routeInformersForTargetNamespace.Route().V1().Routes().Informer(), + // Watches all ConfigMaps in openshift-config because the proxy CA ConfigMap + // name is dynamic (from Authentication CR). A NamesFilter can't be used since + // the name isn't known at construction time. + kubeInformersForSourceNamespace.Core().V1().ConfigMaps().Informer(), }, oauthDeploymentSyncer, eventsRecorder, @@ -196,7 +222,7 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact return nil, false, append(errs, err) } - proxyConfig, err := c.getProxyConfig() + proxy, err := common.ResolveProxy(c.featureGateAccessor, c.operatorAuthLister) if err != nil { return nil, false, append(errs, err) } @@ -209,9 +235,8 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact // TODO move this hash from deployment meta to operatorConfig.status.generations.[...].hash resourceVersions := []string{} - if len(proxyConfig.Name) > 0 { - resourceVersions = append(resourceVersions, "proxy:"+proxyConfig.Name+":"+proxyConfig.ResourceVersion) - } + resourceVersions = append(resourceVersions, + fmt.Sprintf("proxy:%s:%s:%s", proxy.HTTPProxy, proxy.HTTPSProxy, proxy.NoProxy)) configResourceVersions, err := c.getConfigResourceVersions() if err != nil { @@ -231,7 +256,7 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact } // deployment, have RV of all resources - expectedDeployment, err := getOAuthServerDeployment(operatorSpec, proxyConfig, c.bootstrapUserChangeRollOut, resourceVersions...) + expectedDeployment, err := getOAuthServerDeployment(operatorSpec, proxy.HTTPProxy, proxy.HTTPSProxy, proxy.NoProxy, c.bootstrapUserChangeRollOut, resourceVersions...) if err != nil { return nil, false, append(errs, err) } @@ -252,6 +277,10 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact }) } + if err := c.syncComponentProxyCA(ctx, proxy.TrustedCAName, expectedDeployment); err != nil { + return nil, false, append(errs, fmt.Errorf("syncing component proxy CA: %v", err)) + } + err = c.ensureAtMostOnePodPerNode(&expectedDeployment.Spec, "oauth-openshift") if err != nil { return nil, false, append(errs, fmt.Errorf("unable to ensure at most one pod per node: %v", err)) @@ -281,18 +310,6 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact return deployment, true, errs } -func (c *oauthServerDeploymentSyncer) getProxyConfig() (*configv1.Proxy, error) { - proxyConfig, err := c.proxyLister.Get("cluster") - if err != nil { - if errors.IsNotFound(err) { - klog.V(4).Infof("No proxy configuration found, defaulting to empty") - return &configv1.Proxy{}, nil - } - return nil, fmt.Errorf("unable to get cluster proxy configuration: %v", err) - } - return proxyConfig, nil -} - func (c *oauthServerDeploymentSyncer) getConfigResourceVersions() ([]string, error) { var configRVs []string @@ -301,8 +318,9 @@ func (c *oauthServerDeploymentSyncer) getConfigResourceVersions() ([]string, err return nil, fmt.Errorf("unable to list configmaps in %q namespace: %v", "openshift-authentication", err) } for _, cm := range configMaps { - if strings.HasPrefix(cm.Name, "v4-0-config-") { - // prefix the RV to make it clear where it came from since each resource can be from different etcd + // Exclude the proxy CA configmap: its content is hot-reloaded by the + // OAuth server, so CA updates must not trigger a rollout. + if strings.HasPrefix(cm.Name, "v4-0-config-") && cm.Name != componentProxyCAConfigMapName { configRVs = append(configRVs, "configmaps:"+cm.Name+":"+cm.ResourceVersion) } } @@ -331,3 +349,66 @@ func setRollingUpdateParameters(controlPlaneCount int32, deployment *appsv1.Depl deployment.Spec.Strategy.RollingUpdate.MaxUnavailable = &maxUnavailable deployment.Spec.Strategy.RollingUpdate.MaxSurge = &maxSurge } + +// syncComponentProxyCA copies the component-scoped proxy CA ConfigMap from +// openshift-config to openshift-authentication and adds volume/mount to the +// OAuth Server deployment. The OAuth Server hot-reloads the CA file on change, +// so no redeployment is needed for CA content updates. +func (c *oauthServerDeploymentSyncer) syncComponentProxyCA( + ctx context.Context, + trustedCAName string, + deployment *appsv1.Deployment, +) error { + if len(trustedCAName) == 0 { + // Check the lister before calling Delete: the manifestclient used in + // integration tests rejects Delete requests whose body (DeleteOptions) + // carries no namespace, so we avoid an unnecessary call when the + // configmap doesn't exist. + _, err := c.configMapLister.ConfigMaps("openshift-authentication").Get(componentProxyCAConfigMapName) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("failed to check component proxy CA configmap \"%s/%s\": %w", "openshift-authentication", componentProxyCAConfigMapName, err) + } + if err := c.configMaps.ConfigMaps("openshift-authentication").Delete( + ctx, componentProxyCAConfigMapName, metav1.DeleteOptions{}, + ); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete component proxy CA configmap \"%s/%s\" : %w", "openshift-authentication", componentProxyCAConfigMapName, err) + } + return nil + } + + sourceCM, err := c.sourceConfigMapLister.ConfigMaps("openshift-config").Get(trustedCAName) + if err != nil { + return fmt.Errorf("failed to get component proxy CA configmap \"%s/%s\": %w", "openshift-config", trustedCAName, err) + } + + targetCM := corev1ac.ConfigMap(componentProxyCAConfigMapName, "openshift-authentication").WithData(sourceCM.Data) + if _, err := c.configMaps.ConfigMaps("openshift-authentication").Apply( + ctx, targetCM, metav1.ApplyOptions{FieldManager: c.name, Force: true}, + ); err != nil { + return fmt.Errorf("failed to apply component proxy CA configmap \"%s/%s\": %w", "openshift-authentication", componentProxyCAConfigMapName, err) + } + + deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: componentProxyCAConfigMapName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: componentProxyCAConfigMapName, + }, + Optional: ptr.To(true), + }, + }, + }) + deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append( + deployment.Spec.Template.Spec.Containers[0].VolumeMounts, + corev1.VolumeMount{ + Name: componentProxyCAConfigMapName, + ReadOnly: true, + MountPath: componentProxyCAMountPath, + }, + ) + return nil +} diff --git a/pkg/controllers/deployment/deployment_controller_test.go b/pkg/controllers/deployment/deployment_controller_test.go index b7385a9f3d..684c4f2a72 100644 --- a/pkg/controllers/deployment/deployment_controller_test.go +++ b/pkg/controllers/deployment/deployment_controller_test.go @@ -1,11 +1,24 @@ package deployment import ( + "context" + stderrors "errors" "testing" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" + corev1listers "k8s.io/client-go/listers/core/v1" + k8stesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/ptr" ) func TestSetRollingUpdateParameters(t *testing.T) { @@ -99,3 +112,159 @@ func TestSetRollingUpdateParameters(t *testing.T) { }) } } + +func TestSyncComponentProxyCA(t *testing.T) { + sourceCA := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "my-proxy-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": "test-ca-data"}, + } + + t.Run("trustedCA set applies configmap and adds volume and mount", func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, indexer.Add(sourceCA)) + + cs := fake.NewClientset() + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + sourceConfigMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + require.NoError(t, syncer.syncComponentProxyCA(context.Background(), "my-proxy-ca", dep)) + + applied, err := cs.CoreV1().ConfigMaps("openshift-authentication").Get(context.Background(), componentProxyCAConfigMapName, metav1.GetOptions{}) + require.NoError(t, err) + if diff := cmp.Diff(map[string]string{"ca-bundle.crt": "test-ca-data"}, applied.Data); diff != "" { + t.Errorf("applied configmap data mismatch (-want +got):\n%s", diff) + } + + wantVolumes := []corev1.Volume{{ + Name: componentProxyCAConfigMapName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: componentProxyCAConfigMapName}, + Optional: ptr.To(true), + }, + }, + }} + if diff := cmp.Diff(wantVolumes, dep.Spec.Template.Spec.Volumes); diff != "" { + t.Errorf("volumes mismatch (-want +got):\n%s", diff) + } + + wantMounts := []corev1.VolumeMount{{ + Name: componentProxyCAConfigMapName, + ReadOnly: true, + MountPath: componentProxyCAMountPath, + }} + if diff := cmp.Diff(wantMounts, dep.Spec.Template.Spec.Containers[0].VolumeMounts); diff != "" { + t.Errorf("volume mounts mismatch (-want +got):\n%s", diff) + } + }) + + t.Run("trustedCA empty deletes stale configmap", func(t *testing.T) { + existing := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: componentProxyCAConfigMapName, Namespace: "openshift-authentication"}, + } + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, indexer.Add(existing)) + + cs := fake.NewClientset(existing) + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + configMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + require.NoError(t, syncer.syncComponentProxyCA(context.Background(), "", dep)) + + _, err := cs.CoreV1().ConfigMaps("openshift-authentication").Get(context.Background(), componentProxyCAConfigMapName, metav1.GetOptions{}) + require.True(t, errors.IsNotFound(err), "expected NotFound, got: %v", err) + + require.Empty(t, dep.Spec.Template.Spec.Volumes) + }) + + t.Run("trustedCA empty no stale configmap is noop", func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + cs := fake.NewClientset() + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + configMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + require.NoError(t, syncer.syncComponentProxyCA(context.Background(), "", dep)) + }) + + t.Run("source configmap not found returns error", func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + cs := fake.NewClientset() + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + sourceConfigMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + err := syncer.syncComponentProxyCA(context.Background(), "nonexistent-ca", dep) + require.True(t, errors.IsNotFound(stderrors.Unwrap(err)), "expected wrapped NotFound, got: %v", err) + }) + + t.Run("delete error propagates", func(t *testing.T) { + existing := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: componentProxyCAConfigMapName, Namespace: "openshift-authentication"}, + } + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, indexer.Add(existing)) + + cs := fake.NewClientset() + cs.PrependReactor("delete", "configmaps", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, stderrors.New("delete failed") + }) + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + configMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + err := syncer.syncComponentProxyCA(context.Background(), "", dep) + require.ErrorContains(t, err, "delete failed") + }) + + t.Run("apply error propagates", func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + require.NoError(t, indexer.Add(sourceCA)) + + cs := fake.NewClientset() + cs.PrependReactor("patch", "configmaps", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, stderrors.New("apply failed") + }) + syncer := &oauthServerDeploymentSyncer{ + name: "TestController", + configMaps: cs.CoreV1(), + sourceConfigMapLister: corev1listers.NewConfigMapLister(indexer), + } + + dep := testDeployment() + err := syncer.syncComponentProxyCA(context.Background(), "my-proxy-ca", dep) + require.ErrorContains(t, err, "apply failed") + + _, getErr := cs.CoreV1().ConfigMaps("openshift-authentication").Get(context.Background(), componentProxyCAConfigMapName, metav1.GetOptions{}) + require.True(t, errors.IsNotFound(getErr), "configmap should not exist after apply failure") + }) +} + +func testDeployment() *appsv1.Deployment { + return &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "oauth-server"}}, + }, + }, + }, + } +} diff --git a/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go b/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go index 2a966550a5..bccda81afc 100644 --- a/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go +++ b/pkg/controllers/oauthendpoints/oauth_endpoints_controller.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "fmt" "net" + "net/url" "strconv" "k8s.io/apimachinery/pkg/util/sets" @@ -17,14 +18,17 @@ import ( routev1 "github.com/openshift/api/route/v1" configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" configv1lister "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" routev1informers "github.com/openshift/client-go/route/informers/externalversions/route/v1" routev1listers "github.com/openshift/client-go/route/listers/route/v1" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/cluster-authentication-operator/pkg/libs/endpointaccessible" + "github.com/openshift/cluster-authentication-operator/pkg/transport" ) // NewOAuthRouteCheckController returns a controller that checks the health of authentication route. @@ -32,14 +36,18 @@ func NewOAuthRouteCheckController( operatorClient v1helpers.OperatorClient, kubeInformersForTargetNS informers.SharedInformerFactory, kubeInformersForConfigManagedNS informers.SharedInformerFactory, + kubeInformersForConfigNS informers.SharedInformerFactory, routeInformerNamespaces routev1informers.RouteInformer, ingressInformerAllNamespaces configv1informers.IngressInformer, + operatorAuthInformer operatorv1informers.AuthenticationInformer, + featureGateAccessor featuregates.FeatureGateAccess, authConfigChecker common.AuthConfigChecker, systemCABundle []byte, recorder events.Recorder, ) factory.Controller { cmLister := kubeInformersForConfigManagedNS.Core().V1().ConfigMaps().Lister() cmInformer := kubeInformersForConfigManagedNS.Core().V1().ConfigMaps().Informer() + configNSCMLister := kubeInformersForConfigNS.Core().V1().ConfigMaps().Lister() secretLister := kubeInformersForTargetNS.Core().V1().Secrets().Lister() secretInformer := kubeInformersForTargetNS.Core().V1().Secrets().Informer() @@ -53,9 +61,21 @@ func NewOAuthRouteCheckController( } getTLSConfigFunc := func() (*tls.Config, error) { - return getOAuthRouteTLSConfig(cmLister, secretLister, ingressLister, systemCABundle) + proxy, err := common.ResolveProxy(featureGateAccessor, operatorAuthInformer.Lister()) + if err != nil { + return nil, err + } + return getOAuthRouteTLSConfig(cmLister, configNSCMLister, secretLister, ingressLister, systemCABundle, proxy.TrustedCAName) } + getProxyFn := transport.ProxyFunc(func(reqURL *url.URL) (*url.URL, error) { + proxy, err := common.ResolveProxy(featureGateAccessor, operatorAuthInformer.Lister()) + if err != nil { + return nil, err + } + return proxy.ProxyFunc()(reqURL) + }) + endpointCheckDisabledFunc := authConfigChecker.OIDCAvailable informers := []factory.Informer{ @@ -63,13 +83,14 @@ func NewOAuthRouteCheckController( secretInformer, routeInformer, ingressInformer, + operatorAuthInformer.Informer(), } informers = append(informers, common.AuthConfigCheckerInformers[factory.Informer](&authConfigChecker)...) - return endpointaccessible.NewEndpointAccessibleController( + return endpointaccessible.NewEndpointAccessibleControllerWithProxy( "OAuthServerRoute", operatorClient, - endpointListFunc, getTLSConfigFunc, endpointCheckDisabledFunc, + endpointListFunc, getTLSConfigFunc, getProxyFn, endpointCheckDisabledFunc, informers, recorder) } @@ -221,7 +242,7 @@ func listOAuthRoutes(ingressConfigLister configv1lister.IngressLister, routeList return toHealthzURL(results), nil } -func getOAuthRouteTLSConfig(cmLister corev1listers.ConfigMapLister, secretLister corev1listers.SecretLister, ingressLister configv1lister.IngressLister, systemCABundle []byte) (*tls.Config, error) { +func getOAuthRouteTLSConfig(cmLister, configNSCMLister corev1listers.ConfigMapLister, secretLister corev1listers.SecretLister, ingressLister configv1lister.IngressLister, systemCABundle []byte, trustedCAName string) (*tls.Config, error) { // get default router CA cert cm defaultIngressCertCM, err := cmLister.ConfigMaps("openshift-config-managed").Get("default-ingress-cert") if err != nil { @@ -268,6 +289,16 @@ func getOAuthRouteTLSConfig(cmLister corev1listers.ConfigMapLister, secretLister } } + if len(trustedCAName) > 0 { + caData, err := transport.LoadCAData(configNSCMLister, trustedCAName, "ca-bundle.crt") + if err != nil { + return nil, fmt.Errorf("failed to load proxy trustedCA from \"openshift-config/%s\": %w", trustedCAName, err) + } + if ok := rootCAs.AppendCertsFromPEM(caData); !ok { + klog.V(5).Infof("the proxy trustedCA bundle from \"openshift-config/%s\" did not contain any PEM certificates", trustedCAName) + } + } + return &tls.Config{ RootCAs: rootCAs, }, nil diff --git a/pkg/controllers/proxyconfig/proxyconfig_controller.go b/pkg/controllers/proxyconfig/proxyconfig_controller.go index 5dd748ef91..e5cc125884 100644 --- a/pkg/controllers/proxyconfig/proxyconfig_controller.go +++ b/pkg/controllers/proxyconfig/proxyconfig_controller.go @@ -2,26 +2,33 @@ package proxyconfig import ( "context" + "crypto/sha256" "crypto/tls" "crypto/x509" "fmt" "net/http" "net/url" + "strings" "sync" "time" - "golang.org/x/net/http/httpproxy" - corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" + configv1 "github.com/openshift/api/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" routeinformer "github.com/openshift/client-go/route/informers/externalversions/route/v1" v1 "github.com/openshift/client-go/route/listers/route/v1" - "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/library-go/pkg/route/routeapihelpers" + + "github.com/openshift/cluster-authentication-operator/pkg/controllers/common" + "github.com/openshift/cluster-authentication-operator/pkg/transport" ) // proxyConfigChecker reports bad proxy configurations. @@ -32,7 +39,13 @@ type proxyConfigChecker struct { routeNamespace string caConfigMaps map[string][]string // ns -> []configmapNames + oauthLister configv1listers.OAuthLister + operatorAuthLister operatorv1listers.AuthenticationLister + featureGateAccessor featuregates.FeatureGateAccess + authConfigChecker common.AuthConfigChecker + + lastIdPValidationHash string } func NewProxyConfigChecker( @@ -43,14 +56,21 @@ func NewProxyConfigChecker( routeName string, caConfigMaps map[string][]string, recorder events.Recorder, - operatorClient v1helpers.OperatorClient) factory.Controller { + operatorClient v1helpers.OperatorClient, + oauthLister configv1listers.OAuthLister, + operatorAuthInformer operatorv1informers.AuthenticationInformer, + featureGateAccessor featuregates.FeatureGateAccess, +) factory.Controller { p := proxyConfigChecker{ - routeLister: routeInformer.Lister(), - configMapLister: configMapInformers.ConfigMapLister(), - routeName: routeName, - routeNamespace: routeNamespace, - caConfigMaps: caConfigMaps, - authConfigChecker: authConfigChecker, + routeLister: routeInformer.Lister(), + configMapLister: configMapInformers.ConfigMapLister(), + routeName: routeName, + routeNamespace: routeNamespace, + caConfigMaps: caConfigMaps, + oauthLister: oauthLister, + operatorAuthLister: operatorAuthInformer.Lister(), + featureGateAccessor: featureGateAccessor, + authConfigChecker: authConfigChecker, } c := factory.New(). @@ -59,6 +79,7 @@ func NewProxyConfigChecker( routeInformer.Informer(), ). WithInformers(common.AuthConfigCheckerInformers[factory.Informer](&authConfigChecker)...). + WithInformers(operatorAuthInformer.Informer()). ResyncEvery(60 * time.Minute). WithSyncDegradedOnError(operatorClient) @@ -72,37 +93,111 @@ func NewProxyConfigChecker( return c.ToController("ProxyConfigController", recorder.WithComponentSuffix("proxy-config-controller")) } -// sync attempts to connect to route using configured proxy settings and reports any error. -func (p *proxyConfigChecker) sync(ctx context.Context, _ factory.SyncContext) error { +// sync attempts to connect to the route using the active proxy settings and reports +// any misconfiguration. ResolveProxy returns effective proxy settings for both +// cluster-wide and component-scoped proxies, so the validation logic is the same. +func (p *proxyConfigChecker) sync(ctx context.Context, syncCtx factory.SyncContext) error { if oidcAvailable, err := p.authConfigChecker.OIDCAvailable(); err != nil { return err } else if oidcAvailable { return nil } - proxyConfig := httpproxy.FromEnvironment() - if !isProxyConfigured(proxyConfig) { - // If proxy is not configured, then it is a no-op. + proxy, err := common.ResolveProxy(p.featureGateAccessor, p.operatorAuthLister) + if err != nil { + return err + } + if !proxy.IsProxyConfigured() { return nil } - route, err := p.routeLister.Routes(p.routeNamespace).Get(p.routeName) + routeURL, err := p.getRouteHealthzURL() if err != nil { return err } - routeURL, _, err := routeapihelpers.IngressURI(route, "") + clientWithProxy, clientWithoutProxy, err := p.createHTTPClients(proxy) if err != nil { return err } - routeURL.Path = "healthz" - clientWithProxy, clientWithoutProxy, err := p.createHTTPClients() - if err != nil { + if err := checkProxyConfig(ctx, routeURL, proxy.NoProxy, clientWithProxy, clientWithoutProxy); err != nil { return err } - return checkProxyConfig(ctx, routeURL, proxyConfig.NoProxy, clientWithProxy, clientWithoutProxy) + p.validateIdPConnectivity(ctx, syncCtx.Recorder(), clientWithProxy, proxy.HTTPProxy, proxy.HTTPSProxy, proxy.NoProxy) + return nil +} + +// validateIdPConnectivity tests that configured IdP endpoints are reachable through +// the proxy. Only runs on config change (tracked by hash). Reports warnings +// for transient IdP failures instead of returning errors (which would set Degraded), +// since external IdPs can be unreachable for reasons unrelated to proxy configuration. +func (p *proxyConfigChecker) validateIdPConnectivity(ctx context.Context, recorder events.Recorder, client *http.Client, httpProxy, httpsProxy, noProxy string) { + oauthConfig, err := p.oauthLister.Get("cluster") + if err != nil { + klog.Warningf("unable to get oauth config for IdP validation: %v", err) + return + } + + idpURLs := extractIdPURLs(oauthConfig) + if len(idpURLs) == 0 { + return + } + + hash := computeIdPValidationHash(httpProxy, httpsProxy, noProxy, idpURLs) + if hash == p.lastIdPValidationHash { + return + } + + var unreachable []string + for _, idpURL := range idpURLs { + if err := isEndpointReachable(ctx, idpURL, client); err != nil { + klog.Warningf("IdP endpoint unreachable through proxy: %v", err) + unreachable = append(unreachable, err.Error()) + } + } + if len(unreachable) > 0 { + recorder.Warningf("IdPEndpointUnreachable", "IdP endpoints unreachable through proxy: %s", strings.Join(unreachable, "; ")) + } else { + p.lastIdPValidationHash = hash + } +} + +// extractIdPURLs returns external URLs from configured identity providers. +func extractIdPURLs(oauthConfig *configv1.OAuth) []string { + var urls []string + for _, idp := range oauthConfig.Spec.IdentityProviders { + switch { + case idp.OpenID != nil && len(idp.OpenID.Issuer) > 0: + issuer := strings.TrimSuffix(idp.OpenID.Issuer, "/") + urls = append(urls, issuer+"/.well-known/openid-configuration") + case idp.GitHub != nil: + host := idp.GitHub.Hostname + if len(host) == 0 { + host = "github.com" + } + urls = append(urls, "https://"+host) + case idp.GitLab != nil && len(idp.GitLab.URL) > 0: + urls = append(urls, idp.GitLab.URL) + case idp.Keystone != nil && len(idp.Keystone.URL) > 0: + urls = append(urls, idp.Keystone.URL) + case idp.BasicAuth != nil && len(idp.BasicAuth.URL) > 0: + urls = append(urls, idp.BasicAuth.URL) + case idp.Google != nil: + urls = append(urls, "https://accounts.google.com") + } + } + return urls +} + +func computeIdPValidationHash(httpProxy, httpsProxy, noProxy string, idpURLs []string) string { + h := sha256.New() + fmt.Fprintf(h, "%s\n%s\n%s\n", httpProxy, httpsProxy, noProxy) + for _, u := range idpURLs { + fmt.Fprintf(h, "%s\n", u) + } + return fmt.Sprintf("%x", h.Sum(nil)) } // checkProxyConfig determines any mis-configuration in proxy settings by attempting @@ -129,21 +224,43 @@ func checkProxyConfig(ctx context.Context, endpointURL *url.URL, noProxy string, return nil } -// createHTTPClients returns two http clients, one with proxy and another without proxy -func (p *proxyConfigChecker) createHTTPClients() (*http.Client, *http.Client, error) { +func (p *proxyConfigChecker) getRouteHealthzURL() (*url.URL, error) { + route, err := p.routeLister.Routes(p.routeNamespace).Get(p.routeName) + if err != nil { + return nil, err + } + routeURL, _, err := routeapihelpers.IngressURI(route, "") + if err != nil { + return nil, err + } + routeURL.Path = "healthz" + return routeURL, nil +} + +// createHTTPClients returns two HTTP clients — one that routes through the proxy +// and one that connects directly. The proxy's trustedCA bundle (if any) is loaded +// and appended to the system CA pool. +func (p *proxyConfigChecker) createHTTPClients(proxy *common.ResolvedProxy) (*http.Client, *http.Client, error) { caPool, err := p.getCACerts() if err != nil { return nil, nil, err } - tlsConfig := &tls.Config{ - RootCAs: caPool, + if len(proxy.TrustedCAName) > 0 { + caData, err := transport.LoadCAData(p.configMapLister, proxy.TrustedCAName, "ca-bundle.crt") + if err != nil { + return nil, nil, fmt.Errorf("failed to load proxy CA: %w", err) + } + caPool.AppendCertsFromPEM(caData) } + tlsConfig := &tls.Config{RootCAs: caPool} + proxyFn := func(req *http.Request) (*url.URL, error) { return proxy.ProxyFunc()(req.URL) } + return &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsConfig, - Proxy: proxyFunc, + Proxy: proxyFn, }, }, &http.Client{ Transport: &http.Transport{ @@ -197,29 +314,6 @@ func isEndpointReachable(ctx context.Context, endpointURL string, client *http.C return nil } -func isProxyConfigured(proxyConfig *httpproxy.Config) bool { - return proxyConfig != nil && (len(proxyConfig.HTTPProxy) != 0 || len(proxyConfig.HTTPSProxy) != 0) -} - -// proxyFunc returns the proxy URL to be used for a given request -// when NO_PROXY is ignored. -func proxyFunc(req *http.Request) (*url.URL, error) { - proxyConfig := httpproxy.FromEnvironment() - if req.URL.Scheme == "https" && len(proxyConfig.HTTPSProxy) > 0 { - proxyURL, err := url.Parse(proxyConfig.HTTPSProxy) - if err == nil { - return proxyURL, nil - } - klog.V(4).Infof("failed to parse https proxy %q", proxyConfig.HTTPSProxy) - } - - proxyURL, err := url.Parse(proxyConfig.HTTPProxy) - if err != nil { - return nil, err - } - return proxyURL, nil -} - // newLazyChecker returns a function that calculates an error value once // and returns that error in subsequent calls func newLazyChecker(f func() error) func() error { diff --git a/pkg/controllers/proxyconfig/proxyconfig_controller_test.go b/pkg/controllers/proxyconfig/proxyconfig_controller_test.go index b76afe8a63..1440e6a9aa 100644 --- a/pkg/controllers/proxyconfig/proxyconfig_controller_test.go +++ b/pkg/controllers/proxyconfig/proxyconfig_controller_test.go @@ -5,150 +5,18 @@ import ( "fmt" "net/http" "net/url" - "os" "reflect" "testing" + "time" - "golang.org/x/net/http/httpproxy" -) - -func Test_isProxyConfigured(t *testing.T) { - tests := []struct { - name string - proxyConfig *httpproxy.Config - want bool - }{ - { - name: "without proxy", - }, - { - name: "with http proxy", - proxyConfig: &httpproxy.Config{ - HTTPProxy: "proxy-url", - }, - want: true, - }, - { - name: "with https proxy", - proxyConfig: &httpproxy.Config{ - HTTPSProxy: "proxy-url", - }, - want: true, - }, - { - name: "with http and https proxy", - proxyConfig: &httpproxy.Config{ - HTTPProxy: "proxy-url", - HTTPSProxy: "proxy-url", - }, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := isProxyConfigured(tt.proxyConfig); got != tt.want { - t.Errorf("isProxyConfigured() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_proxyFunc(t *testing.T) { - httpsProxy := "https://test.com:443" - httpsProxyURL, err := url.Parse(httpsProxy) - if err != nil { - t.Fatal(err) - } + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" + clocktesting "k8s.io/utils/clock/testing" - httpProxy := "test.com:80" - httpProxyURL, err := url.Parse(httpProxy) - if err != nil { - t.Fatal(err) - } - - tests := []struct { - name string - httpsProxy string - httpProxy string - req *http.Request - want *url.URL - wantErr bool - }{ - { - name: "valid https proxy with https url scheme", - httpsProxy: httpsProxy, - req: &http.Request{ - URL: httpsProxyURL, - }, - want: httpsProxyURL, - }, - { - name: "valid http proxy with http url scheme", - httpProxy: httpProxy, - req: &http.Request{ - URL: httpProxyURL, - }, - want: httpProxyURL, - }, - { - name: "valid http proxy with https url scheme", - httpProxy: httpProxy, - req: &http.Request{ - URL: httpsProxyURL, - }, - want: httpProxyURL, - }, - { - name: "invalid https proxy but valid http proxy", - httpsProxy: "this-url-is-invalid%1^", - httpProxy: httpProxy, - req: &http.Request{ - URL: httpsProxyURL, - }, - want: httpProxyURL, - }, - { - name: "invalid https proxy and invalid http proxy", - httpsProxy: "this-url-is-invalid%1^", - httpProxy: "this-url-is-invalid%1^", - req: &http.Request{ - URL: httpsProxyURL, - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := os.Setenv("HTTPS_PROXY", tt.httpsProxy); err != nil { - t.Error(err) - return - } - - if err := os.Setenv("HTTP_PROXY", tt.httpProxy); err != nil { - t.Error(err) - return - } - - got, err := proxyFunc(tt.req) - if (err != nil) != tt.wantErr { - t.Errorf("proxyFunc() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("proxyFunc() got = %v, want %v", got, tt.want) - } - - if err := os.Unsetenv("HTTPS_PROXY"); err != nil { - t.Error(err) - return - } - if err := os.Unsetenv("HTTP_PROXY"); err != nil { - t.Error(err) - return - } - }) - } -} + configv1 "github.com/openshift/api/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/operator/events" +) func Test_checkProxyConfig(t *testing.T) { endpoint := "https://proxy.testing.com:443" @@ -222,13 +90,294 @@ func Test_checkProxyConfig(t *testing.T) { } } +func Test_extractIdPURLs(t *testing.T) { + tests := []struct { + name string + idps []configv1.IdentityProvider + want []string + }{ + { + name: "no providers", + }, + { + name: "OpenID appends well-known path", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeOpenID, + OpenID: &configv1.OpenIDIdentityProvider{Issuer: "https://sso.example.com/"}, + }, + }}, + want: []string{"https://sso.example.com/.well-known/openid-configuration"}, + }, + { + name: "GitHub with custom hostname", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitHub, + GitHub: &configv1.GitHubIdentityProvider{Hostname: "github.corp.example.com"}, + }, + }}, + want: []string{"https://github.corp.example.com"}, + }, + { + name: "GitHub without hostname defaults to github.com", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitHub, + GitHub: &configv1.GitHubIdentityProvider{}, + }, + }}, + want: []string{"https://github.com"}, + }, + { + name: "GitLab", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }, + }}, + want: []string{"https://gitlab.example.com"}, + }, + { + name: "Keystone", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeKeystone, + Keystone: &configv1.KeystoneIdentityProvider{OAuthRemoteConnectionInfo: configv1.OAuthRemoteConnectionInfo{URL: "https://keystone.example.com"}}, + }, + }}, + want: []string{"https://keystone.example.com"}, + }, + { + name: "BasicAuth", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeBasicAuth, + BasicAuth: &configv1.BasicAuthIdentityProvider{OAuthRemoteConnectionInfo: configv1.OAuthRemoteConnectionInfo{URL: "https://auth.example.com"}}, + }, + }}, + want: []string{"https://auth.example.com"}, + }, + { + name: "Google uses fixed endpoint", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGoogle, + Google: &configv1.GoogleIdentityProvider{ClientID: "id"}, + }, + }}, + want: []string{"https://accounts.google.com"}, + }, + { + name: "LDAP is skipped", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeLDAP, + LDAP: &configv1.LDAPIdentityProvider{URL: "ldap://ldap.example.com"}, + }, + }}, + }, + { + name: "HTPasswd is skipped", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeHTPasswd, + HTPasswd: &configv1.HTPasswdIdentityProvider{}, + }, + }}, + }, + { + name: "RequestHeader is skipped", + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeRequestHeader, + RequestHeader: &configv1.RequestHeaderIdentityProvider{}, + }, + }}, + }, + { + name: "multiple providers", + idps: []configv1.IdentityProvider{ + {IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeOpenID, + OpenID: &configv1.OpenIDIdentityProvider{Issuer: "https://sso.example.com"}, + }}, + {IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }}, + {IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeHTPasswd, + HTPasswd: &configv1.HTPasswdIdentityProvider{}, + }}, + }, + want: []string{ + "https://sso.example.com/.well-known/openid-configuration", + "https://gitlab.example.com", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractIdPURLs(&configv1.OAuth{ + Spec: configv1.OAuthSpec{IdentityProviders: tt.idps}, + }) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("extractIdPURLs() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_validateIdPConnectivity(t *testing.T) { + tests := []struct { + name string + idps []configv1.IdentityProvider + client *http.Client + wantReason string + wantMessage string + }{ + { + name: "all reachable emits no event", + client: &http.Client{Transport: &workingHTTPRoundTripper{}}, + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }, + }}, + }, + { + name: "unreachable emits warning", + client: &http.Client{Transport: &faultyHTTPRoundTripper{}}, + idps: []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }, + }}, + wantReason: "IdPEndpointUnreachable", + wantMessage: `IdP endpoints unreachable through proxy: "https://gitlab.example.com" returned 404`, + }, + { + name: "multiple unreachable emits single event", + client: &http.Client{Transport: &faultyHTTPRoundTripper{}}, + idps: []configv1.IdentityProvider{ + {IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }}, + {IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeOpenID, + OpenID: &configv1.OpenIDIdentityProvider{Issuer: "https://sso.example.com"}, + }}, + }, + wantReason: "IdPEndpointUnreachable", + wantMessage: `IdP endpoints unreachable through proxy: "https://gitlab.example.com" returned 404; "https://sso.example.com/.well-known/openid-configuration" returned 404`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(&configv1.OAuth{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.OAuthSpec{IdentityProviders: tt.idps}, + }); err != nil { + t.Fatal(err) + } + + recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + p := &proxyConfigChecker{ + oauthLister: configv1listers.NewOAuthLister(indexer), + } + + p.validateIdPConnectivity(context.Background(), recorder, tt.client, "", "", "") + + recordedEvents := recorder.Events() + if tt.wantReason == "" { + if len(recordedEvents) > 0 { + t.Fatalf("expected no events but got %d: %v", len(recordedEvents), recordedEvents) + } + return + } + if len(recordedEvents) != 1 { + t.Fatalf("expected 1 event but got %d", len(recordedEvents)) + } + if recordedEvents[0].Reason != tt.wantReason { + t.Errorf("reason = %q, want %q", recordedEvents[0].Reason, tt.wantReason) + } + if recordedEvents[0].Message != tt.wantMessage { + t.Errorf("message = %q, want %q", recordedEvents[0].Message, tt.wantMessage) + } + }) + } +} + +func Test_validateIdPConnectivity_hashDedup(t *testing.T) { + idps := []configv1.IdentityProvider{{ + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeGitLab, + GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"}, + }, + }} + oauthConfig := &configv1.OAuth{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.OAuthSpec{IdentityProviders: idps}, + } + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(oauthConfig); err != nil { + t.Fatal(err) + } + + t.Run("second call with same config skips validation", func(t *testing.T) { + recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + p := &proxyConfigChecker{ + oauthLister: configv1listers.NewOAuthLister(indexer), + } + + reachable := &http.Client{Transport: &workingHTTPRoundTripper{}} + p.validateIdPConnectivity(context.Background(), recorder, reachable, "http://proxy:3128", "", "") + if len(recorder.Events()) != 0 { + t.Fatalf("first call should emit no events for reachable endpoints, got %d", len(recorder.Events())) + } + + followupRecorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + unreachable := &http.Client{Transport: &faultyHTTPRoundTripper{}} + p.validateIdPConnectivity(context.Background(), followupRecorder, unreachable, "http://proxy:3128", "", "") + if len(followupRecorder.Events()) != 0 { + t.Fatalf("second call with same config should skip validation, got %d events", len(followupRecorder.Events())) + } + }) + + t.Run("hash not saved on failure allows retry", func(t *testing.T) { + recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + p := &proxyConfigChecker{ + oauthLister: configv1listers.NewOAuthLister(indexer), + } + + unreachable := &http.Client{Transport: &faultyHTTPRoundTripper{}} + p.validateIdPConnectivity(context.Background(), recorder, unreachable, "http://proxy:3128", "", "") + if len(recorder.Events()) != 1 { + t.Fatalf("expected 1 event on failure, got %d", len(recorder.Events())) + } + + followupRecorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())) + p.validateIdPConnectivity(context.Background(), followupRecorder, unreachable, "http://proxy:3128", "", "") + if len(followupRecorder.Events()) != 1 { + t.Fatalf("expected 1 event on retry after failure, got %d", len(followupRecorder.Events())) + } + }) +} + type workingHTTPRoundTripper struct{} -type faultyHTTPRoundTripper struct{} func (s *workingHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) { return &http.Response{StatusCode: 200}, nil } +type faultyHTTPRoundTripper struct{} + func (s *faultyHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) { return &http.Response{StatusCode: 404}, nil } diff --git a/pkg/internal/transporttest/transporttest.go b/pkg/internal/transporttest/transporttest.go new file mode 100644 index 0000000000..1cc783914e --- /dev/null +++ b/pkg/internal/transporttest/transporttest.go @@ -0,0 +1,60 @@ +package transporttest + +import ( + "net/http" + "net/url" + "path" + "testing" + "time" + + "github.com/openshift/library-go/pkg/crypto" +) + +// UnwrapTransport recursively unwraps a RoundTripper (e.g. from +// DebugWrappers) until it reaches the underlying *http.Transport. +func UnwrapTransport(t *testing.T, rt http.RoundTripper) *http.Transport { + t.Helper() + type unwrapper interface { + WrappedRoundTripper() http.RoundTripper + } + for { + if tr, ok := rt.(*http.Transport); ok { + return tr + } + u, ok := rt.(unwrapper) + if !ok { + t.Fatalf("cannot unwrap %T to *http.Transport", rt) + } + rt = u.WrappedRoundTripper() + } +} + +// MakeSelfSignedCA generates a self-signed CA certificate and returns +// the CA object and its PEM-encoded certificate bytes. +func MakeSelfSignedCA(t *testing.T) (*crypto.CA, []byte) { + t.Helper() + tmpDir := t.TempDir() + ca, err := crypto.MakeSelfSignedCA( + path.Join(tmpDir, "ca.crt"), + path.Join(tmpDir, "ca.key"), + "", "testCA", time.Hour*24, + ) + if err != nil { + t.Fatalf("failed to create self-signed CA: %v", err) + } + certPEM, _, err := ca.Config.GetPEMBytes() + if err != nil { + t.Fatalf("failed to get CA PEM bytes: %v", err) + } + return ca, certPEM +} + +// MustParseURL parses a raw URL string and fails the test if it is invalid. +func MustParseURL(t *testing.T, rawURL string) *url.URL { + t.Helper() + u, err := url.Parse(rawURL) + if err != nil { + t.Fatalf("failed to parse URL %q: %v", rawURL, err) + } + return u +} diff --git a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go index 686d52630f..837578c295 100644 --- a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go +++ b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "net/http" + "net/url" "strings" "sync" "time" @@ -15,16 +16,20 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + "github.com/openshift/cluster-authentication-operator/pkg/transport" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" ) type endpointAccessibleController struct { - controllerInstanceName string - operatorClient v1helpers.OperatorClient - endpointListFn EndpointListFunc - getTLSConfigFn EndpointTLSConfigFunc + controllerInstanceName string + operatorClient v1helpers.OperatorClient + endpointListFn EndpointListFunc + getTLSConfigFn EndpointTLSConfigFunc + // Only a proxy function is needed, not the full proxy config with trusted + // CA, because this controller skips TLS verification (InsecureSkipVerify). + getProxyFn transport.ProxyFunc availableConditionName string endpointCheckDisabledFunc EndpointCheckDisabledFunc } @@ -43,6 +48,34 @@ func NewEndpointAccessibleController( endpointCheckDisabledFunc EndpointCheckDisabledFunc, triggers []factory.Informer, recorder events.Recorder, +) factory.Controller { + return newEndpointAccessibleController(name, operatorClient, endpointListFn, getTLSConfigFn, nil, endpointCheckDisabledFunc, triggers, recorder) +} + +// NewEndpointAccessibleControllerWithProxy is like NewEndpointAccessibleController +// but accepts a proxy function that overrides http.ProxyFromEnvironment. +func NewEndpointAccessibleControllerWithProxy( + name string, + operatorClient v1helpers.OperatorClient, + endpointListFn EndpointListFunc, + getTLSConfigFn EndpointTLSConfigFunc, + getProxyFn transport.ProxyFunc, + endpointCheckDisabledFunc EndpointCheckDisabledFunc, + triggers []factory.Informer, + recorder events.Recorder, +) factory.Controller { + return newEndpointAccessibleController(name, operatorClient, endpointListFn, getTLSConfigFn, getProxyFn, endpointCheckDisabledFunc, triggers, recorder) +} + +func newEndpointAccessibleController( + name string, + operatorClient v1helpers.OperatorClient, + endpointListFn EndpointListFunc, + getTLSConfigFn EndpointTLSConfigFunc, + getProxyFn transport.ProxyFunc, + endpointCheckDisabledFunc EndpointCheckDisabledFunc, + triggers []factory.Informer, + recorder events.Recorder, ) factory.Controller { controllerName := name + "EndpointAccessibleController" @@ -51,6 +84,7 @@ func NewEndpointAccessibleController( operatorClient: operatorClient, endpointListFn: endpointListFn, getTLSConfigFn: getTLSConfigFn, + getProxyFn: getProxyFn, availableConditionName: name + "EndpointAccessibleControllerAvailable", endpointCheckDisabledFunc: endpointCheckDisabledFunc, } @@ -175,8 +209,13 @@ func (c *endpointAccessibleController) sync(ctx context.Context, syncCtx factory } func (c *endpointAccessibleController) buildTLSClient() (*http.Client, error) { + proxyFn := http.ProxyFromEnvironment + if c.getProxyFn != nil { + proxyFn = func(req *http.Request) (*url.URL, error) { return c.getProxyFn(req.URL) } + } + transport := &http.Transport{ - Proxy: http.ProxyFromEnvironment, + Proxy: proxyFn, TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, }, diff --git a/pkg/operator/replacement_starter.go b/pkg/operator/replacement_starter.go index 6d8b856592..bbbe4a3972 100644 --- a/pkg/operator/replacement_starter.go +++ b/pkg/operator/replacement_starter.go @@ -143,7 +143,10 @@ func CreateOperatorInputFromMOM(ctx context.Context, momInput libraryapplyconfig apiextensionClient: apiextensionClient, eventRecorder: eventRecorder, clock: momInput.Clock, - featureGateAccessor: staticFeatureGateAccessor([]ocpconfigv1.FeatureGateName{features.FeatureGateExternalOIDC}, []ocpconfigv1.FeatureGateName{features.FeatureGateKMSEncryption, features.FeatureGateExternalOIDCExternalClaimsSourcing}), + featureGateAccessor: staticFeatureGateAccessor( + []ocpconfigv1.FeatureGateName{features.FeatureGateExternalOIDC}, + []ocpconfigv1.FeatureGateName{features.FeatureGateKMSEncryption, features.FeatureGateExternalOIDCExternalClaimsSourcing, features.FeatureGateAuthenticationComponentProxy}, + ), informerFactories: []libraryapplyconfiguration.SimplifiedInformerFactory{ libraryapplyconfiguration.DynamicInformerFactoryAdapter(dynamicInformers), // we don't share the dynamic informers, but we only want to start when requested }, diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index d75e5fe2ed..c7464ab704 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -189,6 +189,8 @@ func prepareOauthOperator( resourceSyncController, enabledClusterCapabilities, authOperatorInput.eventRecorder, + informerFactories.operatorInformer.Operator().V1().Authentications(), + featureGateAccessor, ) routerCertsController := routercerts.NewRouterCertsDomainValidationController( @@ -281,7 +283,10 @@ func prepareOauthOperator( authOperatorInput.eventRecorder, versionRecorder, informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config"), authConfigChecker, + informerFactories.operatorInformer.Operator().V1().Authentications(), + featureGateAccessor, ) workersAvailableController := ingressnodesavailable.NewIngressNodesAvailableController( @@ -302,8 +307,11 @@ func prepareOauthOperator( authOperatorInput.authenticationOperatorClient, informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"), informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config-managed"), + informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config"), informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), informerFactories.operatorConfigInformer.Config().V1().Ingresses(), + informerFactories.operatorInformer.Operator().V1().Authentications(), + featureGateAccessor, authConfigChecker, systemCABundle, authOperatorInput.eventRecorder, @@ -335,6 +343,9 @@ func prepareOauthOperator( }, authOperatorInput.eventRecorder, authOperatorInput.authenticationOperatorClient, + informerFactories.operatorConfigInformer.Config().V1().OAuths().Lister(), + informerFactories.operatorInformer.Operator().V1().Authentications(), + featureGateAccessor, ) customRouteController := componentroutesecretsync.NewCustomRouteController( @@ -347,6 +358,8 @@ func prepareOauthOperator( informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(), authOperatorInput.routeClient.RouteV1().Routes("openshift-authentication"), informerFactories.kubeInformersForNamespaces, + informerFactories.operatorInformer.Operator().V1().Authentications(), + featureGateAccessor, authOperatorInput.authenticationOperatorClient, authConfigChecker, authOperatorInput.eventRecorder, diff --git a/pkg/transport/transport.go b/pkg/transport/transport.go index cd2f525418..99cafd4414 100644 --- a/pkg/transport/transport.go +++ b/pkg/transport/transport.go @@ -6,64 +6,125 @@ import ( "errors" "fmt" "net/http" + "net/url" - "k8s.io/apimachinery/pkg/util/net" + "golang.org/x/net/http/httpproxy" + + knet "k8s.io/apimachinery/pkg/util/net" corelistersv1 "k8s.io/client-go/listers/core/v1" ktransport "k8s.io/client-go/transport" ) // TODO move all this to library-go -// TransportFor returns an http.Transport for the given ca and client cert data (which may be empty) +// ProxyFunc returns the proxy URL for a given request URL. +type ProxyFunc func(reqURL *url.URL) (*url.URL, error) + +// TransportFor returns an http.Transport for the given CA and client cert data (which may be empty). func TransportFor(serverName string, caData, certData, keyData []byte) (http.RoundTripper, error) { - transport, err := transportForInner(serverName, caData, certData, keyData) + if len(caData) == 0 && len(certData) == 0 && len(keyData) == 0 { + return ktransport.DebugWrappers(http.DefaultTransport), nil + } + transport, err := newTransport(serverName, caData, certData, keyData) if err != nil { return nil, err } return ktransport.DebugWrappers(transport), nil } -func TransportForCARef(cmLister corelistersv1.ConfigMapLister, caConfigMapName, key string) (http.RoundTripper, error) { - if len(caConfigMapName) == 0 { +// CAReference identifies a CA bundle stored in a ConfigMap. +type CAReference struct { + ConfigMapName string + ConfigMapKey string +} + +// TransportForCARef creates an http.RoundTripper with TLS configured from +// the given CA ConfigMap references and explicit proxy settings. Each +// CAReference is loaded via the lister and appended to the trust pool. +// When httpProxy or httpsProxy is non-empty, the transport routes requests +// through the proxy. When both are empty, no proxy is used. +func TransportForCARef( + cmLister corelistersv1.ConfigMapLister, + caRefs []CAReference, + httpProxy, httpsProxy, noProxy string, +) (http.RoundTripper, error) { + var caData []byte + for _, ref := range caRefs { + data, err := LoadCAData(cmLister, ref.ConfigMapName, ref.ConfigMapKey) + if err != nil { + return nil, err + } + caData = append(caData, data...) + } + + if len(caData) == 0 && len(httpProxy) == 0 && len(httpsProxy) == 0 { return TransportFor("", nil, nil, nil) } - cm, err := cmLister.ConfigMaps("openshift-config").Get(caConfigMapName) + transport, err := newTransport("", caData, nil, nil) if err != nil { return nil, err } + + if len(httpProxy) > 0 || len(httpsProxy) > 0 { + proxyCfg := httpproxy.Config{ + HTTPProxy: httpProxy, + HTTPSProxy: httpsProxy, + NoProxy: noProxy, + } + proxyFunc := proxyCfg.ProxyFunc() + transport.Proxy = func(req *http.Request) (*url.URL, error) { + return proxyFunc(req.URL) + } + } else { + transport.Proxy = nil + } + + return ktransport.DebugWrappers(transport), nil +} + +// LoadCAData reads CA bundle bytes from a ConfigMap in openshift-config. +// It checks Data first and falls back to BinaryData for the given key. +func LoadCAData(cmLister corelistersv1.ConfigMapLister, caConfigMapName, key string) ([]byte, error) { + cm, err := cmLister.ConfigMaps("openshift-config").Get(caConfigMapName) + if err != nil { + return nil, fmt.Errorf("unable to get configmap \"%s/%s\": %w", "openshift-config", caConfigMapName, err) + } + caData := []byte(cm.Data[key]) if len(caData) == 0 { caData = cm.BinaryData[key] } if len(caData) == 0 { - return nil, fmt.Errorf("config map %s/%s has no ca data at key %s", "openshift-config", caConfigMapName, key) + return nil, fmt.Errorf("configmap \"%s/%s\" has no CA data at key %q", "openshift-config", caConfigMapName, key) } - return TransportFor("", caData, nil, nil) + return caData, nil } -func transportForInner(serverName string, caData, certData, keyData []byte) (http.RoundTripper, error) { - if len(caData) == 0 && len(certData) == 0 && len(keyData) == 0 { - return http.DefaultTransport, nil - } - +// newTransport creates a fresh *http.Transport with TLS configured from the given parameters. +func newTransport(serverName string, caData, certData, keyData []byte) (*http.Transport, error) { if (len(certData) == 0) != (len(keyData) == 0) { return nil, errors.New("cert and key data must be specified together") } // copy default transport - transport := net.SetTransportDefaults(&http.Transport{ + transport := knet.SetTransportDefaults(&http.Transport{ TLSClientConfig: &tls.Config{ ServerName: serverName, }, }) if len(caData) != 0 { - roots := x509.NewCertPool() + roots, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("error loading system cert pool: %w", err) + } + if ok := roots.AppendCertsFromPEM(caData); !ok { // avoid logging data that could contain keys - return nil, errors.New("error loading cert pool from ca data") + return nil, errors.New("error loading cert pool from CA data") } + transport.TLSClientConfig.RootCAs = roots } @@ -73,6 +134,7 @@ func transportForInner(serverName string, caData, certData, keyData []byte) (htt // avoid logging data that will contain keys return nil, errors.New("error loading x509 keypair from cert and key data") } + transport.TLSClientConfig.Certificates = []tls.Certificate{cert} } diff --git a/pkg/transport/transport_test.go b/pkg/transport/transport_test.go new file mode 100644 index 0000000000..0d80b61b42 --- /dev/null +++ b/pkg/transport/transport_test.go @@ -0,0 +1,335 @@ +package transport + +import ( + "crypto/x509" + "encoding/pem" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/authentication/user" + corelistersv1 "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" + + "github.com/openshift/cluster-authentication-operator/pkg/internal/transporttest" +) + +func TestTransportForCARef(t *testing.T) { + _, caPEM := transporttest.MakeSelfSignedCA(t) + _, extraPEM := transporttest.MakeSelfSignedCA(t) + emptyLister := newConfigMapLister() + + ref := func(name, key string) CAReference { return CAReference{ConfigMapName: name, ConfigMapKey: key} } + + t.Run("no refs no proxy returns default transport", func(t *testing.T) { + rt, err := TransportForCARef(emptyLister, nil, "", "", "") + require.NoError(t, err) + require.NotNil(t, rt) + }) + + t.Run("single CA ref is loaded into TLS root CAs", func(t *testing.T) { + lister := newConfigMapLister(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "my-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(caPEM)}, + }) + + rt, err := TransportForCARef(lister, []CAReference{ref("my-ca", "ca-bundle.crt")}, "", "", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + requirePoolContains(t, rootCAs(t, tr), caPEM) + }) + + t.Run("configmap not found returns error", func(t *testing.T) { + _, err := TransportForCARef(emptyLister, []CAReference{ref("missing-cm", "ca-bundle.crt")}, "", "", "") + require.ErrorContains(t, err, `"missing-cm" not found`) + }) + + t.Run("configmap with empty key returns error", func(t *testing.T) { + lister := newConfigMapLister(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "empty-ca", Namespace: "openshift-config"}, + Data: map[string]string{}, + }) + + _, err := TransportForCARef(lister, []CAReference{ref("empty-ca", "ca-bundle.crt")}, "", "", "") + require.ErrorContains(t, err, `has no CA data at key "ca-bundle.crt"`) + }) + + t.Run("multiple CA refs are all loaded", func(t *testing.T) { + lister := newConfigMapLister( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "ca-1", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(caPEM)}, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "ca-2", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(extraPEM)}, + }, + ) + + rt, err := TransportForCARef(lister, []CAReference{ + ref("ca-1", "ca-bundle.crt"), + ref("ca-2", "ca-bundle.crt"), + }, "", "", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + requirePoolContains(t, rootCAs(t, tr), caPEM, extraPEM) + }) + + t.Run("BinaryData key is used when Data key is absent", func(t *testing.T) { + lister := newConfigMapLister(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "binary-ca", Namespace: "openshift-config"}, + BinaryData: map[string][]byte{"ca-bundle.crt": caPEM}, + }) + + rt, err := TransportForCARef(lister, []CAReference{ref("binary-ca", "ca-bundle.crt")}, "", "", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + requirePoolContains(t, rootCAs(t, tr), caPEM) + }) + + t.Run("HTTP proxy is set on transport", func(t *testing.T) { + rt, err := TransportForCARef(emptyLister, nil, + "http://proxy.example.com:8080", "", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + require.NotNil(t, tr.Proxy) + + proxyURL, err := tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "http://target.example.com")}) + require.NoError(t, err) + require.Equal(t, "http://proxy.example.com:8080", proxyURL.String()) + + proxyURL, err = tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "https://target.example.com")}) + require.NoError(t, err) + require.Nil(t, proxyURL, "HTTPS request should not use HTTP proxy") + }) + + t.Run("HTTPS proxy is set on transport", func(t *testing.T) { + rt, err := TransportForCARef(emptyLister, nil, + "", "https://secure-proxy.example.com:443", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + require.NotNil(t, tr.Proxy) + + proxyURL, err := tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "https://target.example.com")}) + require.NoError(t, err) + require.Equal(t, "https://secure-proxy.example.com:443", proxyURL.String()) + + proxyURL, err = tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "http://target.example.com")}) + require.NoError(t, err) + require.Nil(t, proxyURL, "HTTP request should not use HTTPS proxy") + }) + + t.Run("noProxy excludes matching hosts from proxy", func(t *testing.T) { + rt, err := TransportForCARef(emptyLister, nil, + "http://proxy.example.com:8080", "http://proxy.example.com:8080", + "noproxy.example.com") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + require.NotNil(t, tr.Proxy) + + proxyURL, err := tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "http://noproxy.example.com/path")}) + require.NoError(t, err) + require.Nil(t, proxyURL, "request matching noProxy should not be proxied") + + proxyURL, err = tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "http://other.example.com/path")}) + require.NoError(t, err) + require.NotNil(t, proxyURL, "request not matching noProxy should be proxied") + }) + + t.Run("proxy with multiple CA refs", func(t *testing.T) { + lister := newConfigMapLister( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "idp-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(caPEM)}, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(extraPEM)}, + }, + ) + + rt, err := TransportForCARef(lister, []CAReference{ + ref("idp-ca", "ca-bundle.crt"), + ref("proxy-ca", "ca-bundle.crt"), + }, "http://proxy.example.com:8080", "", "") + require.NoError(t, err) + + tr := transporttest.UnwrapTransport(t, rt) + require.NotNil(t, tr.Proxy) + requirePoolContains(t, rootCAs(t, tr), caPEM, extraPEM) + }) +} + +func TestNewTransport(t *testing.T) { + _, caPEM := transporttest.MakeSelfSignedCA(t) + + t.Run("nil caData returns transport without RootCAs", func(t *testing.T) { + tr, err := newTransport("", nil, nil, nil) + require.NoError(t, err) + require.Nil(t, rootCAs(t, tr)) + }) + + t.Run("cert without key returns error", func(t *testing.T) { + _, err := newTransport("", nil, []byte("cert"), nil) + require.ErrorContains(t, err, "cert and key data must be specified together") + }) + + t.Run("key without cert returns error", func(t *testing.T) { + _, err := newTransport("", nil, nil, []byte("key")) + require.ErrorContains(t, err, "cert and key data must be specified together") + }) + + t.Run("valid CA configures RootCAs", func(t *testing.T) { + tr, err := newTransport("", caPEM, nil, nil) + require.NoError(t, err) + requirePoolContains(t, rootCAs(t, tr), caPEM) + }) + + t.Run("server name is propagated", func(t *testing.T) { + tr, err := newTransport("my-server", nil, nil, nil) + require.NoError(t, err) + require.NotNil(t, tr.TLSClientConfig) + require.Equal(t, "my-server", tr.TLSClientConfig.ServerName) + }) + + t.Run("valid cert and key pair is loaded", func(t *testing.T) { + ca, _ := transporttest.MakeSelfSignedCA(t) + + clientCfg, err := ca.MakeClientCertificateForDuration(&user.DefaultInfo{Name: "test-client"}, time.Hour) + require.NoError(t, err) + + certPEM, keyPEM, err := clientCfg.GetPEMBytes() + require.NoError(t, err) + + tr, err := newTransport("", nil, certPEM, keyPEM) + require.NoError(t, err) + require.NotNil(t, tr.TLSClientConfig) + require.Len(t, tr.TLSClientConfig.Certificates, 1) + + block, _ := pem.Decode(certPEM) + require.NotNil(t, block) + require.Equal(t, block.Bytes, tr.TLSClientConfig.Certificates[0].Certificate[0]) + }) + + t.Run("invalid cert and key pair returns error", func(t *testing.T) { + _, err := newTransport("", nil, []byte("bad-cert"), []byte("bad-key")) + require.ErrorContains(t, err, "error loading x509 keypair from cert and key data") + }) +} + +func TestLoadCAData(t *testing.T) { + caData := []byte("-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----") + + tests := []struct { + name string + cms []*corev1.ConfigMap + cmName string + cmKey string + want []byte + wantErr string + }{ + { + name: "returns Data value", + cms: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Name: "my-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": string(caData)}, + }}, + cmName: "my-ca", + cmKey: "ca-bundle.crt", + want: caData, + }, + { + name: "falls back to BinaryData", + cms: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Name: "my-ca", Namespace: "openshift-config"}, + BinaryData: map[string][]byte{"ca-bundle.crt": caData}, + }}, + cmName: "my-ca", + cmKey: "ca-bundle.crt", + want: caData, + }, + { + name: "Data takes precedence over BinaryData", + cms: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Name: "my-ca", Namespace: "openshift-config"}, + Data: map[string]string{"ca-bundle.crt": "from-data"}, + BinaryData: map[string][]byte{"ca-bundle.crt": []byte("from-binary")}, + }}, + cmName: "my-ca", + cmKey: "ca-bundle.crt", + want: []byte("from-data"), + }, + { + name: "configmap not found", + cmName: "nonexistent", + cmKey: "ca-bundle.crt", + wantErr: `unable to get configmap "openshift-config/nonexistent": configmap "nonexistent" not found`, + }, + { + name: "key missing from both Data and BinaryData", + cms: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Name: "my-ca", Namespace: "openshift-config"}, + Data: map[string]string{"other-key": "value"}, + }}, + cmName: "my-ca", + cmKey: "ca-bundle.crt", + wantErr: `configmap "openshift-config/my-ca" has no CA data at key "ca-bundle.crt"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lister := newConfigMapLister(tt.cms...) + got, err := LoadCAData(lister, tt.cmName, tt.cmKey) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} + +func newConfigMapLister(cms ...*corev1.ConfigMap) corelistersv1.ConfigMapLister { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, cm := range cms { + _ = indexer.Add(cm) + } + return corelistersv1.NewConfigMapLister(indexer) +} + +func rootCAs(t *testing.T, tr *http.Transport) *x509.CertPool { + t.Helper() + require.NotNil(t, tr.TLSClientConfig) + return tr.TLSClientConfig.RootCAs +} + +func requirePoolContains(t *testing.T, pool *x509.CertPool, pemData ...[]byte) { + t.Helper() + require.NotNil(t, pool) + opts := x509.VerifyOptions{Roots: pool} + for _, p := range pemData { + cert := mustCertFromPEM(t, p) + _, err := cert.Verify(opts) + require.NoError(t, err) + } +} + +func mustCertFromPEM(t *testing.T, data []byte) *x509.Certificate { + t.Helper() + block, _ := pem.Decode(data) + require.NotNil(t, block, "no PEM block found") + cert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + return cert +} diff --git a/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/810c-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml b/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/9806-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml similarity index 66% rename from test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/810c-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml rename to test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/9806-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml index 2e16ed8280..dbcd556f20 100644 --- a/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/810c-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml +++ b/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/9806-body-system-COLON-openshift-COLON-openshift-authenticator-.yaml @@ -5,7 +5,7 @@ metadata: labels: authentication.openshift.io/csr: openshift-authenticator spec: - request: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURSBSRVFVRVNULS0tLS0KTUlJQkRUQ0J0QUlCQURCU01WQXdUZ1lEVlFRREUwZHplWE4wWlcwNmMyVnlkbWxqWldGalkyOTFiblE2YjNCbApibk5vYVdaMExXOWhkWFJvTFdGd2FYTmxjblpsY2pwdmNHVnVjMmhwWm5RdFlYVjBhR1Z1ZEdsallYUnZjakJaCk1CTUdCeXFHU000OUFnRUdDQ3FHU000OUF3RUhBMElBQkw2VkhyZENabnBmaEp2Wnl0R09lWlhRVEhsQ3RSVE0KT1ZPRGY3aGI1SnJabjRNYzZuQzFRdkExM3NjaWxIVUZ3ei9oNXdsV25kalBTSE5qeFZ1K05HR2dBREFLQmdncQpoa2pPUFFRREFnTklBREJGQWlBWmljc3FzNHZITjFCV0ViZGxkZzNSYStGZmtrelRUZ3h0eEpjZDFqNVprZ0loCkFMSit3YjhxRytJWjdoZ2ZiY24wYzFiV2xYSDM3SE1PbXArcFpDUHN4bjhWCi0tLS0tRU5EIENFUlRJRklDQVRFIFJFUVVFU1QtLS0tLQo= + request: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURSBSRVFVRVNULS0tLS0KTUlJQkRUQ0J0QUlCQURCU01WQXdUZ1lEVlFRREUwZHplWE4wWlcwNmMyVnlkbWxqWldGalkyOTFiblE2YjNCbApibk5vYVdaMExXOWhkWFJvTFdGd2FYTmxjblpsY2pwdmNHVnVjMmhwWm5RdFlYVjBhR1Z1ZEdsallYUnZjakJaCk1CTUdCeXFHU000OUFnRUdDQ3FHU000OUF3RUhBMElBQkJZQ2wyaXdqamgwS2xDb0JhbklLRVB4SXVaOEFuNFcKNm96TGNQdWhKWDkwSmgrQzVmLzVyWEtZRkZRdFFrMnZ2QlJYSUhyNlhJUVhYUlZWUXNOMnA4bWdBREFLQmdncQpoa2pPUFFRREFnTklBREJGQWlFQXFDZTBjOFVEQXRPdlVQSjhvRTZXcUFqdk9BUkIrakxOVG5GR3dDNFdnY1VDCklHeU15TGY2YVVTRUVERmUvQ2F5Vm5nQ2o2bDJ5a1JBLzJuckhZc0hpYUpsCi0tLS0tRU5EIENFUlRJRklDQVRFIFJFUVVFU1QtLS0tLQo= signerName: kubernetes.io/kube-apiserver-client usages: - digital signature diff --git a/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/810c-metadata-system-COLON-openshift-COLON-openshift-authenticator-.yaml b/test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/9806-metadata-system-COLON-openshift-COLON-openshift-authenticator-.yaml similarity index 100% rename from test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/810c-metadata-system-COLON-openshift-COLON-openshift-authenticator-.yaml rename to test-data/apply-configuration/overall/minimal-cluster/expected-output/UserWorkload/Create/cluster-scoped-resources/certificates.k8s.io/certificatesigningrequests/9806-metadata-system-COLON-openshift-COLON-openshift-authenticator-.yaml diff --git a/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/2280-body-oauth-openshift.yaml b/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/b3b2-body-oauth-openshift.yaml similarity index 94% rename from test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/2280-body-oauth-openshift.yaml rename to test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/b3b2-body-oauth-openshift.yaml index b90f7962ab..c84c848623 100644 --- a/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/2280-body-oauth-openshift.yaml +++ b/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/b3b2-body-oauth-openshift.yaml @@ -2,8 +2,8 @@ apiVersion: apps/v1 kind: Deployment metadata: annotations: - operator.openshift.io/rvs-hash: f4V-TOKKLhC7zxXahsybviIQ6XFZf_Ua2SFe2jckw9gL4UuCiEXYmFPtjUvFGC13xB72tEYqR0N1somiZq0-JQ - operator.openshift.io/spec-hash: 0a0b624d81acaa84b703ae631aa8552ec31f538924445ffe696cf9a6c39d7dac + operator.openshift.io/rvs-hash: Yj6uU3fZPpZ50nwdBul3p49-JwgX5Sns2Ui5BAoiC-VwIumwdefgcFdMwEya9wke3gaB2ZRCP990BhbUBThRIw + operator.openshift.io/spec-hash: 51680330e02df4ec8b4c856301ac4881110b29e76b501b75050a79bd3ebb73ef labels: app: oauth-openshift name: oauth-openshift @@ -22,7 +22,7 @@ spec: metadata: annotations: openshift.io/required-scc: privileged - operator.openshift.io/rvs-hash: f4V-TOKKLhC7zxXahsybviIQ6XFZf_Ua2SFe2jckw9gL4UuCiEXYmFPtjUvFGC13xB72tEYqR0N1somiZq0-JQ + operator.openshift.io/rvs-hash: Yj6uU3fZPpZ50nwdBul3p49-JwgX5Sns2Ui5BAoiC-VwIumwdefgcFdMwEya9wke3gaB2ZRCP990BhbUBThRIw target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' labels: app: oauth-openshift diff --git a/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/2280-metadata-oauth-openshift.yaml b/test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/b3b2-metadata-oauth-openshift.yaml similarity index 100% rename from test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/2280-metadata-oauth-openshift.yaml rename to test-data/apply-configuration/overall/oauth-server-creation-minimal/expected-output/Management/Create/namespaces/openshift-authentication/apps/deployments/b3b2-metadata-oauth-openshift.yaml diff --git a/test/e2e-component-proxy/component_proxy.go b/test/e2e-component-proxy/component_proxy.go new file mode 100644 index 0000000000..4262edc9a0 --- /dev/null +++ b/test/e2e-component-proxy/component_proxy.go @@ -0,0 +1,387 @@ +package component_proxy + +import ( + "context" + "fmt" + "time" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/operator/v1helpers" + + test "github.com/openshift/cluster-authentication-operator/test/library" +) + +var _ = g.Describe("[sig-auth] authentication operator", func() { + g.It("[Serial][Operator][ComponentProxy] should validate OIDC IdP through component proxy", func() { + testOIDCIdPThroughComponentProxy(false) + }) + g.It("[Serial][Operator][ComponentProxy] should validate OIDC IdP through component proxy with trustedCA", func() { + testOIDCIdPThroughComponentProxy(true) + }) + g.It("[Serial][Operator][ComponentProxy] should fall back on spec.proxy removal", func() { + testFallbackOnProxyRemoval() + }) + g.It("[Serial][Operator][ComponentProxy] should set Degraded when spec.proxy points to an unreachable proxy", func() { + testDegradedOnBadProxyURL() + }) + g.It("[Serial][Operator][ComponentProxy] should emit IdPEndpointUnreachable warning when IdP is unreachable through proxy", func() { + testWarningOnUnreachableIdP() + }) +}) + +func testOIDCIdPThroughComponentProxy(withTrustedCA bool) { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + + g.By("Creating test clients") + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(proxyCleanup) + + var proxyURL string + const trustedCAConfigMapName = "e2e-proxy-ca" + if withTrustedCA { + proxyURL = httpsProxyURL + + g.By("Creating trustedCA ConfigMap in openshift-config") + _, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCAConfigMapName, + Labels: test.CAOE2ETestLabels(), + }, + Data: map[string]string{ + "ca-bundle.crt": string(caCertPEM), + }, + }, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing trustedCA ConfigMap") + _ = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, trustedCAConfigMapName, metav1.DeleteOptions{}) + }) + } else { + proxyURL = httpProxyURL + } + g.GinkgoWriter.Printf("Squid proxy URL: %s\n", proxyURL) + + g.By("Deploying Keycloak (without registering IdP yet)") + kcSetup := test.DeployKeycloak(t, kubeConfig) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak") + for _, cleanup := range kcSetup.Cleanups { + cleanup() + } + })) + g.GinkgoWriter.Printf("Keycloak issuer URL: %s\n", kcSetup.IssuerURL) + g.GinkgoWriter.Printf("Keycloak namespace: %s\n", kcSetup.Namespace) + + g.By("Deploying NetworkPolicy to restrict Keycloak ingress to proxy namespace only") + networkPolicyCleanup := test.DeployProxyNetworkPolicies(t, clients.KubeClient, proxyNamespace, kcSetup.Namespace) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing proxy NetworkPolicies") + networkPolicyCleanup() + }) + + g.By("Setting component-scoped proxy") + operatorAuth, proxyRestore := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyRestore) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: proxyURL, + } + if withTrustedCA { + operatorAuth.Spec.Proxy.TrustedCA = operatorv1.AuthenticationConfigMapReference{Name: trustedCAConfigMapName} + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") + idpCleanups := test.AddKeycloakOIDCIdP(t, kubeConfig, kcSetup, false) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing OIDC IdP") + for _, cleanup := range idpCleanups { + cleanup() + } + })) + + g.By("Verifying operator is Available and not Degraded") + err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying OAuth server deployment has proxy env vars and trustedCA volume/mount") + test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, "", proxyURL, ".cluster.local,.svc,127.0.0.1,localhost", withTrustedCA) + + if withTrustedCA { + g.By("Verifying trustedCA ConfigMap was synced to openshift-authentication") + test.VerifyTrustedCAConfigMapSynced(t, clients.KubeClient, trustedCAConfigMapName) + } + + g.By("Verifying traffic went through the Squid proxy") + err = test.WaitForSquidProxyTraffic(t, clients.KubeClient, proxyNamespace, 5*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) +} + +// No NetworkPolicy is deployed here intentionally: after proxy removal the +// operator must fall back to direct connectivity, so Keycloak must remain +// reachable without a proxy. +func testFallbackOnProxyRemoval() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + + g.By("Creating test clients") + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + httpProxyURL, _, _, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(proxyCleanup) + g.GinkgoWriter.Printf("Squid proxy URL: %s\n", httpProxyURL) + + g.By("Saving original proxy config and setting component-scoped proxy") + operatorAuth, proxyRestore := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyRestore) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Keycloak and adding OIDC IdP") + _, _, keycloakCleanups := test.AddKeycloakIDP(t, kubeConfig, false) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + g.By("Verifying operator is stable with proxy configured") + err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + // Removing spec.proxy causes the operator to contact Keycloak again. + g.By("Removing spec.proxy from Authentication CR") + operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{} + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to pick up proxy removal and stabilize") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying proxy env vars are no longer set on OAuth server deployment") + test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, "", "", "", false) +} + +func testDegradedOnBadProxyURL() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + + g.By("Creating test clients") + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + httpProxyURL, _, _, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(proxyCleanup) + + g.By("Saving original proxy config and setting a working proxy") + operatorAuth, proxyRestore := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyRestore) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Keycloak and adding OIDC IdP") + _, _, keycloakCleanups := test.AddKeycloakIDP(t, kubeConfig, false) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + g.By("Verifying operator is stable with working proxy") + err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Setting spec.proxy.httpsProxy to an unreachable host") + operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: "http://does-not-exist.invalid:3128", + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for ProxyConfigControllerDegraded=True on the operator CR") + var lastCondition *operatorv1.OperatorCondition + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + config, err := clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + g.GinkgoWriter.Printf("failed to get operator auth: %v\n", err) + return false, nil + } + lastCondition = v1helpers.FindOperatorCondition(config.Status.Conditions, "ProxyConfigControllerDegraded") + return lastCondition != nil && lastCondition.Status == operatorv1.ConditionTrue, nil + }) + o.Expect(err).NotTo(o.HaveOccurred(), "ProxyConfigControllerDegraded never became True") + g.GinkgoWriter.Printf("ProxyConfigControllerDegraded: status=%s reason=%s message=%s\n", lastCondition.Status, lastCondition.Reason, lastCondition.Message) + + g.By("Verifying ClusterOperator authentication is Degraded") + err = test.WaitForClusterOperatorDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) +} + +func testWarningOnUnreachableIdP() { + ctx := context.Background() + t := g.GinkgoTB() + + g.By("Creating test clients") + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + httpProxyURL, _, _, proxyNamespace, squidCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(squidCleanup) + proxyURL := httpProxyURL + g.GinkgoWriter.Printf("Squid proxy URL: %s\n", proxyURL) + + g.By("Saving original proxy config for cleanup") + operatorAuth, proxyRestore := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + + const ( + fakeIDPName = "e2e-unreachable-idp" + fakeIDPSecretName = "e2e-unreachable-idp-secret" + ) + + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing fake IdP from OAuth config") + test.CleanIDPConfigByName(t, clients.ConfigClient.ConfigV1().OAuths(), fakeIDPName) + + g.GinkgoWriter.Println("cleaning up: deleting fake IdP secret") + _ = clients.KubeClient.CoreV1().Secrets("openshift-config").Delete(ctx, fakeIDPSecretName, metav1.DeleteOptions{}) + + proxyRestore() + }) + + g.By("Creating fake IdP client secret in openshift-config") + _, err = clients.KubeClient.CoreV1().Secrets("openshift-config").Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: fakeIDPSecretName, + Labels: test.CAOE2ETestLabels(), + }, + Data: map[string][]byte{ + "clientSecret": []byte("fake-secret"), + }, + }, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Adding fake OpenID IdP to OAuth config") + oauthConfig, err := clients.ConfigClient.ConfigV1().OAuths().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + oauthCopy := oauthConfig.DeepCopy() + oauthCopy.Spec.IdentityProviders = append(oauthCopy.Spec.IdentityProviders, configv1.IdentityProvider{ + Name: fakeIDPName, + MappingMethod: configv1.MappingMethodClaim, + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeOpenID, + OpenID: &configv1.OpenIDIdentityProvider{ + ClientID: "fake-client", + ClientSecret: configv1.SecretNameReference{ + Name: fakeIDPSecretName, + }, + Issuer: "https://unreachable-idp.invalid", + Claims: configv1.OpenIDClaims{ + PreferredUsername: []string{"preferred_username"}, + }, + }, + }, + }) + _, err = clients.ConfigClient.ConfigV1().OAuths().Update(ctx, oauthCopy, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Setting component-scoped proxy pointing to the Squid instance") + startTime := time.Now() + operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: proxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for IdPEndpointUnreachable warning event") + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + events, err := clients.KubeClient.CoreV1().Events("openshift-authentication-operator").List(ctx, metav1.ListOptions{ + FieldSelector: "reason=IdPEndpointUnreachable", + }) + if err != nil { + g.GinkgoWriter.Printf("failed to list events: %v\n", err) + return false, nil + } + for _, event := range events.Items { + eventTime := event.LastTimestamp.Time + if eventTime.IsZero() { + eventTime = event.EventTime.Time + } + if event.Type == "Warning" && eventTime.After(startTime) { + g.GinkgoWriter.Printf("found IdPEndpointUnreachable event: %s\n", event.Message) + return true, nil + } + } + return false, nil + }) + o.Expect(err).NotTo(o.HaveOccurred(), "IdPEndpointUnreachable warning event was not emitted") + + g.By("Verifying the request went through the Squid proxy") + err = test.WaitForSquidProxyTraffic(t, clients.KubeClient, proxyNamespace, 2*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying operator is NOT Degraded") + ok, conditions, checkErr := test.CheckClusterOperatorStatus(t, ctx, clients.ConfigClient.ConfigV1(), "authentication", + configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorDegraded, Status: configv1.ConditionFalse}, + ) + o.Expect(checkErr).NotTo(o.HaveOccurred()) + o.Expect(ok).To(o.BeTrue(), fmt.Sprintf("operator should NOT be degraded, conditions: %v", conditions)) +} diff --git a/test/e2e-component-proxy/component_proxy_oidc_login.go b/test/e2e-component-proxy/component_proxy_oidc_login.go new file mode 100644 index 0000000000..bc67a6d054 --- /dev/null +++ b/test/e2e-component-proxy/component_proxy_oidc_login.go @@ -0,0 +1,575 @@ +package component_proxy + +import ( + "context" + "crypto/x509" + "encoding/pem" + "fmt" + "io" + "net/url" + "os/exec" + "testing" + "time" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + authnv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + + "github.com/openshift/api/features" + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/oauth/tokenrequest" + "github.com/openshift/library-go/pkg/oauth/tokenrequest/challengehandlers" + + test "github.com/openshift/cluster-authentication-operator/test/library" +) + +var _ = g.Describe("[sig-auth] authentication operator", func() { + g.It("[Serial][Operator][ComponentProxy] should set partial and full env vars when configured", func() { + testPartialFullProxyEnvVars() + }) + g.It("[Serial][Operator][ComponentProxy] should apply proxy config and perform full OIDC login flow", func() { + testProxyOIDCLoginFlow() + }) + g.It("[Serial][Operator][ComponentProxy] should fall back to direct IdP connectivity for OIDC login", func() { + testDirectIdPFallback() + }) + g.It("[Serial][Operator][ComponentProxy] should hot-reload mounted CA file on change when spec.proxy.trustedCA is set", func() { + testTrustedCAHotReload() + }) + g.It("[Serial][Operator][ComponentProxy] should bypass proxy for noProxy hosts", func() { + testNoProxy() + }) +}) + +func assertOIDCLogin(t testing.TB, kubeConfig *rest.Config, routeClient test.TestClients, username, password, expectedGroup string) { + g.GinkgoHelper() + ctx := context.Background() + + route, err := routeClient.RouteClient.RouteV1().Routes("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to get the OAuth server route") + oauthServerURL := fmt.Sprintf("https://%s", route.Spec.Host) + + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + tokenOpts := tokenrequest.NewRequestTokenOptions(kubeConfig, false) + tokenOpts, err := tokenOpts.WithChallengeHandlers( + challengehandlers.NewBasicChallengeHandler(oauthServerURL, "", nil, io.Discard, nil, username, password), + ) + if err != nil { + t.Logf("failed to create challenge handler: %v", err) + return false, nil + } + + if err := tokenOpts.SetDefaultOsinConfig("openshift-challenging-client", nil); err != nil { + t.Logf("failed to discover OAuth metadata: %v", err) + return false, nil + } + + token, err := tokenOpts.RequestToken() + if err != nil { + t.Logf("failed to request token: %v", err) + return false, nil + } + if token == "" { + t.Log("received empty token") + return false, nil + } + + tokenConfig := rest.AnonymousClientConfig(kubeConfig) + tokenConfig.BearerToken = token + tokenKubeClient, err := kubernetes.NewForConfig(tokenConfig) + if err != nil { + t.Logf("failed to create kube client with token: %v", err) + return false, nil + } + + ssr, err := tokenKubeClient.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + t.Logf("failed to create SelfSubjectReview: %v", err) + return false, nil + } + + if ssr.Status.UserInfo.Username == "" { + t.Log("SelfSubjectReview returned empty username") + return false, nil + } + + for _, g := range ssr.Status.UserInfo.Groups { + if g == expectedGroup { + return true, nil + } + } + t.Logf("expected group %q not found in groups: %v", expectedGroup, ssr.Status.UserInfo.Groups) + return false, nil + }) + o.Expect(err).NotTo(o.HaveOccurred(), "OIDC login flow should succeed") +} + +func testPartialFullProxyEnvVars() { + ctx := context.Background() + t := g.GinkgoTB() + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + httpProxyURL, httpsProxyURL, caPEM, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + proxyCleanup() + }) + + g.By("Saving original proxy config") + operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyConfigCleanup) + + g.By("Setting only httpsProxy in component proxy config") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying oauth-server has HTTPS_PROXY but not HTTP_PROXY") + test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, "", httpProxyURL, ".cluster.local,.svc,127.0.0.1,localhost", false) + + configMapName := "e2e-proxy-trusted-ca" + caConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configMapName, + Namespace: "openshift-config", + }, + Data: map[string]string{ + "ca-bundle.crt": string(caPEM), + }, + } + _, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to create trustedCA ConfigMap") + g.DeferCleanup(func() { + if err := clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, configMapName, metav1.DeleteOptions{}); err != nil { + g.GinkgoWriter.Printf("failed to clean up ConfigMap %s: %v\n", configMapName, err) + } + }) + + noProxyHost := "noproxy.example.com" + + operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Setting httpProxy, httpsProxy, noProxy, and trustedCA in component proxy config") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPProxy: httpProxyURL, + HTTPSProxy: httpsProxyURL, + NoProxy: []string{noProxyHost}, + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: configMapName, + }, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying oauth-server has HTTP_PROXY, HTTPS_PROXY, and NO_PROXY with custom entry") + test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, httpProxyURL, httpsProxyURL, ".cluster.local,.svc,127.0.0.1,localhost,noproxy.example.com", true) +} + +func testProxyOIDCLoginFlow() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + _, httpsProxyURL, _, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + proxyCleanup() + }) + + g.By("Saving original proxy config and setting component-scoped proxy") + operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyConfigCleanup) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpsProxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Keycloak and adding OIDC IdP") + + setup := test.DeployKeycloak(t, kubeConfig) + keycloakCleanups := setup.Cleanups + + idpCleans := test.AddKeycloakOIDCIdP(t, kubeConfig, setup, false) + keycloakCleanups = append(keycloakCleanups, idpCleans...) + + kcClient := setup.Client + + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + g.By("Deploying NetworkPolicy to restrict Keycloak ingress to proxy namespace only") + keycloakNamespace := setup.Namespace + networkPolicyCleanup := test.DeployProxyNetworkPolicies(t, clients.KubeClient, proxyNamespace, keycloakNamespace) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing proxy NetworkPolicies") + networkPolicyCleanup() + }) + + g.By("Waiting for operator to reconcile proxy config") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Creating Keycloak test user and group") + group := "ocp-test-proxy-login-group" + o.Expect(kcClient.CreateGroup(group)).To(o.Succeed()) + + username := "proxy-login-test-user" + password := "proxy-login-test-password" + o.Expect(kcClient.CreateUser(username, "", password, []string{group}, nil)).To(o.Succeed()) + + g.By("Performing full OIDC login flow through component proxy") + assertOIDCLogin(t, kubeConfig, *clients, username, password, group) + + g.By("Verifying traffic went through the Squid proxy") + err = test.WaitForSquidProxyTraffic(t, clients.KubeClient, proxyNamespace, 5*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) +} + +func testDirectIdPFallback() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + _, httpsProxyURL, _, proxyNS, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + proxyCleanup() + }) + + g.By("Saving original proxy config") + operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyConfigCleanup) + + g.By("Setting component-scoped proxy config") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpsProxyURL, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Keycloak and adding OIDC IdP") + kcClient, _, keycloakCleanups := test.AddKeycloakIDP(t, kubeConfig, false) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + issuerURL, err := url.Parse(kcClient.IssuerURL()) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Creating Keycloak test user and group") + group := "ocp-test-direct-fallback-group" + o.Expect(kcClient.CreateGroup(group)).To(o.Succeed()) + + username := "direct-fallback-test-user" + password := "direct-fallback-test-password" + o.Expect(kcClient.CreateUser(username, "", password, []string{group}, nil)).To(o.Succeed()) + + g.By("Removing component-scoped proxy config") + operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{} + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy removal") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Performing OIDC login flow via direct IdP connectivity after proxy removal") + assertOIDCLogin(t, kubeConfig, *clients, username, password, group) + + logs, err := test.GetSquidProxyLogs(clients.KubeClient, proxyNS) + o.Expect(err).NotTo(o.HaveOccurred()) + + keycloakHost := issuerURL.Hostname() + o.Expect(logs).NotTo(o.ContainSubstring(keycloakHost), "squid logs should not contain keycloak connect") +} + +func testTrustedCAHotReload() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + _, httpsProxyURL, caFile, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + proxyCleanup() + }) + + g.By("Saving original proxy config") + operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyConfigCleanup) + + g.By("Deploying Keycloak and adding OIDC IdP") + + setup := test.DeployKeycloak(t, kubeConfig) + keycloakCleanups := setup.Cleanups + + idpCleans := test.AddKeycloakOIDCIdP(t, kubeConfig, setup, false) + keycloakCleanups = append(keycloakCleanups, idpCleans...) + + kcClient := setup.Client + + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + keycloakNamespace := setup.Namespace + networkPolicyCleanup := test.DeployProxyNetworkPolicies(t, clients.KubeClient, proxyNamespace, keycloakNamespace) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing proxy NetworkPolicies") + networkPolicyCleanup() + }) + + configMapName := "e2e-proxy-trusted-ca" + caConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configMapName, + Namespace: "openshift-config", + }, + Data: map[string]string{ + "ca-bundle.crt": string(caFile), + }, + } + _, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to create trustedCA ConfigMap") + g.DeferCleanup(func() { + if err := clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, configMapName, metav1.DeleteOptions{}); err != nil { + g.GinkgoWriter.Printf("failed to clean up ConfigMap %s: %v\n", configMapName, err) + } + }) + + g.By("Setting component-scoped proxy with trustedCA") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpsProxyURL, + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: configMapName, + }, + } + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config with trustedCA") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Creating Keycloak test user and group") + group := "ocp-test-ca-reload-group" + o.Expect(kcClient.CreateGroup(group)).To(o.Succeed()) + + username := "ca-reload-test-user" + password := "ca-reload-test-password" + o.Expect(kcClient.CreateUser(username, "", password, []string{group}, nil)).To(o.Succeed()) + + g.By("Verifying OIDC login works after setting proxy with trustedCA") + assertOIDCLogin(t, kubeConfig, *clients, username, password, group) + + g.By("Verifying traffic went through the Squid proxy") + err = test.WaitForSquidProxyTraffic(t, clients.KubeClient, proxyNamespace, 5*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying trustedCA ConfigMap is synced to openshift-authentication namespace") + test.VerifyTrustedCAConfigMapSynced(t, clients.KubeClient, configMapName) + + g.By("Recording oauth-server pod names before CA rotation") + oauthServerPodList, err := clients.KubeClient.CoreV1().Pods("openshift-authentication").List(ctx, metav1.ListOptions{LabelSelector: "app=oauth-openshift"}) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(oauthServerPodList.Items).NotTo(o.BeEmpty()) + + podNamesBefore := sets.New[string]() + for _, pod := range oauthServerPodList.Items { + podNamesBefore.Insert(pod.Name) + } + + g.By("Rotating CA: generating new CA and server cert") + newCA := test.NewCertificateAuthorityCertificate(t, nil) + serviceDNS := fmt.Sprintf("squid-proxy.%s.svc.cluster.local", proxyNamespace) + newServerCert := test.NewServerCertificate(t, newCA, serviceDNS) + + newCACertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newCA.Certificate.Raw}) + newServerCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newServerCert.Certificate.Raw}) + newServerKeyDER, err := x509.MarshalPKCS8PrivateKey(newServerCert.PrivateKey) + o.Expect(err).NotTo(o.HaveOccurred()) + newServerKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: newServerKeyDER}) + + g.By("Updating squid-tls Secret with rotated cert") + tlsSecret, err := clients.KubeClient.CoreV1().Secrets(proxyNamespace).Get(ctx, "squid-tls", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + tlsSecret.Data["tls.crt"] = newServerCertPEM + tlsSecret.Data["tls.key"] = newServerKeyPEM + _, err = clients.KubeClient.CoreV1().Secrets(proxyNamespace).Update(ctx, tlsSecret, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Reconfiguring Squid to pick up new cert") + squidPods, err := clients.KubeClient.CoreV1().Pods(proxyNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: "app=squid-proxy", + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(squidPods.Items).NotTo(o.BeEmpty()) + + reconfigureCmd := exec.Command("oc", "exec", + "-n", proxyNamespace, + squidPods.Items[0].Name, + "-c", "squid", + "--", "squid", "-k", "reconfigure", + ) + output, err := reconfigureCmd.CombinedOutput() + o.Expect(err).NotTo(o.HaveOccurred(), "squid reconfigure failed: %s", string(output)) + + g.By("Updating trustedCA ConfigMap with new CA") + cm, err := clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Get(ctx, configMapName, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + cm.Data["ca-bundle.crt"] = string(newCACertPEM) + _, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Update(ctx, cm, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying OIDC login works after CA rotation") + assertOIDCLogin(t, kubeConfig, *clients, username, password, group) + + g.By("Verifying traffic went through the Squid proxy") + err = test.WaitForSquidProxyTraffic(t, clients.KubeClient, proxyNamespace, 5*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying oauth-server pods were NOT redeployed after CA rotation") + oauthServerPodListAfter, err := clients.KubeClient.CoreV1().Pods("openshift-authentication").List(ctx, metav1.ListOptions{LabelSelector: "app=oauth-openshift"}) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(oauthServerPodListAfter.Items).NotTo(o.BeEmpty()) + + podNamesAfter := sets.New[string]() + for _, pod := range oauthServerPodListAfter.Items { + podNamesAfter.Insert(pod.Name) + } + + o.Expect(podNamesAfter.Equal(podNamesBefore)).To(o.BeTrue(), "oauth-server pods should not have been redeployed after CA file change") +} + +func testNoProxy() { + ctx := context.Background() + t := g.GinkgoTB() + kubeConfig := test.NewClientConfigForTest(t) + clients := test.NewTestClients(t) + + test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy) + + g.By("Waiting for authentication operator to be stable before test") + err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deploying Squid forward proxy") + _, httpsProxyURL, _, proxyNS, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + proxyCleanup() + }) + + g.By("Saving original proxy config") + operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient) + g.DeferCleanup(proxyConfigCleanup) + + g.By("Deploying Keycloak and adding OIDC IdP") + kcClient, _, keycloakCleanups := test.AddKeycloakIDP(t, kubeConfig, false) + g.DeferCleanup(test.IDPCleanupWrapper(func() { + g.GinkgoWriter.Println("cleaning up: removing Keycloak and IdP") + for _, cleanup := range keycloakCleanups { + cleanup() + } + })) + + issuerURL, err := url.Parse(kcClient.IssuerURL()) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Setting component-scoped proxy with noProxy") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpsProxyURL, + NoProxy: []string{issuerURL.Hostname()}, + } + + _, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Creating Keycloak test user and group") + group := "ocp-test-no-proxy-group" + o.Expect(kcClient.CreateGroup(group)).To(o.Succeed()) + + username := "ca-no-proxy-test-user" + password := "ca-no-proxy-test-password" + o.Expect(kcClient.CreateUser(username, "", password, []string{group}, nil)).To(o.Succeed()) + + g.By("Verifying OIDC login works after setting proxy with noProxy") + assertOIDCLogin(t, kubeConfig, *clients, username, password, group) + + logs, err := test.GetSquidProxyLogs(clients.KubeClient, proxyNS) + o.Expect(err).NotTo(o.HaveOccurred()) + + keycloakHost := issuerURL.Hostname() + o.Expect(logs).NotTo(o.ContainSubstring(keycloakHost), "squid logs should not contain keycloak connect") +} diff --git a/test/library/client.go b/test/library/client.go index 2397cb2313..c23a41db37 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -40,6 +40,10 @@ func NewClientConfigForTest(t testing.TB) *rest.Config { } require.NoError(t, err) + + config.QPS = 40 + config.Burst = 60 + return config } diff --git a/test/library/idpdeployment.go b/test/library/idpdeployment.go index 7c6e082b4d..53369425a1 100644 --- a/test/library/idpdeployment.go +++ b/test/library/idpdeployment.go @@ -307,6 +307,15 @@ func addOIDCIDentityProvider( directExternalOIDC bool) ([]func(), error) { var cleanups []func() + success := false + defer func() { + if !success { + for _, c := range cleanups { + c() + } + } + }() + secretName := idpName + "-secret" _, err := kubeClients.CoreV1().Secrets("openshift-config").Create(context.TODO(), &corev1.Secret{ @@ -321,7 +330,7 @@ func addOIDCIDentityProvider( metav1.CreateOptions{}, ) if err != nil { - return cleanups, fmt.Errorf("failed to create keycloak client secret: %v", err) + return nil, fmt.Errorf("failed to create keycloak client secret: %v", err) } cleanups = append(cleanups, func() { if err := kubeClients.CoreV1().Secrets("openshift-config").Delete(context.TODO(), secretName, metav1.DeleteOptions{}); err != nil { @@ -330,7 +339,6 @@ func addOIDCIDentityProvider( }) caCMName := idpName + "-ca" - // configure the default ingress CA as the CA for the IdP in the openshift-config NS cleanups = append(cleanups, SyncDefaultIngressCAToConfig(t, kubeClients.CoreV1(), caCMName)) if !directExternalOIDC { @@ -354,14 +362,14 @@ func addOIDCIDentityProvider( }, }, }) + cleanups = append(cleanups, idpClean...) if err != nil { - return cleanups, fmt.Errorf("failed to add identity provider to oauth server: %v", err) + return nil, fmt.Errorf("failed to add identity provider to oauth server: %v", err) } - - cleanups = append(cleanups, idpClean...) } - return cleanups, err + success = true + return cleanups, nil } func addIdentityProvider(t testing.TB, configClient *configv1client.ConfigV1Client, idp *configv1.IdentityProvider) ([]func(), error) { diff --git a/test/library/keycloakidp.go b/test/library/keycloakidp.go index 8d3b254879..418a14167e 100644 --- a/test/library/keycloakidp.go +++ b/test/library/keycloakidp.go @@ -27,20 +27,28 @@ import ( routev1client "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" ) -func AddKeycloakIDP( - t testing.TB, - kubeconfig *rest.Config, - directOIDC bool, -) (kcClient *KeycloakClient, idpName string, cleanups []func()) { +// KeycloakSetup holds the results of deploying Keycloak, before the IdP is +// registered in OpenShift. Use AddKeycloakOIDCIdP to register the IdP. +type KeycloakSetup struct { + Client *KeycloakClient + IDPName string + Namespace string + ClientID string + ClientSecret string + IssuerURL string + Cleanups []func() +} + +// DeployKeycloak deploys Keycloak in a test namespace, configures a client and +// group mapper, and returns a KeycloakSetup. The IdP is NOT registered in +// OpenShift — call AddKeycloakOIDCIdP separately when ready. +func DeployKeycloak(t testing.TB, kubeconfig *rest.Config) *KeycloakSetup { kubeClients, err := kubernetes.NewForConfig(kubeconfig) require.NoError(t, err) routeClient, err := routev1client.NewForConfig(kubeconfig) require.NoError(t, err) - configClient, err := configv1client.NewForConfig(kubeconfig) - require.NoError(t, err) - readinessProbe := corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ @@ -66,7 +74,6 @@ func AddKeycloakIDP( "keycloak", "quay.io/keycloak/keycloak:25.0", []corev1.EnvVar{ - // configure password for Keycloak root user {Name: "KEYCLOAK_ADMIN", Value: "admin"}, {Name: "KEYCLOAK_ADMIN_PASSWORD", Value: "password"}, {Name: "KC_HEALTH_ENABLED", Value: "true"}, @@ -105,10 +112,15 @@ func AddKeycloakIDP( true, "/opt/keycloak/bin/kc.sh", "start-dev", ) - cleanups = []func(){cleanup} + + setup := &KeycloakSetup{ + IDPName: fmt.Sprintf("keycloak-test-%s", nsName), + Namespace: nsName, + Cleanups: []func(){cleanup}, + } defer func() { if err != nil { - for _, c := range cleanups { + for _, c := range setup.Cleanups { c() } } @@ -119,19 +131,13 @@ func AddKeycloakIDP( transport, err := rest.TransportFor(kubeconfig) require.NoError(t, err) - openshiftIDPName := fmt.Sprintf("keycloak-test-%s", nsName) - keycloakURL := keycloakBaseURL + "/realms/master" + setup.IssuerURL = keycloakURL - // create a keycloak REST client and authenticate to the API - kcClient = KeycloakClientFor(t, transport, keycloakURL, "master") + setup.Client = KeycloakClientFor(t, transport, keycloakURL, "master") - // even though configured via env vars and even though we checked Keycloak reports - // ready on /health/ready, it still appears that we may need some time to log in properly - // In resource-constrained CI environments with parallel test execution, Keycloak can take - // 40-60+ seconds to fully initialize its admin API even after passing readiness probes err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { - err := kcClient.AuthenticatePassword("admin-cli", "", "admin", "password") + err := setup.Client.AuthenticatePassword("admin-cli", "", "admin", "password") if err != nil { t.Logf("failed to authenticate to Keycloak: %v", err) return false, nil @@ -140,17 +146,16 @@ func AddKeycloakIDP( }) require.NoError(t, err) - clientList, err := kcClient.ListClients() + clientList, err := setup.Client.ListClients() require.NoError(t, err) - var adminClientId, passwdClientId, passwdClientClientId string + var adminClientId, passwdClientId string for _, c := range clientList { if clientID := c["clientId"].(string); clientID == "admin-cli" { adminClientId = c["id"].(string) } else if len(c["redirectUris"].([]interface{})) > 0 { - // just reuse one other client that's already there passwdClientId = c["id"].(string) - passwdClientClientId = clientID + setup.ClientID = clientID } if len(passwdClientId) > 0 && len(adminClientId) > 0 { @@ -158,14 +163,11 @@ func AddKeycloakIDP( } } - // change the client's access token timeout just in case we need it for the test - // Wrap in retry logic as Keycloak may still be unstable after initial authentication err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { - err := kcClient.UpdateClientAccessTokenTimeout(adminClientId, 60*30) + err := setup.Client.UpdateClientAccessTokenTimeout(adminClientId, 60*30) if err != nil { t.Logf("failed to update client access token timeout: %v, retrying", err) - // Re-authenticate in case the connection was dropped - if authErr := kcClient.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { + if authErr := setup.Client.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { t.Logf("failed to re-authenticate: %v", authErr) } return false, nil @@ -174,19 +176,15 @@ func AddKeycloakIDP( }) require.NoError(t, err) - // reauthenticate for a new, longer-lived token - err = kcClient.AuthenticatePassword("admin-cli", "", "admin", "password") + err = setup.Client.AuthenticatePassword("admin-cli", "", "admin", "password") require.NoError(t, err) - // Regenerate client secret with retry logic for Keycloak stability - var clientSecret string err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { var err error - clientSecret, err = kcClient.RegenerateClientSecret(passwdClientId) + setup.ClientSecret, err = setup.Client.RegenerateClientSecret(passwdClientId) if err != nil { t.Logf("failed to regenerate client secret: %v, retrying", err) - // Re-authenticate in case the connection was dropped - if authErr := kcClient.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { + if authErr := setup.Client.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { t.Logf("failed to re-authenticate: %v", authErr) } return false, nil @@ -195,14 +193,12 @@ func AddKeycloakIDP( }) require.NoError(t, err) - // Create client group mapper with retry logic const groupsClaimName = "groups" err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { - err := kcClient.CreateClientGroupMapper(passwdClientId, "test-groups-mapper", groupsClaimName) + err := setup.Client.CreateClientGroupMapper(passwdClientId, "test-groups-mapper", groupsClaimName) if err != nil { t.Logf("failed to create client group mapper: %v, retrying", err) - // Re-authenticate in case the connection was dropped - if authErr := kcClient.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { + if authErr := setup.Client.AuthenticatePassword("admin-cli", "", "admin", "password"); authErr != nil { t.Logf("failed to re-authenticate: %v", authErr) } return false, nil @@ -211,22 +207,50 @@ func AddKeycloakIDP( }) require.NoError(t, err) + return setup +} + +// AddKeycloakOIDCIdP registers the Keycloak instance from a KeycloakSetup as an +// OIDC identity provider in OpenShift. When directOIDC is true, secrets and CA +// are created but the IdP is not added to the OAuth config. +func AddKeycloakOIDCIdP(t testing.TB, kubeconfig *rest.Config, setup *KeycloakSetup, directOIDC bool) []func() { + kubeClients, err := kubernetes.NewForConfig(kubeconfig) + require.NoError(t, err) + + configClient, err := configv1client.NewForConfig(kubeconfig) + require.NoError(t, err) + idpCleans, err := addOIDCIDentityProvider(t, kubeClients, configClient, - passwdClientClientId, clientSecret, - openshiftIDPName, - keycloakURL, + setup.ClientID, setup.ClientSecret, + setup.IDPName, + setup.IssuerURL, configv1.OpenIDClaims{ PreferredUsername: []string{"preferred_username"}, - Groups: []configv1.OpenIDClaim{groupsClaimName}, + Groups: []configv1.OpenIDClaim{"groups"}, }, directOIDC, ) - cleanups = append(cleanups, idpCleans...) require.NoError(t, err, "failed to configure the identity provider") - return kcClient, openshiftIDPName, cleanups + return idpCleans +} + +// AddKeycloakIDP deploys Keycloak and registers it as an OIDC IdP in one call. +// This is a convenience wrapper around DeployKeycloak + AddKeycloakOIDCIdP. +func AddKeycloakIDP( + t testing.TB, + kubeconfig *rest.Config, + directOIDC bool, +) (kcClient *KeycloakClient, idpName string, cleanups []func()) { + setup := DeployKeycloak(t, kubeconfig) + cleanups = setup.Cleanups + + idpCleans := AddKeycloakOIDCIdP(t, kubeconfig, setup, directOIDC) + cleanups = append(cleanups, idpCleans...) + + return setup.Client, setup.IDPName, cleanups } type KeycloakClient struct { diff --git a/test/library/proxy.go b/test/library/proxy.go new file mode 100644 index 0000000000..ee2582fefd --- /dev/null +++ b/test/library/proxy.go @@ -0,0 +1,529 @@ +package library + +import ( + "context" + "crypto/x509" + "encoding/pem" + "fmt" + "reflect" + "strings" + "sync" + "testing" + "time" + + g "github.com/onsi/ginkgo/v2" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" + watchtools "k8s.io/client-go/tools/watch" + "k8s.io/utils/ptr" + + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + configclient "github.com/openshift/client-go/config/clientset/versioned" + operatorclient "github.com/openshift/client-go/operator/clientset/versioned" +) + +const ( + squidImage = "docker.io/ubuntu/squid:7.2-26.04_edge" + squidHTTPPort = int32(3128) + squidHTTPSPort = int32(3129) + squidServiceName = "squid-proxy" + + componentProxyCAConfigMapName = "v4-0-config-system-auth-proxy-ca" +) + +// SaveAndRestoreProxyConfig snapshots the current spec.proxy on the operator +// Authentication CR and returns a cleanup function that restores it and waits +// for the operator to reconcile. +func SaveAndRestoreProxyConfig(t testing.TB, operatorClient *operatorclient.Clientset, configClient *configclient.Clientset) (operatorAuth *operatorv1.Authentication, cleanup func()) { + ctx := context.TODO() + + auth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get operator authentication CR: %v", err) + } + originalProxy := auth.Spec.Proxy.DeepCopy() + + return auth, sync.OnceFunc(func() { + t.Log("cleaning up: restoring original proxy config") + var changed bool + err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 30*time.Second, true, func(ctx context.Context) (bool, error) { + fresh, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Logf("cleanup: failed to get operator auth: %v", err) + return false, nil + } + target := operatorv1.AuthenticationProxyConfig{} + if originalProxy != nil { + target = *originalProxy + } + if reflect.DeepEqual(fresh.Spec.Proxy, target) { + t.Log("cleanup: proxy config already matches original, no update needed") + return true, nil + } + fresh.Spec.Proxy = target + if _, err := operatorClient.OperatorV1().Authentications().Update(ctx, fresh, metav1.UpdateOptions{}); err != nil { + t.Logf("cleanup: failed to update operator auth (will retry): %v", err) + return false, nil + } + changed = true + return true, nil + }) + if err != nil { + t.Logf("cleanup: failed to restore proxy config: %v", err) + return + } + if !changed { + return + } + t.Log("cleanup: waiting for operator to pick up changes and stabilize") + if err := WaitForOperatorToPickUpChanges(t, configClient.ConfigV1(), "authentication"); err != nil { + t.Logf("cleanup: operator did not recover: %v", err) + } + }) +} + +// DeploySquidProxy deploys a Squid forward proxy that listens on both plain +// HTTP (port 3128) and HTTPS (port 3129). It generates a self-signed CA and +// serving certificate internally. Returns the HTTP and HTTPS proxy URLs, +// the PEM-encoded CA certificate (for trustedCA ConfigMaps when using https), +// the namespace name, and a cleanup function. +func DeploySquidProxy(t testing.TB, kubeClient kubernetes.Interface) (httpProxyURL, httpsProxyURL string, caCertPEM []byte, namespace string, cleanup func()) { + ctx := context.TODO() + + namespace = NewTestNamespaceBuilder("e2e-proxy-"). + WithBaselinePSaEnforcement(). + WithLabels(CAOE2ETestLabels()). + Create(t, kubeClient.CoreV1().Namespaces()) + + cleanup = sync.OnceFunc(func() { + g.GinkgoWriter.Println("cleaning up: removing Squid proxy") + if err := kubeClient.CoreV1().Namespaces().Delete(ctx, namespace, metav1.DeleteOptions{}); err != nil { + t.Logf("error cleaning up proxy namespace %q: %v", namespace, err) + } + }) + + success := false + defer func() { + if !success { + cleanup() + } + }() + + ca := NewCertificateAuthorityCertificate(t, nil) + serviceDNS := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) + serverCert := NewServerCertificate(t, ca, serviceDNS) + + caCertPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ca.Certificate.Raw}) + serverCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: serverCert.Certificate.Raw}) + + serverKeyDER, err := x509.MarshalPKCS8PrivateKey(serverCert.PrivateKey) + if err != nil { + t.Fatalf("failed to marshal server private key: %v", err) + } + serverKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: serverKeyDER}) + + squidConfig := fmt.Sprintf(`http_port %d +https_port %d tls-cert=/etc/squid/tls/tls.crt tls-key=/etc/squid/tls/tls.key +pid_filename /tmp/squid.pid +acl all src all +http_access allow all +access_log /tmp/squid/access.log +cache_log /tmp/squid/cache.log +cache deny all +buffered_logs off +`, squidHTTPPort, squidHTTPSPort) + + _, err = kubeClient.CoreV1().ConfigMaps(namespace).Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "squid-config"}, + Data: map[string]string{"squid.conf": squidConfig}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create squid config: %v", err) + } + + _, err = kubeClient.CoreV1().Secrets(namespace).Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "squid-tls"}, + Data: map[string][]byte{ + "tls.crt": serverCertPEM, + "tls.key": serverKeyPEM, + }, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create squid TLS secret: %v", err) + } + + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: squidServiceName, + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: ptr.To(int32(1)), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": squidServiceName}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "squid", + Image: squidImage, + Ports: []corev1.ContainerPort{ + {ContainerPort: squidHTTPPort, Protocol: corev1.ProtocolTCP}, + {ContainerPort: squidHTTPSPort, Protocol: corev1.ProtocolTCP}, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "squid-config", + MountPath: "/etc/squid/squid.conf", + SubPath: "squid.conf", + }, + { + Name: "squid-tls", + MountPath: "/etc/squid/tls", + ReadOnly: true, + }, + { + Name: "squid-logs", + MountPath: "/tmp/squid", + }, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt32(squidHTTPPort), + }, + }, + InitialDelaySeconds: 5, + PeriodSeconds: 5, + }, + }, + { + Name: "log", + Image: squidImage, + Command: []string{"tail", "-F", "/tmp/squid/access.log"}, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "squid-logs", + MountPath: "/tmp/squid", + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "squid-config", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "squid-config", + }, + }, + }, + }, + { + Name: "squid-tls", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "squid-tls", + }, + }, + }, + { + Name: "squid-logs", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, + }, + }, + }, + } + + _, err = kubeClient.AppsV1().Deployments(namespace).Create(ctx, deployment, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create squid deployment: %v", err) + } + + _, err = kubeClient.CoreV1().Services(namespace).Create(ctx, &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: squidServiceName, + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": squidServiceName}, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: squidHTTPPort, + TargetPort: intstr.FromInt32(squidHTTPPort), + Protocol: corev1.ProtocolTCP, + }, + { + Name: "https", + Port: squidHTTPSPort, + TargetPort: intstr.FromInt32(squidHTTPSPort), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create squid service: %v", err) + } + + t.Logf("waiting for squid proxy deployment in %s to be ready", namespace) + timeLimitedCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + _, err = watchtools.UntilWithSync(timeLimitedCtx, + cache.NewListWatchFromClient( + kubeClient.AppsV1().RESTClient(), "deployments", namespace, + fields.OneTermEqualSelector("metadata.name", squidServiceName)), + &appsv1.Deployment{}, + nil, + func(event watch.Event) (bool, error) { + d := event.Object.(*appsv1.Deployment) + return d.Status.ReadyReplicas > 0, nil + }, + ) + if err != nil { + t.Fatalf("squid proxy deployment did not become ready: %v", err) + } + + success = true + + serviceHost := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) + httpProxyURL = fmt.Sprintf("http://%s:%d", serviceHost, squidHTTPPort) + httpsProxyURL = fmt.Sprintf("https://%s:%d", serviceHost, squidHTTPSPort) + success = true + t.Logf("squid proxy deployed: http=%s https=%s", httpProxyURL, httpsProxyURL) + return httpProxyURL, httpsProxyURL, caCertPEM, namespace, cleanup +} + +// DeployProxyNetworkPolicies creates a NetworkPolicy on the Keycloak namespace +// that restricts ingress to only the proxy namespace. This ensures auth +// components can only reach Keycloak through the proxy. +// +// Note: egress policies on auth namespaces are not created because the +// operator-managed NetworkPolicies already have allow-all egress rules that +// cannot be overridden additively. +func DeployProxyNetworkPolicies(t testing.TB, kubeClient kubernetes.Interface, proxyNamespace, keycloakNamespace string) func() { + ctx := context.TODO() + + keycloakPolicy := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-e2e-allow-only-from-proxy", + Namespace: keycloakNamespace, + Labels: CAOE2ETestLabels(), + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": proxyNamespace, + }, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "policy-group.network.openshift.io/ingress": "", + }, + }, + }, + }, + }, + }, + }, + } + + _, err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Create(ctx, keycloakPolicy, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create NetworkPolicy in %s: %v", keycloakNamespace, err) + } + t.Logf("created NetworkPolicy proxy-e2e-allow-only-from-proxy in %s", keycloakNamespace) + + return func() { + if err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Delete(ctx, "proxy-e2e-allow-only-from-proxy", metav1.DeleteOptions{}); err != nil { + t.Logf("error cleaning up NetworkPolicy in %s: %v", keycloakNamespace, err) + } + } +} + +// GetSquidProxyLogs reads the Squid access log from the proxy pod via +// the log sidecar container that tails the access log file. +func GetSquidProxyLogs(kubeClient kubernetes.Interface, namespace string) (string, error) { + ctx := context.TODO() + + pods, err := kubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("app=%s", squidServiceName), + }) + if err != nil { + return "", fmt.Errorf("failed to list squid pods in %s: %w", namespace, err) + } + if len(pods.Items) == 0 { + return "", fmt.Errorf("no squid proxy pods found in namespace %s", namespace) + } + + container := "log" + logBytes, err := kubeClient.CoreV1().Pods(namespace).GetLogs(pods.Items[0].Name, &corev1.PodLogOptions{ + Container: container, + }).DoRaw(ctx) + if err != nil { + return "", fmt.Errorf("failed to get logs from container %s: %w", container, err) + } + + return string(logBytes), nil +} + +// WaitForSquidProxyTraffic polls the Squid proxy logs until it sees CONNECT or +// TCP_ entries, indicating traffic went through the proxy. +func WaitForSquidProxyTraffic(t testing.TB, kubeClient kubernetes.Interface, namespace string, timeout time.Duration) error { + t.Logf("waiting up to %s for traffic in squid proxy logs", timeout) + return wait.PollUntilContextTimeout(context.TODO(), 10*time.Second, timeout, true, func(ctx context.Context) (bool, error) { + logs, err := GetSquidProxyLogs(kubeClient, namespace) + if err != nil { + t.Logf("failed to read squid logs: %v", err) + return false, nil + } + if strings.Contains(logs, "CONNECT") || strings.Contains(logs, "TCP_") { + t.Logf("detected proxy traffic in squid logs") + return true, nil + } + return false, nil + }) +} + +// VerifyOAuthServerDeploymentProxyConfig asserts that the OAuth server +// deployment has the expected proxy env var values and trustedCA volume/mount. +// Proxy env vars are always set; pass empty string to assert an unset proxy. +// When expectTrustedCAVolume is true, the v4-0-config-system-auth-proxy-ca +// volume and mount must exist; when false, they must be absent. +func VerifyOAuthServerDeploymentProxyConfig(t testing.TB, kubeClient kubernetes.Interface, expectedHTTPProxy, expectedHTTPSProxy, expectedNoProxy string, expectTrustedCAVolume bool) { + ctx := context.TODO() + + var deployment *appsv1.Deployment + err := wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + var err error + deployment, err = kubeClient.AppsV1().Deployments("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) + if err != nil { + t.Logf("failed to get oauth-openshift deployment: %v", err) + return false, nil + } + + envVars := make(map[string]string) + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, env := range container.Env { + switch env.Name { + case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY": + envVars[env.Name] = env.Value + } + } + } + + if envVars["HTTP_PROXY"] != expectedHTTPProxy || envVars["HTTPS_PROXY"] != expectedHTTPSProxy { + return false, nil + } + // Use superset check: the operator may add extra entries to NO_PROXY beyond + // what the caller specifies (e.g. the kubernetes service IP for KUBERNETES_SERVICE_HOST). + actualNoProxy := sets.New[string](strings.Split(envVars["NO_PROXY"], ",")...) + expectedNoProxyEntries := sets.New[string](strings.Split(expectedNoProxy, ",")...) + if !actualNoProxy.IsSuperset(expectedNoProxyEntries) { + return false, nil + } + + if matchTrustedCAVolume(deployment, expectTrustedCAVolume) { + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatalf("OAuth server deployment proxy config did not match expected values within timeout") + } +} + +func matchTrustedCAVolume(deployment *appsv1.Deployment, expectPresent bool) bool { + foundVolume := false + for _, vol := range deployment.Spec.Template.Spec.Volumes { + if vol.ConfigMap != nil && vol.ConfigMap.Name == componentProxyCAConfigMapName { + foundVolume = true + break + } + } + + foundMount := false + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, mount := range container.VolumeMounts { + if mount.Name == componentProxyCAConfigMapName { + foundMount = true + break + } + } + } + + if expectPresent { + return foundVolume && foundMount + } + return !foundVolume && !foundMount +} + +// VerifyTrustedCAConfigMapSynced checks that the trustedCA ConfigMap has been +// synced to the openshift-authentication namespace under the operator's +// hardcoded name (v4-0-config-system-auth-proxy-ca). +func VerifyTrustedCAConfigMapSynced(t testing.TB, kubeClient kubernetes.Interface, configMapName string) { + ctx := context.TODO() + + err := wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + cm, err := kubeClient.CoreV1().ConfigMaps("openshift-authentication").Get(ctx, componentProxyCAConfigMapName, metav1.GetOptions{}) + if err != nil { + return false, nil + } + return len(cm.Data) > 0, nil + }) + if err != nil { + t.Fatalf("trustedCA ConfigMap %s was not synced to openshift-authentication as %s within timeout", configMapName, componentProxyCAConfigMapName) + } +} + +// CheckFeatureGateEnabledOrSkip skips the test if the given feature gate is not enabled. +func CheckFeatureGateEnabledOrSkip(t testing.TB, configClient *configclient.Clientset, featureGateName configv1.FeatureGateName) { + ctx := context.TODO() + + featureGates, err := configClient.ConfigV1().FeatureGates().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get feature gates: %v", err) + } + + if len(featureGates.Status.FeatureGates) != 1 { + t.Fatalf("multiple feature gate versions detected — cluster may be upgrading") + } + + for _, gate := range featureGates.Status.FeatureGates[0].Enabled { + if gate.Name == featureGateName { + t.Logf("feature gate %s is enabled", featureGateName) + return + } + } + + t.Skipf("skipping: feature gate %s is not enabled", featureGateName) +}