Skip to content

Commit

Permalink
support custom API server address & CA bundle (#303)
Browse files Browse the repository at this point in the history
Signed-off-by: Yang Le <[email protected]>
  • Loading branch information
elgnay authored Jan 29, 2024
1 parent 72c5a19 commit 7dd6c13
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
23 changes: 17 additions & 6 deletions pkg/bootstrap/boostrapkubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
18 changes: 16 additions & 2 deletions pkg/bootstrap/bootstrapkubeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 10 additions & 6 deletions pkg/controller/importconfig/cluster_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -130,26 +130,30 @@ 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
}

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
}
Expand Down
34 changes: 27 additions & 7 deletions pkg/controller/importconfig/cluster_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ func TestValidateKubeAPIServerAddress(t *testing.T) {
name string
kubeAPIServer string
infraKubeAPIServer string
klusterletConfig *klusterletconfigv1alpha1.KlusterletConfig
valid bool
}{
{
Expand All @@ -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",
Expand All @@ -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)
}
Expand All @@ -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",
Expand All @@ -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"),
Expand Down Expand Up @@ -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)
}
Expand Down
51 changes: 49 additions & 2 deletions test/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,19 @@ 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"
"k8s.io/apimachinery/pkg/api/equality"
"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"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 7dd6c13

Please sign in to comment.