OAuth related API Services can be managed/served by an external server/operator

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/gogo/protobuf v1.3.1 // indirect
 	github.com/gonum/graph v0.0.0-20170401004347-50b27dea7ebb
 	github.com/jteeuwen/go-bindata v3.0.8-0.20151023091102-a0ff2567cfb7+incompatible
-	github.com/openshift/api v0.0.0-20200109182645-c3cf38ec5571
+	github.com/openshift/api v0.0.0-20200115130134-f472aa214b03
 	github.com/openshift/client-go v0.0.0-20200109173103-2763c6378941
 	github.com/openshift/library-go v0.0.0-20200114124611-9ace650367d2
 	github.com/pkg/errors v0.8.1

go.sum

@@ -308,6 +308,8 @@ github.com/openshift/api v0.0.0-20191217141120-791af96035a5/go.mod h1:dOo9oLY4le
 github.com/openshift/api v0.0.0-20200109154256-35a64c701b10/go.mod h1:dv+J0b/HWai0QnMVb37/H0v36klkLBi2TNpPeWDxX10=
 github.com/openshift/api v0.0.0-20200109182645-c3cf38ec5571 h1:cREUBoPv0hEFnWvSg12TVjjsYcEfNqglyE12Ody4gJk=
 github.com/openshift/api v0.0.0-20200109182645-c3cf38ec5571/go.mod h1:N1jYLqdomc/eHHrU/wphMsZBzRvxv5FBc7ATIdSQelI=
+github.com/openshift/api v0.0.0-20200115130134-f472aa214b03 h1:FH/0yrv+sii6Z9fU9x4G/eClVMs7yN9jtYOArzuIFzs=
+github.com/openshift/api v0.0.0-20200115130134-f472aa214b03/go.mod h1:WKvUfsZJ454fpKTZ2V+R+4/DI0pT4g9aDIizmZMkRto=
 github.com/openshift/client-go v0.0.0-20191216194936-57f413491e9e/go.mod h1:nLJaHFCQ5Mavh98g2ejEnWYFWBMGVdphrKNjLErOn/w=
 github.com/openshift/client-go v0.0.0-20200109173103-2763c6378941 h1:r9oaIRvM0JRem87eHGTCIJCWqRjRhZHcA0uc3cdc+mY=
 github.com/openshift/client-go v0.0.0-20200109173103-2763c6378941/go.mod h1:zMqD3jZrS8UB+n7ZBz/PtyFvkbKExD8i/Dfye5wgFqE=
@@ -502,6 +504,8 @@ golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72 h1:bw9doJza/SFBEweII/rHQh3
 golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200108203644-89082a384178 h1:f5gMxb6FbpY48csegk9UPd7IAHVrBD013CU7N4pWzoE=
 golang.org/x/tools v0.0.0-20200108203644-89082a384178/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
+golang.org/x/tools v0.0.0-20200115044656-831fdb1e1868 h1:6VZw2h4iwEB4GwgQU3Jvcsm8l9+yReTrErAEK1k6AC4=
+golang.org/x/tools v0.0.0-20200115044656-831fdb1e1868/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

pkg/operator/apiservercontrollerset/apiservercontrollerset.go

