Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #317 from p0lyn0mial/release-4.5-clean-up-oauth-ap…
…iserver Bug 1860922: adds OAuthAPIServer pruner controller
- Loading branch information
Showing
72 changed files
with
4,660 additions
and
111 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package apiservices | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
||
| operatorconfigclient "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1" | ||
| operatorclientinformers "github.com/openshift/client-go/operator/informers/externalversions" | ||
| operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" | ||
| "github.com/openshift/library-go/pkg/controller/factory" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
| ) | ||
|
|
||
| // NewUnmanageAPIServicesController sets ManagingOAuthAPIServer flag to false. | ||
| // This will make OAuth APIServer (introduced in 4.6) to step down and traffic | ||
| // will be routed to the OpenShift APIServer. It's a measure required for 4.6->4.5 | ||
| // downgrade. | ||
| // | ||
| // see https://github.com/openshift/enhancements/blob/master/enhancements/authentication/separate-oauth-resources.md | ||
| func NewUnmanageAPIServicesController( | ||
| name string, | ||
| authOperatorClient operatorconfigclient.AuthenticationsGetter, | ||
| authOperatorInformers operatorclientinformers.SharedInformerFactory, | ||
| eventRecorder events.Recorder, | ||
| ) factory.Controller { | ||
|
|
||
| authInformers := authOperatorInformers.Operator().V1().Authentications() | ||
| return factory.New(). | ||
| WithSync(syncUnmanageAPIServicesController(name, authOperatorClient, authInformers.Lister())). | ||
| WithInformers(authInformers.Informer()). | ||
| ToController(name, eventRecorder.WithComponentSuffix("unmanage-oauth-api-controller")) | ||
| } | ||
|
|
||
| func syncUnmanageAPIServicesController(controllerName string, authOperatorClient operatorconfigclient.AuthenticationsGetter, authOperatorLister operatorlistersv1.AuthenticationLister) factory.SyncFunc { | ||
| return func(ctx context.Context, syncContext factory.SyncContext) error { | ||
| operator, err := authOperatorLister.Get("cluster") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if operator.Status.ManagingOAuthAPIServer { | ||
| operatorCopy := operator.DeepCopy() | ||
| operatorCopy.Status.ManagingOAuthAPIServer = false | ||
| _, err = authOperatorClient.Authentications().UpdateStatus(ctx, operatorCopy, metav1.UpdateOptions{}) | ||
| if err == nil { | ||
| syncContext.Recorder().Eventf(controllerName, "turned OAuth API managing off") | ||
| } | ||
| } | ||
|
|
||
| return err | ||
| } | ||
| } |
94 changes: 94 additions & 0 deletions
94
pkg/controller/apiservices/unmanageoauthapicontroller_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package apiservices | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "k8s.io/client-go/tools/cache" | ||
| "k8s.io/client-go/util/workqueue" | ||
|
|
||
| operatorv1 "github.com/openshift/api/operator/v1" | ||
| fakeoperator "github.com/openshift/client-go/operator/clientset/versioned/fake" | ||
| operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
| ) | ||
|
|
||
| func Test_syncUnmanageAPIServicesController(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| operatorStatus *operatorv1.AuthenticationStatus | ||
| changed bool | ||
| expectErr bool | ||
| }{ | ||
| { | ||
| name: "operator not found", | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "managed set to true", | ||
| operatorStatus: &operatorv1.AuthenticationStatus{ | ||
| ManagingOAuthAPIServer: true, | ||
| }, | ||
| changed: true, | ||
| }, | ||
| { | ||
| name: "managed set to false = no action", | ||
| operatorStatus: &operatorv1.AuthenticationStatus{ | ||
| ManagingOAuthAPIServer: false, | ||
| }, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| authOps := []runtime.Object{} | ||
| indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) | ||
| if tt.operatorStatus != nil { | ||
| operatorObj := &operatorv1.Authentication{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "cluster", | ||
| }, | ||
| Status: *tt.operatorStatus, | ||
| } | ||
|
|
||
| if err := indexer.Add(operatorObj); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| authOps = append(authOps, operatorObj) | ||
| } | ||
| fakeOperatorClient := fakeoperator.NewSimpleClientset(authOps...) | ||
| operatorLister := operatorv1listers.NewAuthenticationLister(indexer) | ||
|
|
||
| testRecorder := events.NewInMemoryRecorder("test") | ||
| if gotErr := syncUnmanageAPIServicesController("testUnmanagedController", fakeOperatorClient.OperatorV1(), operatorLister)(context.TODO(), testSyncContext{recorder: testRecorder}); tt.expectErr != (gotErr != nil) { | ||
| t.Errorf("syncUnmanageAPIServicesController() => expected error: %v, but got %v", tt.expectErr, gotErr) | ||
| } | ||
|
|
||
| var updateObserved bool | ||
| for _, a := range fakeOperatorClient.Actions() { | ||
| if a.GetVerb() == "update" { | ||
| updateObserved = true | ||
| break | ||
| } | ||
| } | ||
| if !tt.changed { | ||
| if len(testRecorder.Events()) > 0 || updateObserved { | ||
| t.Errorf("expected the operator status to be the same, but that did not happen; update observed: %v; events: %v", updateObserved, testRecorder.Events()) | ||
| } | ||
| } else if len(testRecorder.Events()) == 0 || !updateObserved { | ||
| t.Errorf("expected change of the operator status:, but that did not happen; update observed: %v; events: %v", updateObserved, testRecorder.Events()) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| type testSyncContext struct { | ||
| recorder events.Recorder | ||
| } | ||
|
|
||
| func (c testSyncContext) Recorder() events.Recorder { return c.recorder } | ||
|
|
||
| func (c testSyncContext) Queue() workqueue.RateLimitingInterface { return nil } | ||
|
|
||
| func (c testSyncContext) QueueKey() string { return "testkey" } |
137 changes: 137 additions & 0 deletions
137
pkg/controller/oauthapiserverpruner/oauth_apiserver_pruner.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| package oauthapiserverpruner | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| utilerrors "k8s.io/apimachinery/pkg/util/errors" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
| corev1client "k8s.io/client-go/kubernetes/typed/core/v1" | ||
| corev1listers "k8s.io/client-go/listers/core/v1" | ||
| "k8s.io/klog" | ||
| apiregistrationv1informer "k8s.io/kube-aggregator/pkg/client/informers/externalversions/apiregistration/v1" | ||
| apiregistrationv1lister "k8s.io/kube-aggregator/pkg/client/listers/apiregistration/v1" | ||
|
|
||
| operatorclientinformers "github.com/openshift/client-go/operator/informers/externalversions" | ||
| operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" | ||
| "github.com/openshift/library-go/pkg/controller/factory" | ||
| "github.com/openshift/library-go/pkg/operator/encryption/secrets" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
| "github.com/openshift/library-go/pkg/operator/v1helpers" | ||
| ) | ||
|
|
||
| const ( | ||
| OAuthApiServerNsToRemove = "openshift-oauth-apiserver" | ||
| ) | ||
|
|
||
| type oauthAPIServerPrunerController struct { | ||
| apiServicesManagedByOAS []string | ||
|
|
||
| namespaceClient corev1client.NamespaceInterface | ||
| secretClient corev1client.SecretInterface | ||
|
|
||
| apiregistrationv1Lister apiregistrationv1lister.APIServiceLister | ||
| authOperatorLister operatorlistersv1.AuthenticationLister | ||
| namespaceLister corev1listers.NamespaceLister | ||
| } | ||
|
|
||
| // NewOAuthAPIServerPrunerController removes "openshift-oauth-apiserver" namespace. | ||
| // | ||
| // The namespace holds the OAuth API Server and all necessary resources that are valid only for 4.6+ clusters | ||
| // This controller is intended to clean up in case of a downgrade from 4.6 to 4.5. | ||
| func NewOAuthAPIServerPrunerController( | ||
| name string, | ||
| apiServicesManagedByOAS []string, | ||
| kubeClient corev1client.CoreV1Interface, | ||
| kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, | ||
| authOperatorInformers operatorclientinformers.SharedInformerFactory, | ||
| apiregistrationv1Informer apiregistrationv1informer.APIServiceInformer, | ||
| eventRecorder events.Recorder) factory.Controller { | ||
|
|
||
| authInformers := authOperatorInformers.Operator().V1().Authentications() | ||
|
|
||
| c := &oauthAPIServerPrunerController{ | ||
| apiServicesManagedByOAS: apiServicesManagedByOAS, | ||
| namespaceClient: kubeClient.Namespaces(), | ||
| secretClient: kubeClient.Secrets(OAuthApiServerNsToRemove), | ||
| apiregistrationv1Lister: apiregistrationv1Informer.Lister(), | ||
| authOperatorLister: authInformers.Lister(), | ||
| namespaceLister: kubeInformersForNamespaces.InformersFor("").Core().V1().Namespaces().Lister(), | ||
| } | ||
|
|
||
| controllerFactory := factory.New() | ||
| controllerFactory.WithInformers(authInformers.Informer(), kubeInformersForNamespaces.InformersFor("").Core().V1().Namespaces().Informer(), apiregistrationv1Informer.Informer()) | ||
| controllerFactory.WithSync(c.sync) | ||
|
|
||
| return controllerFactory.ToController(name, eventRecorder.WithComponentSuffix("oauth-apiserver-cleaner-controller")) | ||
| } | ||
|
|
||
| func (c *oauthAPIServerPrunerController) sync(ctx context.Context, _ factory.SyncContext) error { | ||
| // check the ManagingOAuthAPIServer field | ||
| operator, err := c.authOperatorLister.Get("cluster") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if operator.Status.ManagingOAuthAPIServer { | ||
| klog.V(2).Info("waiting for ManagingOAuthAPIServer field to be set to false") | ||
| return nil // we will be called again once the operator status changes | ||
| } | ||
|
|
||
| // be graceful and check if the API Services that were managed by CAO in 4.6 are now being managed by OAS-O | ||
| for _, apiServiceName := range c.apiServicesManagedByOAS { | ||
| managedByOAS, err := c.isAPIServiceMangedByOAS(apiServiceName) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if !managedByOAS { | ||
| klog.V(2).Infof("waiting for the api service %s to be managed by OAS-O", apiServiceName) | ||
| return nil // we will be called again once the api services change | ||
| } | ||
| } | ||
|
|
||
| // remove the namespace | ||
| if _, err := c.namespaceLister.Get(OAuthApiServerNsToRemove); err != nil { | ||
| if errors.IsNotFound(err) { | ||
| return nil // no-op the namespace was already removed | ||
| } | ||
| return err | ||
| } | ||
| if err := c.namespaceClient.Delete(ctx, OAuthApiServerNsToRemove, metav1.DeleteOptions{}); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // remove the finalizer from the encryption config secrets (encryption-config ,encryption-config-REVISION) | ||
| // otherwise the namespace won't be removed | ||
| allSecrets, err := c.secretClient.List(ctx, metav1.ListOptions{}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| var finalizerDeletionErrs []error | ||
| for _, secret := range allSecrets.Items { | ||
| if finalizers := sets.NewString(secret.Finalizers...); finalizers.Has(secrets.EncryptionSecretFinalizer) { | ||
| delete(finalizers, secrets.EncryptionSecretFinalizer) | ||
| secret.Finalizers = finalizers.List() | ||
| if _, err := c.secretClient.Update(ctx, &secret, metav1.UpdateOptions{}); err != nil { | ||
| finalizerDeletionErrs = append(finalizerDeletionErrs, err) | ||
| } | ||
| } | ||
| } | ||
| return utilerrors.NewAggregate(finalizerDeletionErrs) | ||
| } | ||
|
|
||
| func (c *oauthAPIServerPrunerController) isAPIServiceMangedByOAS(apiServiceName string) (bool, error) { | ||
| existingApiService, err := c.apiregistrationv1Lister.Get(apiServiceName) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| // we don't check the "authentication.operator.openshift.io/managed" annotation because it is only set by CAO (4.6) to true. | ||
| // there is no component that sets it back to false or removes it | ||
| if existingApiService.Spec.Service != nil && existingApiService.Spec.Service.Namespace != OAuthApiServerNsToRemove { | ||
| return true, nil | ||
| } | ||
|
|
||
| return false, nil | ||
| } |
Oops, something went wrong.