diff --git a/pkg/controllers/externaloidc/externaloidc_controller.go b/pkg/controllers/externaloidc/externaloidc_controller.go new file mode 100644 index 000000000..0ba09e31f --- /dev/null +++ b/pkg/controllers/externaloidc/externaloidc_controller.go @@ -0,0 +1,292 @@ +package externaloidc + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" + + configv1 "github.com/openshift/api/config/v1" + configinformers "github.com/openshift/client-go/config/informers/externalversions" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/retry" + "github.com/openshift/library-go/pkg/operator/v1helpers" + "golang.org/x/net/http/httpproxy" + + "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apiserverv1beta1 "k8s.io/apiserver/pkg/apis/apiserver/v1beta1" + corev1ac "k8s.io/client-go/applyconfigurations/core/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/util/cert" + "k8s.io/utils/ptr" +) + +const ( + configNamespace = "openshift-config" + managedNamespace = "openshift-config-managed" + targetAuthConfigCMName = "auth-config" + authConfigDataKey = "auth-config.json" + oidcDiscoveryEndpointPath = "/.well-known/openid-configuration" +) + +type externalOIDCController struct { + name string + eventName string + authLister configv1listers.AuthenticationLister + configMapLister corev1listers.ConfigMapLister + configMaps corev1client.ConfigMapsGetter +} + +func NewExternalOIDCController( + kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, + configInformer configinformers.SharedInformerFactory, + operatorClient v1helpers.OperatorClient, + configMaps corev1client.ConfigMapsGetter, + recorder events.Recorder, +) factory.Controller { + + c := &externalOIDCController{ + name: "ExternalOIDCController", + eventName: "external-oidc-controller", + + authLister: configInformer.Config().V1().Authentications().Lister(), + configMapLister: kubeInformersForNamespaces.ConfigMapLister(), + configMaps: configMaps, + } + + return factory.New().WithInformers( + // track openshift-config for changes to the provider's CA bundle + kubeInformersForNamespaces.InformersFor(configNamespace).Core().V1().ConfigMaps().Informer(), + // track auth resource + configInformer.Config().V1().Authentications().Informer(), + ).WithFilteredEventsInformers( + // track openshift-config-managed/auth-config cm in case it gets changed externally + factory.NamesFilter(targetAuthConfigCMName), + kubeInformersForNamespaces.InformersFor(managedNamespace).Core().V1().ConfigMaps().Informer(), + ).WithSync(c.sync). + WithSyncDegradedOnError(operatorClient). + ToController(c.name, recorder.WithComponentSuffix(c.eventName)) +} + +func (c *externalOIDCController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + auth, err := c.authLister.Get("cluster") + if err != nil { + return fmt.Errorf("could not get authentication/cluster: %v", err) + } + + if auth.Spec.Type != configv1.AuthenticationTypeOIDC { + // auth type is "IntegratedOAuth", "" or "None"; delete structured auth configmap if it exists + if _, err := c.configMapLister.ConfigMaps(managedNamespace).Get(targetAuthConfigCMName); errors.IsNotFound(err) { + return nil + } else if err != nil { + return err + } + + if err := c.configMaps.ConfigMaps(managedNamespace).Delete(ctx, targetAuthConfigCMName, metav1.DeleteOptions{}); err == nil { + syncCtx.Recorder().Eventf(c.eventName, "Removed auth configmap %s/%s", managedNamespace, targetAuthConfigCMName) + + } else if !apierrors.IsNotFound(err) { + return fmt.Errorf("could not delete existing configmap %s/%s: %v", managedNamespace, targetAuthConfigCMName, err) + } + + return nil + } + + authConfig, err := c.generateAuthConfig(*auth) + if err != nil { + return err + } + + b, err := json.Marshal(authConfig) + if err != nil { + return fmt.Errorf("could not marshal auth config into JSON: %v", err) + } + authConfigJSON := string(b) + + existingCM, err := c.configMapLister.ConfigMaps(managedNamespace).Get(targetAuthConfigCMName) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("could not retrieve auth configmap %s/%s to check data before sync: %v", managedNamespace, targetAuthConfigCMName, err) + } + + if existingCM != nil && existingCM.Data[authConfigDataKey] == authConfigJSON { + return nil + } + + if err := validateAuthenticationConfiguration(*authConfig); err != nil { + return fmt.Errorf("auth config validation failed: %v", err) + } + + cm := corev1ac.ConfigMap(targetAuthConfigCMName, managedNamespace).WithData(map[string]string{authConfigDataKey: authConfigJSON}) + if _, err := c.configMaps.ConfigMaps(managedNamespace).Apply(ctx, cm, metav1.ApplyOptions{FieldManager: c.name, Force: true}); err != nil { + return fmt.Errorf("could not apply changes to auth configmap %s/%s: %v", managedNamespace, targetAuthConfigCMName, err) + } + + syncCtx.Recorder().Eventf(c.eventName, "Synced auth configmap %s/%s", managedNamespace, targetAuthConfigCMName) + + return nil +} + +// generateAuthConfig creates a structured JWT AuthenticationConfiguration for OIDC +// from the configuration found in the authentication/cluster resource +func (c *externalOIDCController) generateAuthConfig(auth configv1.Authentication) (*apiserverv1beta1.AuthenticationConfiguration, error) { + authConfig := apiserverv1beta1.AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "AuthenticationConfiguration", + APIVersion: "apiserver.config.k8s.io/v1beta1", + }, + } + + for _, provider := range auth.Spec.OIDCProviders { + jwt := apiserverv1beta1.JWTAuthenticator{ + Issuer: apiserverv1beta1.Issuer{ + URL: provider.Issuer.URL, + AudienceMatchPolicy: apiserverv1beta1.AudienceMatchPolicyMatchAny, + }, + ClaimMappings: apiserverv1beta1.ClaimMappings{ + Username: apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: provider.ClaimMappings.Username.Claim, + }, + Groups: apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: provider.ClaimMappings.Groups.Claim, + Prefix: &provider.ClaimMappings.Groups.Prefix, + }, + }, + } + + if len(provider.Issuer.Audiences) > 0 { + jwt.Issuer.Audiences = make([]string, 0, len(provider.Issuer.Audiences)) + for _, aud := range provider.Issuer.Audiences { + jwt.Issuer.Audiences = append(jwt.Issuer.Audiences, string(aud)) + } + } + + if len(provider.Issuer.CertificateAuthority.Name) > 0 { + caConfigMap, err := c.configMapLister.ConfigMaps(configNamespace).Get(provider.Issuer.CertificateAuthority.Name) + if err != nil { + return nil, fmt.Errorf("could not retrieve auth configmap %s/%s to check CA bundle: %v", configNamespace, provider.Issuer.CertificateAuthority.Name, err) + } + + caData, ok := caConfigMap.Data["ca-bundle.crt"] + if !ok || len(caData) == 0 { + return nil, fmt.Errorf("configmap %s/%s key \"ca-bundle.crt\" missing or empty", configNamespace, provider.Issuer.CertificateAuthority.Name) + } + + jwt.Issuer.CertificateAuthority = caData + } + + switch provider.ClaimMappings.Username.PrefixPolicy { + case configv1.NoOpinion: + jwt.ClaimMappings.Username.Prefix = ptr.To("") + case configv1.NoPrefix: + jwt.ClaimMappings.Username.Prefix = ptr.To("-") + case configv1.Prefix: + if provider.ClaimMappings.Username.Prefix == nil { + return nil, fmt.Errorf("nil username prefix while policy expects one") + } else { + jwt.ClaimMappings.Username.Prefix = &provider.ClaimMappings.Username.Prefix.PrefixString + } + default: + return nil, fmt.Errorf("invalid username prefix policy: %s", provider.ClaimMappings.Username.PrefixPolicy) + } + + for i, rule := range provider.ClaimValidationRules { + if rule.RequiredClaim == nil { + return nil, fmt.Errorf("empty validation rule at index %d", i) + } + + jwt.ClaimValidationRules = append(jwt.ClaimValidationRules, apiserverv1beta1.ClaimValidationRule{ + Claim: rule.RequiredClaim.Claim, + RequiredValue: rule.RequiredClaim.RequiredValue, + }) + } + + authConfig.JWT = append(authConfig.JWT, jwt) + } + + return &authConfig, nil +} + +// validateAuthenticationConfiguration performs validations that are not done at the server-side, +// including validation that the provided CA cert (or system CAs if not specified) can be used for +// TLS cert verification +func validateAuthenticationConfiguration(auth apiserverv1beta1.AuthenticationConfiguration) error { + for _, jwt := range auth.JWT { + var caCertPool *x509.CertPool + var err error + if len(jwt.Issuer.CertificateAuthority) > 0 { + caCertPool, err = cert.NewPoolFromBytes([]byte(jwt.Issuer.CertificateAuthority)) + if err != nil { + return fmt.Errorf("issuer CA is invalid: %v", err) + } + } + + // make sure we can access the issuer with the given cert pool (system CAs used if pool is empty) + if err := validateCACert(jwt.Issuer.URL, caCertPool); err != nil { + certMessage := "using the specified CA cert" + if caCertPool == nil { + certMessage = "using the system CAs" + } + return fmt.Errorf("could not validate IDP URL %s: %v", certMessage, err) + } + } + + return nil +} + +// validateCACert makes a request to the provider's well-known endpoint using the +// specified CA cert pool to validate that the certs in the pool match the host +func validateCACert(hostURL string, caCertPool *x509.CertPool) error { + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{RootCAs: caCertPool}, + Proxy: func(*http.Request) (*url.URL, error) { + if proxyConfig := httpproxy.FromEnvironment(); len(proxyConfig.HTTPSProxy) > 0 { + return url.Parse(proxyConfig.HTTPSProxy) + } + return nil, nil + }, + }, + Timeout: 5 * time.Second, + } + + wellKnown := strings.TrimSuffix(hostURL, "/") + oidcDiscoveryEndpointPath + req, err := http.NewRequest(http.MethodGet, wellKnown, nil) + if err != nil { + return fmt.Errorf("could not create well-known HTTP request: %v", err) + } + + var resp *http.Response + var connErr error + retryCtx, cancel := context.WithTimeout(req.Context(), 10*time.Second) + defer cancel() + retry.RetryOnConnectionErrors(retryCtx, func(ctx context.Context) (done bool, err error) { + resp, connErr = client.Do(req) + return connErr == nil, connErr + }) + if connErr != nil { + return fmt.Errorf("GET well-known error: %v", connErr) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("unable to read response body; HTTP status: %s; error: %v", resp.Status, err) + } + + return fmt.Errorf("unexpected well-known status code %s: %s", resp.Status, body) + } + + return nil +} diff --git a/pkg/controllers/externaloidc/externaloidc_controller_test.go b/pkg/controllers/externaloidc/externaloidc_controller_test.go new file mode 100644 index 000000000..fea385b1e --- /dev/null +++ b/pkg/controllers/externaloidc/externaloidc_controller_test.go @@ -0,0 +1,964 @@ +package externaloidc + +import ( + "bytes" + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + configv1 "github.com/openshift/api/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" + apiserverv1beta1 "k8s.io/apiserver/pkg/apis/apiserver/v1beta1" + "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" + certutil "k8s.io/client-go/util/cert" + "k8s.io/utils/ptr" +) + +var ( + baseCACert, baseCAPrivateKey, testCertData = func() (*x509.Certificate, crypto.Signer, string) { + cert, key, err := generateCAKeyPair() + if err != nil { + panic(err) + } + return cert, key, string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})) + }() + + baseAuthResource = *newAuthWithSpec(configv1.AuthenticationSpec{ + Type: configv1.AuthenticationTypeOIDC, + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test-oidc-provider", + Issuer: configv1.TokenIssuer{ + CertificateAuthority: configv1.ConfigMapNameReference{Name: "oidc-ca-bundle"}, + Audiences: []configv1.TokenAudience{"my-test-aud", "another-aud"}, + }, + OIDCClients: []configv1.OIDCClientConfig{ + { + ComponentName: "console", + ComponentNamespace: "openshift-console", + ClientID: "console-oidc-client", + }, + { + ComponentName: "kube-apiserver", + ComponentNamespace: "openshift-kube-apiserver", + ClientID: "test-oidc-client", + }, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "username", + }, + PrefixPolicy: configv1.Prefix, + Prefix: &configv1.UsernamePrefix{ + PrefixString: "oidc-user:", + }, + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "groups", + }, + Prefix: "oidc-group:", + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeRequiredClaim, + RequiredClaim: &configv1.TokenRequiredClaim{ + Claim: "username", + RequiredValue: "test-username", + }, + }, + { + Type: configv1.TokenValidationRuleTypeRequiredClaim, + RequiredClaim: &configv1.TokenRequiredClaim{ + Claim: "email", + RequiredValue: "test-email", + }, + }, + }, + }, + }, + }) + + baseAuthConfig = apiserverv1beta1.AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "AuthenticationConfiguration", + APIVersion: "apiserver.config.k8s.io/v1beta1", + }, + JWT: []apiserverv1beta1.JWTAuthenticator{ + { + Issuer: apiserverv1beta1.Issuer{ + Audiences: []string{"my-test-aud", "another-aud"}, + CertificateAuthority: testCertData, + AudienceMatchPolicy: apiserverv1beta1.AudienceMatchPolicyMatchAny, + }, + ClaimMappings: apiserverv1beta1.ClaimMappings{ + Username: apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: ptr.To("oidc-user:"), + }, + Groups: apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: "groups", + Prefix: ptr.To("oidc-group:"), + }, + }, + ClaimValidationRules: []apiserverv1beta1.ClaimValidationRule{ + { + Claim: "username", + RequiredValue: "test-username", + }, + { + Claim: "email", + RequiredValue: "test-email", + }, + }, + }, + }, + } + + baseAuthConfigJSON = fmt.Sprintf(`{"kind":"AuthenticationConfiguration","apiVersion":"apiserver.config.k8s.io/v1beta1","jwt":[{"issuer":{"url":"$URL","certificateAuthority":"%s","audiences":["my-test-aud","another-aud"],"audienceMatchPolicy":"MatchAny"},"claimValidationRules":[{"claim":"username","requiredValue":"test-username"},{"claim":"email","requiredValue":"test-email"}],"claimMappings":{"username":{"claim":"username","prefix":"oidc-user:"},"groups":{"claim":"groups","prefix":"oidc-group:"},"uid":{}}}]}`, strings.ReplaceAll(testCertData, "\n", "\\n")) + + baseAuthConfigCM = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: targetAuthConfigCMName, + Namespace: managedNamespace, + }, + Data: map[string]string{ + authConfigDataKey: baseAuthConfigJSON, + }, + } + + baseCABundleConfigMap = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oidc-ca-bundle", + Namespace: configNamespace, + }, + Data: map[string]string{ + "ca-bundle.crt": testCertData, + }, + } + + caBundleConfigMapInvalidKey = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oidc-ca-bundle", + Namespace: configNamespace, + }, + Data: map[string]string{ + "invalid": testCertData, + }, + } + + caBundleConfigMapInvalidData = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oidc-ca-bundle", + Namespace: configNamespace, + }, + Data: map[string]string{ + "ca-bundle.crt": "not a cert", + }, + } + + caBundleConfigMapNoData = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oidc-ca-bundle", + Namespace: configNamespace, + }, + Data: map[string]string{ + "ca-bundle.crt": "", + }, + } +) + +func TestExternalOIDCController_sync(t *testing.T) { + testCtx := context.Background() + + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, nil) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + for _, tt := range []struct { + name string + + configMapIndexer cache.Indexer + existingAuthConfigCM *corev1.ConfigMap + caBundleConfigMap *corev1.ConfigMap + auth *configv1.Authentication + cmApplyReaction k8stesting.ReactionFunc + cmDeleteReaction k8stesting.ReactionFunc + + expectedAuthConfigJSON string + expectEvents bool + expectError bool + }{ + { + name: "auth resource not found", + expectError: true, + }, + { + name: "auth type IntegratedOAuth and failing to delete cm", + existingAuthConfigCM: &baseAuthConfigCM, + cmDeleteReaction: func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("error") + }, + auth: newAuthWithSpec(configv1.AuthenticationSpec{Type: configv1.AuthenticationTypeIntegratedOAuth}), + expectError: true, + }, + { + name: "auth type IntegratedOAuth configmap lister error", + configMapIndexer: &everFailingIndexer{}, + auth: newAuthWithSpec(configv1.AuthenticationSpec{Type: configv1.AuthenticationTypeIntegratedOAuth}), + expectError: true, + }, + { + name: "auth type IntegratedOAuth and no auth configmap", + auth: newAuthWithSpec(configv1.AuthenticationSpec{Type: configv1.AuthenticationTypeIntegratedOAuth}), + expectError: false, + }, + { + name: "auth type IntegratedOAuth with auth configmap", + existingAuthConfigCM: &baseAuthConfigCM, + caBundleConfigMap: &baseCABundleConfigMap, + auth: newAuthWithSpec(configv1.AuthenticationSpec{Type: configv1.AuthenticationTypeIntegratedOAuth}), + expectEvents: true, + expectError: false, + }, + { + name: "auth type OIDC but config map lister fails", + configMapIndexer: cache.Indexer(&everFailingIndexer{}), + expectEvents: false, + expectError: true, + }, + { + name: "auth type OIDC config same as existing", + existingAuthConfigCM: authConfigCMWithIssuerURL(&baseAuthConfigCM, testServer.URL), + auth: authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + auth.Spec.OIDCProviders[0].Issuer.URL = testServer.URL + }, + }), + expectedAuthConfigJSON: strings.ReplaceAll(baseAuthConfigJSON, "$URL", testServer.URL), + caBundleConfigMap: &baseCABundleConfigMap, + expectEvents: false, + expectError: false, + }, + { + name: "auth type OIDC error while applying config", + cmApplyReaction: func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("error") + }, + existingAuthConfigCM: &baseAuthConfigCM, + auth: authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + auth.Spec.OIDCProviders[0].Issuer.Audiences = []configv1.TokenAudience{"my-test-aud", "yet-another-aud"} + }, + }), + expectEvents: false, + expectError: true, + }, + { + name: "auth type OIDC apply config", + caBundleConfigMap: &baseCABundleConfigMap, + existingAuthConfigCM: authConfigCMWithIssuerURL(&baseAuthConfigCM, testServer.URL), + auth: authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + auth.Spec.OIDCProviders[0].Issuer.URL = testServer.URL + auth.Spec.OIDCProviders[0].Issuer.Audiences = []configv1.TokenAudience{"my-test-aud", "yet-another-aud"} + }, + }), + expectedAuthConfigJSON: func() string { + str := strings.ReplaceAll(baseAuthConfigJSON, "$URL", testServer.URL) + str = strings.ReplaceAll(str, "another-aud", "yet-another-aud") + return str + }(), + expectEvents: true, + expectError: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + objects := []runtime.Object{} + + authIndexer := cache.NewIndexer(func(obj interface{}) (string, error) { + return "cluster", nil + }, cache.Indexers{}) + + if tt.configMapIndexer == nil { + tt.configMapIndexer = cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + } + + if tt.auth != nil { + authIndexer.Add(tt.auth) + } + + if tt.caBundleConfigMap != nil { + tt.configMapIndexer.Add(&baseCABundleConfigMap) + } + + if tt.existingAuthConfigCM != nil { + tt.configMapIndexer.Add(tt.existingAuthConfigCM) + objects = append(objects, tt.existingAuthConfigCM) + } + + cs := fake.NewSimpleClientset(objects...) + if tt.cmApplyReaction != nil { + // fake client's Apply() is a patch under the hood + cs.PrependReactor("patch", "configmaps", tt.cmApplyReaction) + } + + if tt.cmDeleteReaction != nil { + cs.PrependReactor("delete", "configmaps", tt.cmDeleteReaction) + } + + c := externalOIDCController{ + configMaps: cs.CoreV1(), + authLister: configv1listers.NewAuthenticationLister(authIndexer), + configMapLister: corev1listers.NewConfigMapLister(tt.configMapIndexer), + } + + eventRecorder := events.NewInMemoryRecorder("externaloidc-test") + err := c.sync(testCtx, factory.NewSyncContext("externaloidc-test-context", eventRecorder)) + if tt.expectError && err == nil { + t.Errorf("expected error but didn't get any") + } + + if !tt.expectError && err != nil { + t.Errorf("did not expect any error but got: %v", err) + } + + if tt.expectEvents != (len(eventRecorder.Events()) > 0) { + t.Errorf("expected events: %v; got %v", tt.expectEvents, eventRecorder.Events()) + } + + if tt.auth == nil || err != nil { + // stop assertions here; the ones that follow are not relevant + return + } + + cm, err := c.configMaps.ConfigMaps(managedNamespace).Get(testCtx, targetAuthConfigCMName, metav1.GetOptions{}) + if len(tt.expectedAuthConfigJSON) == 0 && err == nil { + t.Errorf("expected auth configmap to be missing, but it was found") + } else if len(tt.expectedAuthConfigJSON) > 0 && errors.IsNotFound(err) { + t.Errorf("expected auth configmap to exist but it was not found; error = %v", err) + } + + if len(tt.expectedAuthConfigJSON) > 0 && tt.expectedAuthConfigJSON != cm.Data[authConfigDataKey] { + t.Errorf("got unexpected auth-config data: '%s'\nexpected: '%s'", cm.Data[authConfigDataKey], tt.expectedAuthConfigJSON) + } + }) + } +} + +func TestExternalOIDCController_generateAuthConfig(t *testing.T) { + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, nil) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + for _, tt := range []struct { + name string + + auth configv1.Authentication + caBundleConfigMap *corev1.ConfigMap + configMapIndexer cache.Indexer + + expectedAuthConfig *apiserverv1beta1.AuthenticationConfiguration + expectError bool + }{ + { + name: "ca bundle configmap lister error", + auth: baseAuthResource, + configMapIndexer: cache.Indexer(&everFailingIndexer{}), + expectError: true, + }, + { + name: "ca bundle configmap without required key", + auth: baseAuthResource, + caBundleConfigMap: &caBundleConfigMapInvalidKey, + expectError: true, + }, + { + name: "ca bundle configmap with no data", + auth: baseAuthResource, + caBundleConfigMap: &caBundleConfigMapNoData, + expectError: true, + }, + { + name: "auth config nil prefix when required", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].ClaimMappings.Username = configv1.UsernameClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "username", + }, + PrefixPolicy: configv1.Prefix, + Prefix: nil, + } + } + }, + }), + expectError: true, + }, + { + name: "auth config invalid prefix policy", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].ClaimMappings.Username = configv1.UsernameClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "username", + }, + PrefixPolicy: configv1.UsernamePrefixPolicy("invalid-policy"), + } + } + }, + }), + expectError: true, + }, + { + name: "auth config with nil claim in validation rule", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(copy *configv1.Authentication) { + for i := range copy.Spec.OIDCProviders { + if len(copy.Spec.OIDCProviders[i].ClaimValidationRules) == 0 { + copy.Spec.OIDCProviders[i].ClaimValidationRules = make([]configv1.TokenClaimValidationRule, 0) + } + copy.Spec.OIDCProviders[i].ClaimValidationRules = append(copy.Spec.OIDCProviders[i].ClaimValidationRules, configv1.TokenClaimValidationRule{ + Type: configv1.TokenValidationRuleTypeRequiredClaim, + RequiredClaim: nil, + }) + } + }, + }), + expectError: true, + }, + { + name: "valid auth config", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].Issuer.URL = testServer.URL + } + }, + }), + expectedAuthConfig: authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + authConfig.JWT[0].Issuer.URL = testServer.URL + }, + }), + expectError: false, + }, + { + name: "valid auth config with empty CA name", + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].Issuer.CertificateAuthority.Name = "" + } + }, + }), + expectedAuthConfig: authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + for i := range authConfig.JWT { + authConfig.JWT[i].Issuer.CertificateAuthority = "" + } + }, + }), + expectError: false, + }, + { + name: "auth config with default prefix policy", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].Issuer.URL = testServer.URL + auth.Spec.OIDCProviders[i].ClaimMappings.Username = configv1.UsernameClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "username", + }, + PrefixPolicy: configv1.NoOpinion, + } + } + }, + }), + expectedAuthConfig: authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + for i := range authConfig.JWT { + authConfig.JWT[i].Issuer.URL = testServer.URL + authConfig.JWT[i].ClaimMappings.Username = apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: ptr.To(""), + } + } + }, + }), + expectError: false, + }, + { + name: "auth config with no prefix policy", + caBundleConfigMap: &baseCABundleConfigMap, + auth: *authWithUpdates(baseAuthResource, []func(auth *configv1.Authentication){ + func(auth *configv1.Authentication) { + for i := range auth.Spec.OIDCProviders { + auth.Spec.OIDCProviders[i].Issuer.URL = testServer.URL + auth.Spec.OIDCProviders[i].ClaimMappings.Username = configv1.UsernameClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Claim: "username", + }, + PrefixPolicy: configv1.NoPrefix, + } + } + }, + }), + expectedAuthConfig: authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + for i := range authConfig.JWT { + authConfig.JWT[i].Issuer.URL = testServer.URL + authConfig.JWT[i].ClaimMappings.Username = apiserverv1beta1.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: ptr.To("-"), + } + } + }, + }), + expectError: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + + if tt.configMapIndexer == nil { + tt.configMapIndexer = cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + } + + if tt.caBundleConfigMap != nil { + tt.configMapIndexer.Add(tt.caBundleConfigMap) + } + + c := externalOIDCController{ + configMapLister: corev1listers.NewConfigMapLister(tt.configMapIndexer), + } + + gotConfig, err := c.generateAuthConfig(tt.auth) + if tt.expectError && err == nil { + t.Errorf("expected error but didn't get any") + } + + if !tt.expectError && err != nil { + t.Errorf("did not expect any error but got: %v", err) + } + + if !equality.Semantic.DeepEqual(tt.expectedAuthConfig, gotConfig) { + t.Errorf("unexpected config diff: %s", diff.ObjectReflectDiff(tt.expectedAuthConfig, gotConfig)) + } + }) + } +} + +func TestExternalOIDCController_validateAuthenticationConfiguration(t *testing.T) { + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, nil) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + for _, tt := range []struct { + name string + authConfig apiserverv1beta1.AuthenticationConfiguration + expectError bool + }{ + { + name: "empty config", + authConfig: apiserverv1beta1.AuthenticationConfiguration{}, + expectError: false, + }, + { + name: "issuer with empty URL", + authConfig: *authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + authConfig.JWT[0].Issuer.URL = "" + }, + }), + expectError: true, + }, + { + name: "issuer with http URL", + authConfig: *authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + authConfig.JWT[0].Issuer.URL = "http://insecure.com" + }, + }), + expectError: true, + }, + { + name: "issuer with invalid CA", + authConfig: *authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + authConfig.JWT[0].Issuer.CertificateAuthority = "invalid CA" + }, + }), + expectError: true, + }, + { + name: "valid auth config", + authConfig: *authConfigWithUpdates(baseAuthConfig, []func(authConfig *apiserverv1beta1.AuthenticationConfiguration){ + func(authConfig *apiserverv1beta1.AuthenticationConfiguration) { + authConfig.JWT[0].Issuer.URL = testServer.URL + }, + }), + expectError: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + err := validateAuthenticationConfiguration(tt.authConfig) + if tt.expectError && err == nil { + t.Errorf("expected error but didn't get any") + } + + if !tt.expectError && err != nil { + t.Errorf("did not expect any error but got: %v", err) + } + + }) + } +} + +func TestExternalOIDCController_validateCACert(t *testing.T) { + certPool := x509.NewCertPool() + certPool.AddCert(baseCACert) + + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, nil) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + t.Run("nil CA cert to use system CAs", func(t *testing.T) { + err := validateCACert(testServer.URL, nil) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("valid CA cert", func(t *testing.T) { + err := validateCACert(testServer.URL, certPool) + if err != nil { + t.Errorf("got error while not expecting one: %v", err) + } + }) + + t.Run("mismatched CA cert", func(t *testing.T) { + anotherCACert, _, err := generateCAKeyPair() + if err != nil { + t.Errorf("could not generate CA keypair: %v", err) + } + certPool := x509.NewCertPool() + certPool.AddCert(anotherCACert) + err = validateCACert(testServer.URL, certPool) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("unknown URL", func(t *testing.T) { + err = validateCACert("https://does-not-exist.com", certPool) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("nil cert pool", func(t *testing.T) { + err := validateCACert(testServer.URL, nil) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("empty cert pool", func(t *testing.T) { + err := validateCACert(testServer.URL, x509.NewCertPool()) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("well-known request error", func(t *testing.T) { + handlerFunc := func(w http.ResponseWriter, r *http.Request) { + time.Sleep(6 * time.Second) + w.WriteHeader(http.StatusOK) + } + + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, handlerFunc) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + err = validateCACert(testServer.URL, certPool) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) + + t.Run("well-known status not 200 OK", func(t *testing.T) { + handlerFunc := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusFound) + } + + testServer, err := createTestServer(baseCACert, baseCAPrivateKey, handlerFunc) + if err != nil { + t.Fatalf("could not create test server: %v", err) + } + defer testServer.Close() + testServer.StartTLS() + + err = validateCACert(testServer.URL, certPool) + if err == nil { + t.Errorf("did not get an error while expecting one") + } + }) +} + +func generateCAKeyPair() (*x509.Certificate, crypto.Signer, error) { + caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return nil, nil, err + } + + caCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "test-ca"}, caPrivateKey) + if err != nil { + return nil, nil, err + } + + return caCert, caPrivateKey, err +} + +func generateServingCert(caCert *x509.Certificate, caPrivateKey crypto.Signer) (*tls.Certificate, error) { + cert := &x509.Certificate{ + SerialNumber: big.NewInt(2024), + Subject: pkix.Name{ + Organization: []string{"Company, INC."}, + Country: []string{"US"}, + Province: []string{""}, + Locality: []string{"Springfield"}, + StreetAddress: []string{"742 Evergreen Terrace"}, + }, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, + NotBefore: time.Now(), + NotAfter: time.Now().AddDate(10, 0, 0), + SubjectKeyId: []byte{1, 2, 3, 4, 6}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature, + } + + certPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return nil, err + } + + certBytes, err := x509.CreateCertificate(rand.Reader, cert, caCert, &certPrivKey.PublicKey, caPrivateKey) + if err != nil { + return nil, err + } + + certPEM := new(bytes.Buffer) + pem.Encode(certPEM, &pem.Block{ + Type: "CERTIFICATE", + Bytes: certBytes, + }) + + certPrivKeyPEM := new(bytes.Buffer) + pem.Encode(certPrivKeyPEM, &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(certPrivKey), + }) + + serverCert, err := tls.X509KeyPair(certPEM.Bytes(), certPrivKeyPEM.Bytes()) + if err != nil { + return nil, err + } + + return &serverCert, nil +} + +func createTestServer(caCert *x509.Certificate, caPrivateKey crypto.Signer, handlerFunc http.HandlerFunc) (*httptest.Server, error) { + cert := caCert + key := caPrivateKey + var err error + if caCert == nil { + cert, key, err = generateCAKeyPair() + if err != nil { + return nil, err + } + } + + servingCertPair, err := generateServingCert(cert, key) + if err != nil { + return nil, err + } + + if handlerFunc == nil { + handlerFunc = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + } + + testServer := httptest.NewUnstartedServer(handlerFunc) + testServer.TLS = &tls.Config{ + Certificates: []tls.Certificate{*servingCertPair}, + } + + return testServer, nil +} + +func makeClosedChannel() chan struct{} { + ch := make(chan struct{}) + close(ch) + return ch +} + +func newAuthWithSpec(spec configv1.AuthenticationSpec) *configv1.Authentication { + return &configv1.Authentication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: spec, + } +} + +func authWithUpdates(auth configv1.Authentication, updateFuncs []func(auth *configv1.Authentication)) *configv1.Authentication { + copy := auth.DeepCopy() + for _, updateFunc := range updateFuncs { + updateFunc(copy) + } + return copy +} + +func authConfigWithUpdates(authConfig apiserverv1beta1.AuthenticationConfiguration, updateFuncs []func(authConfig *apiserverv1beta1.AuthenticationConfiguration)) *apiserverv1beta1.AuthenticationConfiguration { + copy := authConfig.DeepCopy() + for _, updateFunc := range updateFuncs { + updateFunc(copy) + } + return copy +} + +func authConfigCMWithIssuerURL(cm *corev1.ConfigMap, issuerURL string) *corev1.ConfigMap { + copy := cm.DeepCopy() + copy.Data[authConfigDataKey] = strings.ReplaceAll(baseAuthConfigJSON, "$URL", issuerURL) + return copy +} + +type everFailingIndexer struct{} + +// Index always returns an error +func (i *everFailingIndexer) Index(indexName string, obj interface{}) ([]interface{}, error) { + return nil, fmt.Errorf("Index method not implemented") +} + +// IndexKeys always returns an error +func (i *everFailingIndexer) IndexKeys(indexName, indexedValue string) ([]string, error) { + return nil, fmt.Errorf("IndexKeys method not implemented") +} + +// ListIndexFuncValues always returns an error +func (i *everFailingIndexer) ListIndexFuncValues(indexName string) []string { + return nil +} + +// ByIndex always returns an error +func (i *everFailingIndexer) ByIndex(indexName, indexedValue string) ([]interface{}, error) { + return nil, fmt.Errorf("ByIndex method not implemented") +} + +// GetIndexers always returns an error +func (i *everFailingIndexer) GetIndexers() cache.Indexers { + return nil +} + +// AddIndexers always returns an error +func (i *everFailingIndexer) AddIndexers(newIndexers cache.Indexers) error { + return fmt.Errorf("AddIndexers method not implemented") +} + +// Add always returns an error +func (s *everFailingIndexer) Add(obj interface{}) error { + return fmt.Errorf("Add method not implemented") +} + +// Update always returns an error +func (s *everFailingIndexer) Update(obj interface{}) error { + return fmt.Errorf("Update method not implemented") +} + +// Delete always returns an error +func (s *everFailingIndexer) Delete(obj interface{}) error { + return fmt.Errorf("Delete method not implemented") +} + +// List always returns nil +func (s *everFailingIndexer) List() []interface{} { + return nil +} + +// ListKeys always returns nil +func (s *everFailingIndexer) ListKeys() []string { + return nil +} + +// Get always returns an error +func (s *everFailingIndexer) Get(obj interface{}) (item interface{}, exists bool, err error) { + return nil, false, fmt.Errorf("Get method not implemented") +} + +// GetByKey always returns an error +func (s *everFailingIndexer) GetByKey(key string) (item interface{}, exists bool, err error) { + return nil, false, fmt.Errorf("GetByKey method not implemented") +} + +// Replace always returns an error +func (s *everFailingIndexer) Replace(objects []interface{}, sKey string) error { + return fmt.Errorf("Replace method not implemented") +} + +// Resync always returns an error +func (s *everFailingIndexer) Resync() error { + return fmt.Errorf("Resync method not implemented") +} diff --git a/pkg/operator/replacement_starter.go b/pkg/operator/replacement_starter.go index 2b08fb14e..d9d5d115f 100644 --- a/pkg/operator/replacement_starter.go +++ b/pkg/operator/replacement_starter.go @@ -318,5 +318,12 @@ func CreateOperatorStarter(ctx context.Context, authOperatorInput *authenticatio ret.ControllerRunFns = append(ret.ControllerRunFns, oauthAPIServerRunFns...) ret.ControllerNamedRunOnceFns = append(ret.ControllerNamedRunOnceFns, oauthAPIServerRunOnceFns...) + externalOIDCRunOnceFns, externalOIDCRunFns, err := prepareExternalOIDC(ctx, authOperatorInput, informerFactories) + if err != nil { + return nil, fmt.Errorf("unable to prepare external OIDC: %w", err) + } + ret.ControllerRunFns = append(ret.ControllerRunFns, externalOIDCRunFns...) + ret.ControllerNamedRunOnceFns = append(ret.ControllerNamedRunOnceFns, externalOIDCRunOnceFns...) + return ret, nil } diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index d0ea159bc..48297372b 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -12,6 +12,7 @@ import ( "github.com/openshift/multi-operator-manager/pkg/library/libraryapplyconfiguration" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" @@ -19,6 +20,7 @@ import ( "github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation/configobservercontroller" componentroutesecretsync "github.com/openshift/cluster-authentication-operator/pkg/controllers/customroute" "github.com/openshift/cluster-authentication-operator/pkg/controllers/deployment" + "github.com/openshift/cluster-authentication-operator/pkg/controllers/externaloidc" "github.com/openshift/cluster-authentication-operator/pkg/controllers/ingressnodesavailable" "github.com/openshift/cluster-authentication-operator/pkg/controllers/ingressstate" "github.com/openshift/cluster-authentication-operator/pkg/controllers/metadata" @@ -39,6 +41,7 @@ import ( workloadcontroller "github.com/openshift/library-go/pkg/operator/apiserver/controller/workload" apiservercontrollerset "github.com/openshift/library-go/pkg/operator/apiserver/controllerset" "github.com/openshift/library-go/pkg/operator/certrotation" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/csr" "github.com/openshift/library-go/pkg/operator/encryption" "github.com/openshift/library-go/pkg/operator/encryption/controllers/migrators" @@ -671,6 +674,52 @@ func prepareOauthAPIServerOperator( return runOnceFns, runFns, nil } +func prepareExternalOIDC( + ctx context.Context, + authOperatorInput *authenticationOperatorInput, + informerFactories authenticationOperatorInformerFactories, +) ([]libraryapplyconfiguration.NamedRunOnce, []libraryapplyconfiguration.RunFunc, error) { + + // By default, this will exit(0) if the featuregates change + featureGateAccessor := featuregates.NewFeatureGateAccess( + status.VersionForOperatorFromEnv(), "0.0.1-snapshot", + informerFactories.operatorConfigInformer.Config().V1().ClusterVersions(), + informerFactories.operatorConfigInformer.Config().V1().FeatureGates(), + authOperatorInput.eventRecorder, + ) + go featureGateAccessor.Run(ctx) + go informerFactories.operatorConfigInformer.Start(ctx.Done()) + + var featureGates featuregates.FeatureGate + select { + case <-featureGateAccessor.InitialFeatureGatesObserved(): + featureGates, _ = featureGateAccessor.CurrentFeatureGates() + case <-time.After(1 * time.Minute): + return nil, nil, fmt.Errorf("timed out waiting for FeatureGate detection") + } + + if !featureGates.Enabled(features.FeatureGateExternalOIDC) { + return nil, nil, nil + } + + externalOIDCController := externaloidc.NewExternalOIDCController( + informerFactories.kubeInformersForNamespaces, + informerFactories.operatorConfigInformer, + authOperatorInput.authenticationOperatorClient, + authOperatorInput.kubeClient.CoreV1(), + authOperatorInput.eventRecorder, + ) + + runOnceFns := []libraryapplyconfiguration.NamedRunOnce{ + libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, "TODO-other-externalOIDCController", externalOIDCController.Sync), + } + runFns := []libraryapplyconfiguration.RunFunc{ + libraryapplyconfiguration.AdaptRunFn(externalOIDCController.Run), + } + + return runOnceFns, runFns, nil +} + func singleNameListOptions(name string) func(opts *metav1.ListOptions) { return func(opts *metav1.ListOptions) { opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", name).String() diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/featuregate.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/featuregate.go new file mode 100644 index 000000000..5792ada3a --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/featuregate.go @@ -0,0 +1,48 @@ +package featuregates + +import ( + "fmt" + "slices" + + configv1 "github.com/openshift/api/config/v1" +) + +// FeatureGate indicates whether a given feature is enabled or not +// This interface is heavily influenced by k8s.io/component-base, but not exactly compatible. +type FeatureGate interface { + // Enabled returns true if the key is enabled. + Enabled(key configv1.FeatureGateName) bool + // KnownFeatures returns a slice of strings describing the FeatureGate's known features. + KnownFeatures() []configv1.FeatureGateName +} + +type featureGate struct { + enabled []configv1.FeatureGateName + disabled []configv1.FeatureGateName +} + +func NewFeatureGate(enabled, disabled []configv1.FeatureGateName) FeatureGate { + return &featureGate{ + enabled: enabled, + disabled: disabled, + } +} + +func (f *featureGate) Enabled(key configv1.FeatureGateName) bool { + if slices.Contains(f.enabled, key) { + return true + } + if slices.Contains(f.disabled, key) { + return false + } + + panic(fmt.Errorf("feature %q is not registered in FeatureGates %v", key, f.KnownFeatures())) +} + +func (f *featureGate) KnownFeatures() []configv1.FeatureGateName { + allKnown := make([]configv1.FeatureGateName, 0, len(f.enabled)+len(f.disabled)) + allKnown = append(allKnown, f.enabled...) + allKnown = append(allKnown, f.disabled...) + + return allKnown +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/hardcoded_featuregate_reader.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/hardcoded_featuregate_reader.go new file mode 100644 index 000000000..58ae71763 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/hardcoded_featuregate_reader.go @@ -0,0 +1,78 @@ +package featuregates + +import ( + "context" + "fmt" + + configv1 "github.com/openshift/api/config/v1" +) + +type hardcodedFeatureGateAccess struct { + enabled []configv1.FeatureGateName + disabled []configv1.FeatureGateName + readErr error + + initialFeatureGatesObserved chan struct{} +} + +// NewHardcodedFeatureGateAccess returns a FeatureGateAccess that is always initialized and always +// returns the provided feature gates. +func NewHardcodedFeatureGateAccess(enabled, disabled []configv1.FeatureGateName) FeatureGateAccess { + initialFeatureGatesObserved := make(chan struct{}) + close(initialFeatureGatesObserved) + c := &hardcodedFeatureGateAccess{ + enabled: enabled, + disabled: disabled, + initialFeatureGatesObserved: initialFeatureGatesObserved, + } + + return c +} + +// NewHardcodedFeatureGateAccessForTesting returns a FeatureGateAccess that returns stub responses +// using caller-supplied values. +func NewHardcodedFeatureGateAccessForTesting(enabled, disabled []configv1.FeatureGateName, initialFeatureGatesObserved chan struct{}, readErr error) FeatureGateAccess { + return &hardcodedFeatureGateAccess{ + enabled: enabled, + disabled: disabled, + initialFeatureGatesObserved: initialFeatureGatesObserved, + readErr: readErr, + } +} + +func (c *hardcodedFeatureGateAccess) SetChangeHandler(featureGateChangeHandlerFn FeatureGateChangeHandlerFunc) { + // ignore +} + +func (c *hardcodedFeatureGateAccess) Run(ctx context.Context) { + // ignore +} + +func (c *hardcodedFeatureGateAccess) InitialFeatureGatesObserved() <-chan struct{} { + return c.initialFeatureGatesObserved +} + +func (c *hardcodedFeatureGateAccess) AreInitialFeatureGatesObserved() bool { + select { + case <-c.InitialFeatureGatesObserved(): + return true + default: + return false + } +} + +func (c *hardcodedFeatureGateAccess) CurrentFeatureGates() (FeatureGate, error) { + return NewFeatureGate(c.enabled, c.disabled), c.readErr +} + +// NewHardcodedFeatureGateAccessFromFeatureGate returns a FeatureGateAccess that is static and initialised from +// a populated FeatureGate status. +// If the desired version is missing, this will return an error. +func NewHardcodedFeatureGateAccessFromFeatureGate(featureGate *configv1.FeatureGate, desiredVersion string) (FeatureGateAccess, error) { + features, err := featuresFromFeatureGate(featureGate, desiredVersion) + if err != nil { + return nil, fmt.Errorf("unable to determine features: %w", err) + } + + return NewHardcodedFeatureGateAccess(features.Enabled, features.Disabled), nil +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/observe_featuregates.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/observe_featuregates.go new file mode 100644 index 000000000..0f2cb85fd --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/observe_featuregates.go @@ -0,0 +1,118 @@ +package featuregates + +import ( + "fmt" + "reflect" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/sets" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/events" +) + +// NewObserveFeatureFlagsFunc produces a configobserver for feature gates. If non-nil, the featureWhitelist filters +// feature gates to a known subset (instead of everything). The featureBlacklist will stop certain features from making +// it through the list. The featureBlacklist should be empty, but for a brief time, some featuregates may need to skipped. +// @smarterclayton will live forever in shame for being the first to require this for "IPv6DualStack". +func NewObserveFeatureFlagsFunc(featureWhitelist sets.Set[configv1.FeatureGateName], featureBlacklist sets.Set[configv1.FeatureGateName], configPath []string, featureGateAccess FeatureGateAccess) configobserver.ObserveConfigFunc { + return (&featureFlags{ + allowAll: len(featureWhitelist) == 0, + featureWhitelist: featureWhitelist, + featureBlacklist: featureBlacklist, + configPath: configPath, + featureGateAccess: featureGateAccess, + }).ObserveFeatureFlags +} + +type featureFlags struct { + allowAll bool + featureWhitelist sets.Set[configv1.FeatureGateName] + // we add a forceDisableFeature list because we've now had bad featuregates break individual operators. Awesome. + featureBlacklist sets.Set[configv1.FeatureGateName] + configPath []string + featureGateAccess FeatureGateAccess +} + +// ObserveFeatureFlags fills in --feature-flags for the kube-apiserver +func (f *featureFlags) ObserveFeatureFlags(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + prunedExistingConfig := configobserver.Pruned(existingConfig, f.configPath) + + errs := []error{} + + if !f.featureGateAccess.AreInitialFeatureGatesObserved() { + // if we haven't observed featuregates yet, return the existing + return prunedExistingConfig, nil + } + + featureGates, err := f.featureGateAccess.CurrentFeatureGates() + if err != nil { + return prunedExistingConfig, append(errs, err) + } + observedConfig := map[string]interface{}{} + newConfigValue := f.getWhitelistedFeatureNames(featureGates) + + currentConfigValue, _, err := unstructured.NestedStringSlice(existingConfig, f.configPath...) + if err != nil { + errs = append(errs, err) + // keep going on read error from existing config + } + if !reflect.DeepEqual(currentConfigValue, newConfigValue) { + recorder.Eventf("ObserveFeatureFlagsUpdated", "Updated %v to %s", strings.Join(f.configPath, "."), strings.Join(newConfigValue, ",")) + } + + if err := unstructured.SetNestedStringSlice(observedConfig, newConfigValue, f.configPath...); err != nil { + recorder.Warningf("ObserveFeatureFlags", "Failed setting %v: %v", strings.Join(f.configPath, "."), err) + return prunedExistingConfig, append(errs, err) + } + + return configobserver.Pruned(observedConfig, f.configPath), errs +} + +func (f *featureFlags) getWhitelistedFeatureNames(featureGates FeatureGate) []string { + newConfigValue := []string{} + formatEnabledFunc := func(fs configv1.FeatureGateName) string { + return fmt.Sprintf("%v=true", fs) + } + formatDisabledFunc := func(fs configv1.FeatureGateName) string { + return fmt.Sprintf("%v=false", fs) + } + + for _, knownFeatureGate := range featureGates.KnownFeatures() { + if f.featureBlacklist.Has(knownFeatureGate) { + continue + } + // only add whitelisted feature flags + if !f.allowAll && !f.featureWhitelist.Has(knownFeatureGate) { + continue + } + + if featureGates.Enabled(knownFeatureGate) { + newConfigValue = append(newConfigValue, formatEnabledFunc(knownFeatureGate)) + } else { + newConfigValue = append(newConfigValue, formatDisabledFunc(knownFeatureGate)) + } + } + + return newConfigValue +} + +func StringsToFeatureGateNames(in []string) []configv1.FeatureGateName { + out := []configv1.FeatureGateName{} + for _, curr := range in { + out = append(out, configv1.FeatureGateName(curr)) + } + + return out +} + +func FeatureGateNamesToStrings(in []configv1.FeatureGateName) []string { + out := []string{} + for _, curr := range in { + out = append(out, string(curr)) + } + + return out +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/simple_featuregate_reader.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/simple_featuregate_reader.go new file mode 100644 index 000000000..4b2caccd6 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/simple_featuregate_reader.go @@ -0,0 +1,318 @@ +package featuregates + +import ( + "context" + "fmt" + "os" + "reflect" + "sync" + "time" + + configv1 "github.com/openshift/api/config/v1" + + v1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/operator/events" + apierrors "k8s.io/apimachinery/pkg/api/errors" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog/v2" +) + +type FeatureGateChangeHandlerFunc func(featureChange FeatureChange) + +// FeatureGateAccess is used to get a list of enabled and disabled featuregates. +// Create a new instance using NewFeatureGateAccess. +// To create one for unit testing, use NewHardcodedFeatureGateAccess. +type FeatureGateAccess interface { + // SetChangeHandler can only be called before Run. + // The default change handler will exit 0 when the set of featuregates changes. + // That is usually the easiest and simplest thing for an *operator* to do. + // This also discourages direct operand reading since all operands restarting simultaneously is bad. + // This function allows changing that default behavior to something else (perhaps a channel notification for + // all impacted controllers in an operator. + // I doubt this will be worth the effort in the majority of cases. + SetChangeHandler(featureGateChangeHandlerFn FeatureGateChangeHandlerFunc) + + // Run starts a go func that continously watches the set of featuregates enabled in the cluster. + Run(ctx context.Context) + // InitialFeatureGatesObserved returns a channel that is closed once the featuregates have + // been observed. Once closed, the CurrentFeatureGates method will return the current set of + // featuregates and will never return a non-nil error. + InitialFeatureGatesObserved() <-chan struct{} + // CurrentFeatureGates returns the list of enabled and disabled featuregates. + // It returns an error if the current set of featuregates is not known. + CurrentFeatureGates() (FeatureGate, error) + // AreInitialFeatureGatesObserved returns true if the initial featuregates have been observed. + AreInitialFeatureGatesObserved() bool +} + +type Features struct { + Enabled []configv1.FeatureGateName + Disabled []configv1.FeatureGateName +} + +type FeatureChange struct { + Previous *Features + New Features +} + +type defaultFeatureGateAccess struct { + desiredVersion string + missingVersionMarker string + clusterVersionLister configlistersv1.ClusterVersionLister + featureGateLister configlistersv1.FeatureGateLister + initialFeatureGatesObserved chan struct{} + + featureGateChangeHandlerFn FeatureGateChangeHandlerFunc + + lock sync.Mutex + started bool + initialFeatures Features + currentFeatures Features + + queue workqueue.RateLimitingInterface + eventRecorder events.Recorder +} + +// NewFeatureGateAccess returns a controller that keeps the list of enabled/disabled featuregates up to date. +// desiredVersion is the version of this operator that would be set on the clusteroperator.status.versions. +// missingVersionMarker is the stub version provided by the operator. If that is also the desired version, +// then the most either the desired clusterVersion or most recent version will be used. +// clusterVersionInformer is used when desiredVersion and missingVersionMarker are the same to derive the "best" version +// of featuregates to use. +// featureGateInformer is used to track changes to the featureGates once they are initially set. +// By default, when the enabled/disabled list of featuregates changes, os.Exit is called. This behavior can be +// overridden by calling SetChangeHandler to whatever you wish the behavior to be. +// A common construct is: +/* go +featureGateAccessor := NewFeatureGateAccess(args) +go featureGateAccessor.Run(ctx) + +select{ +case <- featureGateAccessor.InitialFeatureGatesObserved(): + featureGates, _ := featureGateAccessor.CurrentFeatureGates() + klog.Infof("FeatureGates initialized: knownFeatureGates=%v", featureGates.KnownFeatures()) +case <- time.After(1*time.Minute): + klog.Errorf("timed out waiting for FeatureGate detection") + return fmt.Errorf("timed out waiting for FeatureGate detection") +} + +// whatever other initialization you have to do, at this point you have FeatureGates to drive your behavior. +*/ +// That construct is easy. It is better to use the .spec.observedConfiguration construct common in library-go operators +// to avoid gating your general startup on FeatureGate determination, but if you haven't already got that mechanism +// this construct is easy. +func NewFeatureGateAccess( + desiredVersion, missingVersionMarker string, + clusterVersionInformer v1.ClusterVersionInformer, + featureGateInformer v1.FeatureGateInformer, + eventRecorder events.Recorder) FeatureGateAccess { + c := &defaultFeatureGateAccess{ + desiredVersion: desiredVersion, + missingVersionMarker: missingVersionMarker, + clusterVersionLister: clusterVersionInformer.Lister(), + featureGateLister: featureGateInformer.Lister(), + initialFeatureGatesObserved: make(chan struct{}), + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "feature-gate-detector"), + eventRecorder: eventRecorder, + } + c.SetChangeHandler(ForceExit) + + // we aren't expecting many + clusterVersionInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + c.queue.Add("cluster") + }, + UpdateFunc: func(old, cur interface{}) { + c.queue.Add("cluster") + }, + DeleteFunc: func(uncast interface{}) { + c.queue.Add("cluster") + }, + }) + featureGateInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + c.queue.Add("cluster") + }, + UpdateFunc: func(old, cur interface{}) { + c.queue.Add("cluster") + }, + DeleteFunc: func(uncast interface{}) { + c.queue.Add("cluster") + }, + }) + + return c +} + +func ForceExit(featureChange FeatureChange) { + if featureChange.Previous != nil { + os.Exit(0) + } +} + +func (c *defaultFeatureGateAccess) SetChangeHandler(featureGateChangeHandlerFn FeatureGateChangeHandlerFunc) { + c.lock.Lock() + defer c.lock.Unlock() + + if c.started { + panic("programmer error, cannot update the change handler after starting") + } + c.featureGateChangeHandlerFn = featureGateChangeHandlerFn +} + +func (c *defaultFeatureGateAccess) Run(ctx context.Context) { + defer utilruntime.HandleCrash() + defer c.queue.ShutDown() + + klog.Infof("Starting feature-gate-detector") + defer klog.Infof("Shutting down feature-gate-detector") + + go wait.UntilWithContext(ctx, c.runWorker, time.Second) + + <-ctx.Done() +} + +func (c *defaultFeatureGateAccess) syncHandler(ctx context.Context) error { + desiredVersion := c.desiredVersion + if c.missingVersionMarker == c.desiredVersion { + clusterVersion, err := c.clusterVersionLister.Get("version") + if apierrors.IsNotFound(err) { + return nil // we will be re-triggered when it is created + } + if err != nil { + return err + } + + desiredVersion = clusterVersion.Status.Desired.Version + if len(desiredVersion) == 0 && len(clusterVersion.Status.History) > 0 { + desiredVersion = clusterVersion.Status.History[0].Version + } + } + + featureGate, err := c.featureGateLister.Get("cluster") + if apierrors.IsNotFound(err) { + return nil // we will be re-triggered when it is created + } + if err != nil { + return err + } + + features, err := featuresFromFeatureGate(featureGate, desiredVersion) + if err != nil { + return fmt.Errorf("unable to determine features: %w", err) + } + + c.setFeatureGates(features) + + return nil +} + +func (c *defaultFeatureGateAccess) setFeatureGates(features Features) { + c.lock.Lock() + defer c.lock.Unlock() + + var previousFeatures *Features + if c.AreInitialFeatureGatesObserved() { + t := c.currentFeatures + previousFeatures = &t + } + + c.currentFeatures = features + + if !c.AreInitialFeatureGatesObserved() { + c.initialFeatures = features + close(c.initialFeatureGatesObserved) + c.eventRecorder.Eventf("FeatureGatesInitialized", "FeatureGates updated to %#v", c.currentFeatures) + } + + if previousFeatures == nil || !reflect.DeepEqual(*previousFeatures, c.currentFeatures) { + if previousFeatures != nil { + c.eventRecorder.Eventf("FeatureGatesModified", "FeatureGates updated to %#v", c.currentFeatures) + } + + c.featureGateChangeHandlerFn(FeatureChange{ + Previous: previousFeatures, + New: c.currentFeatures, + }) + } +} + +func (c *defaultFeatureGateAccess) InitialFeatureGatesObserved() <-chan struct{} { + return c.initialFeatureGatesObserved +} + +func (c *defaultFeatureGateAccess) AreInitialFeatureGatesObserved() bool { + select { + case <-c.InitialFeatureGatesObserved(): + return true + default: + return false + } +} + +func (c *defaultFeatureGateAccess) CurrentFeatureGates() (FeatureGate, error) { + c.lock.Lock() + defer c.lock.Unlock() + + if !c.AreInitialFeatureGatesObserved() { + return nil, fmt.Errorf("featureGates not yet observed") + } + retEnabled := make([]configv1.FeatureGateName, len(c.currentFeatures.Enabled)) + retDisabled := make([]configv1.FeatureGateName, len(c.currentFeatures.Disabled)) + copy(retEnabled, c.currentFeatures.Enabled) + copy(retDisabled, c.currentFeatures.Disabled) + + return NewFeatureGate(retEnabled, retDisabled), nil +} + +func (c *defaultFeatureGateAccess) runWorker(ctx context.Context) { + for c.processNextWorkItem(ctx) { + } +} + +func (c *defaultFeatureGateAccess) processNextWorkItem(ctx context.Context) bool { + dsKey, quit := c.queue.Get() + if quit { + return false + } + defer c.queue.Done(dsKey) + + err := c.syncHandler(ctx) + if err == nil { + c.queue.Forget(dsKey) + return true + } + + utilruntime.HandleError(fmt.Errorf("%v failed with : %v", dsKey, err)) + c.queue.AddRateLimited(dsKey) + + return true +} + +func featuresFromFeatureGate(featureGate *configv1.FeatureGate, desiredVersion string) (Features, error) { + found := false + features := Features{} + for _, featureGateValues := range featureGate.Status.FeatureGates { + if featureGateValues.Version != desiredVersion { + continue + } + found = true + for _, enabled := range featureGateValues.Enabled { + features.Enabled = append(features.Enabled, enabled.Name) + } + for _, disabled := range featureGateValues.Disabled { + features.Disabled = append(features.Disabled, disabled.Name) + } + break + } + + if !found { + return Features{}, fmt.Errorf("missing desired version %q in featuregates.config.openshift.io/cluster", desiredVersion) + } + + return features, nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 64ba98976..66477ae03 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -344,6 +344,7 @@ github.com/openshift/library-go/pkg/operator/condition 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/etcd +github.com/openshift/library-go/pkg/operator/configobserver/featuregates github.com/openshift/library-go/pkg/operator/configobserver/oauth github.com/openshift/library-go/pkg/operator/csr github.com/openshift/library-go/pkg/operator/encryption