From 7dd6c13ebba85a04b71729d73d184425da611f42 Mon Sep 17 00:00:00 2001 From: Yang Le Date: Mon, 29 Jan 2024 11:17:11 +0800 Subject: [PATCH] support custom API server address & CA bundle (#303) Signed-off-by: Yang Le --- ...er-management.io_klusterletconfig.crd.yaml | 15 ++++ go.mod | 2 +- go.sum | 4 +- pkg/bootstrap/boostrapkubeconfig.go | 23 +++++-- pkg/bootstrap/bootstrapkubeconfig_test.go | 18 ++++- pkg/controller/importconfig/cluster_info.go | 16 +++-- .../importconfig/cluster_info_test.go | 34 +++++++-- test/e2e/e2e_suite_test.go | 51 +++++++++++++- test/e2e/klusterletconfig_test.go | 69 +++++++++++++++++++ 9 files changed, 206 insertions(+), 26 deletions(-) diff --git a/deploy/base/config.open-cluster-management.io_klusterletconfig.crd.yaml b/deploy/base/config.open-cluster-management.io_klusterletconfig.crd.yaml index f742046f..bc971a21 100644 --- a/deploy/base/config.open-cluster-management.io_klusterletconfig.crd.yaml +++ b/deploy/base/config.open-cluster-management.io_klusterletconfig.crd.yaml @@ -36,6 +36,21 @@ spec: spec: description: Spec defines the desired state of KlusterletConfig properties: + hubKubeAPIServerCABundle: + description: 'HubKubeAPIServerCABundle is the CA bundle to verify + the server certificate of the hub kube API against. If not present, + CA bundle will be determined with the logic below: 1). Use the certificate + of the named certificate configured in APIServer/cluster if FQDN + matches; 2). Otherwise use the CA certificates from kube-root-ca.crt + ConfigMap in the cluster namespace;' + format: byte + type: string + hubKubeAPIServerEndpoint: + description: HubKubeAPIServerURL is the URL of the hub Kube API server. + If not present, the .status.apiServerURL of Infrastructure/cluster + will be used as the default value. e.g. `oc get infrastructure cluster + -o jsonpath='{.status.apiServerURL}'` + type: string hubKubeAPIServerProxyConfig: description: HubKubeAPIServerProxyConfig holds proxy settings for connections between klusterlet/add-on agents on the managed cluster diff --git a/go.mod b/go.mod index d03db5c4..983f0bad 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/openshift/hive/apis v0.0.0-20230825202726-4418e43e27a3 github.com/openshift/library-go v0.0.0-20230809121909-d7e7beca5bae // https://github.com/openshift/library-go/tree/release-4.14 github.com/spf13/pflag v1.0.5 - github.com/stolostron/cluster-lifecycle-api v0.0.0-20240109072430-f5fe6043d1f8 + github.com/stolostron/cluster-lifecycle-api v0.0.0-20240123023750-d71a6437fb00 go.uber.org/zap v1.24.0 golang.org/x/text v0.9.0 k8s.io/api v0.27.4 diff --git a/go.sum b/go.sum index 95d09e8d..1dc7db47 100644 --- a/go.sum +++ b/go.sum @@ -387,8 +387,8 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= -github.com/stolostron/cluster-lifecycle-api v0.0.0-20240109072430-f5fe6043d1f8 h1:DRFh4ML+WuDovJsrdgszqMQ4+qGznlYlX9/pItxWwQ8= -github.com/stolostron/cluster-lifecycle-api v0.0.0-20240109072430-f5fe6043d1f8/go.mod h1:ZNQ3Rttgk4HEreCHfocrhXavLDaUgHbZaUqk5dP8/As= +github.com/stolostron/cluster-lifecycle-api v0.0.0-20240123023750-d71a6437fb00 h1:q0DY1qdDPYWVrPiEeAIgzZRVt5DUCKae5f3ihBsEwkE= +github.com/stolostron/cluster-lifecycle-api v0.0.0-20240123023750-d71a6437fb00/go.mod h1:ZNQ3Rttgk4HEreCHfocrhXavLDaUgHbZaUqk5dP8/As= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= diff --git a/pkg/bootstrap/boostrapkubeconfig.go b/pkg/bootstrap/boostrapkubeconfig.go index 5388d0b7..9112762a 100644 --- a/pkg/bootstrap/boostrapkubeconfig.go +++ b/pkg/bootstrap/boostrapkubeconfig.go @@ -46,12 +46,12 @@ func CreateBootstrapKubeConfig(ctx context.Context, clientHolder *helpers.Client return nil, nil, err } - kubeAPIServer, err := GetKubeAPIServerAddress(ctx, clientHolder.RuntimeClient) + kubeAPIServer, err := GetKubeAPIServerAddress(ctx, clientHolder.RuntimeClient, klusterletConfig) if err != nil { return nil, nil, err } - certData, err := GetBootstrapCAData(ctx, clientHolder, kubeAPIServer, ns) + certData, err := GetBootstrapCAData(ctx, clientHolder, kubeAPIServer, ns, klusterletConfig) if err != nil { return nil, nil, err } @@ -162,8 +162,13 @@ func getBootstrapToken(ctx context.Context, kubeClient kubernetes.Interface, return []byte(tokenRequest.Status.Token), expiration, nil } -// GetKubeAPIServerAddress get the kube-apiserver URL from ocp infrastructure -func GetKubeAPIServerAddress(ctx context.Context, client client.Client) (string, error) { +func GetKubeAPIServerAddress(ctx context.Context, client client.Client, + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig) (string, error) { + // use the custom hub Kube APIServer URL if specified + if klusterletConfig != nil && len(klusterletConfig.Spec.HubKubeAPIServerURL) > 0 { + return klusterletConfig.Spec.HubKubeAPIServerURL, nil + } + infraConfig := &ocinfrav1.Infrastructure{} if err := client.Get(ctx, types.NamespacedName{Name: "cluster"}, infraConfig); err != nil { return "", err @@ -172,8 +177,14 @@ func GetKubeAPIServerAddress(ctx context.Context, client client.Client) (string, return infraConfig.Status.APIServerURL, nil } -func GetBootstrapCAData(ctx context.Context, clientHolder *helpers.ClientHolder, kubeAPIServer string, caNamespace string) ([]byte, error) { - // get the ca cert from ocp apiserver firstly +func GetBootstrapCAData(ctx context.Context, clientHolder *helpers.ClientHolder, kubeAPIServer string, + caNamespace string, klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig) ([]byte, error) { + // use the custom hub Kube APIServer CA bundle if specified + if klusterletConfig != nil && len(klusterletConfig.Spec.HubKubeAPIServerCABundle) > 0 { + return klusterletConfig.Spec.HubKubeAPIServerCABundle, nil + } + + // and then get the ca cert from ocp apiserver firstly if u, err := url.Parse(kubeAPIServer); err == nil { apiServerCertSecretName, err := getKubeAPIServerSecretName(ctx, clientHolder.RuntimeClient, u.Hostname()) if err != nil { diff --git a/pkg/bootstrap/bootstrapkubeconfig_test.go b/pkg/bootstrap/bootstrapkubeconfig_test.go index d2715934..f05086cf 100644 --- a/pkg/bootstrap/bootstrapkubeconfig_test.go +++ b/pkg/bootstrap/bootstrapkubeconfig_test.go @@ -407,7 +407,8 @@ func TestGetKubeAPIServerAddress(t *testing.T) { } type args struct { - client client.Client + client client.Client + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig } tests := []struct { name string @@ -431,10 +432,23 @@ func TestGetKubeAPIServerAddress(t *testing.T) { want: "http://127.0.0.1:6443", wantErr: false, }, + { + name: "use custom address", + args: args{ + client: fake.NewClientBuilder().WithScheme(testscheme).WithObjects(infraConfig).Build(), + klusterletConfig: &klusterletconfigv1alpha1.KlusterletConfig{ + Spec: klusterletconfigv1alpha1.KlusterletConfigSpec{ + HubKubeAPIServerURL: "https://api.acm.example.com:6443", + }, + }, + }, + want: "https://api.acm.example.com:6443", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetKubeAPIServerAddress(context.Background(), tt.args.client) + got, err := GetKubeAPIServerAddress(context.Background(), tt.args.client, tt.args.klusterletConfig) if (err != nil) != tt.wantErr { t.Errorf("getKubeAPIServerAddress() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/controller/importconfig/cluster_info.go b/pkg/controller/importconfig/cluster_info.go index 8f2473bc..f15b4717 100644 --- a/pkg/controller/importconfig/cluster_info.go +++ b/pkg/controller/importconfig/cluster_info.go @@ -48,7 +48,7 @@ func getBootstrapKubeConfigDataFromImportSecret(ctx context.Context, clientHolde } // check if the kube apiserver address is changed - validKubeAPIServer, err := validateKubeAPIServerAddress(ctx, kubeAPIServer, clientHolder) + validKubeAPIServer, err := validateKubeAPIServerAddress(ctx, kubeAPIServer, klusterletConfig, clientHolder) if err != nil { return nil, nil, fmt.Errorf("failed to validate kube apiserver address: %v", err) } @@ -58,7 +58,7 @@ func getBootstrapKubeConfigDataFromImportSecret(ctx context.Context, clientHolde } // check if the CA data is changed - validCAData, err := validateCAData(ctx, caData, kubeAPIServer, clientHolder, clusterName) + validCAData, err := validateCAData(ctx, caData, kubeAPIServer, klusterletConfig, clientHolder, clusterName) if err != nil { return nil, nil, fmt.Errorf("failed to validate CA data: %v", err) } @@ -130,12 +130,14 @@ func parseKubeConfigData(kubeConfigData []byte) (kubeAPIServer, proxyURL string, return } -func validateKubeAPIServerAddress(ctx context.Context, kubeAPIServer string, clientHolder *helpers.ClientHolder) (bool, error) { +func validateKubeAPIServerAddress(ctx context.Context, kubeAPIServer string, + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig, + clientHolder *helpers.ClientHolder) (bool, error) { if len(kubeAPIServer) == 0 { return false, nil } - currentKubeAPIServer, err := bootstrap.GetKubeAPIServerAddress(ctx, clientHolder.RuntimeClient) + currentKubeAPIServer, err := bootstrap.GetKubeAPIServerAddress(ctx, clientHolder.RuntimeClient, klusterletConfig) if err != nil { return false, err } @@ -143,13 +145,15 @@ func validateKubeAPIServerAddress(ctx context.Context, kubeAPIServer string, cli return kubeAPIServer == currentKubeAPIServer, nil } -func validateCAData(ctx context.Context, caData []byte, kubeAPIServer string, clientHolder *helpers.ClientHolder, clusterName string) (bool, error) { +func validateCAData(ctx context.Context, caData []byte, kubeAPIServer string, + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig, + clientHolder *helpers.ClientHolder, clusterName string) (bool, error) { if len(caData) == 0 { // CA data is empty return false, nil } - currentCAData, err := bootstrap.GetBootstrapCAData(ctx, clientHolder, kubeAPIServer, clusterName) + currentCAData, err := bootstrap.GetBootstrapCAData(ctx, clientHolder, kubeAPIServer, clusterName, klusterletConfig) if err != nil { return false, err } diff --git a/pkg/controller/importconfig/cluster_info_test.go b/pkg/controller/importconfig/cluster_info_test.go index 4d6e3cda..f7ee4592 100644 --- a/pkg/controller/importconfig/cluster_info_test.go +++ b/pkg/controller/importconfig/cluster_info_test.go @@ -264,6 +264,7 @@ func TestValidateKubeAPIServerAddress(t *testing.T) { name string kubeAPIServer string infraKubeAPIServer string + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig valid bool }{ { @@ -274,6 +275,15 @@ func TestValidateKubeAPIServerAddress(t *testing.T) { kubeAPIServer: "https://api.my-cluster.example.com:6443", infraKubeAPIServer: "https://api-int.my-cluster.example.com:6443", }, + { + name: "address overridden", + kubeAPIServer: "https://api.my-cluster.example.com:6443", + klusterletConfig: &klusterletconfigv1alpha1.KlusterletConfig{ + Spec: klusterletconfigv1alpha1.KlusterletConfigSpec{ + HubKubeAPIServerURL: "https://api.acm.example.com:6443", + }, + }, + }, { name: "no change", kubeAPIServer: "https://api.my-cluster.example.com:6443", @@ -298,7 +308,7 @@ func TestValidateKubeAPIServerAddress(t *testing.T) { }).Build(), } - valid, err := validateKubeAPIServerAddress(context.TODO(), c.kubeAPIServer, clientHolder) + valid, err := validateKubeAPIServerAddress(context.TODO(), c.kubeAPIServer, c.klusterletConfig, clientHolder) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -311,11 +321,12 @@ func TestValidateKubeAPIServerAddress(t *testing.T) { func TestValidateCAData(t *testing.T) { cases := []struct { - name string - clusterName string - bootstrapCAData []byte - currentCAData []byte - valid bool + name string + clusterName string + bootstrapCAData []byte + currentCAData []byte + klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig + valid bool }{ { name: "CA data is empty", @@ -325,6 +336,15 @@ func TestValidateCAData(t *testing.T) { bootstrapCAData: []byte("my-ca-bundle"), currentCAData: []byte("my-new-ca-bundle"), }, + { + name: "cert overridden", + bootstrapCAData: []byte("my-ca-bundle"), + klusterletConfig: &klusterletconfigv1alpha1.KlusterletConfig{ + Spec: klusterletconfigv1alpha1.KlusterletConfigSpec{ + HubKubeAPIServerCABundle: []byte("my-custom-ca-bundle"), + }, + }, + }, { name: "no cert change", bootstrapCAData: []byte("my-ca-bundle"), @@ -370,7 +390,7 @@ func TestValidateCAData(t *testing.T) { KubeClient: fakeKubeClient, } - valid, err := validateCAData(context.TODO(), c.bootstrapCAData, kubeAPIServer, clientHolder, "cluster") + valid, err := validateCAData(context.TODO(), c.bootstrapCAData, kubeAPIServer, c.klusterletConfig, clientHolder, "cluster") if err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index ed2b8f1c..4956f652 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -17,6 +17,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" + ocinfrav1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/operator/events" corev1 "k8s.io/api/core/v1" apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -24,8 +25,11 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sruntime "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" + k8sscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" certutil "k8s.io/client-go/util/cert" @@ -58,8 +62,15 @@ var ( hubRuntimeClient crclient.Client hubRecorder events.Recorder hubMapper meta.RESTMapper + + scheme = k8sruntime.NewScheme() ) +func init() { + utilruntime.Must(k8sscheme.AddToScheme(scheme)) + utilruntime.Must(ocinfrav1.AddToScheme(scheme)) +} + func TestE2E(t *testing.T) { gomega.RegisterFailHandler(ginkgo.Fail) ginkgo.RunSpecs(t, "End-to-end Test Suite") @@ -98,7 +109,9 @@ var _ = ginkgo.BeforeSuite(func() { klusterletconfigClient, err = klusterletconfigclient.NewForConfig(clusterCfg) gomega.Expect(err).Should(gomega.BeNil()) - hubRuntimeClient, err = crclient.New(clusterCfg, crclient.Options{}) + hubRuntimeClient, err = crclient.New(clusterCfg, crclient.Options{ + Scheme: scheme, + }) gomega.Expect(err).Should(gomega.BeNil()) hubRecorder = helpers.NewEventRecorder(hubKubeClient, "e2e-test") @@ -329,7 +342,7 @@ func assertManagedClusterDeletedFromHub(clusterName string) { } return fmt.Errorf("managed cluster %s still exists", clusterName) - }, 60*time.Second, 1*time.Second).Should(gomega.Succeed()) + }, 120*time.Second, 1*time.Second).Should(gomega.Succeed()) }) util.Logf("spending time: %.2f seconds", time.Since(start).Seconds()) @@ -739,6 +752,40 @@ func assertBootstrapKubeconfigWithProxyConfig(proxyURL string, caDataIncluded, c }) } +func assertBootstrapKubeconfigServerURLAndCABundle(serverURL string, caData []byte) { + ginkgo.By("Klusterlet should have bootstrap kubeconfig with expected serverURL & CA bundle", func() { + var bootstrapKubeconfigSecret *corev1.Secret + gomega.Eventually(func() error { + var err error + bootstrapKubeconfigSecret, err = hubKubeClient.CoreV1().Secrets("open-cluster-management-agent").Get(context.TODO(), "bootstrap-hub-kubeconfig", metav1.GetOptions{}) + if err != nil { + return err + } + + config, err := clientcmd.Load(bootstrapKubeconfigSecret.Data["kubeconfig"]) + if err != nil { + return err + } + + // check server url + cluster, ok := config.Clusters["default-cluster"] + if !ok { + return fmt.Errorf("default-cluster not found") + } + if cluster.Server != serverURL { + return fmt.Errorf("expected server url %q but got: %s", serverURL, cluster.Server) + } + + // check ca data + if !reflect.DeepEqual(cluster.CertificateAuthorityData, caData) { + return fmt.Errorf("unexpected CA bundle is included in the bootstrap kubeconfig: open-cluster-management-agent/bootstrap-hub-kubeconfig") + } + + return nil + }, 60*time.Second, 1*time.Second).Should(gomega.Succeed()) + }) +} + func hasCertificate(certs []*x509.Certificate, cert *x509.Certificate) bool { if cert == nil { return true diff --git a/test/e2e/klusterletconfig_test.go b/test/e2e/klusterletconfig_test.go index b5e2cd79..46617e9a 100644 --- a/test/e2e/klusterletconfig_test.go +++ b/test/e2e/klusterletconfig_test.go @@ -15,6 +15,8 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" klusterletconfigv1alpha1 "github.com/stolostron/cluster-lifecycle-api/klusterletconfig/v1alpha1" + "github.com/stolostron/managedcluster-import-controller/pkg/bootstrap" + "github.com/stolostron/managedcluster-import-controller/pkg/helpers" "github.com/stolostron/managedcluster-import-controller/test/e2e/util" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -206,6 +208,73 @@ var _ = Describe("Use KlusterletConfig to customize klusterlet manifests", func( // cluster should become available because no proxy is used assertManagedClusterAvailable(managedClusterName) }) + + It("Should deploy the klusterlet with custom server URL and CA bundle", func() { + By("Create managed cluster", func() { + _, err := util.CreateManagedClusterWithShortLeaseDuration( + hubClusterClient, + managedClusterName, + map[string]string{ + "agent.open-cluster-management.io/klusterlet-config": klusterletConfigName, + }, + util.NewLable("local-cluster", "true")) + Expect(err).ToNot(HaveOccurred()) + }) + + // klusterletconfig is missing and it will be ignored + defaultServerUrl, err := bootstrap.GetKubeAPIServerAddress(context.TODO(), hubRuntimeClient, nil) + Expect(err).ToNot(HaveOccurred()) + defaultCABundle, err := bootstrap.GetBootstrapCAData(context.TODO(), &helpers.ClientHolder{ + KubeClient: hubKubeClient, + RuntimeClient: hubRuntimeClient, + }, defaultServerUrl, managedClusterName, nil) + Expect(err).ToNot(HaveOccurred()) + assertBootstrapKubeconfigServerURLAndCABundle(defaultServerUrl, defaultCABundle) + assertManagedClusterAvailable(managedClusterName) + + customServerURL := "https://invalid.server.url:6443" + customCAData, _, err := newCert("custom CA for hub Kube API server") + Expect(err).ToNot(HaveOccurred()) + + By("Create KlusterletConfig with custom server URL & CA bundle", func() { + _, err := klusterletconfigClient.ConfigV1alpha1().KlusterletConfigs().Create(context.TODO(), &klusterletconfigv1alpha1.KlusterletConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: klusterletConfigName, + }, + Spec: klusterletconfigv1alpha1.KlusterletConfigSpec{ + HubKubeAPIServerURL: customServerURL, + HubKubeAPIServerCABundle: customCAData, + }, + }, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + }) + + assertBootstrapKubeconfigServerURLAndCABundle(customServerURL, customCAData) + + // cluster should become offline because the custom server URL and CA bundle is invalid + assertManagedClusterOffline(managedClusterName, 120*time.Second) + + By("Delete Klusterletconfig", func() { + err := klusterletconfigClient.ConfigV1alpha1().KlusterletConfigs().Delete(context.TODO(), klusterletConfigName, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + }) + + assertBootstrapKubeconfigServerURLAndCABundle(defaultServerUrl, defaultCABundle) + + // delete agent deployment to rebootstrap + deploys, err := hubKubeClient.AppsV1().Deployments("open-cluster-management-agent").List(context.TODO(), metav1.ListOptions{}) + Expect(err).ToNot(HaveOccurred()) + for _, deploy := range deploys.Items { + if deploy.Name == "klusterlet" { + continue + } + err = hubKubeClient.AppsV1().Deployments(deploy.Namespace).Delete(context.TODO(), deploy.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + } + + // cluster should become available because custom server URL and CA bundle is removed + assertManagedClusterAvailable(managedClusterName) + }) }) func newCert(commoneName string) ([]byte, []byte, error) {