@@ -3,7 +3,6 @@ package apiservercontrollerset
 import (
 	"context"
 	"fmt"
-
 	configv1 "github.com/openshift/api/config/v1"
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
@@ -18,7 +17,6 @@ import (
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
-	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	apiregistrationv1client "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1"
 	apiregistrationinformers "k8s.io/kube-aggregator/pkg/client/informers/externalversions"
 )
@@ -123,15 +121,15 @@ func (cs *APIServerControllerSet) WithoutClusterOperatorStatusController() *APIS
 
 func (cs *APIServerControllerSet) WithAPIServiceController(
 	controllerName string,
-	apiServices []*apiregistrationv1.APIService,
+	getAPIServicesToManageFn apiservicecontroller.GetAPIServicesToMangeFunc,
 	apiregistrationInformers apiregistrationinformers.SharedInformerFactory,
 	apiregistrationv1Client apiregistrationv1client.ApiregistrationV1Interface,
 	kubeInformersForTargetNamesace kubeinformers.SharedInformerFactory,
 	kubeClient kubernetes.Interface,
 ) *APIServerControllerSet {
 	cs.apiServiceController.controller = apiservicecontroller.NewAPIServiceController(
 		controllerName,
-		apiServices,
+		getAPIServicesToManageFn,
 		cs.operatorClient,
 		apiregistrationInformers,
 		apiregistrationv1Client,

pkg/operator/apiservicecontroller/apigroup.go

@@ -5,36 +5,37 @@ import (
 	"net/http"
 
 	"github.com/openshift/library-go/pkg/operator/events"
-	"k8s.io/apimachinery/pkg/util/wait"
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/rest"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 )
 
-func NewEndpointPrecondition(kubeInformers kubeinformers.SharedInformerFactory, apiServices []*apiregistrationv1.APIService) wait.ConditionFunc {
+func newEndpointPrecondition(kubeInformers kubeinformers.SharedInformerFactory) func(apiServices []*apiregistrationv1.APIService) (bool, error) {
 	// this is outside the func so it always registers before the informers start
 	endpointsLister := kubeInformers.Core().V1().Endpoints().Lister()
 
 	type coordinate struct {
 		namespace string
 		name      string
 	}
-	coordinates := []coordinate{}
-	for _, apiService := range apiServices {
-		curr := coordinate{namespace: apiService.Spec.Service.Namespace, name: apiService.Spec.Service.Name}
-		exists := false
-		for _, j := range coordinates {
-			if j == curr {
-				exists = true
-				break
+
+	return func(apiServices []*apiregistrationv1.APIService) (bool, error) {
+
+		coordinates := []coordinate{}
+		for _, apiService := range apiServices {
+			curr := coordinate{namespace: apiService.Spec.Service.Namespace, name: apiService.Spec.Service.Name}
+			exists := false
+			for _, j := range coordinates {
+				if j == curr {
+					exists = true
+					break
+				}
+			}
+			if !exists {
+				coordinates = append(coordinates, curr)
 			}
 		}
-		if !exists {
-			coordinates = append(coordinates, curr)
-		}
-	}
 
-	return func() (bool, error) {
 		for _, curr := range coordinates {
 			endpoints, err := endpointsLister.Endpoints(curr.namespace).Get(curr.name)
 			if err != nil {

pkg/operator/apiservicecontroller/apiservice_controller.go

@@ -7,14 +7,10 @@ import (
 	"strings"
 	"time"
 
-	operatorsv1 "github.com/openshift/api/operator/v1"
-	operatorv1 "github.com/openshift/api/operator/v1"
-	"github.com/openshift/library-go/pkg/operator/events"
-	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
-	"github.com/openshift/library-go/pkg/operator/status"
-	"github.com/openshift/library-go/pkg/operator/v1helpers"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
@@ -24,19 +20,28 @@ import (
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	apiregistrationv1client "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1"
 	apiregistrationinformers "k8s.io/kube-aggregator/pkg/client/informers/externalversions"
+
+	operatorsv1 "github.com/openshift/api/operator/v1"
+	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1"
+	"github.com/openshift/library-go/pkg/operator/events"
+	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
+	"github.com/openshift/library-go/pkg/operator/v1helpers"
 )
 
 const (
 	workQueueKey = "key"
 )
 
+type GetAPIServicesToMangeFunc func() ([]*apiregistrationv1.APIService, error)
+type apiServicesPreconditionFuncType func([]*apiregistrationv1.APIService) (bool, error)
+
 type APIServiceController struct {
-	name        string
-	apiServices []*apiregistrationv1.APIService
+	name                     string
+	getAPIServicesToManageFn GetAPIServicesToMangeFunc
 	// precondition must return true before the apiservices will be created
-	precondition wait.ConditionFunc
+	precondition apiServicesPreconditionFuncType
 
-	versionRecorder         status.VersionGetter
 	operatorClient          v1helpers.OperatorClient
 	kubeClient              kubernetes.Interface
 	apiregistrationv1Client apiregistrationv1client.ApiregistrationV1Interface
@@ -48,7 +53,7 @@ type APIServiceController struct {
 
 func NewAPIServiceController(
 	name string,
-	apiServices []*apiregistrationv1.APIService,
+	getAPIServicesToManageFunc GetAPIServicesToMangeFunc,
 	operatorClient v1helpers.OperatorClient,
 	apiregistrationInformers apiregistrationinformers.SharedInformerFactory,
 	apiregistrationv1Client apiregistrationv1client.ApiregistrationV1Interface,
@@ -58,9 +63,9 @@ func NewAPIServiceController(
 ) *APIServiceController {
 	fullname := "APIServiceController_" + name
 	c := &APIServiceController{
-		name:         fullname,
-		apiServices:  apiServices,
-		precondition: NewEndpointPrecondition(kubeInformersForOperandNamespace, apiServices),
+		name:                     fullname,
+		precondition:             newEndpointPrecondition(kubeInformersForOperandNamespace),
+		getAPIServicesToManageFn: getAPIServicesToManageFunc,
 
 		operatorClient:          operatorClient,
 		apiregistrationv1Client: apiregistrationv1Client,
@@ -89,7 +94,12 @@ func (c *APIServiceController) sync() error {
 		return nil
 	case operatorsv1.Removed:
 		errs := []error{}
-		for _, apiService := range c.apiServices {
+		apiServices, err := c.getAPIServicesToManageFn()
+		if err != nil {
+			errs = append(errs, err)
+			return errors.NewAggregate(errs)
+		}
+		for _, apiService := range apiServices {
 			if err := c.apiregistrationv1Client.APIServices().Delete(apiService.Name, nil); err != nil {
 				errs = append(errs, err)
 			}
@@ -100,7 +110,11 @@ func (c *APIServiceController) sync() error {
 		return nil
 	}
 
-	ready, err := c.precondition()
+	apiServices, err := c.getAPIServicesToManageFn()
+	if err != nil {
+		return err
+	}
+	ready, err := c.precondition(apiServices)
 	if err != nil {
 		v1helpers.UpdateStatus(c.operatorClient, v1helpers.UpdateConditionFn(operatorv1.OperatorCondition{
 			Type:    "APIServicesAvailable",
@@ -120,7 +134,7 @@ func (c *APIServiceController) sync() error {
 		return err
 	}
 
-	err = c.syncAPIServices()
+	err = c.syncAPIServices(apiServices)
 
 	// update failing condition
 	cond := operatorv1.OperatorCondition{
@@ -141,11 +155,11 @@ func (c *APIServiceController) sync() error {
 	return err
 }
 
-func (c *APIServiceController) syncAPIServices() error {
+func (c *APIServiceController) syncAPIServices(apiServices []*apiregistrationv1.APIService) error {
 	errs := []error{}
 	var availableConditionMessages []string
 
-	for _, apiService := range c.apiServices {
+	for _, apiService := range apiServices {
 		apiregistrationv1.SetDefaults_ServiceReference(apiService.Spec.Service)
 		apiService, _, err := resourceapply.ApplyAPIService(c.apiregistrationv1Client, c.eventRecorder, apiService)
 		if err != nil {
@@ -173,7 +187,7 @@ func (c *APIServiceController) syncAPIServices() error {
 	// if the apiservices themselves check out ok, try to actually hit the discovery endpoints.  We have a history in clusterup
 	// of something delaying them.  This isn't perfect because of round-robining, but let's see if we get an improvement
 	if c.kubeClient.Discovery().RESTClient() != nil {
-		missingAPIMessages := checkDiscoveryForByAPIServices(c.eventRecorder, c.kubeClient.Discovery().RESTClient(), c.apiServices)
+		missingAPIMessages := checkDiscoveryForByAPIServices(c.eventRecorder, c.kubeClient.Discovery().RESTClient(), apiServices)
 		availableConditionMessages = append(availableConditionMessages, missingAPIMessages...)
 	}
 
@@ -232,3 +246,100 @@ func (c *APIServiceController) eventHandler() cache.ResourceEventHandler {
 		DeleteFunc: func(obj interface{}) { c.queue.Add(workQueueKey) },
 	}
 }
+
+// APIServicesToMange preserve state and clients required to return an authoritative list of API services this operate must manage
+type APIServicesToManage struct {
+	authOperatorLister                         operatorlistersv1.AuthenticationLister
+	apiregistrationv1Client                    apiregistrationv1client.ApiregistrationV1Interface
+	allPossibleAPIServices                     []*apiregistrationv1.APIService
+	eventRecorder                              events.Recorder
+	apiGroupsManagedByExternalServer           sets.String
+	apiGroupsManagedByExternalServerAnnotation string
+	currentAPIServicesToManage                 []*apiregistrationv1.APIService
+}
+
+// NewAPIServicesToManage returns an object that knows how to construct an authoritative list of API services this operate must manage
+func NewAPIServicesToManage(apiregistrationv1Client apiregistrationv1client.ApiregistrationV1Interface,
+	authOperatorLister operatorlistersv1.AuthenticationLister,
+	allPossibleAPIServices []*apiregistrationv1.APIService,
+	eventRecorder events.Recorder,
+	apiGroupsManagedByExternalServer sets.String,
+	apiGroupsManagedByExternalServerAnnotation string) *APIServicesToManage {
+	return &APIServicesToManage{
+		authOperatorLister:                         authOperatorLister,
+		apiregistrationv1Client:                    apiregistrationv1Client,
+		allPossibleAPIServices:                     allPossibleAPIServices,
+		eventRecorder:                              eventRecorder,
+		apiGroupsManagedByExternalServer:           apiGroupsManagedByExternalServer,
+		apiGroupsManagedByExternalServerAnnotation: apiGroupsManagedByExternalServerAnnotation,
+		currentAPIServicesToManage:                 allPossibleAPIServices,
+	}
+}
+
+// GetAPIServicesToManage returns the desired list of API Services that will be managed by this operator
+// note that some services might be managed by an external operators/servers
+func (a *APIServicesToManage) GetAPIServicesToManage() ([]*apiregistrationv1.APIService, error) {
+	if externalOperatorPreconditionErr := a.externalOperatorPrecondition(); externalOperatorPreconditionErr != nil {
+		klog.V(4).Infof("unable to determine if an external operator should take OAuth APIs over due to %v, returning authoritative/initial API Services list", externalOperatorPreconditionErr)
+		return a.allPossibleAPIServices, nil
+	}
+
+	newAPIServicesToManage := []*apiregistrationv1.APIService{}
+	for _, apiService := range a.allPossibleAPIServices {
+		if a.apiGroupsManagedByExternalServer.Has(apiService.Name) && a.isAPIServiceAnnotatedByExternalServer(apiService) {
+			continue
+		}
+		newAPIServicesToManage = append(newAPIServicesToManage, apiService)
+	}
+
+	if changed, newAPIServicesSet := apiServicesChanged(a.currentAPIServicesToManage, newAPIServicesToManage); changed {
+		a.eventRecorder.Eventf("APIServicesToManageChanged", "The new API Services list this operator will manage is %v", newAPIServicesSet.List())
+	}
+
+	a.currentAPIServicesToManage = newAPIServicesToManage
+	return a.currentAPIServicesToManage, nil
+}
+
+func (a *APIServicesToManage) isAPIServiceAnnotatedByExternalServer(apiService *apiregistrationv1.APIService) bool {
+	existingApiService, err := a.apiregistrationv1Client.APIServices().Get(apiService.Name, metav1.GetOptions{})
+	if err != nil {
+		a.eventRecorder.Warningf("APIServicesToManageAnnotation", "unable to determine if the following API Service %s was annotated by an external operator (it should be) due to %v", apiService.Name, err)
+		return false
+	}
+
+	if _, ok := existingApiService.Annotations[a.apiGroupsManagedByExternalServerAnnotation]; ok {
+		return true
+
+	}
+	return false
+}
+
+// externalOperatorPrecondition checks whether authentication operator will manage OAuth API Resources by checking ManagingOAuthAPIServer status field
+func (a *APIServicesToManage) externalOperatorPrecondition() error {
+	authOperator, err := a.authOperatorLister.Get("cluster")
+	if err != nil {
+		return err
+	}
+
+	if !authOperator.Status.ManagingOAuthAPIServer {
+		return fmt.Errorf("%q status field set to false", "ManagingOAuthAPIServer")
+	}
+
+	return nil
+}
+
+func apiServicesChanged(old []*apiregistrationv1.APIService, new []*apiregistrationv1.APIService) (bool, sets.String) {
+	oldSet := sets.String{}
+	for _, oldService := range old {
+		oldSet.Insert(oldService.Name)
+	}
+
+	newSet := sets.String{}
+	for _, newService := range new {
+		newSet.Insert(newService.Name)
+	}
+
+	removed := oldSet.Difference(newSet).List()
+	added := newSet.Difference(oldSet).List()
+	return len(removed) > 0 || len(added) > 0, newSet
+}