Alternative path (then default usage of RBAC) for kubefed cluster access

pkg/kubefed/init/init.go

@@ -284,21 +284,31 @@ func (i *initFederation) Complete(cmd *cobra.Command, args []string) error {
 // See the design doc in https://github.com/kubernetes/kubernetes/pull/34484
 // for details.
 func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
-	hostFactory := config.ClusterFactory(i.commonOptions.Host, i.commonOptions.Kubeconfig)
+	var hostFactory cmdutil.Factory
+	useRBAC := true
+	if i.commonOptions.CredentialsKubeconfig != "" {
+		hostFactory = config.ClusterFactory(i.commonOptions.Host, i.commonOptions.CredentialsKubeconfig)
+		useRBAC = false
+	} else {
+		hostFactory = config.ClusterFactory(i.commonOptions.Host, i.commonOptions.Kubeconfig)
+	}
+
 	hostClientset, err := hostFactory.ClientSet()
 	if err != nil {
 		return err
 	}
 
-	rbacAvailable := true
-	rbacVersionedClientset, err := util.GetVersionedClientForRBACOrFail(hostFactory)
-	if err != nil {
-		if _, ok := err.(*util.NoRBACAPIError); !ok {
-			return err
+	var rbacVersionedClientset client.Interface
+	if useRBAC {
+		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 absence of RBAC support).
+			useRBAC = false
 		}
-		// If the error is type NoRBACAPIError, We continue to create the rest of
-		// the resources, without the SA and roles (in the absence of RBAC support).
-		rbacAvailable = false
 	}
 
 	serverName := APIServerNameSuffix
@@ -384,7 +394,7 @@ func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
 	sa.Name = ""
 	// Create a service account and related RBAC roles if the host cluster has RBAC support.
 	// TODO: We must evaluate creating a separate service account even when RBAC support is missing
-	if rbacAvailable {
+	if useRBAC {
 		glog.V(4).Info("Creating service account for federation controller manager in the host cluster")
 		sa, err = createControllerManagerSA(rbacVersionedClientset, i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, i.options.dryRun)
 		if err != nil {

pkg/kubefed/init/init_test.go

@@ -92,6 +92,7 @@ func TestInitFederation(t *testing.T) {
 		federation                   string
 		kubeconfigGlobal             string
 		kubeconfigExplicit           string
+		kubeconfigForCredentials     string
 		dnsZoneName                  string
 		lbIP                         string
 		apiserverServiceType         v1.ServiceType
@@ -221,6 +222,28 @@ func TestInitFederation(t *testing.T) {
 			apiserverEnableTokenAuth:     true,
 			isRBACAPIAvailable:           true,
 		},
+		// This test checks if init works ok when a RBAC usage is overridden
+		// using a credentials kubeconfig (even when RBAC API is available).
+		// The same (or default) kubeconfig can be used for both flags.
+		{
+			federation:               "union",
+			kubeconfigGlobal:         fakeKubeFiles[0],
+			kubeconfigForCredentials: fakeKubeFiles[0],
+			dnsZoneName:              "example.test.",
+			apiserverServiceType:     v1.ServiceTypeNodePort,
+			advertiseAddress:         nodeIP,
+			serverImage:              "example.test/foo:bar",
+			imagePullPolicy:          "IfNotPresent",
+			etcdImage:                "gcr.io/google_containers/etcd:latest",
+			etcdPVCapacity:           "5Gi",
+			etcdPVStorageClass:       "fast",
+			etcdPersistence:          "true",
+			expectedErr:              "",
+			dryRun:                   "",
+			apiserverEnableHTTPBasicAuth: true,
+			apiserverEnableTokenAuth:     true,
+			isRBACAPIAvailable:           true,
+		},
 	}
 
 	defaultEtcdImage := "gcr.io/google_containers/etcd:3.1.10"
@@ -261,7 +284,7 @@ func TestInitFederation(t *testing.T) {
 			tc.imagePullPolicy = "IfNotPresent"
 		}
 
-		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.serverImage, tc.etcdImage, tc.dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.etcdPVStorageClass, tc.apiserverArgOverrides, tc.cmArgOverrides, tmpDirPath, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth, tc.isRBACAPIAvailable, tc.nodeSelector, tc.imagePullPolicy, tc.imagePullSecrets)
+		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.serverImage, tc.etcdImage, tc.dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.etcdPVStorageClass, tc.apiserverArgOverrides, tc.cmArgOverrides, tmpDirPath, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth, tc.isRBACAPIAvailable, tc.nodeSelector, tc.imagePullPolicy, tc.imagePullSecrets, tc.kubeconfigForCredentials)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -274,6 +297,7 @@ func TestInitFederation(t *testing.T) {
 		cmd := NewCmdInit(buf, adminConfig, "serverImage", defaultEtcdImage)
 
 		cmd.Flags().Set("kubeconfig", tc.kubeconfigExplicit)
+		cmd.Flags().Set("use-credentials-kubeconfig", tc.kubeconfigForCredentials)
 		cmd.Flags().Set("host-cluster-context", "substrate")
 		cmd.Flags().Set("dns-zone-name", tc.dnsZoneName)
 		cmd.Flags().Set("image", tc.serverImage)
@@ -643,7 +667,7 @@ func TestCertsHTTPS(t *testing.T) {
 	}
 }
 
-func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, serverImage, etcdImage, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, etcdPVStorageClass, apiserverOverrideArg, cmOverrideArg, tmpDirPath string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth, isRBACAPIAvailable bool, nodeSelectorString string, imagePullPolicy, imagePullSecrets string) (cmdutil.Factory, error) {
+func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, serverImage, etcdImage, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, etcdPVStorageClass, apiserverOverrideArg, cmOverrideArg, tmpDirPath string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth, isRBACAPIAvailable bool, nodeSelectorString string, imagePullPolicy, imagePullSecrets, kubeconfigForCredentials string) (cmdutil.Factory, error) {
 	svcName := "apiserver"
 	svcUrlPrefix := "/api/v1/namespaces/federation-system/services"
 	credSecretName := "apiserver" + "-credentials"
@@ -1106,7 +1130,7 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 			},
 		},
 	}
