diff --git a/pkg/controllers/common/constants.go b/pkg/controllers/common/constants.go index 8e8ed27d..30dee559 100644 --- a/pkg/controllers/common/constants.go +++ b/pkg/controllers/common/constants.go @@ -141,6 +141,7 @@ const ( ClusterClientKeyKey = "client-key-data" ClusterCertificateAuthorityKey = "certificate-authority-data" ClusterServiceAccountTokenKey = "service-account-token-data" + ClusterBootstrapTokenKey = "bootstrap-token-data" ) const ( diff --git a/pkg/controllers/federatedcluster/controller.go b/pkg/controllers/federatedcluster/controller.go index 013014a9..f76ae4b2 100644 --- a/pkg/controllers/federatedcluster/controller.go +++ b/pkg/controllers/federatedcluster/controller.go @@ -475,7 +475,7 @@ func (c *FederatedClusterController) getClusterClient( return nil, nil, fmt.Errorf("failed to get cluster secret: %w", err) } - if err := clusterutil.PopulateAuthDetailsFromSecret(restConfig, cluster.Spec.Insecure, clusterSecret, false); err != nil { + if err := clusterutil.PopulateAuthDetailsFromSecret(restConfig, cluster.Spec.Insecure, clusterSecret, true); err != nil { return nil, nil, fmt.Errorf("cluster secret malformed: %w", err) } diff --git a/pkg/util/cluster/client.go b/pkg/util/cluster/client.go index fa46080f..da5dc932 100644 --- a/pkg/util/cluster/client.go +++ b/pkg/util/cluster/client.go @@ -27,18 +27,7 @@ import ( "k8s.io/client-go/tools/clientcmd" fedcorev1a1 "github.com/kubewharf/kubeadmiral/pkg/apis/core/v1alpha1" -) - -// User account keys -const ( - ClientCertificateKey = "client-certificate-data" - ClientKeyKey = "client-key-data" - CertificateAuthorityKey = "certificate-authority-data" -) - -// Service account keys -const ( - ServiceAccountTokenKey = "service-account-token-data" + "github.com/kubewharf/kubeadmiral/pkg/controllers/common" ) func BuildClusterConfig( @@ -52,7 +41,7 @@ func BuildClusterConfig( fedClient, restConfig, fedSystemNamespace, - cluster.Spec.UseServiceAccountToken, + !cluster.Spec.UseServiceAccountToken, ) } @@ -69,7 +58,7 @@ func BuildRawClusterConfig( fedClient, restConfig, fedSystemNamespace, - false, + true, ) } @@ -78,7 +67,7 @@ func buildClusterConfig( fedClient kubernetes.Interface, restConfig *rest.Config, fedSystemNamespace string, - useServiceAccountToken bool, + useBootstrap bool, ) (*rest.Config, error) { apiEndpoint := cluster.Spec.APIEndpoint if len(apiEndpoint) == 0 { @@ -107,7 +96,7 @@ func buildClusterConfig( return nil, err } - err = PopulateAuthDetailsFromSecret(clusterConfig, cluster.Spec.Insecure, secret, useServiceAccountToken) + err = PopulateAuthDetailsFromSecret(clusterConfig, cluster.Spec.Insecure, secret, useBootstrap) if err != nil { return nil, fmt.Errorf("cannot build rest config from cluster secret: %w", err) } @@ -118,45 +107,39 @@ func PopulateAuthDetailsFromSecret( clusterConfig *rest.Config, insecure bool, secret *corev1.Secret, - useServiceAccount bool, + useBootstrap bool, ) error { - var exists bool - - if useServiceAccount { - serviceAccountToken, exists := secret.Data[ServiceAccountTokenKey] - if !exists { - return fmt.Errorf("%q data is missing from secret", ServiceAccountTokenKey) - } - clusterConfig.BearerToken = string(serviceAccountToken) - - if insecure { - clusterConfig.Insecure = true - } else { - clusterConfig.CAData, exists = secret.Data[CertificateAuthorityKey] - if !exists { - return fmt.Errorf("%q data is missing from secret and insecure is false", CertificateAuthorityKey) - } - } + if insecure { + clusterConfig.Insecure = true } else { - clusterConfig.CertData, exists = secret.Data[ClientCertificateKey] - if !exists { - return fmt.Errorf("%q data is missing from secret", ClientCertificateKey) + // if federatedCluster.Spec.Insecure is false, ca data is required + if clusterConfig.CAData = secret.Data[common.ClusterCertificateAuthorityKey]; len(clusterConfig.CAData) == 0 { + return fmt.Errorf("%q data is missing from secret and insecure is false", common.ClusterCertificateAuthorityKey) } + } - clusterConfig.KeyData, exists = secret.Data[ClientKeyKey] - if !exists { - return fmt.Errorf("%q data is missing from secret", ClientKeyKey) - } + // if useBootstrap is true, we will use the client authentication specified by user + // else, we will use the token created by admiral + if useBootstrap { + var bootstrapTokenExist bool + clusterConfig.BearerToken, bootstrapTokenExist = getTokenDataFromSecret(secret, common.ClusterBootstrapTokenKey) + clusterConfig.CertData, clusterConfig.KeyData = secret.Data[common.ClusterClientCertificateKey], secret.Data[common.ClusterClientKeyKey] - if insecure { - clusterConfig.Insecure = true - } else { - clusterConfig.CAData, exists = secret.Data[CertificateAuthorityKey] - if !exists { - return fmt.Errorf("%q data is missing from secret", CertificateAuthorityKey) - } + if !bootstrapTokenExist && (len(clusterConfig.CertData) == 0 || len(clusterConfig.KeyData) == 0) { + return fmt.Errorf("the client authentication information is missing from secret, " + + "at least token or (certificate, key) information is required") + } + } else { + var saTokenExist bool + if clusterConfig.BearerToken, saTokenExist = getTokenDataFromSecret(secret, common.ClusterServiceAccountTokenKey); !saTokenExist { + return fmt.Errorf("%q data is missing from secret", common.ClusterServiceAccountTokenKey) } } return nil } + +func getTokenDataFromSecret(secret *corev1.Secret, tokenKey string) (string, bool) { + bootstrapToken := secret.Data[tokenKey] + return string(bootstrapToken), len(bootstrapToken) != 0 +} diff --git a/pkg/util/cluster/client_test.go b/pkg/util/cluster/client_test.go new file mode 100644 index 00000000..f5f19d6c --- /dev/null +++ b/pkg/util/cluster/client_test.go @@ -0,0 +1,154 @@ +/* +Copyright 2023 The KubeAdmiral Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cluster + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/rest" + + "github.com/kubewharf/kubeadmiral/pkg/controllers/common" +) + +func TestPopulateAuthDetailsFromSecret(t *testing.T) { + tests := []struct { + name string + insecure bool + inputSecret *corev1.Secret + useBootstrap bool + expectedConfig *rest.Config + expectedErr error + }{ + { + name: "normal test, insecure=true, useBootstrap=true", + insecure: true, + inputSecret: buildSecretData("ca", "cert", "key", "bootstrapToken", ""), + useBootstrap: true, + expectedConfig: &rest.Config{ + TLSClientConfig: rest.TLSClientConfig{ + Insecure: true, + CertData: []byte("cert"), + KeyData: []byte("key"), + }, + BearerToken: "bootstrapToken", + }, + }, + { + name: "normal test, insecure=false, useBootstrap=true", + insecure: false, + inputSecret: buildSecretData("ca", "cert", "key", "bootstrapToken", ""), + useBootstrap: true, + expectedConfig: &rest.Config{ + TLSClientConfig: rest.TLSClientConfig{ + CertData: []byte("cert"), + KeyData: []byte("key"), + CAData: []byte("ca"), + }, + BearerToken: "bootstrapToken", + }, + }, + { + name: "normal test, useBootstrap==true, (cert,key) is empty, bootstrapToken is not empty", + insecure: false, + inputSecret: buildSecretData("ca", "", "", "bootstrapToken", ""), + useBootstrap: true, + expectedConfig: &rest.Config{ + TLSClientConfig: rest.TLSClientConfig{ + CertData: []byte(""), + KeyData: []byte(""), + CAData: []byte("ca"), + }, + BearerToken: "bootstrapToken", + }, + }, + { + name: "normal test, useBootstrap==true, (cert,key) is not empty, bootstrapToken is empty", + insecure: false, + inputSecret: buildSecretData("ca", "cert", "key", "", ""), + useBootstrap: true, + expectedConfig: &rest.Config{ + TLSClientConfig: rest.TLSClientConfig{ + CertData: []byte("cert"), + KeyData: []byte("key"), + CAData: []byte("ca"), + }, + BearerToken: "", + }, + }, + { + name: "normal test, useBootstrap==false", + insecure: false, + inputSecret: buildSecretData("ca", "", "", "", "sat"), + useBootstrap: false, + expectedConfig: &rest.Config{ + TLSClientConfig: rest.TLSClientConfig{ + CAData: []byte("ca"), + }, + BearerToken: "sat", + }, + }, + // error test + { + name: "insecure=false, but secret.Data[certificate-authority-data] is empty", + insecure: false, + inputSecret: buildSecretData("", "", "", "", ""), + expectedErr: fmt.Errorf( + "%q data is missing from secret and insecure is false", common.ClusterCertificateAuthorityKey), + }, + { + name: "useBootstrap=false, but Secret.Data[service-account-token-data] is empty", + insecure: false, + useBootstrap: false, + inputSecret: buildSecretData("ca", "", "", "", ""), + expectedErr: fmt.Errorf("%q data is missing from secret", common.ClusterServiceAccountTokenKey), + }, + { + name: "useBootstrap=true, but both (cert,key) and bootstrapToken are empty", + insecure: false, + useBootstrap: true, + inputSecret: buildSecretData("ca", "", "", "", ""), + expectedErr: fmt.Errorf("the client authentication information is missing from secret, " + + "at least token or (certificate, key) information is required"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualClusterConfig := &rest.Config{} + err := PopulateAuthDetailsFromSecret(actualClusterConfig, tt.insecure, tt.inputSecret, tt.useBootstrap) + + assert.Equal(t, tt.expectedErr, err) + if tt.expectedConfig != nil { + assert.Equal(t, tt.expectedConfig, actualClusterConfig) + } + }) + } +} + +func buildSecretData(ca, cert, key, bt, sat string) *corev1.Secret { + return &corev1.Secret{ + Data: map[string][]byte{ + common.ClusterCertificateAuthorityKey: []byte(ca), + common.ClusterClientCertificateKey: []byte(cert), + common.ClusterClientKeyKey: []byte(key), + common.ClusterBootstrapTokenKey: []byte(bt), + common.ClusterServiceAccountTokenKey: []byte(sat), + }, + } +}