Merge pull request #41830 from irfanurrehman/fed-rbac-1

Automatic merge from submit-queue

[Federation] Kubefed Init should use the right RBAC API version clientset

**What this PR does / why we need it**:
Implements the need as described in https://github.com/kubernetes/kubernetes/issues/41263
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
https://github.com/kubernetes/kubernetes/issues/41263

**Special notes for your reviewer**:
@madhusudancs @shashidharatd @marun 
cc @kubernetes/sig-federation-bugs

**Release note**:

```
NONE
```

federation/pkg/kubefed/init/BUILD

@@ -48,6 +48,7 @@ go_test(
         "//pkg/api/testapi:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/apis/extensions/v1beta1:go_default_library",
+        "//pkg/apis/rbac:go_default_library",
         "//pkg/apis/rbac/v1beta1:go_default_library",
         "//pkg/kubectl/cmd/testing:go_default_library",
         "//pkg/kubectl/cmd/util:go_default_library",

federation/pkg/kubefed/init/init.go

@@ -261,6 +261,17 @@ func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
 		return err
 	}
 
+	rbacAvailable := true
+	rbacVersionedClientset, err := util.GetVersionedClientForRBACOrFail(hostFactory)
+	if err != nil {
+		if _, ok := err.(*util.NoRBACAPIError); !ok {
+			return err
+		}
+		// If the error is type NoRBACAPIError, We continue to create the rest of
+		// the resources, without the SA and roles (in the abscense of RBAC support).
+		rbacAvailable = false
+	}
+
 	serverName := fmt.Sprintf("%s-%s", i.commonOptions.Name, APIServerNameSuffix)
 	serverCredName := fmt.Sprintf("%s-%s", serverName, CredentialSuffix)
 	cmName := fmt.Sprintf("%s-%s", i.commonOptions.Name, CMNameSuffix)
@@ -329,19 +340,26 @@ func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
 		return err
 	}
 
-	// 7. Create federation controller manager
+	sa := &api.ServiceAccount{}
-	// 7a. Create a service account in the host cluster for federation
+	sa.Name = ""
-	// controller manager.
+	// 7. Create deployment for federation controller manager
-	sa, err := createControllerManagerSA(hostClientset, i.commonOptions.FederationSystemNamespace, i.options.dryRun)
+	// The below code either creates the SA and the related roles or skips
-	if err != nil {
+	// creating the same if the RBAC support is not found in the base cluster
-		return err
+	// TODO: We must evaluate creating a separate service account even when RBAC support is missing
-	}
+	if rbacAvailable {
+		// 7a. Create a service account in the host cluster for federation
+		// controller manager.
+		sa, err = createControllerManagerSA(rbacVersionedClientset, i.commonOptions.FederationSystemNamespace, i.options.dryRun)
+		if err != nil {
+			return err
+		}
 
-	// 7b. Create RBAC role and role binding for federation controller
+		// 7b. Create RBAC role and role binding for federation controller
-	// manager service account.
+		// manager service account.
-	_, _, err = createRoleBindings(hostClientset, i.commonOptions.FederationSystemNamespace, sa.Name, i.options.dryRun)
+		_, _, err = createRoleBindings(rbacVersionedClientset, i.commonOptions.FederationSystemNamespace, sa.Name, i.options.dryRun)
-	if err != nil {
+		if err != nil {
-		return err
+			return err
+		}
 	}
 
 	// 7c. Create a dns-provider config secret
@@ -876,12 +894,15 @@ func createControllerManager(clientset client.Interface, namespace, name, svcNam
 							},
 						},
 					},
-					ServiceAccountName: saName,
 				},
 			},
 		},
 	}
 
+	if saName != "" {
+		dep.Spec.Template.Spec.ServiceAccountName = saName
+	}
+
 	if dryRun {
 		return dep, nil
 	}

federation/pkg/kubefed/init/init_test.go