-	if isRBACAPIAvailable {
+	if isRBACAPIAvailable && (kubeconfigForCredentials == "") {
 		cm.Spec.Template.Spec.ServiceAccountName = "federation-controller-manager"
 		cm.Spec.Template.Spec.DeprecatedServiceAccount = "federation-controller-manager"
 	}

pkg/kubefed/join.go

@@ -34,6 +34,7 @@ import (
 	extensions "k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
+	client "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/kubectl"
 	kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
@@ -170,6 +171,9 @@ func (j *joinFederation) performPreflightChecks(config util.AdminConfig) error {
 
 // joiningClusterClientset returns a factory for the joining cluster.
 func (j *joinFederation) joningClusterFactory(config util.AdminConfig) cmdutil.Factory {
+	if j.commonOptions.CredentialsKubeconfig != "" {
+		return config.ClusterFactory(j.options.clusterContext, j.commonOptions.CredentialsKubeconfig)
+	}
 	return config.ClusterFactory(j.options.clusterContext, j.commonOptions.Kubeconfig)
 }
 
@@ -228,37 +232,45 @@ func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.Ad
 	// a service account and use its credentials; otherwise, use the credentials
 	// from the local kubeconfig.
 	glog.V(2).Info("Creating cluster credentials secret")
-	rbacClientset, err := util.GetVersionedClientForRBACOrFail(joiningClusterFactory)
-	if err == nil {
+
+	useRBAC := false
+	var rbacClientset client.Interface
+	if j.commonOptions.CredentialsKubeconfig == "" {
+		rbacClientset, err = util.GetVersionedClientForRBACOrFail(joiningClusterFactory)
+		if err != nil {
+			if _, ok := err.(*util.NoRBACAPIError); !ok {
+				glog.V(2).Infof("Failed to get or verify absence of RBAC client: %v", err)
+				return err
+			}
+		} else {
+			useRBAC = true
+		}
+	}
+
+	if useRBAC {
 		if _, serviceAccountName, clusterRoleName, err = createRBACSecret(hostClientset, rbacClientset, federationNamespace, federationName, joiningClusterName, host, clusterContext, secretName, dryRun); err != nil {
 			glog.V(2).Infof("Could not create cluster credentials secret: %v", err)
 			return err
 		}
 	} else {
-		if _, ok := err.(*util.NoRBACAPIError); ok {
-
-			// We are not using the `kubectl create secret` machinery through
-			// `RunCreateSubcommand` as we do to the cluster resource below
-			// because we have a bunch of requirements that the machinery does
-			// not satisfy.
-			// 1. We want to create the secret in a specific namespace, which
-			//    is neither the "default" namespace nor the one specified
-			//    via the `--namespace` flag.
-			// 2. `SecretGeneratorV1` requires LiteralSources in a string-ified
-			//    form that it parses to generate the secret data key-value
-			//    pairs. We, however, have the key-value pairs ready without a
-			//    need for parsing.
-			// 3. The result printing mechanism needs to be mostly quiet. We
-			//    don't have to print the created secret in the default case.
-			// Having said that, secret generation machinery could be altered to
-			// suit our needs, but it is far less invasive and readable this way.
-			_, err = createSecret(hostClientset, clientConfig, federationNamespace, federationName, joiningClusterName, clusterContext, secretName, dryRun)
-			if err != nil {
-				glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err)
-				return err
-			}
-		} else {
-			glog.V(2).Infof("Failed to get or verify absence of RBAC client: %v", err)
+		// We are not using the `kubectl create secret` machinery through
+		// `RunCreateSubcommand` as we do to the cluster resource below
+		// because we have a bunch of requirements that the machinery does
+		// not satisfy.
+		// 1. We want to create the secret in a specific namespace, which
+		//    is neither the "default" namespace nor the one specified
+		//    via the `--namespace` flag.
+		// 2. `SecretGeneratorV1` requires LiteralSources in a string-ified
+		//    form that it parses to generate the secret data key-value
+		//    pairs. We, however, have the key-value pairs ready without a
+		//    need for parsing.
+		// 3. The result printing mechanism needs to be mostly quiet. We
+		//    don't have to print the created secret in the default case.
+		// Having said that, secret generation machinery could be altered to
+		// suit our needs, but it is far less invasive and readable this way.
+		_, err = createSecret(hostClientset, clientConfig, federationNamespace, federationName, joiningClusterName, clusterContext, secretName, dryRun)
+		if err != nil {
+			glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err)
 			return err
 		}
 	}

pkg/kubefed/join_test.go

@@ -69,17 +69,18 @@ func TestJoinFederation(t *testing.T) {
 	defer kubefedtesting.RemoveFakeKubeconfigFiles(fakeKubeFiles)
 
 	testCases := []struct {
-		cluster            string
-		clusterCtx         string
-		secret             string
-		server             string
-		token              string
-		kubeconfigGlobal   string
-		kubeconfigExplicit string
-		expectedServer     string
-		expectedErr        string
-		dnsProvider        string
-		isRBACAPIAvailable bool
+		cluster                  string
+		clusterCtx               string
+		secret                   string
+		server                   string
+		token                    string
+		kubeconfigGlobal         string
+		kubeconfigExplicit       string
+		kubeconfigForCredentials string
+		expectedServer           string
+		expectedErr              string
+		dnsProvider              string
+		isRBACAPIAvailable       bool
 	}{
 		{
 			cluster:            "syndicate",
@@ -93,6 +94,22 @@ func TestJoinFederation(t *testing.T) {
 			dnsProvider:        util.FedDNSProviderCoreDNS,
 			isRBACAPIAvailable: true,
 		},
+		// This test checks if join works ok when a RBAC usage is overridden
+		// using a credentials kubeconfig (even when RBAC API is available).
+		// The same (or default) kubeconfig can be used for both flags.
+		{
+			cluster:                  "syndicate",
+			clusterCtx:               "",
+			server:                   "https://10.20.30.40",
+			token:                    "badge",
+			kubeconfigGlobal:         fakeKubeFiles[0],
+			kubeconfigForCredentials: fakeKubeFiles[0],
+			kubeconfigExplicit:       "",
+			expectedServer:           "https://10.20.30.40",
+			expectedErr:              "",
+			dnsProvider:              util.FedDNSProviderCoreDNS,
+			isRBACAPIAvailable:       true,
+		},
 		{
 			cluster:            "syndicate",
 			clusterCtx:         "",
@@ -164,6 +181,9 @@ func TestJoinFederation(t *testing.T) {
 
 	for i, tc := range testCases {
 		cmdErrMsg = ""
+		if tc.kubeconfigForCredentials != "" {
+			tc.isRBACAPIAvailable = false
+		}
 		f := testJoinFederationFactory(tc.cluster, tc.secret, tc.expectedServer, tc.isRBACAPIAvailable)
 		buf := bytes.NewBuffer([]byte{})
 

pkg/kubefed/unjoin.go

@@ -124,19 +124,27 @@ func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, conf
 		return err
 	}
 
-	unjoiningClusterFactory := config.ClusterFactory(u.options.clusterContext, u.commonOptions.Kubeconfig)
-	unjoiningClusterClientset, err := util.GetVersionedClientForRBACOrFail(unjoiningClusterFactory)
-	// If the cluster does not support the RBAC facility get a simple internal clientset
-	if err != nil {
+	useRBAC := true
+	var rbacErr error
+	var unjoiningClusterClientset internalclientset.Interface
+	var unjoiningClusterFactory cmdutil.Factory
+	if u.commonOptions.CredentialsKubeconfig == "" {
+		unjoiningClusterFactory = config.ClusterFactory(u.options.clusterContext, u.commonOptions.Kubeconfig)
+		unjoiningClusterClientset, rbacErr = util.GetVersionedClientForRBACOrFail(unjoiningClusterFactory)
+	} else {
+		unjoiningClusterFactory = config.ClusterFactory(u.options.clusterContext, u.commonOptions.CredentialsKubeconfig)
+		useRBAC = false
+	}
+
+	if !useRBAC || rbacErr != nil {
 		unjoiningClusterClientset, err = unjoiningClusterFactory.ClientSet()
-	}
-
-	outerErr := err
-	if err != nil {
-		// Attempt to get a clientset using information from the cluster.
-		unjoiningClusterClientset, err = getClientsetFromCluster(secret, cluster)
+		outerErr := err
 		if err != nil {
-			return fmt.Errorf("unable to get clientset from kubeconfig or cluster: %v, %v", outerErr, err)
+			// Attempt to get a clientset using information from the cluster.
+			unjoiningClusterClientset, err = getClientsetFromCluster(secret, cluster)
+			if err != nil {
+				return fmt.Errorf("unable to get clientset from kubeconfig or cluster: %v, %v", outerErr, err)
+			}
 		}
 	}
 
@@ -149,24 +157,28 @@ func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, conf
 	// We need to ensure updating the config map created in the deregistered cluster
 	// This configmap was created/updated when the cluster joined this federation to aid
 	// the kube-dns of that cluster to aid service discovery.
-	err = updateConfigMapFromCluster(hostClientset, unjoiningClusterClientset, u.commonOptions.FederationSystemNamespace)
+	err = updateConfigMapInCluster(hostClientset, unjoiningClusterClientset, u.commonOptions.FederationSystemNamespace)
 	if err != nil {
 		fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting kube-dns configmap: %v", err)
 		// We anyways continue to print success message but with above warning
 	}
 
-	// Delete the service account in the unjoining cluster.
-	err = deleteServiceAccountFromCluster(unjoiningClusterClientset, cluster, u.commonOptions.FederationSystemNamespace)
-	if err != nil {
-		fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting service account: %v", err)
-		return err
-	}
+	// Don't try to delete the SA and cluster role binding from unjoining cluster
+	// if user explicitly requested non RBAC use by providing --use-credentials-kubeconfig.
+	if useRBAC {
+		// Delete the service account in the unjoining cluster.
+		err = deleteServiceAccountFromCluster(unjoiningClusterClientset, cluster, u.commonOptions.FederationSystemNamespace)
+		if err != nil {
+			fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting service account: %v", err)
+			return err
+		}
 
-	// Delete the cluster role and role binding in the unjoining cluster.
-	err = deleteClusterRoleBindingFromCluster(unjoiningClusterClientset, cluster)
-	if err != nil {
-		fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting cluster role bindings: %v", err)
-		return err
+		// Delete the cluster role and role binding in the unjoining cluster.
+		err = deleteClusterRoleBindingFromCluster(unjoiningClusterClientset, cluster)
+		if err != nil {
+			fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting cluster role bindings: %v", err)
+			return err
+		}
 	}
 
 	_, err = fmt.Fprintf(cmdOut, "Successfully removed cluster %q from federation\n", u.commonOptions.Name)
@@ -223,7 +235,7 @@ func popCluster(f cmdutil.Factory, name string) (*federationapi.Cluster, error)
 	return v1beta1Cluster, rh.Delete("", name)
 }
 
-func updateConfigMapFromCluster(hostClientset, unjoiningClusterClientset internalclientset.Interface, fedSystemNamespace string) error {
+func updateConfigMapInCluster(hostClientset, unjoiningClusterClientset internalclientset.Interface, fedSystemNamespace string) error {
 	cmDep, err := getCMDeployment(hostClientset, fedSystemNamespace)
 	if err != nil {
 		return err