Merge pull request #42513 from perotinus/generatedsecrets

Automatic merge from submit-queue (batch tested with PRs 45100, 45152, 42513, 44796, 45222)

[Federation] Generate the secret name in kubefed join.

Addresses part of #42324. A follow-up PR will address annotating Federation resources.

```release-note
Remove the `--secret-name` flag from `kubefed join`, instead generating the secret name arbitrarily.
```

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
 }
 

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",

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

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))
 				}