@@ -48,6 +48,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+	"k8s.io/kubernetes/pkg/apis/rbac"
 	rbacv1beta1 "k8s.io/kubernetes/pkg/apis/rbac/v1beta1"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -63,6 +64,9 @@ const (
 	lbIP     = "10.20.30.40"
 	nodeIP   = "10.20.30.50"
 	nodePort = 32111
+
+	testAPIGroup   = "testGroup"
+	testAPIVersion = "testVersion"
 )
 
 func TestInitFederation(t *testing.T) {
@@ -96,6 +100,7 @@ func TestInitFederation(t *testing.T) {
 		cmArgOverrides               string
 		apiserverEnableHTTPBasicAuth bool
 		apiserverEnableTokenAuth     bool
+		isRBACAPIAvailable           bool
 	}{
 		{
 			federation:            "union",
@@ -191,6 +196,7 @@ func TestInitFederation(t *testing.T) {
 			dryRun:               "",
 			apiserverEnableHTTPBasicAuth: true,
 			apiserverEnableTokenAuth:     true,
+			isRBACAPIAvailable:           true,
 		},
 	}
 
@@ -198,6 +204,7 @@ func TestInitFederation(t *testing.T) {
 
 	for i, tc := range testCases {
 		cmdErrMsg = ""
+		tmpDirPath := ""
 		buf := bytes.NewBuffer([]byte{})
 
 		if tc.dnsProviderConfig != "" {
@@ -208,7 +215,16 @@ func TestInitFederation(t *testing.T) {
 			tc.dnsProviderConfig = tmpfile.Name()
 			defer os.Remove(tmpfile.Name())
 		}
-		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.apiserverArgOverrides, tc.cmArgOverrides, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth)
+
+		// Check pkg/kubectl/cmd/testing/fake (fakeAPIFactory.DiscoveryClient()) for details of tmpDir
+		// We want an unique discovery cache path for each test run, else the case from previous case would be used
+		tmpDirPath, err = ioutil.TempDir("", "")
+		if err != nil {
+			t.Fatalf("[%d] unexpected error: %v", i, err)
+		}
+		defer os.Remove(tmpDirPath)
+
+		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.apiserverArgOverrides, tc.cmArgOverrides, tmpDirPath, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth, tc.isRBACAPIAvailable)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -577,7 +593,7 @@ func TestCertsHTTPS(t *testing.T) {
 	}
 }
 
-func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, image, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, apiserverOverrideArg, cmOverrideArg string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth bool) (cmdutil.Factory, error) {
+func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, image, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, apiserverOverrideArg, cmOverrideArg, tmpDirPath string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth, isRBACAPIAvailable bool) (cmdutil.Factory, error) {
 	svcName := federationName + "-apiserver"
 	svcUrlPrefix := "/api/v1/namespaces/federation-system/services"
 	credSecretName := svcName + "-credentials"
@@ -982,12 +998,14 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 							},
 						},
 					},
-					ServiceAccountName:       "federation-controller-manager",
-					DeprecatedServiceAccount: "federation-controller-manager",
 				},
 			},
 		},
 	}
+	if isRBACAPIAvailable {
+		cm.Spec.Template.Spec.ServiceAccountName = "federation-controller-manager"
+		cm.Spec.Template.Spec.DeprecatedServiceAccount = "federation-controller-manager"
+	}
 	if dnsProviderConfig != "" {
 		cm = addDNSProviderConfigTest(cm, cmDNSProviderSecret.Name)
 	}
@@ -1024,11 +1042,37 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 	podList.Items = append(podList.Items, apiServerPod)
 	podList.Items = append(podList.Items, cmPod)
 
+	apiGroupList := &metav1.APIGroupList{}
+	testGroup := metav1.APIGroup{
+		Name: testAPIGroup,
+		Versions: []metav1.GroupVersionForDiscovery{
+			{
+				GroupVersion: testAPIGroup + "/" + testAPIVersion,
+				Version:      testAPIVersion,
+			},
+		},
+	}
+	rbacGroup := metav1.APIGroup{
+		Name: rbac.GroupName,
+		Versions: []metav1.GroupVersionForDiscovery{
+			{
+				GroupVersion: rbac.GroupName + "/v1beta1",
+				Version:      "v1beta1",
+			},
+		},
+	}
+
+	apiGroupList.Groups = append(apiGroupList.Groups, testGroup)
+	if isRBACAPIAvailable {
+		apiGroupList.Groups = append(apiGroupList.Groups, rbacGroup)
+	}
+
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
 	extCodec := testapi.Extensions.Codec()
 	rbacCodec := testapi.Rbac.Codec()
 	ns := dynamic.ContentConfig().NegotiatedSerializer
 	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
