diff --git a/federation/pkg/federation-controller/cluster/BUILD b/federation/pkg/federation-controller/cluster/BUILD index d418f3d123b..d6d0eed853f 100644 --- a/federation/pkg/federation-controller/cluster/BUILD +++ b/federation/pkg/federation-controller/cluster/BUILD @@ -47,6 +47,7 @@ go_test( "//federation/apis/federation/v1beta1:go_default_library", "//federation/client/clientset_generated/federation_clientset:go_default_library", "//federation/pkg/federation-controller/util:go_default_library", + "//pkg/api:go_default_library", "//pkg/api/testapi:go_default_library", "//pkg/api/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/federation/pkg/federation-controller/cluster/clustercontroller_test.go b/federation/pkg/federation-controller/cluster/clustercontroller_test.go index 5d4b04ca590..69fca4f016c 100644 --- a/federation/pkg/federation-controller/cluster/clustercontroller_test.go +++ b/federation/pkg/federation-controller/cluster/clustercontroller_test.go @@ -31,6 +31,7 @@ import ( federationv1beta1 "k8s.io/kubernetes/federation/apis/federation/v1beta1" federationclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset" controllerutil "k8s.io/kubernetes/federation/pkg/federation-controller/util" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/v1" ) @@ -124,9 +125,9 @@ func TestUpdateClusterStatusOK(t *testing.T) { } federationClientSet := federationclientset.NewForConfigOrDie(restclient.AddUserAgent(restClientCfg, "cluster-controller")) - // Override KubeconfigGetterForCluster to avoid having to setup service accounts and mount files with secret tokens. - originalGetter := controllerutil.KubeconfigGetterForCluster - controllerutil.KubeconfigGetterForCluster = func(c *federationv1beta1.Cluster) clientcmd.KubeconfigGetter { + // Override KubeconfigGetterForSecret to avoid having to setup service accounts and mount files with secret tokens. + originalGetter := controllerutil.KubeconfigGetterForSecret + controllerutil.KubeconfigGetterForSecret = func(s *api.Secret) clientcmd.KubeconfigGetter { return func() (*clientcmdapi.Config, error) { return &clientcmdapi.Config{}, nil } @@ -146,6 +147,6 @@ func TestUpdateClusterStatusOK(t *testing.T) { } } - // Reset KubeconfigGetterForCluster - controllerutil.KubeconfigGetterForCluster = originalGetter + // Reset KubeconfigGetterForSecret + controllerutil.KubeconfigGetterForSecret = originalGetter } diff --git a/federation/pkg/federation-controller/util/cluster_util.go b/federation/pkg/federation-controller/util/cluster_util.go index 33499190d43..c95cd27b038 100644 --- a/federation/pkg/federation-controller/util/cluster_util.go +++ b/federation/pkg/federation-controller/util/cluster_util.go @@ -65,8 +65,29 @@ func BuildClusterConfig(c *federation_v1beta1.Cluster) (*restclient.Config, erro glog.Infof("didn't find secretRef for cluster %s. Trying insecure access", c.Name) clusterConfig, err = clientcmd.BuildConfigFromFlags(serverAddress, "") } else { - kubeconfigGetter := KubeconfigGetterForCluster(c) - clusterConfig, err = clientcmd.BuildConfigFromKubeconfigGetter(serverAddress, kubeconfigGetter) + if c.Spec.SecretRef.Name == "" { + return nil, fmt.Errorf("found secretRef but no secret name for cluster %s", c.Name) + } + secret, err := getSecret(c.Spec.SecretRef.Name) + if err != nil { + return nil, err + } + // Pre-1.7, the secret contained a serialized kubeconfig which contained appropriate credentials. + // Post-1.7, the secret contains credentials for a service account. + // Check for the service account credentials, and use them if they exist; if not, use the + // serialized kubeconfig. + token, tokenFound := secret.Data["token"] + ca, caFound := secret.Data["ca.crt"] + if tokenFound != caFound { + return nil, fmt.Errorf("secret should have values for either both 'ca.crt' and 'token' in its Data, or neither: %v", secret) + } else if tokenFound && caFound { + clusterConfig, err = clientcmd.BuildConfigFromFlags(serverAddress, "") + clusterConfig.CAData = ca + clusterConfig.BearerToken = string(token) + } else { + kubeconfigGetter := KubeconfigGetterForSecret(secret) + clusterConfig, err = clientcmd.BuildConfigFromKubeconfigGetter(serverAddress, kubeconfigGetter) + } } if err != nil { return nil, err @@ -77,60 +98,49 @@ func BuildClusterConfig(c *federation_v1beta1.Cluster) (*restclient.Config, erro return clusterConfig, nil } -// This is to inject a different kubeconfigGetter in tests. -// We don't use the standard one which calls NewInCluster in tests to avoid having to setup service accounts and mount files with secret tokens. -var KubeconfigGetterForCluster = func(c *federation_v1beta1.Cluster) clientcmd.KubeconfigGetter { - return func() (*clientcmdapi.Config, error) { - secretRefName := "" - if c.Spec.SecretRef != nil { - secretRefName = c.Spec.SecretRef.Name - } else { - glog.Infof("didn't find secretRef for cluster %s. Trying insecure access", c.Name) - } - return KubeconfigGetterForSecret(secretRefName)() +// getSecret gets a secret from the cluster. +func getSecret(secretName string) (*api.Secret, error) { + // Get the namespace this is running in from the env variable. + namespace := os.Getenv("POD_NAMESPACE") + if namespace == "" { + return nil, fmt.Errorf("unexpected: POD_NAMESPACE env var returned empty string") } + // Get a client to talk to the k8s apiserver, to fetch secrets from it. + cc, err := restclient.InClusterConfig() + if err != nil { + return nil, fmt.Errorf("error in creating in-cluster config: %s", err) + } + client, err := clientset.NewForConfig(cc) + if err != nil { + return nil, fmt.Errorf("error in creating in-cluster client: %s", err) + } + var secret *api.Secret + err = wait.PollImmediate(1*time.Second, getSecretTimeout, func() (bool, error) { + secret, err = client.Core().Secrets(namespace).Get(secretName, metav1.GetOptions{}) + if err == nil { + return true, nil + } + glog.Warningf("error in fetching secret: %s", err) + return false, nil + }) + if err != nil { + return nil, fmt.Errorf("timed out waiting for secret: %s", err) + } + if secret == nil { + return nil, fmt.Errorf("unexpected: received null secret %s", secretName) + } + return secret, nil } -// KubeconfigGetterForSecret is used to get the kubeconfig from the given secret. -var KubeconfigGetterForSecret = func(secretName string) clientcmd.KubeconfigGetter { +// KubeconfigGetterForSecret gets the kubeconfig from the given secret. +// This is to inject a different KubeconfigGetter in tests. We don't use +// the standard one which calls NewInCluster in tests to avoid having to +// set up service accounts and mount files with secret tokens. +var KubeconfigGetterForSecret = func(secret *api.Secret) clientcmd.KubeconfigGetter { return func() (*clientcmdapi.Config, error) { - var data []byte - if secretName != "" { - // Get the namespace this is running in from the env variable. - namespace := os.Getenv("POD_NAMESPACE") - if namespace == "" { - return nil, fmt.Errorf("unexpected: POD_NAMESPACE env var returned empty string") - } - // Get a client to talk to the k8s apiserver, to fetch secrets from it. - cc, err := restclient.InClusterConfig() - if err != nil { - return nil, fmt.Errorf("error in creating in-cluster client: %s", err) - } - client, err := clientset.NewForConfig(cc) - if err != nil { - return nil, fmt.Errorf("error in creating in-cluster client: %s", err) - } - data = []byte{} - var secret *api.Secret - err = wait.PollImmediate(1*time.Second, getSecretTimeout, func() (bool, error) { - secret, err = client.Core().Secrets(namespace).Get(secretName, metav1.GetOptions{}) - if err == nil { - return true, nil - } - glog.Warningf("error in fetching secret: %s", err) - return false, nil - }) - if err != nil { - return nil, fmt.Errorf("timed out waiting for secret: %s", err) - } - if secret == nil { - return nil, fmt.Errorf("unexpected: received null secret %s", secretName) - } - ok := false - data, ok = secret.Data[KubeconfigSecretDataKey] - if !ok { - return nil, fmt.Errorf("secret does not have data with key: %s", KubeconfigSecretDataKey) - } + data, ok := secret.Data[KubeconfigSecretDataKey] + if !ok { + return nil, fmt.Errorf("secret does not have data with key %s", KubeconfigSecretDataKey) } return clientcmd.Load(data) } diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index 02389cb9a46..b9fab82b3ad 100644 --- a/federation/pkg/kubefed/BUILD +++ b/federation/pkg/kubefed/BUILD @@ -23,6 +23,7 @@ go_library( "//pkg/api:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/apis/extensions:go_default_library", + "//pkg/apis/rbac:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd:go_default_library", @@ -36,6 +37,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd/api:go_default_library", @@ -59,6 +61,8 @@ go_test( "//pkg/api/testapi:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/apis/extensions/v1beta1:go_default_library", + "//pkg/apis/rbac/v1beta1:go_default_library", + "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", diff --git a/federation/pkg/kubefed/join.go b/federation/pkg/kubefed/join.go index 61228a4226b..8027ce73da5 100644 --- a/federation/pkg/kubefed/join.go +++ b/federation/pkg/kubefed/join.go @@ -20,11 +20,19 @@ import ( "fmt" "io" "strings" + "time" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/kubernetes/federation/apis/federation" "k8s.io/kubernetes/federation/pkg/kubefed/util" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/v1" + extensions "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubectl" kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd" @@ -34,10 +42,6 @@ import ( "github.com/golang/glog" "github.com/spf13/cobra" "github.com/spf13/pflag" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/v1" - extensions "k8s.io/kubernetes/pkg/apis/extensions" ) const ( @@ -45,8 +49,9 @@ const ( // joining API server. See `apis/federation.ClusterSpec` for // details. // TODO(madhusudancs): Make this value customizable. - defaultClientCIDR = "0.0.0.0/0" - CMNameSuffix = "controller-manager" + defaultClientCIDR = "0.0.0.0/0" + CMNameSuffix = "controller-manager" + serviceAccountSecretTimeout = 30 * time.Second ) var ( @@ -93,7 +98,7 @@ func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *c Long: join_long, Example: join_example, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(opts.Complete(cmd, args)) + cmdutil.CheckErr(opts.Complete(cmd, args, config)) cmdutil.CheckErr(opts.Run(f, cmdOut, config, cmd)) }, } @@ -111,7 +116,7 @@ func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *c } // Complete ensures that options are valid and marshals them if necessary. -func (j *joinFederation) Complete(cmd *cobra.Command, args []string) error { +func (j *joinFederation) Complete(cmd *cobra.Command, args []string, config util.AdminConfig) error { err := j.commonOptions.SetName(cmd, args) if err != nil { return err @@ -125,82 +130,171 @@ func (j *joinFederation) Complete(cmd *cobra.Command, args []string) error { glog.V(2).Infof("Args and flags: name %s, host: %s, host-system-namespace: %s, kubeconfig: %s, cluster-context: %s, secret-name: %s, dry-run: %s", j.commonOptions.Name, j.commonOptions.Host, j.commonOptions.FederationSystemNamespace, j.commonOptions.Kubeconfig, j.options.clusterContext, j.options.secretName, j.options.dryRun) + glog.V(2).Infof("Performing preflight checks.") + err = j.performPreflightChecks(config) + if err != nil { + return err + } return nil } +// performPreflightChecks checks that the host and joining clusters are in +// a consistent state. +// TODO: This currently only verifies a few things. Add more checks. +func (j *joinFederation) performPreflightChecks(config util.AdminConfig) error { + joiningClusterFactory := j.joningClusterFactory(config) + + // If RBAC is not available, then skip checking for a service account. + // If RBAC availability cannot be determined, return an error. + rbacVersionedClientset, err := util.GetVersionedClientForRBACOrFail(joiningClusterFactory) + if err != nil { + if _, ok := err.(*util.NoRBACAPIError); ok { + return nil + } + return err + } + + // Make sure there is no existing service account in the joining cluster. + saName := util.ClusterServiceAccountName(j.commonOptions.Name, j.commonOptions.Host) + sa, err := rbacVersionedClientset.Core().ServiceAccounts(j.commonOptions.FederationSystemNamespace).Get(saName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + return nil + } else if err != nil { + return err + } else if sa != nil { + return fmt.Errorf("service account already exists in joining cluster") + } + + return nil +} + +// joiningClusterClientset returns a factory for the joining cluster. +func (j *joinFederation) joningClusterFactory(config util.AdminConfig) cmdutil.Factory { + return config.ClusterFactory(j.options.clusterContext, j.commonOptions.Kubeconfig) +} + // Run is the implementation of the `join federation` command. func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command) error { - po := config.PathOptions() - po.LoadingRules.ExplicitPath = j.commonOptions.Kubeconfig - clientConfig, err := po.GetStartingConfig() - if err != nil { - return err - } + clusterContext := j.options.clusterContext + dryRun := j.options.dryRun + federationNamespace := j.commonOptions.FederationSystemNamespace + host := j.commonOptions.Host + kubeconfig := j.commonOptions.Kubeconfig + joiningClusterName := j.commonOptions.Name secretName := j.options.secretName if secretName == "" { secretName = v1.SimpleNameGenerator.GenerateName(j.commonOptions.Name + "-") } - generator, err := clusterGenerator(clientConfig, j.commonOptions.Name, j.options.clusterContext, secretName) + joiningClusterFactory := j.joningClusterFactory(config) + joiningClusterClientset, err := joiningClusterFactory.ClientSet() if err != nil { - glog.V(2).Infof("Failed creating cluster generator: %v", err) + glog.V(2).Infof("Could not create client for joining cluster: %v", err) return err } - glog.V(2).Infof("Created cluster generator: %#v", generator) - hostFactory := config.ClusterFactory(j.commonOptions.Host, j.commonOptions.Kubeconfig) + hostFactory := config.ClusterFactory(host, kubeconfig) hostClientset, err := hostFactory.ClientSet() if err != nil { - glog.V(2).Infof("Failed to get the cluster client for the host cluster: %q", j.commonOptions.Host, err) + glog.V(2).Infof("Could not create client for host cluster: %v", err) return err } - federationName, err := getFederationName(hostClientset, j.commonOptions.FederationSystemNamespace) + federationName, err := getFederationName(hostClientset, federationNamespace) if err != nil { glog.V(2).Infof("Failed to get the federation name: %v", err) return 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, j.commonOptions.FederationSystemNamespace, federationName, j.commonOptions.Name, j.options.clusterContext, secretName, j.options.dryRun) + glog.V(2).Info("Creating federation system namespace in joining cluster") + _, err = createFederationSystemNamespace(joiningClusterClientset, federationNamespace, federationName, joiningClusterName, dryRun) if err != nil { - glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err) + glog.V(2).Info("Error creating federation system namespace in joining cluster: %v", err) return err } - glog.V(2).Infof("Cluster credentials secret created") + glog.V(2).Info("Created federation system namespace in joining cluster") + po := config.PathOptions() + po.LoadingRules.ExplicitPath = kubeconfig + clientConfig, err := po.GetStartingConfig() + if err != nil { + glog.V(2).Info("Could not load clientConfig from %s: %v", kubeconfig, err) + return err + } + + serviceAccountName := "" + clusterRoleName := "" + // Check for RBAC in the joining cluster. If it supports RBAC, then create + // 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 { + 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) + return err + } + } + glog.V(2).Info("Cluster credentials secret created") + + glog.V(2).Info("Creating a generator for the cluster API object") + generator, err := clusterGenerator(clientConfig, joiningClusterName, clusterContext, secretName, serviceAccountName, clusterRoleName) + if err != nil { + glog.V(2).Infof("Failed to create a generator for the cluster API object: %v", err) + return err + } + glog.V(2).Info("Created a generator for the cluster API object") + + glog.V(2).Info("Running create cluster command against the federation API server") err = kubectlcmd.RunCreateSubcommand(f, cmd, cmdOut, &kubectlcmd.CreateSubcommandOptions{ - Name: j.commonOptions.Name, + Name: joiningClusterName, StructuredGenerator: generator, - DryRun: j.options.dryRun, + DryRun: dryRun, OutputFormat: cmdutil.GetFlagString(cmd, "output"), }) if err != nil { + glog.V(2).Infof("Failed running create cluster command against the federation API server: %v", err) return err } + glog.V(2).Info("Successfully ran create cluster command against the federation API server") // We further need to create a configmap named kube-config in the // just registered cluster which will be consumed by the kube-dns // of this cluster. - _, err = createConfigMap(hostClientset, config, j.commonOptions.FederationSystemNamespace, federationName, j.commonOptions.Name, j.options.clusterContext, j.commonOptions.Kubeconfig, j.options.dryRun) + glog.V(2).Info("Creating configmap in host cluster") + _, err = createConfigMap(hostClientset, config, federationNamespace, federationName, joiningClusterName, clusterContext, kubeconfig, dryRun) if err != nil { - glog.V(2).Infof("Failed creating the config map in cluster: %v", err) + glog.V(2).Infof("Failed to create configmap in cluster: %v", err) return err } + glog.V(2).Info("Created configmap in host cluster") return err } @@ -224,7 +318,7 @@ func minifyConfig(clientConfig *clientcmdapi.Config, context string) (*clientcmd // createSecret extracts the kubeconfig for a given cluster and populates // a secret with that kubeconfig. -func createSecret(clientset internalclientset.Interface, clientConfig *clientcmdapi.Config, namespace, federationName, clusterName, contextName, secretName string, dryRun bool) (runtime.Object, error) { +func createSecret(clientset internalclientset.Interface, clientConfig *clientcmdapi.Config, namespace, federationName, joiningClusterName, contextName, secretName string, dryRun bool) (runtime.Object, error) { // Minify the kubeconfig to ensure that there is only information // relevant to the cluster we are registering. newClientConfig, err := minifyConfig(clientConfig, contextName) @@ -241,13 +335,13 @@ func createSecret(clientset internalclientset.Interface, clientConfig *clientcmd return nil, err } - return util.CreateKubeconfigSecret(clientset, newClientConfig, namespace, secretName, federationName, clusterName, dryRun) + return util.CreateKubeconfigSecret(clientset, newClientConfig, namespace, secretName, federationName, joiningClusterName, dryRun) } // createConfigMap creates a configmap with name kube-dns in the joining cluster // which stores the information about this federation zone name. // If the configmap with this name already exists, its updated with this information. -func createConfigMap(hostClientSet internalclientset.Interface, config util.AdminConfig, fedSystemNamespace, federationName, targetClusterName, targetClusterContext, kubeconfigPath string, dryRun bool) (*api.ConfigMap, error) { +func createConfigMap(hostClientSet internalclientset.Interface, config util.AdminConfig, fedSystemNamespace, federationName, joiningClusterName, targetClusterContext, kubeconfigPath string, dryRun bool) (*api.ConfigMap, error) { cmDep, err := getCMDeployment(hostClientSet, fedSystemNamespace) if err != nil { return nil, err @@ -271,7 +365,7 @@ func createConfigMap(hostClientSet internalclientset.Interface, config util.Admi Namespace: metav1.NamespaceSystem, Annotations: map[string]string{ federation.FederationNameAnnotation: federationName, - federation.ClusterNameAnnotation: targetClusterName, + federation.ClusterNameAnnotation: joiningClusterName, }, }, Data: map[string]string{ @@ -310,7 +404,7 @@ func createConfigMap(hostClientSet internalclientset.Interface, config util.Admi // clusterGenerator extracts the cluster information from the supplied // kubeconfig and builds a StructuredGenerator for the // `federation/cluster` API resource. -func clusterGenerator(clientConfig *clientcmdapi.Config, name, contextName, secretName string) (kubectl.StructuredGenerator, error) { +func clusterGenerator(clientConfig *clientcmdapi.Config, name, contextName, secretName, serviceAccountName, clusterRoleName string) (kubectl.StructuredGenerator, error) { // Get the context from the config. ctx, found := clientConfig.Contexts[contextName] if !found { @@ -334,10 +428,12 @@ func clusterGenerator(clientConfig *clientcmdapi.Config, name, contextName, secr } generator := &kubectl.ClusterGeneratorV1Beta1{ - Name: name, - ClientCIDR: defaultClientCIDR, - ServerAddress: serverAddress, - SecretName: secretName, + Name: name, + ClientCIDR: defaultClientCIDR, + ServerAddress: serverAddress, + SecretName: secretName, + ServiceAccountName: serviceAccountName, + ClusterRoleName: clusterRoleName, } return generator, nil } @@ -410,3 +506,195 @@ func populateStubDomainsIfRequired(configMap *api.ConfigMap, annotations map[str configMap.Data[util.KubeDnsStubDomains] = fmt.Sprintf(`{"%s":["%s"]}`, dnsZoneName, nameServer) return configMap } + +// createFederationSystemNamespace creates the federation-system namespace in the cluster +// associated with clusterClientset, if it doesn't already exist. +func createFederationSystemNamespace(clusterClientset internalclientset.Interface, federationNamespace, federationName, joiningClusterName string, dryRun bool) (*api.Namespace, error) { + federationNS := &api.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: federationNamespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: federationName, + federation.ClusterNameAnnotation: joiningClusterName, + }, + }, + } + + if dryRun { + return federationNS, nil + } + + _, err := clusterClientset.Core().Namespaces().Create(federationNS) + if err != nil && !errors.IsAlreadyExists(err) { + glog.V(2).Infof("Could not create federation-system namespace in client: %v", err) + return nil, err + } + return federationNS, nil +} + +// createRBACSecret creates a secret in the joining cluster using a service account, and +// populates that secret into the host cluster to allow it to access the joining cluster. +func createRBACSecret(hostClusterClientset, joiningClusterClientset internalclientset.Interface, namespace, federationName, joiningClusterName, hostClusterContext, joiningClusterContext, secretName string, dryRun bool) (*api.Secret, string, string, error) { + glog.V(2).Info("Creating service account in joining cluster") + saName, err := createServiceAccount(joiningClusterClientset, namespace, federationName, joiningClusterName, hostClusterContext, dryRun) + if err != nil { + glog.V(2).Infof("Error creating service account in joining cluster: %v", err) + return nil, "", "", err + } + glog.V(2).Infof("Created service account in joining cluster") + + glog.V(2).Info("Creating role binding for service account in joining cluster") + crb, err := createClusterRoleBinding(joiningClusterClientset, saName, namespace, federationName, joiningClusterName, dryRun) + if err != nil { + glog.V(2).Infof("Error creating role binding for service account in joining cluster: %v", err) + return nil, "", "", err + } + glog.V(2).Info("Created role binding for service account in joining cluster") + + glog.V(2).Info("Creating secret in host cluster") + secret, err := populateSecretInHostCluster(joiningClusterClientset, hostClusterClientset, saName, namespace, federationName, joiningClusterName, secretName, dryRun) + if err != nil { + glog.V(2).Infof("Error creating secret in host cluster: %v", err) + return nil, "", "", err + } + glog.V(2).Info("Created secret in host cluster") + return secret, saName, crb.Name, nil +} + +// createServiceAccount creates a service account in the cluster associated with clusterClientset with +// credentials that will be used by the host cluster to access its API server. +func createServiceAccount(clusterClientset internalclientset.Interface, namespace, federationName, joiningClusterName, hostContext string, dryRun bool) (string, error) { + saName := util.ClusterServiceAccountName(joiningClusterName, hostContext) + sa := &api.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Namespace: namespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: federationName, + federation.ClusterNameAnnotation: joiningClusterName, + }, + }, + } + + if dryRun { + return saName, nil + } + + // Create a new service account. + _, err := clusterClientset.Core().ServiceAccounts(namespace).Create(sa) + if err != nil { + return "", err + } + + return saName, nil +} + +// createClusterRoleBinding creates an RBAC role and binding that allows the +// service account identified by saName to access all resources in all namespaces +// in the cluster associated with clusterClientset. +func createClusterRoleBinding(clusterClientset internalclientset.Interface, saName, namespace, federationName, joiningClusterName string, dryRun bool) (*rbac.ClusterRoleBinding, error) { + roleName := util.ClusterRoleName(saName) + role := &rbac.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + Namespace: namespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: federationName, + federation.ClusterNameAnnotation: joiningClusterName, + }, + }, + Rules: []rbac.PolicyRule{ + rbac.NewRule(rbac.VerbAll).Groups(rbac.APIGroupAll).Resources(rbac.ResourceAll).RuleOrDie(), + rbac.NewRule("get").URLs("/healthz").RuleOrDie(), + }, + } + + // TODO: This should limit its access to only necessary resources. + rolebinding, err := rbac.NewClusterBinding(roleName).SAs(namespace, saName).Binding() + rolebinding.ObjectMeta.Namespace = namespace + rolebinding.ObjectMeta.Annotations = map[string]string{ + federation.FederationNameAnnotation: federationName, + federation.ClusterNameAnnotation: joiningClusterName, + } + if err != nil { + glog.V(2).Infof("Could not create role binding for service account: %v", err) + return nil, err + } + + if dryRun { + return &rolebinding, nil + } + + _, err = clusterClientset.Rbac().ClusterRoles().Create(role) + if err != nil { + glog.V(2).Infof("Could not create role for service account in joining cluster: %v", err) + return nil, err + } + + _, err = clusterClientset.Rbac().ClusterRoleBindings().Create(&rolebinding) + if err != nil { + glog.V(2).Infof("Could not create role binding for service account in joining cluster: %v", err) + return nil, err + } + + return &rolebinding, nil +} + +// populateSecretInHostCluster copies the service account secret for saName from the cluster +// referenced by clusterClientset to the client referenced by hostClientset, putting it in a secret +// named secretName in the provided namespace. +func populateSecretInHostCluster(clusterClientset, hostClientset internalclientset.Interface, saName, namespace, federationName, joiningClusterName, secretName string, dryRun bool) (*api.Secret, error) { + if dryRun { + // The secret is created indirectly with the service account, and so there is no local copy to return in a dry run. + return nil, nil + } + // Get the secret from the joining cluster. + var sa *api.ServiceAccount + err := wait.PollImmediate(1*time.Second, serviceAccountSecretTimeout, func() (bool, error) { + var err error + sa, err = clusterClientset.Core().ServiceAccounts(namespace).Get(saName, metav1.GetOptions{}) + if err != nil { + return false, nil + } + return len(sa.Secrets) == 1, nil + }) + if err != nil { + return nil, err + } + + glog.V(2).Info("Getting secret named: %s", sa.Secrets[0].Name) + var secret *api.Secret + err = wait.PollImmediate(1*time.Second, serviceAccountSecretTimeout, func() (bool, error) { + var err error + secret, err = clusterClientset.Core().Secrets(namespace).Get(sa.Secrets[0].Name, metav1.GetOptions{}) + if err != nil { + return false, nil + } + return true, nil + }) + if err != nil { + glog.V(2).Infof("Could not get service account secret from joining cluster: %v", err) + return nil, err + } + + // Create a parallel secret in the host cluster. + v1Secret := api.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: federationName, + federation.ClusterNameAnnotation: joiningClusterName, + }, + }, + Data: secret.Data, + } + + glog.V(2).Infof("Creating secret in host cluster named: %s", v1Secret.Name) + _, err = hostClientset.Core().Secrets(namespace).Create(&v1Secret) + if err != nil { + glog.V(2).Infof("Could not create secret in host cluster: %v", err) + return nil, err + } + return &v1Secret, nil +} diff --git a/federation/pkg/kubefed/join_test.go b/federation/pkg/kubefed/join_test.go index 916b9c7e96c..e87b9596998 100644 --- a/federation/pkg/kubefed/join_test.go +++ b/federation/pkg/kubefed/join_test.go @@ -21,6 +21,7 @@ import ( "fmt" "io/ioutil" "net/http" + "os" "testing" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -38,6 +39,8 @@ import ( "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/apis/extensions/v1beta1" + rbacv1beta1 "k8s.io/kubernetes/pkg/apis/rbac/v1beta1" + "k8s.io/kubernetes/pkg/kubectl" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" ) @@ -75,6 +78,7 @@ func TestJoinFederation(t *testing.T) { expectedServer string expectedErr string dnsProvider string + isRBACAPIAvailable bool }{ { cluster: "syndicate", @@ -86,6 +90,19 @@ func TestJoinFederation(t *testing.T) { expectedServer: "https://10.20.30.40", expectedErr: "", dnsProvider: util.FedDNSProviderCoreDNS, + isRBACAPIAvailable: true, + }, + { + cluster: "syndicate", + clusterCtx: "", + secret: "", + server: "https://10.20.30.40", + token: "badge", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + expectedServer: "https://10.20.30.40", + expectedErr: "", + isRBACAPIAvailable: false, }, { cluster: "ally", @@ -96,6 +113,7 @@ func TestJoinFederation(t *testing.T) { kubeconfigExplicit: fakeKubeFiles[1], expectedServer: "https://ally256.example.com:80", expectedErr: "", + isRBACAPIAvailable: true, }, { cluster: "confederate", @@ -106,6 +124,7 @@ func TestJoinFederation(t *testing.T) { kubeconfigExplicit: fakeKubeFiles[2], expectedServer: "https://10.8.8.8", expectedErr: "", + isRBACAPIAvailable: true, }, { cluster: "associate", @@ -116,6 +135,7 @@ func TestJoinFederation(t *testing.T) { kubeconfigExplicit: fakeKubeFiles[2], expectedServer: "https://10.8.8.8", expectedErr: "", + isRBACAPIAvailable: true, }, { cluster: "affiliate", @@ -126,6 +146,7 @@ func TestJoinFederation(t *testing.T) { kubeconfigExplicit: "", expectedServer: "https://10.20.30.40", expectedErr: fmt.Sprintf("error: cluster context %q not found", "affiliate"), + isRBACAPIAvailable: true, }, { cluster: "associate", @@ -142,15 +163,23 @@ func TestJoinFederation(t *testing.T) { for i, tc := range testCases { cmdErrMsg = "" - f := testJoinFederationFactory(tc.cluster, tc.secret, tc.expectedServer) + f := testJoinFederationFactory(tc.cluster, tc.secret, tc.expectedServer, tc.isRBACAPIAvailable) buf := bytes.NewBuffer([]byte{}) - hostFactory, err := fakeJoinHostFactory(tc.cluster, tc.clusterCtx, tc.secret, tc.server, tc.token, tc.dnsProvider) + hostFactory, err := fakeJoinHostFactory(tc.cluster, tc.clusterCtx, tc.secret, tc.server, tc.token, tc.dnsProvider, tc.isRBACAPIAvailable) if err != nil { t.Fatalf("[%d] unexpected error: %v", i, err) } - targetClusterFactory, err := fakeJoinTargetClusterFactory(tc.cluster, tc.clusterCtx, tc.dnsProvider) + // The fake discovery client caches results by default, so invalidate it by modifying the temporary directory. + // Refer to pkg/kubectl/cmd/testing/fake (fakeAPIFactory.DiscoveryClient()) for details of tmpDir + tmpDirPath, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("[%d] unexpected error: %v", i, err) + } + defer os.Remove(tmpDirPath) + + targetClusterFactory, err := fakeJoinTargetClusterFactory(tc.cluster, tc.clusterCtx, tc.dnsProvider, tmpDirPath, tc.isRBACAPIAvailable) if err != nil { t.Fatalf("[%d] unexpected error: %v", i, err) } @@ -195,9 +224,9 @@ func TestJoinFederation(t *testing.T) { } } -func testJoinFederationFactory(clusterName, secretName, server string) cmdutil.Factory { +func testJoinFederationFactory(clusterName, secretName, server string, isRBACAPIAvailable bool) cmdutil.Factory { - want := fakeCluster(clusterName, secretName, server) + want := fakeCluster(clusterName, secretName, server, isRBACAPIAvailable) f, tf, _, _ := cmdtesting.NewAPIFactory() codec := testapi.Federation.Codec() ns := dynamic.ContentConfig().NegotiatedSerializer @@ -236,55 +265,77 @@ func testJoinFederationFactory(clusterName, secretName, server string) cmdutil.F return f } -func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token, dnsProvider string) (cmdutil.Factory, error) { +func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token, dnsProvider string, isRBACAPIAvailable bool) (cmdutil.Factory, error) { if clusterCtx == "" { clusterCtx = clusterName } - kubeconfig := clientcmdapi.Config{ - Clusters: map[string]*clientcmdapi.Cluster{ - clusterCtx: { - Server: server, - }, - }, - AuthInfos: map[string]*clientcmdapi.AuthInfo{ - clusterCtx: { - Token: token, - }, - }, - Contexts: map[string]*clientcmdapi.Context{ - clusterCtx: { - Cluster: clusterCtx, - AuthInfo: clusterCtx, - }, - }, - CurrentContext: clusterCtx, - } - configBytes, err := clientcmd.Write(kubeconfig) - if err != nil { - return nil, err - } - placeholderSecretName := secretName if placeholderSecretName == "" { placeholderSecretName = "secretName" } - secretObject := v1.Secret{ - TypeMeta: metav1.TypeMeta{ - Kind: "Secret", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: placeholderSecretName, - Namespace: util.DefaultFederationSystemNamespace, - Annotations: map[string]string{ - federation.FederationNameAnnotation: testFederationName, - federation.ClusterNameAnnotation: clusterName, + var secretObject v1.Secret + if isRBACAPIAvailable { + secretObject = v1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", }, - }, - Data: map[string][]byte{ - "kubeconfig": configBytes, - }, + ObjectMeta: metav1.ObjectMeta{ + Name: placeholderSecretName, + Namespace: util.DefaultFederationSystemNamespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + Data: map[string][]byte{ + "ca.crt": []byte("cert"), + "token": []byte("token"), + }, + } + } else { + kubeconfig := clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + clusterCtx: { + Server: server, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + clusterCtx: { + Token: token, + }, + }, + Contexts: map[string]*clientcmdapi.Context{ + clusterCtx: { + Cluster: clusterCtx, + AuthInfo: clusterCtx, + }, + }, + CurrentContext: clusterCtx, + } + configBytes, err := clientcmd.Write(kubeconfig) + if err != nil { + return nil, err + } + + secretObject = v1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: placeholderSecretName, + Namespace: util.DefaultFederationSystemNamespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + Data: map[string][]byte{ + "kubeconfig": configBytes, + }, + } } cmName := "controller-manager" @@ -351,7 +402,11 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token, dns return f, nil } -func fakeJoinTargetClusterFactory(clusterName, clusterCtx, dnsProvider string) (cmdutil.Factory, error) { +func serviceAccountName(clusterName string) string { + return fmt.Sprintf("%s-substrate", clusterName) +} + +func fakeJoinTargetClusterFactory(clusterName, clusterCtx, dnsProvider, tmpDirPath string, isRBACAPIAvailable bool) (cmdutil.Factory, error) { if clusterCtx == "" { clusterCtx = clusterName } @@ -369,6 +424,38 @@ func fakeJoinTargetClusterFactory(clusterName, clusterCtx, dnsProvider string) ( util.FedDomainMapKey: fmt.Sprintf("%s=%s", clusterCtx, zoneName), }, } + + saSecretName := "serviceaccountsecret" + saSecret := v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: saSecretName, + Namespace: util.DefaultFederationSystemNamespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + Data: map[string][]byte{ + "ca.crt": []byte("cert"), + "token": []byte("token"), + }, + Type: v1.SecretTypeServiceAccountToken, + } + + saName := serviceAccountName(clusterName) + + serviceAccount := v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + Secrets: []v1.ObjectReference{ + {Name: saSecretName}, + }, + } if dnsProvider == util.FedDNSProviderCoreDNS { annotations := map[string]string{ util.FedDNSProvider: util.FedDNSProviderCoreDNS, @@ -378,14 +465,92 @@ func fakeJoinTargetClusterFactory(clusterName, clusterCtx, dnsProvider string) ( configmapObject = populateStubDomainsIfRequiredTest(configmapObject, annotations) } + namespace := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "federation-system", + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + } + + roleName := util.ClusterRoleName(saName) + clusterRole := rbacv1beta1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + Namespace: util.DefaultFederationSystemNamespace, + Annotations: map[string]string{ + federation.FederationNameAnnotation: testFederationName, + federation.ClusterNameAnnotation: clusterName, + }, + }, + Rules: []rbacv1beta1.PolicyRule{ + rbacv1beta1.NewRule(rbacv1beta1.VerbAll).Groups(rbacv1beta1.APIGroupAll).Resources(rbacv1beta1.ResourceAll).RuleOrDie(), + }, + } + + clusterRoleBinding, err := rbacv1beta1.NewClusterBinding(roleName).SAs(util.DefaultFederationSystemNamespace, saName).Binding() + if err != nil { + return nil, err + } + + testGroup := metav1.APIGroup{ + Name: "testAPIGroup", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "testAPIGroup/testAPIVersion", + Version: "testAPIVersion", + }, + }, + } + apiGroupList := &metav1.APIGroupList{} + apiGroupList.Groups = append(apiGroupList.Groups, testGroup) + if isRBACAPIAvailable { + rbacGroup := metav1.APIGroup{ + Name: rbacv1beta1.GroupName, + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: rbacv1beta1.GroupName + "/v1beta1", + Version: "v1beta1", + }, + }, + } + apiGroupList.Groups = append(apiGroupList.Groups, rbacGroup) + } + f, tf, codec, _ := cmdtesting.NewAPIFactory() + defaultCodec := testapi.Default.Codec() + rbacCodec := testapi.Rbac.Codec() ns := dynamic.ContentConfig().NegotiatedSerializer + tf.TmpDir = tmpDirPath tf.ClientConfig = kubefedtesting.DefaultClientConfig() tf.Client = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - switch p, m := req.URL.Path, req.Method; { + switch p, m, r := req.URL.Path, req.Method, isRBACAPIAvailable; { + case p == "/api/v1/namespaces" && m == http.MethodPost: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(defaultCodec, &namespace)}, 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 == fmt.Sprintf("/api/v1/namespaces/federation-system/serviceaccounts/%s", saName) && m == http.MethodGet && r: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(defaultCodec, &serviceAccount)}, nil + case p == "/api/v1/namespaces/federation-system/serviceaccounts" && m == http.MethodPost && r: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(defaultCodec, &serviceAccount)}, nil + + case p == "/apis/rbac.authorization.k8s.io/v1beta1/clusterroles" && m == http.MethodPost && r: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(rbacCodec, &clusterRole)}, nil + case p == "/apis/rbac.authorization.k8s.io/v1beta1/clusterrolebindings" && m == http.MethodPost && r: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(rbacCodec, &clusterRoleBinding)}, nil + + case p == "/api/v1/namespaces/federation-system/secrets/serviceaccountsecret" && m == http.MethodGet && r: + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(defaultCodec, &saSecret)}, nil + case p == "/api/v1/namespaces/kube-system/configmaps/" && m == http.MethodPost: body, err := ioutil.ReadAll(req.Body) if err != nil { @@ -405,11 +570,12 @@ func fakeJoinTargetClusterFactory(clusterName, clusterCtx, dnsProvider string) ( } }), } + return f, nil } -func fakeCluster(clusterName, secretName, server string) federationapi.Cluster { - return federationapi.Cluster{ +func fakeCluster(clusterName, secretName, server string, isRBACAPIAvailable bool) federationapi.Cluster { + cluster := federationapi.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: clusterName, }, @@ -425,6 +591,15 @@ func fakeCluster(clusterName, secretName, server string) federationapi.Cluster { }, }, } + if isRBACAPIAvailable { + saName := serviceAccountName(clusterName) + annotations := map[string]string{ + kubectl.ServiceAccountNameAnnotation: saName, + kubectl.ClusterRoleNameAnnotation: util.ClusterRoleName(saName), + } + cluster.ObjectMeta.SetAnnotations(annotations) + } + return cluster } // TODO: Reuse the function populateStubDomainsIfRequired once that function is converted to use versioned objects. diff --git a/federation/pkg/kubefed/unjoin.go b/federation/pkg/kubefed/unjoin.go index bbb3de99d24..963908937c8 100644 --- a/federation/pkg/kubefed/unjoin.go +++ b/federation/pkg/kubefed/unjoin.go @@ -29,11 +29,13 @@ import ( "k8s.io/kubernetes/federation/pkg/kubefed/util" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" + "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) var ( @@ -47,11 +49,20 @@ var ( # Federation control plane's host cluster context name # must be specified via the --host-cluster-context flag # to properly cleanup the credentials. - kubectl unjoin foo --host-cluster-context=bar`) + kubectl unjoin foo --host-cluster-context=bar --cluster-context=baz`) ) type unjoinFederation struct { commonOptions util.SubcommandOptions + options unjoinFederationOptions +} + +type unjoinFederationOptions struct { + clusterContext string +} + +func (o *unjoinFederationOptions) Bind(flags *pflag.FlagSet) { + flags.StringVar(&o.clusterContext, "cluster-context", "", "Name of the cluster's context in the local kubeconfig. Defaults to cluster name if unspecified.") } // NewCmdUnjoin defines the `unjoin` command that removes a cluster @@ -72,12 +83,17 @@ func NewCmdUnjoin(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.Admin flags := cmd.Flags() opts.commonOptions.Bind(flags) + opts.options.Bind(flags) return cmd } // unjoinFederation is the implementation of the `unjoin` command. func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.AdminConfig) error { + if u.options.clusterContext == "" { + u.options.clusterContext = u.commonOptions.Name + } + cluster, err := popCluster(f, u.commonOptions.Name) if err != nil { return err @@ -101,26 +117,53 @@ func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, conf // If this is the case, we cannot get the cluster clientset to delete the // config map from that cluster and obviously cannot delete the not existing secret. // We just publish the warning as cluster has already been removed from federation. - fmt.Fprintf(cmdErr, "WARNING: secret %q not found in the host cluster, so it couldn't be deleted", secretName) + fmt.Fprintf(cmdErr, "WARNING: secret %q not found in the host cluster, so it couldn't be deleted. Cluster has already been removed from the federation.", secretName) + return nil } else if err != nil { fmt.Fprintf(cmdErr, "WARNING: Error retrieving secret from the base cluster") - } else { - err := deleteSecret(hostClientset, cluster.Spec.SecretRef.Name, u.commonOptions.FederationSystemNamespace) - if err != nil { - fmt.Fprintf(cmdErr, "WARNING: secret %q could not be deleted: %v", secretName, err) - // We anyways continue to try and delete the config map but with above warning - } + return err + } - // 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, secret, cluster, u.commonOptions.FederationSystemNamespace) + unjoiningClusterFactory := config.ClusterFactory(u.options.clusterContext, u.commonOptions.Kubeconfig) + unjoiningClusterClientset, err := unjoiningClusterFactory.ClientSet() + outerErr := err + if err != nil { + // Attempt to get a clientset using information from the cluster. + unjoiningClusterClientset, err = getClientsetFromCluster(secret, cluster) 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 + return fmt.Errorf("unable to get clientset from kubeconfig or cluster: %v, %v", outerErr, err) } } + err = deleteSecret(hostClientset, cluster.Spec.SecretRef.Name, u.commonOptions.FederationSystemNamespace) + if err != nil { + fmt.Fprintf(cmdErr, "WARNING: secret %q could not be deleted: %v", secretName, err) + // We anyways continue to try and delete the config map but with above warning + } + + // 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) + 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 + } + + // 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) return err } @@ -162,12 +205,7 @@ func popCluster(f cmdutil.Factory, name string) (*federationapi.Cluster, error) return cluster, rh.Delete("", name) } -func updateConfigMapFromCluster(hostClientset internalclientset.Interface, secret *api.Secret, cluster *federationapi.Cluster, fedSystemNamespace string) error { - clientset, err := getClientsetFromCluster(secret, cluster) - if err != nil { - return err - } - +func updateConfigMapFromCluster(hostClientset, unjoiningClusterClientset internalclientset.Interface, fedSystemNamespace string) error { cmDep, err := getCMDeployment(hostClientset, fedSystemNamespace) if err != nil { return err @@ -177,7 +215,7 @@ func updateConfigMapFromCluster(hostClientset internalclientset.Interface, secre return fmt.Errorf("kube-dns config map data missing from controller manager annotations") } - configMap, err := clientset.Core().ConfigMaps(metav1.NamespaceSystem).Get(util.KubeDnsConfigmapName, metav1.GetOptions{}) + configMap, err := unjoiningClusterClientset.Core().ConfigMaps(metav1.NamespaceSystem).Get(util.KubeDnsConfigmapName, metav1.GetOptions{}) if err != nil { return err } @@ -194,7 +232,7 @@ func updateConfigMapFromCluster(hostClientset internalclientset.Interface, secre } if needUpdate { - _, err = clientset.Core().ConfigMaps(metav1.NamespaceSystem).Update(configMap) + _, err = unjoiningClusterClientset.Core().ConfigMaps(metav1.NamespaceSystem).Update(configMap) } return err } @@ -262,3 +300,34 @@ func removeConfigMapString(str string, toRemove string) string { } return strings.Join(values, ",") } + +// deleteServiceAccountFromCluster removes the service account that the federation control plane uses +// to access the cluster from the cluster that is leaving the federation. +func deleteServiceAccountFromCluster(unjoiningClusterClientset internalclientset.Interface, cluster *federationapi.Cluster, fedSystemNamespace string) error { + serviceAccountName, ok := cluster.ObjectMeta.Annotations[kubectl.ServiceAccountNameAnnotation] + if !ok { + // If there is no service account name annotation, assume that this cluster does not have a federation control plane service account. + return nil + } + return unjoiningClusterClientset.Core().ServiceAccounts(fedSystemNamespace).Delete(serviceAccountName, &metav1.DeleteOptions{}) +} + +// deleteClusterRoleBindingFromCluster deletes the ClusterRole and ClusterRoleBinding from the +// cluster that is leaving the federation. +func deleteClusterRoleBindingFromCluster(unjoiningClusterClientset internalclientset.Interface, cluster *federationapi.Cluster) error { + clusterRoleName, ok := cluster.ObjectMeta.Annotations[kubectl.ClusterRoleNameAnnotation] + if !ok { + // If there is no cluster role name annotation, assume that this cluster does not have cluster role bindings. + return nil + } + + err := unjoiningClusterClientset.Rbac().ClusterRoleBindings().Delete(clusterRoleName, &metav1.DeleteOptions{}) + if err != nil && !errors.IsMethodNotSupported(err) && !errors.IsNotFound(err) { + return err + } + err = unjoiningClusterClientset.Rbac().ClusterRoles().Delete(clusterRoleName, &metav1.DeleteOptions{}) + if err != nil && !errors.IsMethodNotSupported(err) && !errors.IsNotFound(err) { + return err + } + return nil +} diff --git a/federation/pkg/kubefed/unjoin_test.go b/federation/pkg/kubefed/unjoin_test.go index c507aecd7d1..0050a5c75d6 100644 --- a/federation/pkg/kubefed/unjoin_test.go +++ b/federation/pkg/kubefed/unjoin_test.go @@ -119,7 +119,7 @@ func TestUnjoinFederation(t *testing.T) { kubeconfigGlobal: fakeKubeFiles[0], kubeconfigExplicit: "", expectedServer: "https://10.20.30.40", - expectedErr: fmt.Sprintf("WARNING: secret %q not found in the host cluster, so it couldn't be deleted", "noexist"), + expectedErr: fmt.Sprintf("WARNING: secret %q not found in the host cluster, so it couldn't be deleted. Cluster has already been removed from the federation.", "noexist"), }, // TODO: Figure out a way to test the scenarios of configmap deletion // As of now we delete the config map after deriving the clientset using @@ -171,7 +171,7 @@ func TestUnjoinFederation(t *testing.T) { func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory { urlPrefix := "/clusters/" - cluster := fakeCluster(name, name, server) + cluster := fakeCluster(name, name, server, true) if secret != "" { cluster.Spec.SecretRef.Name = secret } @@ -212,8 +212,11 @@ func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory { return f } -func fakeUnjoinHostFactory(name string) cmdutil.Factory { - urlPrefix := "/api/v1/namespaces/federation-system/secrets/" +func fakeUnjoinHostFactory(clusterName string) cmdutil.Factory { + secretsPrefix := "/api/v1/namespaces/federation-system/secrets/" + clusterRolePrefix := "/apis/rbac.authorization.k8s.io/v1beta1/clusterroles/" + serviceAccountPrefix := "/api/v1/namespaces/federation-system/serviceaccounts/" + clusterRoleBindingPrefix := "/apis/rbac.authorization.k8s.io/v1beta1/clusterrolebindings/" // Using dummy bytes for now configBytes, _ := clientcmd.Write(clientcmdapi.Config{}) @@ -223,7 +226,7 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: clusterName, Namespace: util.DefaultFederationSystemNamespace, }, Data: map[string][]byte{ @@ -239,11 +242,11 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory { NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { switch p, m := req.URL.Path, req.Method; { - case strings.HasPrefix(p, urlPrefix): + case strings.HasPrefix(p, secretsPrefix): switch m { case http.MethodDelete: - got := strings.TrimPrefix(p, urlPrefix) - if got != name { + got := strings.TrimPrefix(p, secretsPrefix) + if got != clusterName { return nil, errors.NewNotFound(api.Resource("secrets"), got) } status := metav1.Status{ @@ -251,14 +254,47 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory { } return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil case http.MethodGet: - got := strings.TrimPrefix(p, urlPrefix) - if got != name { + got := strings.TrimPrefix(p, secretsPrefix) + if got != clusterName { return nil, errors.NewNotFound(api.Resource("secrets"), got) } return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &secretObject)}, nil default: return nil, fmt.Errorf("unexpected request method: %#v\n%#v", req.URL, req) } + case strings.HasPrefix(p, serviceAccountPrefix) && m == http.MethodDelete: + got := strings.TrimPrefix(p, serviceAccountPrefix) + want := serviceAccountName(clusterName) + if got != want { + return nil, errors.NewNotFound(api.Resource("serviceaccounts"), got) + } + + status := metav1.Status{ + Status: "Success", + } + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil + case strings.HasPrefix(p, clusterRoleBindingPrefix) && m == http.MethodDelete: + got := strings.TrimPrefix(p, clusterRoleBindingPrefix) + want := util.ClusterRoleName(serviceAccountName(clusterName)) + if got != want { + return nil, errors.NewNotFound(api.Resource("clusterrolebindings"), got) + } + + status := metav1.Status{ + Status: "Success", + } + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil + case strings.HasPrefix(p, clusterRolePrefix) && m == http.MethodDelete: + got := strings.TrimPrefix(p, clusterRolePrefix) + want := util.ClusterRoleName(serviceAccountName(clusterName)) + if got != want { + return nil, errors.NewNotFound(api.Resource("clusterroles"), got) + } + + status := metav1.Status{ + Status: "Success", + } + return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil default: return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req) } diff --git a/federation/pkg/kubefed/util/util.go b/federation/pkg/kubefed/util/util.go index 2c505dcfc00..f76d422a6c9 100644 --- a/federation/pkg/kubefed/util/util.go +++ b/federation/pkg/kubefed/util/util.go @@ -226,11 +226,29 @@ func GetServerAddress(c *federationapi.Cluster) (string, error) { } func buildConfigFromSecret(secret *api.Secret, serverAddress string) (*restclient.Config, error) { - kubeconfigGetter := kubeconfigGetterForSecret(secret) - clusterConfig, err := clientcmd.BuildConfigFromKubeconfigGetter(serverAddress, kubeconfigGetter) + var clusterConfig *restclient.Config + var err error + // Pre-1.7, the secret contained a serialized kubeconfig which contained appropriate credentials. + // Post-1.7, the secret contains credentials for a service account. + // Check for the service account credentials, and use them if they exist; if not, use the + // serialized kubeconfig. + token, tokenFound := secret.Data["token"] + ca, caFound := secret.Data["ca.crt"] + if tokenFound != caFound { + return nil, fmt.Errorf("secret should have values for either both 'ca.crt' and 'token' in its Data, or neither: %v", secret) + } else if tokenFound && caFound { + clusterConfig, err = clientcmd.BuildConfigFromFlags(serverAddress, "") + clusterConfig.CAData = ca + clusterConfig.BearerToken = string(token) + } else { + kubeconfigGetter := kubeconfigGetterForSecret(secret) + clusterConfig, err = clientcmd.BuildConfigFromKubeconfigGetter(serverAddress, kubeconfigGetter) + } + if err != nil { return nil, err } + clusterConfig.QPS = KubeAPIQPS clusterConfig.Burst = KubeAPIBurst @@ -273,3 +291,17 @@ func GetVersionedClientForRBACOrFail(hostFactory cmdutil.Factory) (client.Interf return nil, &NoRBACAPIError{rbacAPINotAvailable} } + +// ClusterServiceAccountName returns the name of a service account +// whose credentials are used by the host cluster to access the +// client cluster. +func ClusterServiceAccountName(joiningClusterName, hostContext string) string { + return fmt.Sprintf("%s-%s", joiningClusterName, hostContext) +} + +// ClusterRoleName returns the name of a ClusterRole and its associated +// ClusterRoleBinding that are used to allow the service account to +// access necessary resources on the cluster. +func ClusterRoleName(serviceAccountName string) string { + return fmt.Sprintf("federation-controller-manager:%s", serviceAccountName) +} diff --git a/pkg/kubectl/cluster.go b/pkg/kubectl/cluster.go index 3bb9f0feade..27ad1facdaa 100644 --- a/pkg/kubectl/cluster.go +++ b/pkg/kubectl/cluster.go @@ -25,6 +25,11 @@ import ( "k8s.io/kubernetes/pkg/api/v1" ) +const ( + ServiceAccountNameAnnotation = "federation.kubernetes.io/servive-account-name" + ClusterRoleNameAnnotation = "federation.kubernetes.io/cluster-role-name" +) + // ClusterGeneratorV1Beta1 supports stable generation of a // federation/cluster resource. type ClusterGeneratorV1Beta1 struct { @@ -39,6 +44,15 @@ type ClusterGeneratorV1Beta1 struct { // SecretName is the name of the secret that stores the credentials // for the Kubernetes cluster that is being registered (optional) SecretName string + // ServiceAccountName is the name of the service account that is + // created in the cluster being registered. If this is provided, + // then ClusterRoleName must also be provided (optional) + ServiceAccountName string + // ClusterRoleName is the name of the cluster role and cluster role + // binding that are created in the cluster being registered. If this + // is provided, then ServiceAccountName must also be provided + // (optional) + ClusterRoleName string } // Ensure it supports the generator pattern that uses parameter @@ -68,6 +82,8 @@ func (s ClusterGeneratorV1Beta1) Generate(genericParams map[string]interface{}) clustergen.ClientCIDR = params["client-cidr"] clustergen.ServerAddress = params["server-address"] clustergen.SecretName = params["secret"] + clustergen.ServiceAccountName = params["service-account-name"] + clustergen.ClusterRoleName = params["cluster-role-name"] return clustergen.StructuredGenerate() } @@ -79,6 +95,8 @@ func (s ClusterGeneratorV1Beta1) ParamNames() []GeneratorParam { {"client-cidr", false}, {"server-address", true}, {"secret", false}, + {"service-account-name", false}, + {"cluster-role-name", false}, } } @@ -110,6 +128,21 @@ func (s ClusterGeneratorV1Beta1) StructuredGenerate() (runtime.Object, error) { }, }, } + + annotations := make(map[string]string) + if s.ServiceAccountName != "" { + annotations[ServiceAccountNameAnnotation] = s.ServiceAccountName + } + if s.ClusterRoleName != "" { + annotations[ClusterRoleNameAnnotation] = s.ClusterRoleName + } + if len(annotations) == 1 { + return nil, fmt.Errorf("Either both or neither of ServiceAccountName and ClusterRoleName must be provided.") + } + if len(annotations) > 0 { + cluster.SetAnnotations(annotations) + } + return cluster, nil } diff --git a/pkg/kubectl/cluster_test.go b/pkg/kubectl/cluster_test.go index 65a2e160e0a..197df34e713 100644 --- a/pkg/kubectl/cluster_test.go +++ b/pkg/kubectl/cluster_test.go @@ -106,6 +106,62 @@ func TestClusterGenerate(t *testing.T) { }, expectErr: false, }, + { + params: map[string]interface{}{ + "name": "bar-cluster", + "client-cidr": "10.20.30.40/16", + "server-address": "http://10.20.30.40", + "secret": "credentials", + }, + expected: &federationapi.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-cluster", + }, + Spec: federationapi.ClusterSpec{ + ServerAddressByClientCIDRs: []federationapi.ServerAddressByClientCIDR{ + { + ClientCIDR: "10.20.30.40/16", + ServerAddress: "http://10.20.30.40", + }, + }, + SecretRef: &v1.LocalObjectReference{ + Name: "credentials", + }, + }, + }, + expectErr: false, + }, + { + params: map[string]interface{}{ + "name": "bar-cluster", + "client-cidr": "10.20.30.40/16", + "server-address": "http://10.20.30.40", + "secret": "credentials", + "service-account-name": "service-account", + "cluster-role-name": "cluster-role", + }, + expected: &federationapi.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-cluster", + Annotations: map[string]string{ + ServiceAccountNameAnnotation: "service-account", + ClusterRoleNameAnnotation: "cluster-role", + }, + }, + Spec: federationapi.ClusterSpec{ + ServerAddressByClientCIDRs: []federationapi.ServerAddressByClientCIDR{ + { + ClientCIDR: "10.20.30.40/16", + ServerAddress: "http://10.20.30.40", + }, + }, + SecretRef: &v1.LocalObjectReference{ + Name: "credentials", + }, + }, + }, + expectErr: false, + }, { params: map[string]interface{}{ "server-address": "https://10.20.30.40", @@ -144,6 +200,28 @@ func TestClusterGenerate(t *testing.T) { expected: nil, expectErr: true, }, + { + params: map[string]interface{}{ + "name": "bar-cluster", + "client-cidr": "10.20.30.40/16", + "server-address": "http://10.20.30.40", + "secret": "credentials", + "cluster-role-name": "cluster-role", + }, + expected: nil, + expectErr: true, + }, + { + params: map[string]interface{}{ + "name": "bar-cluster", + "client-cidr": "10.20.30.40/16", + "server-address": "http://10.20.30.40", + "secret": "credentials", + "service-account-name": "service-account", + }, + expected: nil, + expectErr: true, + }, } generator := ClusterGeneratorV1Beta1{} for i, test := range tests {