From 77e71a890e83c4c29e9a31aa5974e51cb9456553 Mon Sep 17 00:00:00 2001 From: Jonathan MacMillan Date: Fri, 3 Mar 2017 17:22:31 -0800 Subject: [PATCH] [Federation] Generate the secret name in kubefed join. --- federation/cluster/federation-up.sh | 3 +- federation/pkg/kubefed/BUILD | 1 + federation/pkg/kubefed/join.go | 14 ++++--- federation/pkg/kubefed/join_test.go | 62 +++++++++++++++++++---------- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/federation/cluster/federation-up.sh b/federation/cluster/federation-up.sh index 3fb8f3bc907..b5c120353a1 100755 --- a/federation/cluster/federation-up.sh +++ b/federation/cluster/federation-up.sh @@ -107,8 +107,7 @@ function join_clusters() { "${context}" \ --federation-system-namespace=${FEDERATION_NAMESPACE} \ --host-cluster-context="${HOST_CLUSTER_CONTEXT}" \ - --context="${FEDERATION_KUBE_CONTEXT}" \ - --secret-name="${context//_/-}" # Replace "_" by "-" + --context="${FEDERATION_KUBE_CONTEXT}" done } diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index 833e5e67e36..02389cb9a46 100644 --- a/federation/pkg/kubefed/BUILD +++ b/federation/pkg/kubefed/BUILD @@ -21,6 +21,7 @@ go_library( "//federation/pkg/kubefed/init:go_default_library", "//federation/pkg/kubefed/util:go_default_library", "//pkg/api:go_default_library", + "//pkg/api/v1:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/kubectl:go_default_library", diff --git a/federation/pkg/kubefed/join.go b/federation/pkg/kubefed/join.go index 7479b367552..74a7eaf7f52 100644 --- a/federation/pkg/kubefed/join.go +++ b/federation/pkg/kubefed/join.go @@ -36,6 +36,7 @@ import ( "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" ) @@ -78,6 +79,7 @@ type joinFederationOptions struct { func (o *joinFederationOptions) 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.") flags.StringVar(&o.secretName, "secret-name", "", "Name of the secret where the cluster's credentials will be stored in the host cluster. This name should be a valid RFC 1035 label. Defaults to cluster name if unspecified.") + flags.MarkDeprecated("secret-name", "kubefed now generates a secret name, and this flag will be removed in a future release.") } // NewCmdJoin defines the `join` command that joins a cluster to a @@ -120,9 +122,6 @@ func (j *joinFederation) Complete(cmd *cobra.Command, args []string) error { if j.options.clusterContext == "" { j.options.clusterContext = j.commonOptions.Name } - if j.options.secretName == "" { - j.options.secretName = j.commonOptions.Name - } 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) @@ -137,7 +136,12 @@ func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.Ad if err != nil { return err } - generator, err := clusterGenerator(clientConfig, j.commonOptions.Name, j.options.clusterContext, j.options.secretName) + secretName := j.options.secretName + if secretName == "" { + secretName = v1.SimpleNameGenerator.GenerateName(j.commonOptions.Name + "-") + } + + generator, err := clusterGenerator(clientConfig, j.commonOptions.Name, j.options.clusterContext, secretName) if err != nil { glog.V(2).Infof("Failed creating cluster generator: %v", err) return err @@ -172,7 +176,7 @@ func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.Ad // 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, j.options.secretName, j.options.dryRun) + _, err = createSecret(hostClientset, clientConfig, j.commonOptions.FederationSystemNamespace, federationName, j.commonOptions.Name, j.options.clusterContext, secretName, j.options.dryRun) if err != nil { glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err) return err diff --git a/federation/pkg/kubefed/join_test.go b/federation/pkg/kubefed/join_test.go index 1b56fd69b18..b18bf9aafd4 100644 --- a/federation/pkg/kubefed/join_test.go +++ b/federation/pkg/kubefed/join_test.go @@ -73,7 +73,6 @@ func TestJoinFederation(t *testing.T) { { cluster: "syndicate", clusterCtx: "", - secret: "", server: "https://10.20.30.40", token: "badge", kubeconfigGlobal: fakeKubeFiles[0], @@ -84,7 +83,6 @@ func TestJoinFederation(t *testing.T) { { cluster: "ally", clusterCtx: "", - secret: "", server: "ally256.example.com:80", token: "souvenir", kubeconfigGlobal: fakeKubeFiles[0], @@ -95,7 +93,6 @@ func TestJoinFederation(t *testing.T) { { cluster: "confederate", clusterCtx: "", - secret: "", server: "10.8.8.8", token: "totem", kubeconfigGlobal: fakeKubeFiles[1], @@ -103,6 +100,26 @@ func TestJoinFederation(t *testing.T) { expectedServer: "https://10.8.8.8", expectedErr: "", }, + { + cluster: "associate", + clusterCtx: "confederate", + server: "10.8.8.8", + token: "totem", + kubeconfigGlobal: fakeKubeFiles[1], + kubeconfigExplicit: fakeKubeFiles[2], + expectedServer: "https://10.8.8.8", + expectedErr: "", + }, + { + cluster: "affiliate", + clusterCtx: "", + server: "https://10.20.30.40", + token: "badge", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + expectedServer: "https://10.20.30.40", + expectedErr: fmt.Sprintf("error: cluster context %q not found", "affiliate"), + }, { cluster: "associate", clusterCtx: "confederate", @@ -114,17 +131,6 @@ func TestJoinFederation(t *testing.T) { expectedServer: "https://10.8.8.8", expectedErr: "", }, - { - cluster: "affiliate", - clusterCtx: "", - secret: "", - server: "https://10.20.30.40", - token: "badge", - kubeconfigGlobal: fakeKubeFiles[0], - kubeconfigExplicit: "", - expectedServer: "https://10.20.30.40", - expectedErr: fmt.Sprintf("error: cluster context %q not found", "affiliate"), - }, } for i, tc := range testCases { @@ -183,9 +189,6 @@ func TestJoinFederation(t *testing.T) { } func testJoinFederationFactory(clusterName, secretName, server string) cmdutil.Factory { - if secretName == "" { - secretName = clusterName - } want := fakeCluster(clusterName, secretName, server) f, tf, _, _ := cmdtesting.NewAPIFactory() @@ -206,6 +209,13 @@ func testJoinFederationFactory(clusterName, secretName, server string) cmdutil.F if err != nil { return nil, err } + // If the secret name was generated, test it separately. + if secretName == "" { + if got.Spec.SecretRef.Name == "" { + return nil, fmt.Errorf("expected a generated secret name, got \"\"") + } + got.Spec.SecretRef.Name = "" + } if !apiequality.Semantic.DeepEqual(got, want) { return nil, fmt.Errorf("Unexpected cluster object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, want)) } @@ -223,9 +233,6 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri if clusterCtx == "" { clusterCtx = clusterName } - if secretName == "" { - secretName = clusterName - } kubeconfig := clientcmdapi.Config{ Clusters: map[string]*clientcmdapi.Cluster{ @@ -251,13 +258,17 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri return nil, err } + placeholderSecretName := secretName + if placeholderSecretName == "" { + placeholderSecretName = "secretName" + } secretObject := v1.Secret{ TypeMeta: metav1.TypeMeta{ Kind: "Secret", APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: secretName, + Name: placeholderSecretName, Namespace: util.DefaultFederationSystemNamespace, Annotations: map[string]string{ federation.FederationNameAnnotation: testFederationName, @@ -312,6 +323,15 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri if err != nil { return nil, err } + + // If the secret name was generated, test it separately. + if secretName == "" { + if got.Name == "" { + return nil, fmt.Errorf("expected a generated secret name, got \"\"") + } + got.Name = placeholderSecretName + } + if !apiequality.Semantic.DeepEqual(got, secretObject) { return nil, fmt.Errorf("Unexpected secret object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, secretObject)) }