+	tf.TmpDir = tmpDirPath
 	tf.Client = &fake.RESTClient{
 		APIRegistry:          api.Registry,
 		NegotiatedSerializer: ns,
@@ -1036,6 +1080,10 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 			switch p, m := req.URL.Path, req.Method; {
 			case p == "/healthz":
 				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: ioutil.NopCloser(bytes.NewReader([]byte("ok")))}, nil
+			case p == "/api" && m == http.MethodGet:
+				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &metav1.APIVersions{})}, nil
+			case p == "/apis" && m == http.MethodGet:
+				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, apiGroupList)}, nil
 			case p == "/api/v1/namespaces" && m == http.MethodPost:
 				body, err := ioutil.ReadAll(req.Body)
 				if err != nil {

federation/pkg/kubefed/util/BUILD

@@ -15,12 +15,14 @@ go_library(
         "//federation/apis/federation:go_default_library",
         "//federation/client/clientset_generated/federation_clientset:go_default_library",
         "//pkg/api:go_default_library",
+        "//pkg/apis/rbac:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/kubectl/cmd:go_default_library",
         "//pkg/kubectl/cmd/util:go_default_library",
         "//vendor:github.com/spf13/cobra",
         "//vendor:github.com/spf13/pflag",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/apimachinery/pkg/util/net",
         "//vendor:k8s.io/client-go/rest",
         "//vendor:k8s.io/client-go/tools/clientcmd",

federation/pkg/kubefed/util/util.go

@@ -21,18 +21,19 @@ import (
 	"net"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	utilnet "k8s.io/apimachinery/pkg/util/net"
+	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
+	federationapi "k8s.io/kubernetes/federation/apis/federation"
 	fedclient "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/rbac"
 	client "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 
-	utilnet "k8s.io/apimachinery/pkg/util/net"
-	restclient "k8s.io/client-go/rest"
-	federationapi "k8s.io/kubernetes/federation/apis/federation"
-
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -54,8 +55,19 @@ const (
 	userAgentName = "kubefed-tool"
 	KubeAPIQPS    = 20.0
 	KubeAPIBurst  = 30
+
+	rbacAPINotAvailable = "RBAC API not available"
 )
 
+// used to identify the rbac api availability error.
+type NoRBACAPIError struct {
+	s string
+}
+
+func (n *NoRBACAPIError) Error() string {
+	return n.s
+}
+
 // AdminConfig provides a filesystem based kubeconfig (via
 // `PathOptions()`) and a mechanism to talk to the federation
 // host cluster and the federation control plane api server.
@@ -211,3 +223,40 @@ func buildConfigFromSecret(secret *api.Secret, serverAddress string) (*restclien
 
 	return clusterConfig, nil
 }
+
+// GetVersionedClientForRBACOrFail discovers the versioned rbac APIs and gets the versioned
+// clientset for either the preferred version or the first listed version (if no preference listed)
+// TODO: We need to evaluate the usage of RESTMapper interface to achieve te same functionality
+func GetVersionedClientForRBACOrFail(hostFactory cmdutil.Factory) (client.Interface, error) {
+	discoveryclient, err := hostFactory.DiscoveryClient()
+	if err != nil {
+		return nil, err
+	}
+	groupList, err := discoveryclient.ServerGroups()
+	if err != nil {
+		return nil, fmt.Errorf("Couldn't get clientset to create RBAC roles in the host cluster: %v", err)
+	}
+
+	for _, g := range groupList.Groups {
+		if g.Name == rbac.GroupName {
+			if g.PreferredVersion.GroupVersion != "" {
+				gv, err := schema.ParseGroupVersion(g.PreferredVersion.GroupVersion)
+				if err != nil {
+					return nil, err
+				}
+				return hostFactory.ClientSetForVersion(&gv)
+			}
+			for i := 0; i < len(g.Versions); i++ {
+				if g.Versions[i].GroupVersion != "" {
+					gv, err := schema.ParseGroupVersion(g.Versions[i].GroupVersion)
+					if err != nil {
+						return nil, err
+					}
+					return hostFactory.ClientSetForVersion(&gv)
+				}
+			}
+		}
+	}
+
+	return nil, &NoRBACAPIError{rbacAPINotAvailable}
+}

pkg/kubectl/cmd/testing/fake.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"path/filepath"
+	"time"
 
 	"github.com/emicklei/go-restful/swagger"
 	"github.com/spf13/cobra"
@@ -224,6 +226,7 @@ type TestFactory struct {
 	Err                error
 	Command            string
 	GenericPrinter     bool
+	TmpDir             string
 
 	ClientForMappingFunc             func(mapping *meta.RESTMapping) (resource.RESTClient, error)
 	UnstructuredClientForMappingFunc func(mapping *meta.RESTMapping) (resource.RESTClient, error)
@@ -567,6 +570,19 @@ func (f *fakeAPIFactory) RESTClient() (*restclient.RESTClient, error) {
 	return restClient, f.tf.Err
 }
 
+func (f *fakeAPIFactory) DiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
+	fakeClient := f.tf.Client.(*fake.RESTClient)
+	discoveryClient := discovery.NewDiscoveryClientForConfigOrDie(f.tf.ClientConfig)
+	discoveryClient.RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
+
+	cacheDir := filepath.Join(f.tf.TmpDir, ".kube", "cache", "discovery")
+	return cmdutil.NewCachedDiscoveryClient(discoveryClient, cacheDir, time.Duration(10*time.Minute)), nil
+}
+
+func (f *fakeAPIFactory) ClientSetForVersion(requiredVersion *schema.GroupVersion) (internalclientset.Interface, error) {
+	return f.ClientSet()
+}
+
 func (f *fakeAPIFactory) ClientConfig() (*restclient.Config, error) {
 	return f.tf.ClientConfig, f.tf.Err
 }