diff --git a/manifests/klusterlet/management/klusterlet-agent-deployment.yaml b/manifests/klusterlet/management/klusterlet-agent-deployment.yaml index 0770dec0b..e64ec3371 100644 --- a/manifests/klusterlet/management/klusterlet-agent-deployment.yaml +++ b/manifests/klusterlet/management/klusterlet-agent-deployment.yaml @@ -125,6 +125,12 @@ spec: {{if .ReservedClusterClaimSuffixes}} - "--reserved-cluster-claim-suffixes={{ .ReservedClusterClaimSuffixes }}" {{end}} + {{if .AddOnKubeClientRegistrationAuth}} + - "--addon-kubeclient-registration-auth={{ .AddOnKubeClientRegistrationAuth }}" + {{end}} + {{if gt .AddOnTokenExpirationSeconds 0}} + - "--addon-token-expiration-seconds={{ .AddOnTokenExpirationSeconds }}" + {{end}} {{if .AppliedManifestWorkEvictionGracePeriod}} - "--appliedmanifestwork-eviction-grace-period={{ .AppliedManifestWorkEvictionGracePeriod }}" {{end}} diff --git a/manifests/klusterlet/management/klusterlet-registration-deployment.yaml b/manifests/klusterlet/management/klusterlet-registration-deployment.yaml index 0f14fb7a9..d9342c281 100644 --- a/manifests/klusterlet/management/klusterlet-registration-deployment.yaml +++ b/manifests/klusterlet/management/klusterlet-registration-deployment.yaml @@ -100,6 +100,12 @@ spec: {{if .ReservedClusterClaimSuffixes}} - "--reserved-cluster-claim-suffixes={{ .ReservedClusterClaimSuffixes }}" {{end}} + {{if .AddOnKubeClientRegistrationAuth}} + - "--addon-kubeclient-registration-auth={{ .AddOnKubeClientRegistrationAuth }}" + {{end}} + {{if gt .AddOnTokenExpirationSeconds 0}} + - "--addon-token-expiration-seconds={{ .AddOnTokenExpirationSeconds }}" + {{end}} {{if eq .RegistrationDriver.AuthType "awsirsa"}} - "--registration-auth={{ .RegistrationDriver.AuthType }}" - "--hub-cluster-arn={{ .RegistrationDriver.AwsIrsa.HubClusterArn }}" diff --git a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index 88e1a6bd6..8f36141e7 100644 --- a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go +++ b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -206,6 +206,11 @@ type klusterletConfig struct { Labels map[string]string RegistrationDriver RegistrationDriver + // AddOnKubeClientRegistrationAuth is the authentication type for add-on registration (csr or token) + AddOnKubeClientRegistrationAuth string + // AddOnTokenExpirationSeconds is the expiration seconds for add-on tokens + AddOnTokenExpirationSeconds int64 + ManagedClusterArn string ManagedClusterRoleArn string ManagedClusterRoleSuffix string @@ -390,6 +395,19 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto annotationsArray = append(annotationsArray, fmt.Sprintf("%s=%s", k, v)) } config.ClusterAnnotationsString = strings.Join(annotationsArray, ",") + + // Set AddOnKubeClientRegistrationAuth from the Klusterlet spec + if klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver != nil && + klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver.AuthType != "" { + config.AddOnKubeClientRegistrationAuth = klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver.AuthType + } + + // Set AddOnTokenExpirationSeconds from the Klusterlet spec + if klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver != nil && + klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver.Token != nil && + klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver.Token.ExpirationSeconds > 0 { + config.AddOnTokenExpirationSeconds = klusterlet.Spec.RegistrationConfiguration.AddOnKubeClientRegistrationDriver.Token.ExpirationSeconds + } } config.AboutAPIEnabled = helpers.FeatureGateEnabled( diff --git a/pkg/registration/register/aws_irsa/aws_irsa.go b/pkg/registration/register/aws_irsa/aws_irsa.go index 12c365757..7de976ae9 100644 --- a/pkg/registration/register/aws_irsa/aws_irsa.go +++ b/pkg/registration/register/aws_irsa/aws_irsa.go @@ -3,11 +3,15 @@ package aws_irsa import ( "context" "fmt" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/klog/v2" clusterv1 "open-cluster-management.io/api/cluster/v1" operatorv1 "open-cluster-management.io/api/operator/v1" @@ -16,6 +20,8 @@ import ( "open-cluster-management.io/ocm/pkg/common/helpers" "open-cluster-management.io/ocm/pkg/registration/register" + "open-cluster-management.io/ocm/pkg/registration/register/csr" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) //TODO: Remove these constants in once we have the function fully implemented for the AWSIRSADriver @@ -36,6 +42,15 @@ type AWSIRSADriver struct { managedClusterRoleSuffix string awsIRSAControl AWSIRSAControl + + // addonClients holds the addon clients and informers + addonClients *register.AddOnClients + + // tokenControl is used for token-based addon authentication + tokenControl token.TokenControl + + // csrControl is used for CSR-based addon authentication + csrControl csr.CSRControl } func (c *AWSIRSADriver) Process( @@ -98,7 +113,7 @@ func (c *AWSIRSADriver) ManagedClusterDecorator(cluster *clusterv1.ManagedCluste return cluster } -func (c *AWSIRSADriver) BuildClients(_ context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { +func (c *AWSIRSADriver) BuildClients(ctx context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { clients, err := register.BuildClientsFromSecretOption(secretOption, bootstrap) if err != nil { return nil, err @@ -107,9 +122,68 @@ func (c *AWSIRSADriver) BuildClients(_ context.Context, secretOption register.Se if err != nil { return nil, fmt.Errorf("failed to create AWS IRSA control: %w", err) } + + // Store addon clients and initialize controls for addon authentication after bootstrap + if !bootstrap { + c.addonClients = ®ister.AddOnClients{ + AddonClient: clients.AddonClient, + AddonInformer: clients.AddonInformer, + } + + kubeConfig, err := register.KubeConfigFromSecretOption(secretOption, bootstrap) + if err != nil { + return nil, err + } + kubeClient, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + return nil, err + } + c.tokenControl = token.NewTokenControl(kubeClient.CoreV1()) + + // Initialize CSR control for CSR-based addon authentication + logger := klog.FromContext(ctx) + kubeInformerFactory := informers.NewSharedInformerFactoryWithOptions( + kubeClient, + 10*time.Minute, + informers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { + listOptions.LabelSelector = fmt.Sprintf("%s=%s", clusterv1.ClusterNameLabelKey, secretOption.ClusterName) + }), + ) + csrControl, err := csr.NewCSRControl(logger, kubeInformerFactory.Certificates(), kubeClient) + if err != nil { + return nil, fmt.Errorf("failed to create CSR control: %w", err) + } + c.csrControl = csrControl + } + return clients, nil } +func (c *AWSIRSADriver) Fork(addonName string, authConfig register.AddonAuthConfig, secretOption register.SecretOption) (register.RegisterDriver, error) { + // Check if token-based authentication should be used (shared helper) + tokenDriver, err := token.TryForkTokenDriver(addonName, authConfig, secretOption, c.tokenControl, c.addonClients) + if err != nil { + return nil, err + } + if tokenDriver != nil { + return tokenDriver, nil + } + + // For CSR driver, create a CSR-based driver for addon authentication + // This handles: + // - CustomSigner type (secretOption.Signer != KubeAPIServerClientSignerName) + // - KubeClient type with CSR authentication + + // Get CSR configuration from AddonAuthConfig (type-safe interface) + csrConfig := authConfig.GetCSRConfiguration() + if csrConfig == nil { + return nil, fmt.Errorf("CSR configuration is nil for addon %s", addonName) + } + + // Note: tokenControl is not set for addon CSR drivers since they use CSR-based auth + return csr.NewCSRDriverForAddOn(addonName, csrConfig, secretOption, c.csrControl), nil +} + func NewAWSIRSADriver(opt *AWSOption, secretOption register.SecretOption) register.RegisterDriver { return &AWSIRSADriver{ managedClusterArn: opt.ManagedClusterArn, @@ -118,3 +192,6 @@ func NewAWSIRSADriver(opt *AWSOption, secretOption register.SecretOption) regist name: secretOption.ClusterName, } } + +var _ register.RegisterDriver = &AWSIRSADriver{} +var _ register.AddonDriverFactory = &AWSIRSADriver{} diff --git a/pkg/registration/register/aws_irsa/aws_irsa_test.go b/pkg/registration/register/aws_irsa/aws_irsa_test.go index d142f468e..5077daf02 100644 --- a/pkg/registration/register/aws_irsa/aws_irsa_test.go +++ b/pkg/registration/register/aws_irsa/aws_irsa_test.go @@ -4,14 +4,25 @@ import ( "context" "os" "path" + "strings" "testing" "time" + certificatesv1 "k8s.io/api/certificates/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + addonfake "open-cluster-management.io/api/client/addon/clientset/versioned/fake" + addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" + "open-cluster-management.io/sdk-go/pkg/basecontroller/events" + testinghelpers "open-cluster-management.io/ocm/pkg/registration/helpers/testing" "open-cluster-management.io/ocm/pkg/registration/register" + "open-cluster-management.io/ocm/pkg/registration/register/csr" + registertesting "open-cluster-management.io/ocm/pkg/registration/register/testing" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) var _ AWSIRSAControl = &mockAWSIRSAControl{} @@ -202,3 +213,266 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { }) } } + +func TestAWSIRSADriver_Fork_TokenAuth(t *testing.T) { + // Setup addon client and informer + addon := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "addon1", + Namespace: "cluster1", + }, + } + addonClient := addonfake.NewSimpleClientset(addon) + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 10*time.Minute) + addonInformer := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns() + + addonClients := ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformer, + } + + // Setup mock AWS IRSA control + mockAWSIRSACtrl := &mockAWSIRSAControl{} + + tests := []struct { + name string + setupDriver func() *AWSIRSADriver + addonName string + secretOption register.SecretOption + regOption register.AddonAuthConfig + expectErr bool + expectErrMsg string + validateResult func(t *testing.T, driver register.RegisterDriver) + }{ + { + name: "token auth - success", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*token.TokenDriver); !ok { + t.Errorf("expected TokenDriver, got %T", driver) + } + }, + }, + { + name: "token auth - invalid token option", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: nil, + }, + expectErr: true, + expectErrMsg: "token authentication requested but TokenConfiguration is nil", + }, + { + name: "token auth - tokenControl not initialized", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: nil, + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "tokenControl is not initialized", + }, + { + name: "token auth - addonClients not initialized", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: nil, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "addonClients is not initialized", + }, + { + name: "csr auth with KubeAPIServerClientSigner - success", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + csrControl: newMockCSRControl(), + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: csr.NewCSROption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*csr.CSRDriver); !ok { + t.Errorf("expected CSRDriver, got %T", driver) + } + }, + }, + { + name: "csr auth with custom signer - success", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + csrControl: newMockCSRControl(), + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: "custom.signer.io/custom", + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: csr.NewCSROption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*csr.CSRDriver); !ok { + t.Errorf("expected CSRDriver, got %T", driver) + } + }, + }, + { + name: "csr auth - invalid CSR option", + setupDriver: func() *AWSIRSADriver { + driver := &AWSIRSADriver{ + tokenControl: ®istertesting.MockTokenControl{}, + csrControl: newMockCSRControl(), + addonClients: addonClients, + awsIRSAControl: mockAWSIRSACtrl, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificatesv1.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: nil, + }, + expectErr: true, + expectErrMsg: "CSR configuration is nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + driver := tt.setupDriver() + + forkedDriver, err := driver.Fork(tt.addonName, tt.regOption, tt.secretOption) + + if tt.expectErr { + if err == nil { + t.Errorf("expected error but got nil") + return + } + if tt.expectErrMsg != "" && !strings.Contains(err.Error(), tt.expectErrMsg) { + t.Errorf("expected error message to contain %q, got %q", tt.expectErrMsg, err.Error()) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + if tt.validateResult != nil { + tt.validateResult(t, forkedDriver) + } + }) + } +} + +// mockCSRControl is a simple mock for testing CSR-based registration +type mockCSRControl struct { + informer cache.SharedIndexInformer +} + +func (m *mockCSRControl) Create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, signerName string, expirationSeconds *int32) (string, error) { + return "mock-csr", nil +} + +func (m *mockCSRControl) IsApproved(name string) (bool, error) { + return true, nil +} + +func (m *mockCSRControl) GetIssuedCertificate(name string) ([]byte, error) { + return []byte("mock-cert"), nil +} + +func (m *mockCSRControl) Informer() cache.SharedIndexInformer { + return m.informer +} + +func newMockCSRControl() *mockCSRControl { + // Create a fake client and informer + return &mockCSRControl{ + informer: cache.NewSharedIndexInformer( + nil, + nil, + 0, + cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, + ), + } +} diff --git a/pkg/registration/register/common.go b/pkg/registration/register/common.go index 9f07f1a7f..8e0715b01 100644 --- a/pkg/registration/register/common.go +++ b/pkg/registration/register/common.go @@ -254,6 +254,12 @@ type Clients struct { AddonInformer addonv1alpha1informers.ManagedClusterAddOnInformer } +// AddOnClients hold clients and informers used by addon driver +type AddOnClients struct { + AddonClient addonclient.Interface + AddonInformer addonv1alpha1informers.ManagedClusterAddOnInformer +} + func KubeConfigFromSecretOption(s SecretOption, bootstrap bool) (*rest.Config, error) { var kubeConfig *rest.Config var err error diff --git a/pkg/registration/register/csr/csr.go b/pkg/registration/register/csr/csr.go index ee0b5151a..d86497994 100644 --- a/pkg/registration/register/csr/csr.go +++ b/pkg/registration/register/csr/csr.go @@ -33,6 +33,7 @@ import ( "open-cluster-management.io/ocm/pkg/registration/hub/user" "open-cluster-management.io/ocm/pkg/registration/register" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) const ( @@ -67,10 +68,16 @@ type CSRDriver struct { csrControl CSRControl + // addonClients holds the addon clients and informers (for addon driver only) + addonClients *register.AddOnClients + + // tokenControl is used for token-based addon authentication + tokenControl token.TokenControl + // HaltCSRCreation halt the csr creation haltCSRCreation func() bool - opt *Option + opt register.CSRConfiguration csrOption *CSROption } @@ -213,7 +220,7 @@ func (c *CSRDriver) Process( } // do not set expiration second if it is 0 - expirationSeconds := pointer.Int32(c.opt.ExpirationSeconds) + expirationSeconds := pointer.Int32(c.opt.GetExpirationSeconds()) if *expirationSeconds == 0 { expirationSeconds = nil } @@ -295,30 +302,28 @@ func (c *CSRDriver) ManagedClusterDecorator(cluster *clusterv1.ManagedCluster) * return cluster } -func (c *CSRDriver) Fork(addonName string, secretOption register.SecretOption) register.RegisterDriver { - csrOption := &CSROption{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("addon-%s-%s-", secretOption.ClusterName, addonName), - Labels: map[string]string{ - // the labels are only hints. Anyone could set/modify them. - clusterv1.ClusterNameLabelKey: secretOption.ClusterName, - addonv1alpha1.AddonLabelKey: addonName, - }, - }, - Subject: secretOption.Subject, - DNSNames: []string{fmt.Sprintf("%s.addon.open-cluster-management.io", addonName)}, - SignerName: secretOption.Signer, - EventFilterFunc: createCSREventFilterFunc(secretOption.ClusterName, addonName, secretOption.Signer), +func (c *CSRDriver) Fork(addonName string, authConfig register.AddonAuthConfig, secretOption register.SecretOption) (register.RegisterDriver, error) { + // Check if token-based authentication should be used (shared helper) + tokenDriver, err := token.TryForkTokenDriver(addonName, authConfig, secretOption, c.tokenControl, c.addonClients) + if err != nil { + return nil, err + } + if tokenDriver != nil { + return tokenDriver, nil } - driver := &CSRDriver{ - csrOption: csrOption, - opt: c.opt, - csrControl: c.csrControl, - haltCSRCreation: haltAddonCSRCreationFunc(c.csrControl.Informer().GetIndexer(), secretOption.ClusterName, addonName), + // For CSR driver, return a CSR-based driver + // This handles: + // - CustomSigner type (secretOption.Signer != KubeAPIServerClientSignerName) + // - KubeClient type with CSR authentication + + // Get CSR configuration from AddonAuthConfig (type-safe interface) + csrConfig := authConfig.GetCSRConfiguration() + if csrConfig == nil { + return nil, fmt.Errorf("CSR configuration is nil for addon %s", addonName) } - return driver + return NewCSRDriverForAddOn(addonName, csrConfig, secretOption, c.csrControl), nil } func (c *CSRDriver) BuildClients(ctx context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { @@ -353,6 +358,16 @@ func (c *CSRDriver) BuildClients(ctx context.Context, secretOption register.Secr if err != nil { return nil, err } + + // Initialize addon clients and token control for addon mode after bootstrap + if !bootstrap { + c.addonClients = ®ister.AddOnClients{ + AddonClient: clients.AddonClient, + AddonInformer: clients.AddonInformer, + } + c.tokenControl = token.NewTokenControl(kubeClient.CoreV1()) + } + return clients, nil } @@ -370,10 +385,45 @@ func (c *CSRDriver) SetCSRControl(csrControl CSRControl, clusterName string) err return nil } -var _ register.RegisterDriver = &CSRDriver{} -var _ register.AddonDriver = &CSRDriver{} +// SetAddonClients sets the addon clients for the CSR driver +func (c *CSRDriver) SetAddonClients(addonClients *register.AddOnClients) { + c.addonClients = addonClients +} -func NewCSRDriver(opt *Option, secretOpts register.SecretOption) (*CSRDriver, error) { +// SetTokenControl sets the token control for the CSR driver +func (c *CSRDriver) SetTokenControl(tokenControl token.TokenControl) { + c.tokenControl = tokenControl +} + +var _ register.RegisterDriver = &CSRDriver{} +var _ register.AddonDriverFactory = &CSRDriver{} + +// NewCSRDriverForAddOn creates a CSRDriver for addon registration with the given parameters +func NewCSRDriverForAddOn(addonName string, csrConfig register.CSRConfiguration, secretOption register.SecretOption, csrControl CSRControl) *CSRDriver { + csrOption := &CSROption{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("addon-%s-%s-", secretOption.ClusterName, addonName), + Labels: map[string]string{ + // the labels are only hints. Anyone could set/modify them. + clusterv1.ClusterNameLabelKey: secretOption.ClusterName, + addonv1alpha1.AddonLabelKey: addonName, + }, + }, + Subject: secretOption.Subject, + DNSNames: []string{fmt.Sprintf("%s.addon.open-cluster-management.io", addonName)}, + SignerName: secretOption.Signer, + EventFilterFunc: createCSREventFilterFunc(secretOption.ClusterName, addonName, secretOption.Signer), + } + + return &CSRDriver{ + csrOption: csrOption, + opt: csrConfig, + csrControl: csrControl, + haltCSRCreation: haltAddonCSRCreationFunc(csrControl.Informer().GetIndexer(), secretOption.ClusterName, addonName), + } +} + +func NewCSRDriver(csrConfig register.CSRConfiguration, secretOpts register.SecretOption) (*CSRDriver, error) { signer := certificates.KubeAPIServerClientSignerName if secretOpts.Signer != "" { signer = secretOpts.Signer @@ -385,7 +435,7 @@ func NewCSRDriver(opt *Option, secretOpts register.SecretOption) (*CSRDriver, er } driver := &CSRDriver{ - opt: opt, + opt: csrConfig, } driver.csrOption = &CSROption{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/registration/register/csr/csr_test.go b/pkg/registration/register/csr/csr_test.go index 9c10445d2..ca037f349 100644 --- a/pkg/registration/register/csr/csr_test.go +++ b/pkg/registration/register/csr/csr_test.go @@ -7,6 +7,7 @@ import ( "os" "path" "reflect" + "strings" "testing" "time" @@ -23,6 +24,8 @@ import ( "k8s.io/klog/v2/ktesting" addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + addonfake "open-cluster-management.io/api/client/addon/clientset/versioned/fake" + addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" clusterv1 "open-cluster-management.io/api/cluster/v1" ocmfeature "open-cluster-management.io/api/feature" "open-cluster-management.io/sdk-go/pkg/basecontroller/events" @@ -32,6 +35,8 @@ import ( testinghelpers "open-cluster-management.io/ocm/pkg/registration/helpers/testing" "open-cluster-management.io/ocm/pkg/registration/hub/user" "open-cluster-management.io/ocm/pkg/registration/register" + registertesting "open-cluster-management.io/ocm/pkg/registration/register/testing" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) const ( @@ -623,8 +628,15 @@ func TestNewCSRDriver(t *testing.T) { CommonName: "addonagent1", }, } - addonDriver := driver.Fork("addon1", addonSecretOptions) - csrAddonDriver := addonDriver.(*CSRDriver) + regOption := ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: NewCSROption(), + } + forkedDriver, err := driver.Fork("addon1", regOption, addonSecretOptions) + if err != nil { + t.Fatal(err) + } + csrAddonDriver := forkedDriver.(*CSRDriver) if csrAddonDriver.csrOption.Subject.CommonName != "addonagent1" { t.Errorf("common name is not set correctly") } @@ -784,3 +796,244 @@ func TestBuildClient(t *testing.T) { }) } } + +func TestCSRDriver_Fork_TokenAuth(t *testing.T) { + // Setup addon client and informer + addon := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "addon1", + Namespace: "cluster1", + }, + } + addonClient := addonfake.NewSimpleClientset(addon) + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 10*time.Minute) + addonInformer := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns() + + ctrl := &mockCSRControl{} + hubKubeClient := kubefake.NewClientset() + ctrl.csrClient = &hubKubeClient.Fake + + addonClients := ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformer, + } + + tests := []struct { + name string + setupDriver func() *CSRDriver + addonName string + secretOption register.SecretOption + regOption register.AddonAuthConfig + expectErr bool + expectErrMsg string + validateResult func(t *testing.T, driver register.RegisterDriver) + }{ + { + name: "token auth - success", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*token.TokenDriver); !ok { + t.Errorf("expected TokenDriver, got %T", driver) + } + }, + }, + { + name: "token auth - invalid token option", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: nil, + }, + expectErr: true, + expectErrMsg: "token authentication requested but TokenConfiguration is nil", + }, + { + name: "token auth - tokenControl not initialized", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: nil, + addonClients: addonClients, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "tokenControl is not initialized", + }, + { + name: "token auth - addonClients not initialized", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: nil, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "addonClients is not initialized", + }, + { + name: "csr auth with custom signer - success", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: "custom.signer.io/custom", + Subject: &pkix.Name{ + CommonName: "custom-addon", + }, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: NewCSROption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*CSRDriver); !ok { + t.Errorf("expected CSRDriver, got %T", driver) + } + }, + }, + { + name: "csr auth - invalid CSR option", + setupDriver: func() *CSRDriver { + driver := &CSRDriver{ + csrControl: ctrl, + tokenControl: ®istertesting.MockTokenControl{}, + addonClients: addonClients, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + CSROption: nil, + }, + expectErr: true, + expectErrMsg: "CSR configuration is nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + driver := tt.setupDriver() + + forkedDriver, err := driver.Fork(tt.addonName, tt.regOption, tt.secretOption) + + if tt.expectErr { + if err == nil { + t.Errorf("expected error but got nil") + return + } + if tt.expectErrMsg != "" && !strings.Contains(err.Error(), tt.expectErrMsg) { + t.Errorf("expected error message to contain %q, got %q", tt.expectErrMsg, err.Error()) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + if tt.validateResult != nil { + tt.validateResult(t, forkedDriver) + } + }) + } +} + +func TestCSRDriver_SetTokenControl(t *testing.T) { + driver := &CSRDriver{} + mockControl := ®istertesting.MockTokenControl{} + + driver.SetTokenControl(mockControl) + + if driver.tokenControl != mockControl { + t.Error("SetTokenControl did not set tokenControl correctly") + } +} + +func TestCSRDriver_SetAddonClients(t *testing.T) { + driver := &CSRDriver{} + + addon := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "addon1", + Namespace: "cluster1", + }, + } + addonClient := addonfake.NewSimpleClientset(addon) + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 10*time.Minute) + addonInformer := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns() + + addonClients := ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformer, + } + + driver.SetAddonClients(addonClients) + + if driver.addonClients != addonClients { + t.Error("SetAddonClients did not set addonClients correctly") + } +} diff --git a/pkg/registration/register/csr/options.go b/pkg/registration/register/csr/options.go index acfc2732d..f6743450d 100644 --- a/pkg/registration/register/csr/options.go +++ b/pkg/registration/register/csr/options.go @@ -10,6 +10,8 @@ import ( "k8s.io/client-go/tools/cache" "open-cluster-management.io/sdk-go/pkg/basecontroller/factory" + + "open-cluster-management.io/ocm/pkg/registration/register" ) // CSROption includes options that is used to create and monitor csrs @@ -43,6 +45,9 @@ type Option struct { ExpirationSeconds int32 } +// Ensure Option implements register.CSRConfiguration interface at compile time +var _ register.CSRConfiguration = &Option{} + func NewCSROption() *Option { return &Option{} } @@ -60,6 +65,10 @@ func (o *Option) Validate() error { return nil } +func (o *Option) GetExpirationSeconds() int32 { + return o.ExpirationSeconds +} + func haltCSRCreationFunc(indexer cache.Indexer, clusterName string) func() bool { return func() bool { items, err := indexer.ByIndex(indexByCluster, clusterName) diff --git a/pkg/registration/register/factory/options.go b/pkg/registration/register/factory/options.go index 167af4051..3ef7c0f09 100644 --- a/pkg/registration/register/factory/options.go +++ b/pkg/registration/register/factory/options.go @@ -1,6 +1,8 @@ package factory import ( + "fmt" + "github.com/spf13/pflag" operatorv1 "open-cluster-management.io/api/operator/v1" @@ -9,6 +11,7 @@ import ( awsirsa "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" "open-cluster-management.io/ocm/pkg/registration/register/csr" "open-cluster-management.io/ocm/pkg/registration/register/grpc" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) type Options struct { @@ -16,25 +19,42 @@ type Options struct { CSROption *csr.Option AWSIRSAOption *awsirsa.AWSOption GRPCOption *grpc.Option + TokenOption *token.Option + + // AddonKubeClientRegistrationAuth specifies the authentication method for addons + // with registration type KubeClient. Possible values are "csr" (default) and "token". + AddonKubeClientRegistrationAuth string } func NewOptions() *Options { return &Options{ - CSROption: csr.NewCSROption(), - AWSIRSAOption: awsirsa.NewAWSOption(), - GRPCOption: grpc.NewOptions(), + CSROption: csr.NewCSROption(), + AWSIRSAOption: awsirsa.NewAWSOption(), + GRPCOption: grpc.NewOptions(), + TokenOption: token.NewTokenOption(), + AddonKubeClientRegistrationAuth: "csr", // default to csr } } func (s *Options) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.RegistrationAuth, "registration-auth", s.RegistrationAuth, "The type of authentication to use to authenticate with hub.") + fs.StringVar(&s.AddonKubeClientRegistrationAuth, "addon-kubeclient-registration-auth", s.AddonKubeClientRegistrationAuth, + "The authentication method for addons with registration type KubeClient. Possible values are 'csr' (default) and 'token'.") s.CSROption.AddFlags(fs) s.AWSIRSAOption.AddFlags(fs) s.GRPCOption.AddFlags(fs) + s.TokenOption.AddFlags(fs) } func (s *Options) Validate() error { + switch s.AddonKubeClientRegistrationAuth { + case "", "csr", "token": + // valid values + default: + return fmt.Errorf("unsupported addon-kubeclient-registration-auth: %s", s.AddonKubeClientRegistrationAuth) + } + switch s.RegistrationAuth { case operatorv1.AwsIrsaAuthType: return s.AWSIRSAOption.Validate() @@ -45,6 +65,18 @@ func (s *Options) Validate() error { } } +func (s *Options) GetKubeClientAuth() string { + return s.AddonKubeClientRegistrationAuth +} + +func (s *Options) GetCSRConfiguration() register.CSRConfiguration { + return s.CSROption +} + +func (s *Options) GetTokenConfiguration() register.TokenConfiguration { + return s.TokenOption +} + func (s *Options) Driver(secretOption register.SecretOption) (register.RegisterDriver, error) { switch s.RegistrationAuth { case operatorv1.AwsIrsaAuthType: diff --git a/pkg/registration/register/grpc/spoke_driver.go b/pkg/registration/register/grpc/spoke_driver.go index d9b204b31..6e4d4ee9d 100644 --- a/pkg/registration/register/grpc/spoke_driver.go +++ b/pkg/registration/register/grpc/spoke_driver.go @@ -8,6 +8,7 @@ import ( "time" "gopkg.in/yaml.v2" + authenticationv1 "k8s.io/api/authentication/v1" certificatesv1 "k8s.io/api/certificates/v1" coordv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" @@ -30,6 +31,7 @@ import ( cloudeventsevent "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/event" cloudeventslease "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/lease" cloudeventsoptions "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/options" + cloudeventsserviceaccount "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/serviceaccount" cloudeventsstore "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/store" "open-cluster-management.io/sdk-go/pkg/cloudevents/constants" "open-cluster-management.io/sdk-go/pkg/cloudevents/generic/options/builder" @@ -38,6 +40,7 @@ import ( "open-cluster-management.io/ocm/pkg/registration/register" "open-cluster-management.io/ocm/pkg/registration/register/csr" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) type GRPCDriver struct { @@ -45,10 +48,12 @@ type GRPCDriver struct { control *ceCSRControl opt *Option configTemplate []byte + addonClients *register.AddOnClients + tokenControl token.TokenControl } var _ register.RegisterDriver = &GRPCDriver{} -var _ register.AddonDriver = &GRPCDriver{} +var _ register.AddonDriverFactory = &GRPCDriver{} func NewGRPCDriver(opt *Option, csrOption *csr.Option, secretOption register.SecretOption) (register.RegisterDriver, error) { secretOption.Signer = operatorv1.GRPCAuthSigner @@ -161,16 +166,34 @@ func (d *GRPCDriver) BuildClients(ctx context.Context, secretOption register.Sec LeaseClient: leaseClient, EventsClient: eventClient, } + + // Initialize addon clients for addon mode + d.addonClients = ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformer, + } + + // Initialize gRPC token control for token-based addon authentication + grpcOptions, ok := config.(*grpc.GRPCOptions) + if !ok { + return nil, fmt.Errorf("invalid gRPC config type") + } + saClient := cloudeventsserviceaccount.NewServiceAccountClient(secretOption.ClusterName, grpcOptions) + d.tokenControl = &grpcTokenControl{ + saClient: saClient, + } + + // Set addonClients and tokenControl on the embedded csrDriver for forked driver instances + d.csrDriver.SetAddonClients(d.addonClients) + d.csrDriver.SetTokenControl(d.tokenControl) + return clients, nil } -func (d *GRPCDriver) Fork(addonName string, secretOption register.SecretOption) register.RegisterDriver { - csrDriver := d.csrDriver.Fork(addonName, secretOption) - return &GRPCDriver{ - control: d.control, - opt: d.opt, - csrDriver: csrDriver.(*csr.CSRDriver), - } +func (d *GRPCDriver) Fork(addonName string, authConfig register.AddonAuthConfig, secretOption register.SecretOption) (register.RegisterDriver, error) { + // Delegate to csrDriver.Fork which handles both token and CSR authentication + // Return the driver directly (either TokenDriver or CSRDriver) without wrapping + return d.csrDriver.Fork(addonName, authConfig, secretOption) } func (d *GRPCDriver) Process( @@ -282,6 +305,32 @@ type ceCSRControl struct { var _ csr.CSRControl = &ceCSRControl{} +type grpcTokenControl struct { + saClient *cloudeventsserviceaccount.ServiceAccountClient +} + +var _ token.TokenControl = &grpcTokenControl{} + +// CreateToken creates a ServiceAccount token using cloud events +func (g *grpcTokenControl) CreateToken(ctx context.Context, serviceAccountName, namespace string, expirationSeconds int64) (string, error) { + if g.saClient == nil { + return "", fmt.Errorf("ServiceAccount client is not initialized") + } + + tokenRequest := &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{ + ExpirationSeconds: &expirationSeconds, + }, + } + + result, err := g.saClient.CreateToken(ctx, serviceAccountName, tokenRequest, metav1.CreateOptions{}) + if err != nil { + return "", fmt.Errorf("failed to create token for ServiceAccount %s/%s: %w", namespace, serviceAccountName, err) + } + + return result.Status.Token, nil +} + func (c *ceCSRControl) IsApproved(name string) (bool, error) { csr, err := c.csrClientHolder.Clients().Get(context.Background(), name, metav1.GetOptions{}) if err != nil { diff --git a/pkg/registration/register/grpc/spoke_driver_test.go b/pkg/registration/register/grpc/spoke_driver_test.go index 0c2ccbc76..3beb6cb52 100644 --- a/pkg/registration/register/grpc/spoke_driver_test.go +++ b/pkg/registration/register/grpc/spoke_driver_test.go @@ -4,14 +4,23 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "time" + certificates "k8s.io/api/certificates/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + addonfake "open-cluster-management.io/api/client/addon/clientset/versioned/fake" + addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" "open-cluster-management.io/sdk-go/pkg/cloudevents/generic/options/grpc" testinghelpers "open-cluster-management.io/ocm/pkg/registration/helpers/testing" "open-cluster-management.io/ocm/pkg/registration/register" "open-cluster-management.io/ocm/pkg/registration/register/csr" + registertesting "open-cluster-management.io/ocm/pkg/registration/register/testing" + "open-cluster-management.io/ocm/pkg/registration/register/token" ) func TestIsHubKubeConfigValid(t *testing.T) { @@ -164,3 +173,267 @@ func TestLoadConfig(t *testing.T) { }) } } + +func TestGRPCDriver_Fork_TokenAuth(t *testing.T) { + // Setup addon client and informer + addon := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "addon1", + Namespace: "cluster1", + }, + } + addonClient := addonfake.NewSimpleClientset(addon) + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 10*time.Minute) + addonInformer := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns() + + addonClients := ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformer, + } + + tests := []struct { + name string + setupDriver func() *GRPCDriver + addonName string + secretOption register.SecretOption + regOption register.AddonAuthConfig + expectErr bool + expectErrMsg string + validateResult func(t *testing.T, driver register.RegisterDriver) + }{ + { + name: "token auth - success", + setupDriver: func() *GRPCDriver { + tokenControl := &grpcTokenControl{} + csrDriver := &csr.CSRDriver{} + csrDriver.SetTokenControl(tokenControl) + csrDriver.SetAddonClients(addonClients) + driver := &GRPCDriver{ + tokenControl: tokenControl, + addonClients: addonClients, + csrDriver: csrDriver, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: false, + validateResult: func(t *testing.T, driver register.RegisterDriver) { + if _, ok := driver.(*token.TokenDriver); !ok { + t.Errorf("expected TokenDriver, got %T", driver) + } + }, + }, + { + name: "token auth - invalid token option", + setupDriver: func() *GRPCDriver { + tokenControl := &grpcTokenControl{} + csrDriver := &csr.CSRDriver{} + csrDriver.SetTokenControl(tokenControl) + csrDriver.SetAddonClients(addonClients) + driver := &GRPCDriver{ + tokenControl: tokenControl, + addonClients: addonClients, + csrDriver: csrDriver, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: nil, + }, + expectErr: true, + expectErrMsg: "token authentication requested but TokenConfiguration is nil", + }, + { + name: "token auth - tokenControl not initialized", + setupDriver: func() *GRPCDriver { + csrDriver := &csr.CSRDriver{} + csrDriver.SetTokenControl(nil) + csrDriver.SetAddonClients(addonClients) + driver := &GRPCDriver{ + tokenControl: nil, + addonClients: addonClients, + csrDriver: csrDriver, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "tokenControl is not initialized", + }, + { + name: "token auth - addonClients not initialized", + setupDriver: func() *GRPCDriver { + tokenControl := &grpcTokenControl{} + csrDriver := &csr.CSRDriver{} + csrDriver.SetTokenControl(tokenControl) + csrDriver.SetAddonClients(nil) + driver := &GRPCDriver{ + tokenControl: tokenControl, + addonClients: nil, + csrDriver: csrDriver, + } + return driver + }, + addonName: "addon1", + secretOption: register.SecretOption{ + ClusterName: "cluster1", + Signer: certificates.KubeAPIServerClientSignerName, + }, + regOption: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "token", + TokenOption: token.NewTokenOption(), + }, + expectErr: true, + expectErrMsg: "addonClients is not initialized", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + driver := tt.setupDriver() + + forkedDriver, err := driver.Fork(tt.addonName, tt.regOption, tt.secretOption) + + if tt.expectErr { + if err == nil { + t.Errorf("expected error but got nil") + return + } + if tt.expectErrMsg != "" && !strings.Contains(err.Error(), tt.expectErrMsg) { + t.Errorf("expected error message to contain %q, got %q", tt.expectErrMsg, err.Error()) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + if tt.validateResult != nil { + tt.validateResult(t, forkedDriver) + } + }) + } +} + +func TestGRPCDriver_BuildClients_InitializesTokenControl(t *testing.T) { + // This test verifies that BuildClients properly initializes tokenControl + // We can't fully test BuildClients without a real gRPC server, so we just + // verify the structure is correct + tempDir, err := os.MkdirTemp("", "grpc-test-build-clients") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + configFile := filepath.Join(tempDir, "config.yaml") + if err := os.WriteFile(configFile, []byte(`url: "https://localhost:8443"`), 0600); err != nil { + t.Fatal(err) + } + + kubeconfig := testinghelpers.NewKubeconfig("cluster1", "https://127.0.0.1:6001", "", "", nil, nil, nil) + kubeconfigFile := filepath.Join(tempDir, "kubeconfig") + if err := os.WriteFile(kubeconfigFile, kubeconfig, 0600); err != nil { + t.Fatal(err) + } + + secretOption := register.SecretOption{ + ClusterName: "cluster1", + AgentName: "agent1", + HubKubeconfigFile: kubeconfigFile, + BootStrapKubeConfigFile: kubeconfigFile, + } + + driverInterface, err := NewGRPCDriver(&Option{ + ConfigFile: configFile, + BootstrapConfigFile: configFile, + }, csr.NewCSROption(), secretOption) + if err != nil { + t.Fatalf("failed to create GRPC driver: %v", err) + } + + // Type assert to access internal fields for verification + driver, ok := driverInterface.(*GRPCDriver) + if !ok { + t.Fatalf("expected *GRPCDriver, got %T", driverInterface) + } + + // Verify driver was created with expected structure + if driver.csrDriver == nil { + t.Error("csrDriver should be initialized") + } + if driver.opt == nil { + t.Error("opt should be initialized") + } +} + +func TestGRPCTokenControl_CreateToken(t *testing.T) { + tests := []struct { + name string + saClient interface{} + expectErr bool + expectErrContains string + }{ + { + name: "saClient is nil", + saClient: nil, + expectErr: true, + expectErrContains: "ServiceAccount client is not initialized", + }, + { + name: "saClient is set but we can't actually call it without a real server", + saClient: &struct{}{}, // Mock object, will fail when we try to use it + expectErr: false, // We just test that nil check passes + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := &grpcTokenControl{} + if tt.saClient != nil { + // We can't easily test the actual CreateToken call without a real gRPC server, + // so we just verify the nil check works + if ctrl.saClient == nil && tt.saClient == nil { + _, err := ctrl.CreateToken(context.Background(), "test-sa", "test-ns", 3600) + if !tt.expectErr { + t.Errorf("expected no error, got: %v", err) + } + if err != nil && !strings.Contains(err.Error(), tt.expectErrContains) { + t.Errorf("expected error to contain %q, got: %v", tt.expectErrContains, err) + } + } + } else { + _, err := ctrl.CreateToken(context.Background(), "test-sa", "test-ns", 3600) + if !tt.expectErr { + t.Errorf("expected no error, got: %v", err) + } + if err != nil && !strings.Contains(err.Error(), tt.expectErrContains) { + t.Errorf("expected error to contain %q, got: %v", tt.expectErrContains, err) + } + } + }) + } +} diff --git a/pkg/registration/register/interface.go b/pkg/registration/register/interface.go index 78e1ee567..9ba780d3d 100644 --- a/pkg/registration/register/interface.go +++ b/pkg/registration/register/interface.go @@ -24,6 +24,18 @@ const ( KubeconfigFile = "kubeconfig" ) +// CSRConfiguration provides configuration for CSR-based authentication. +type CSRConfiguration interface { + // GetExpirationSeconds returns the requested duration of validity of the issued certificate in seconds + GetExpirationSeconds() int32 +} + +// TokenConfiguration provides configuration for token-based authentication. +type TokenConfiguration interface { + // GetExpirationSeconds returns the requested duration of validity of the token in seconds + GetExpirationSeconds() int64 +} + type SecretOption struct { // SecretNamespace is the namespace of the secret containing client certificate. SecretNamespace string @@ -81,9 +93,30 @@ type RegisterDriver interface { BuildClients(ctx context.Context, secretOption SecretOption, bootstrap bool) (*Clients, error) } -// AddonDriver is an interface for the driver to fork a driver for addons registration -type AddonDriver interface { - Fork(addonName string, secretOption SecretOption) RegisterDriver +// AddonAuthConfig provides complete configuration for addon registration, +// including authentication method and access to driver options. +type AddonAuthConfig interface { + // GetKubeClientAuth returns the authentication method for addons with registration type KubeClient. + // Possible values are "csr" (default) and "token". + GetKubeClientAuth() string + + // GetCSRConfiguration returns the CSR driver configuration interface + GetCSRConfiguration() CSRConfiguration + + // GetTokenConfiguration returns the token driver configuration interface + GetTokenConfiguration() TokenConfiguration +} + +// AddonDriverFactory is an interface for creating RegisterDriver instances for addon registration. +// It acts as a factory that creates (forks) driver instances for specific addons. +type AddonDriverFactory interface { + // Fork creates a RegisterDriver instance for a specific addon. + // Parameters: + // - addonName: the name of the addon + // - authConfig: authentication configuration including type and authentication method + // - secretOption: configuration for the secret that will store credentials + // Returns the driver instance and an error if the driver cannot be created + Fork(addonName string, authConfig AddonAuthConfig, secretOption SecretOption) (RegisterDriver, error) } // HubDriver interface is used to implement operations required to complete aws-irsa registration and csr registration. diff --git a/pkg/registration/register/testing/helpers.go b/pkg/registration/register/testing/helpers.go new file mode 100644 index 000000000..498de9970 --- /dev/null +++ b/pkg/registration/register/testing/helpers.go @@ -0,0 +1,33 @@ +package testing + +import ( + "context" + + "open-cluster-management.io/ocm/pkg/registration/register" +) + +// TestAddonAuthConfig is a simple implementation of AddonAuthConfig for testing +type TestAddonAuthConfig struct { + KubeClientAuth string + CSROption register.CSRConfiguration + TokenOption register.TokenConfiguration +} + +func (t *TestAddonAuthConfig) GetKubeClientAuth() string { + return t.KubeClientAuth +} + +func (t *TestAddonAuthConfig) GetCSRConfiguration() register.CSRConfiguration { + return t.CSROption +} + +func (t *TestAddonAuthConfig) GetTokenConfiguration() register.TokenConfiguration { + return t.TokenOption +} + +// MockTokenControl is a simple mock implementation of TokenControl for testing +type MockTokenControl struct{} + +func (m *MockTokenControl) CreateToken(ctx context.Context, serviceAccountName, namespace string, expirationSeconds int64) (string, error) { + return "mock-token", nil +} diff --git a/pkg/registration/register/token/doc.go b/pkg/registration/register/token/doc.go new file mode 100644 index 000000000..16bf9007a --- /dev/null +++ b/pkg/registration/register/token/doc.go @@ -0,0 +1,5 @@ +// Package token provides a token-based authentication driver for addon registration only. +// This driver uses Kubernetes ServiceAccount token projection to authenticate addons with the hub cluster. +// It is designed to be forked from cluster-level drivers (CSR/gRPC/AWS IRSA) and cannot be used +// for cluster registration itself. +package token diff --git a/pkg/registration/register/token/options.go b/pkg/registration/register/token/options.go new file mode 100644 index 000000000..edf163b24 --- /dev/null +++ b/pkg/registration/register/token/options.go @@ -0,0 +1,42 @@ +package token + +import ( + "errors" + + "github.com/spf13/pflag" + + "open-cluster-management.io/ocm/pkg/registration/register" +) + +// Option contains configuration for the token driver +type Option struct { + // ExpirationSeconds is the requested duration of validity of the token. + // This is used to configure the ServiceAccount token projection. + // Default is 31536000 seconds (1 year) + ExpirationSeconds int64 +} + +// Ensure Option implements register.TokenConfiguration interface at compile time +var _ register.TokenConfiguration = &Option{} + +func NewTokenOption() *Option { + return &Option{ + ExpirationSeconds: 31536000, // Default 1 year + } +} + +func (o *Option) AddFlags(fs *pflag.FlagSet) { + fs.Int64Var(&o.ExpirationSeconds, "addon-token-expiration-seconds", o.ExpirationSeconds, + "Requested duration of validity of the token in seconds. Used for ServiceAccount token projection. Minimum 600 seconds (10 minutes)") +} + +func (o *Option) Validate() error { + if o.ExpirationSeconds < 600 { + return errors.New("token expiration seconds must be at least 600 (10 minutes)") + } + return nil +} + +func (o *Option) GetExpirationSeconds() int64 { + return o.ExpirationSeconds +} diff --git a/pkg/registration/register/token/token.go b/pkg/registration/register/token/token.go new file mode 100644 index 000000000..b1330e2dd --- /dev/null +++ b/pkg/registration/register/token/token.go @@ -0,0 +1,481 @@ +package token + +import ( + "context" + "fmt" + "os" + "path" + "strings" + "time" + + authenticationv1 "k8s.io/api/authentication/v1" + certificatesv1 "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/tools/cache" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/klog/v2" + + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + clusterv1 "open-cluster-management.io/api/cluster/v1" + "open-cluster-management.io/sdk-go/pkg/basecontroller/events" + "open-cluster-management.io/sdk-go/pkg/basecontroller/factory" + "open-cluster-management.io/sdk-go/pkg/patcher" + + "open-cluster-management.io/ocm/pkg/registration/register" +) + +const ( + // TokenFile is the name of the token file in the secret + TokenFile = "token" + + // TokenRefreshedCondition is the condition type for addon token refresh status + TokenRefreshedCondition = "TokenRefreshed" + + // TokenInfrastructureReadyCondition is the condition type set by hub to indicate + // that the ServiceAccount infrastructure is ready for token-based authentication + TokenInfrastructureReadyCondition = "TokenInfrastructureReady" +) + +// TokenControl encapsulates the operations needed for token-based authentication +type TokenControl interface { + // CreateToken creates a ServiceAccount token + CreateToken(ctx context.Context, serviceAccountName, namespace string, expirationSeconds int64) (string, error) +} + +// tokenControl implements TokenControl interface +type tokenControl struct { + hubCoreV1Client corev1client.CoreV1Interface +} + +var _ TokenControl = &tokenControl{} + +// CreateToken creates a ServiceAccount token using the TokenRequest API +func (t *tokenControl) CreateToken(ctx context.Context, serviceAccountName, namespace string, expirationSeconds int64) (string, error) { + if t.hubCoreV1Client == nil { + return "", fmt.Errorf("failed to create token for ServiceAccount %s/%s: hub CoreV1 client is not initialized", namespace, serviceAccountName) + } + + tokenRequest := &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{ + ExpirationSeconds: &expirationSeconds, + }, + } + + result, err := t.hubCoreV1Client.ServiceAccounts(namespace).CreateToken( + ctx, + serviceAccountName, + tokenRequest, + metav1.CreateOptions{}, + ) + if err != nil { + return "", fmt.Errorf("failed to create token for ServiceAccount %s/%s: %w", namespace, serviceAccountName, err) + } + + return result.Status.Token, nil +} + +// NewTokenControl creates a new TokenControl instance +func NewTokenControl(hubCoreV1Client corev1client.CoreV1Interface) TokenControl { + return &tokenControl{ + hubCoreV1Client: hubCoreV1Client, + } +} + +// TokenDriver implements token-based authentication for addon registration only. +// It uses ServiceAccount token projection for authentication with the hub cluster. +type TokenDriver struct { + addonName string + clusterName string + opt register.TokenConfiguration + tokenControl TokenControl + + // addonClients holds the addon clients and informers + addonClients *register.AddOnClients + + // addonPatcher for updating addon status + addonPatcher patcher.Patcher[ + *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus] +} + +var _ register.RegisterDriver = &TokenDriver{} + +// NewTokenDriverForAddOn creates a new token driver instance for an addon. +// This should only be called from a cluster driver's Fork() method. +func NewTokenDriverForAddOn(addonName, clusterName string, tokenConfig register.TokenConfiguration, tokenControl TokenControl, addonClients *register.AddOnClients) *TokenDriver { + driver := &TokenDriver{ + addonName: addonName, + clusterName: clusterName, + tokenControl: tokenControl, + addonClients: addonClients, + opt: tokenConfig, + addonPatcher: patcher.NewPatcher[ + *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus]( + addonClients.AddonClient.AddonV1alpha1().ManagedClusterAddOns(clusterName)), + } + + return driver +} + +// Process updates the secret with the current token from the ServiceAccount token file +func (t *TokenDriver) Process( + ctx context.Context, + controllerName string, + secret *corev1.Secret, + additionalSecretData map[string][]byte, + recorder events.Recorder) (*corev1.Secret, *metav1.Condition, error) { + // Get the addon + addon, err := t.addonClients.AddonInformer.Lister().ManagedClusterAddOns(t.clusterName).Get(t.addonName) + if errors.IsNotFound(err) { + // Addon not found (likely deleted), skip processing + return nil, nil, nil + } + if err != nil { + return nil, nil, err + } + + // Ensure subject field is set (driver should already be set by addon registration controller) + updated, err := t.ensureSubject(ctx, addon) + if err != nil || updated { + return nil, nil, err + } + + // Wait for token infrastructure to be ready and get ServiceAccount UID + desiredUID, ready, err := t.ensureTokenInfrastructureReady(ctx, addon) + if err != nil { + return nil, nil, err + } + if !ready { + return nil, nil, nil + } + + // Check if we need to refresh the token + shouldRefresh, err := t.shouldRefreshToken(ctx, secret, desiredUID) + if err != nil { + return nil, nil, err + } + if !shouldRefresh { + return nil, nil, nil + } + + // Refresh the token + return t.refreshToken(ctx, secret, recorder) +} + +// ensureSubject ensures the subject field is set correctly for token-based authentication. +// Subject.user is set to system:serviceaccount::-agent +// Returns (updated, error) where updated indicates if an update was performed +func (t *TokenDriver) ensureSubject(ctx context.Context, addon *addonv1alpha1.ManagedClusterAddOn) (bool, error) { + logger := klog.FromContext(ctx) + + // Find the registration configuration index + var regIndex = -1 + for i := range addon.Status.Registrations { + if addon.Status.Registrations[i].SignerName == certificatesv1.KubeAPIServerClientSignerName { + regIndex = i + break + } + } + + if regIndex == -1 { + return false, nil + } + + // Set subject for token-based authentication + expectedSubjectUser := fmt.Sprintf("system:serviceaccount:%s:%s-agent", t.clusterName, t.addonName) + + // Make a copy and update subject (create new Subject with only User specified) + addonCopy := addon.DeepCopy() + addonCopy.Status.Registrations[regIndex].Subject = addonv1alpha1.Subject{ + User: expectedSubjectUser, + } + + // Update the addon status using addonPatcher + updated, err := t.addonPatcher.PatchStatus(ctx, addonCopy, addonCopy.Status, addon.Status) + if err != nil { + return false, err + } + + if updated { + logger.Info("Updated subject field", "addon", t.addonName, "subject", expectedSubjectUser) + } + + return updated, nil +} + +// ensureTokenInfrastructureReady waits for the TokenInfrastructureReady condition and extracts the ServiceAccount UID +// Returns (uid, ready, error) where: +// - If ready is true, uid is guaranteed to be non-empty +// - If ready is false, infrastructure is not ready yet (caller should wait and retry) +// - If error is non-nil, an actual error occurred +func (t *TokenDriver) ensureTokenInfrastructureReady(ctx context.Context, addon *addonv1alpha1.ManagedClusterAddOn) (string, bool, error) { + logger := klog.FromContext(ctx) + + infraReady := meta.FindStatusCondition(addon.Status.Conditions, TokenInfrastructureReadyCondition) + if infraReady == nil { + logger.Info("TokenInfrastructureReady condition not found, waiting for hub to set it", "addon", t.addonName) + return "", false, nil + } + + if infraReady.Status != metav1.ConditionTrue { + logger.Info("TokenInfrastructureReady condition is not True, waiting", + "addon", t.addonName, + "status", infraReady.Status, + "reason", infraReady.Reason) + return "", false, nil + } + + desiredUID, err := t.parseServiceAccountUIDFromMessage(infraReady.Message) + if err != nil { + logger.Error(err, "Failed to parse ServiceAccount UID from TokenInfrastructureReady condition message", + "addon", t.addonName, + "message", infraReady.Message) + return "", false, err + } + + logger.V(4).Info("Parsed ServiceAccount UID from TokenInfrastructureReady condition", + "addon", t.addonName, + "serviceAccountUID", desiredUID) + + return desiredUID, true, nil +} + +// refreshToken creates a new token and updates the secret +func (t *TokenDriver) refreshToken(ctx context.Context, secret *corev1.Secret, recorder events.Recorder) (*corev1.Secret, *metav1.Condition, error) { + tokenData, expiresAt, err := t.createToken(ctx) + if err != nil { + return nil, &metav1.Condition{ + Type: TokenRefreshedCondition, + Status: metav1.ConditionFalse, + Reason: "TokenCreationFailed", + Message: fmt.Sprintf("Failed to create token: %v", err), + }, err + } + + secret.Data[TokenFile] = tokenData + recorder.Eventf(ctx, "AddonTokenRefreshed", "Token refreshed for addon %s", t.addonName) + + return secret, &metav1.Condition{ + Type: TokenRefreshedCondition, + Status: metav1.ConditionTrue, + Reason: "AddonTokenRefreshed", + Message: fmt.Sprintf("Addon token refreshed, expires at %s", expiresAt.Format(time.RFC3339)), + }, nil +} + +// BuildKubeConfigFromTemplate builds kubeconfig with bearer token authentication +func (t *TokenDriver) BuildKubeConfigFromTemplate(kubeConfig *clientcmdapi.Config) *clientcmdapi.Config { + kubeConfig.AuthInfos = map[string]*clientcmdapi.AuthInfo{ + register.DefaultKubeConfigAuth: { + TokenFile: TokenFile, + }, + } + return kubeConfig +} + +// InformerHandler returns the addon informer with a filter for the specific addon +func (t *TokenDriver) InformerHandler() (cache.SharedIndexInformer, factory.EventFilterFunc) { + if t.addonClients == nil { + return nil, nil + } + + // Create event filter function to only watch the specific addon + // Note: The informer is already scoped to the cluster namespace, so we only need to filter by addon name + eventFilterFunc := func(obj interface{}) bool { + accessor, err := meta.Accessor(obj) + if err != nil { + return false + } + + // Only enqueue the specific addon (name matches addonName) + return accessor.GetName() == t.addonName + } + + return t.addonClients.AddonInformer.Informer(), eventFilterFunc +} + +// IsHubKubeConfigValid checks if the current token is valid +func (t *TokenDriver) IsHubKubeConfigValid(ctx context.Context, secretOption register.SecretOption) (bool, error) { + logger := klog.FromContext(ctx) + + tokenData, err := t.readTokenFile(secretOption.HubKubeconfigDir) + if err != nil { + logger.V(4).Info("Token file not found or unreadable") + return false, nil + } + + desiredUID := t.getDesiredUIDForValidation(ctx) + valid, reason := isTokenValid(tokenData, desiredUID) + if !valid { + logger.V(4).Info("Token is invalid", "reason", reason) + } + return valid, nil +} + +// ManagedClusterDecorator returns the cluster unchanged (no modifications needed) +func (t *TokenDriver) ManagedClusterDecorator(cluster *clusterv1.ManagedCluster) *clusterv1.ManagedCluster { + return cluster +} + +// BuildClients does nothing for TokenDriver +func (t *TokenDriver) BuildClients(ctx context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { + return nil, nil +} + +// shouldRefreshToken determines if the token needs to be refreshed +func (t *TokenDriver) shouldRefreshToken(ctx context.Context, secret *corev1.Secret, desiredUID string) (bool, error) { + logger := klog.FromContext(ctx) + + // If no token in secret, refresh is needed + tokenData, ok := secret.Data[TokenFile] + if !ok || len(tokenData) == 0 { + logger.Info("Token refresh needed: no token found in secret", "addon", t.addonName) + return true, nil + } + + // Check if token is still valid based on expiration and UID + valid, reason := isTokenValid(tokenData, desiredUID) + if !valid { + logger.Info("Token refresh needed", "addon", t.addonName, "reason", reason) + return true, nil + } + + logger.V(4).Info("Token is valid, no refresh needed", "addon", t.addonName) + return false, nil +} + +// readTokenFile reads the token file from the specified directory +func (t *TokenDriver) readTokenFile(hubKubeconfigDir string) ([]byte, error) { + tokenPath := path.Join(hubKubeconfigDir, TokenFile) + return os.ReadFile(path.Clean(tokenPath)) +} + +// getDesiredUIDForValidation retrieves the desired ServiceAccount UID for token validation +// Returns empty string if UID cannot be determined (validation will skip UID check) +func (t *TokenDriver) getDesiredUIDForValidation(ctx context.Context) string { + logger := klog.FromContext(ctx) + + if t.addonClients == nil { + return "" + } + + addon, err := t.addonClients.AddonInformer.Lister().ManagedClusterAddOns(t.clusterName).Get(t.addonName) + if err != nil { + logger.V(4).Info("Failed to get addon for token validation", "addon", t.addonName, "error", err) + return "" + } + + infraReady := meta.FindStatusCondition(addon.Status.Conditions, TokenInfrastructureReadyCondition) + if infraReady == nil || infraReady.Status != metav1.ConditionTrue { + return "" + } + + uid, err := t.parseServiceAccountUIDFromMessage(infraReady.Message) + if err != nil { + logger.Info("Failed to parse ServiceAccount UID for token validation", "error", err) + return "" + } + + return uid +} + +// createToken creates a new ServiceAccount token using the TokenRequest API +// ServiceAccount name format: -agent +// Returns the token data and expiration time +func (t *TokenDriver) createToken(ctx context.Context) ([]byte, time.Time, error) { + if t.tokenControl == nil { + return nil, time.Time{}, fmt.Errorf("token control not initialized") + } + + // ServiceAccount naming convention: -agent + // The ServiceAccount is created by the hub addon manager for token-based registration + serviceAccountName := fmt.Sprintf("%s-agent", t.addonName) + + // Create token using TokenRequest API + token, err := t.tokenControl.CreateToken(ctx, serviceAccountName, t.clusterName, t.opt.GetExpirationSeconds()) + if err != nil { + return nil, time.Time{}, err + } + + tokenData := []byte(token) + + // Parse token to get expiration time + _, expiresAt, _, err := parseToken(tokenData) + if err != nil { + return nil, time.Time{}, fmt.Errorf("failed to parse created token: %w", err) + } + + return tokenData, expiresAt, nil +} + +// parseServiceAccountUIDFromMessage parses the ServiceAccount UID from the TokenInfrastructureReady condition message +// Expected message format: "ServiceAccount / (UID: ) is ready" +func (t *TokenDriver) parseServiceAccountUIDFromMessage(message string) (string, error) { + // Look for "UID: " pattern in the message + const uidPrefix = "UID: " + const uidSuffix = ")" + + startIdx := strings.Index(message, uidPrefix) + if startIdx == -1 { + return "", fmt.Errorf("ServiceAccount UID not found in message: %s", message) + } + + startIdx += len(uidPrefix) + endIdx := strings.Index(message[startIdx:], uidSuffix) + if endIdx == -1 { + return "", fmt.Errorf("malformed ServiceAccount UID in message: %s", message) + } + + uid := message[startIdx : startIdx+endIdx] + if uid == "" { + return "", fmt.Errorf("empty ServiceAccount UID in message: %s", message) + } + + return uid, nil +} + +// TryForkTokenDriver checks if token-based authentication should be used for the addon, +// and if so, creates and returns a TokenDriver. Returns nil if token auth is not needed. +// This helper is shared by all cluster drivers (CSR, AWS IRSA, gRPC) to avoid code duplication. +func TryForkTokenDriver( + addonName string, + authConfig register.AddonAuthConfig, + secretOption register.SecretOption, + tokenControl TokenControl, + addonClients *register.AddOnClients, +) (register.RegisterDriver, error) { + // Determine registration type from signer name + isKubeClientType := secretOption.Signer == certificatesv1.KubeAPIServerClientSignerName + + // Only use token auth for KubeClient type with token authentication + if !isKubeClientType || authConfig.GetKubeClientAuth() != "token" { + return nil, nil // Not using token auth + } + + // Get token configuration from AddonAuthConfig (type-safe interface) + tokenConfig := authConfig.GetTokenConfiguration() + if tokenConfig == nil { + return nil, fmt.Errorf("token authentication requested but TokenConfiguration is nil for addon %s", addonName) + } + + if tokenControl == nil { + return nil, fmt.Errorf("token authentication requested but tokenControl is not initialized for addon %s", addonName) + } + + if addonClients == nil { + return nil, fmt.Errorf("token authentication requested but addonClients is not initialized for addon %s", addonName) + } + + if addonClients.AddonClient == nil { + return nil, fmt.Errorf("token authentication requested but addonClients.AddonClient is nil for addon %s", addonName) + } + + if addonClients.AddonInformer == nil { + return nil, fmt.Errorf("token authentication requested but addonClients.AddonInformer is nil for addon %s", addonName) + } + + return NewTokenDriverForAddOn(addonName, secretOption.ClusterName, tokenConfig, tokenControl, addonClients), nil +} diff --git a/pkg/registration/register/token/token_test.go b/pkg/registration/register/token/token_test.go new file mode 100644 index 000000000..a4ef594b5 --- /dev/null +++ b/pkg/registration/register/token/token_test.go @@ -0,0 +1,820 @@ +package token + +import ( + "context" + "encoding/base64" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + addonfake "open-cluster-management.io/api/client/addon/clientset/versioned/fake" + addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" + clusterv1 "open-cluster-management.io/api/cluster/v1" + + "open-cluster-management.io/ocm/pkg/registration/register" +) + +// newTestAddonClients creates fake addon clients for testing +func newTestAddonClients() *register.AddOnClients { + addonClient := addonfake.NewSimpleClientset() + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 10*time.Minute) + return ®ister.AddOnClients{ + AddonClient: addonClient, + AddonInformer: addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(), + } +} + +func TestTokenDriver_BuildKubeConfigFromTemplate(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + template := &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "hub": { + Server: "https://hub.example.com", + }, + }, + Contexts: map[string]*clientcmdapi.Context{ + register.DefaultKubeConfigContext: { + Cluster: "hub", + AuthInfo: register.DefaultKubeConfigAuth, + }, + }, + CurrentContext: register.DefaultKubeConfigContext, + } + + result := driver.BuildKubeConfigFromTemplate(template) + + if result.AuthInfos == nil { + t.Fatal("AuthInfos should not be nil") + } + + authInfo, ok := result.AuthInfos[register.DefaultKubeConfigAuth] + if !ok { + t.Fatal("AuthInfo not found") + } + + if authInfo.TokenFile != TokenFile { + t.Errorf("Expected TokenFile to be %q, got %q", TokenFile, authInfo.TokenFile) + } +} + +func TestTokenDriver_ManagedClusterDecorator(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + cluster := &clusterv1.ManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + } + + result := driver.ManagedClusterDecorator(cluster) + + if result != cluster { + t.Error("ManagedClusterDecorator should return the same cluster object") + } +} + +func TestTokenDriver_InformerHandler(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + informer, filter := driver.InformerHandler() + + if informer == nil { + t.Error("Expected informer to be non-nil when addonClients is provided") + } + + if filter == nil { + t.Error("Expected filter to be non-nil when addonClients is provided") + } +} + +func TestTokenDriver_Process(t *testing.T) { + t.Skip("Skipping Process test - requires full mock implementation") + // This test requires mocking: + // - AddonInformer with lister + // - AddonClient for status updates + // - TokenControl for token creation + // Consider implementing with fake clients in future +} + +func TestShouldRefreshToken(t *testing.T) { + tests := []struct { + name string + tokenAge time.Duration + tokenExpiry time.Duration + shouldRefresh bool + }{ + { + name: "fresh token - no refresh", + tokenAge: 0, + tokenExpiry: 1 * time.Hour, + shouldRefresh: false, + }, + { + name: "token near expiry - refresh needed", + tokenAge: 50 * time.Minute, + tokenExpiry: 1 * time.Hour, + shouldRefresh: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opt := NewTokenOption() + addonClients := newTestAddonClients() + + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", opt, nil, addonClients) + + now := time.Now() + iat := now.Add(-tt.tokenAge).Unix() + exp := now.Add(tt.tokenExpiry - tt.tokenAge).Unix() + mockToken := createMockJWT(t, exp, iat) + + secret := &corev1.Secret{ + Data: map[string][]byte{ + TokenFile: []byte(mockToken), + }, + } + + shouldRefresh, err := driver.shouldRefreshToken(context.Background(), secret, "") + if err != nil { + t.Fatalf("shouldRefreshToken failed: %v", err) + } + + if shouldRefresh != tt.shouldRefresh { + t.Errorf("Expected shouldRefresh=%v, got %v", tt.shouldRefresh, shouldRefresh) + } + }) + } +} + +func TestShouldRefreshToken_EdgeCases(t *testing.T) { + tests := []struct { + name string + secretData map[string][]byte + desiredUID string + shouldRefresh bool + wantError bool + }{ + { + name: "empty secret data - refresh needed", + secretData: map[string][]byte{}, + shouldRefresh: true, + wantError: false, + }, + { + name: "empty token - refresh needed", + secretData: map[string][]byte{ + TokenFile: []byte(""), + }, + shouldRefresh: true, + wantError: false, + }, + { + name: "invalid token format - refresh needed", + secretData: map[string][]byte{ + TokenFile: []byte("invalid.token"), + }, + shouldRefresh: true, + wantError: false, + }, + { + name: "UID mismatch - refresh needed", + secretData: map[string][]byte{ + TokenFile: []byte(createMockJWTWithUID(t, time.Now().Add(1*time.Hour).Unix(), time.Now().Unix(), "wrong-uid")), + }, + desiredUID: "expected-uid", + shouldRefresh: true, + wantError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + secret := &corev1.Secret{ + Data: tt.secretData, + } + + shouldRefresh, err := driver.shouldRefreshToken(context.Background(), secret, tt.desiredUID) + if (err != nil) != tt.wantError { + t.Errorf("shouldRefreshToken() error = %v, wantError %v", err, tt.wantError) + return + } + + if shouldRefresh != tt.shouldRefresh { + t.Errorf("shouldRefreshToken() = %v, want %v", shouldRefresh, tt.shouldRefresh) + } + }) + } +} + +func TestParseServiceAccountUIDFromMessage(t *testing.T) { + tests := []struct { + name string + message string + wantUID string + wantError bool + }{ + { + name: "valid message", + message: "ServiceAccount cluster1/cluster1-addon1-agent (UID: 12345678-1234-1234-1234-123456789abc) is ready", + wantUID: "12345678-1234-1234-1234-123456789abc", + wantError: false, + }, + { + name: "missing UID prefix", + message: "ServiceAccount cluster1/cluster1-addon1-agent is ready", + wantUID: "", + wantError: true, + }, + { + name: "missing closing parenthesis", + message: "ServiceAccount cluster1/cluster1-addon1-agent (UID: 12345678-1234-1234-1234-123456789abc is ready", + wantUID: "", + wantError: true, + }, + { + name: "empty UID", + message: "ServiceAccount cluster1/cluster1-addon1-agent (UID: ) is ready", + wantUID: "", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + uid, err := driver.parseServiceAccountUIDFromMessage(tt.message) + if (err != nil) != tt.wantError { + t.Errorf("parseServiceAccountUIDFromMessage() error = %v, wantError %v", err, tt.wantError) + return + } + + if uid != tt.wantUID { + t.Errorf("parseServiceAccountUIDFromMessage() = %v, want %v", uid, tt.wantUID) + } + }) + } +} + +func TestIsHubKubeConfigValid(t *testing.T) { + // Create a temporary directory for test files + tmpDir := t.TempDir() + + now := time.Now() + validToken := createMockJWTWithUID(t, now.Add(1*time.Hour).Unix(), now.Unix(), "test-uid") + expiredToken := createMockJWTWithUID(t, now.Add(-1*time.Hour).Unix(), now.Add(-2*time.Hour).Unix(), "test-uid") + + tests := []struct { + name string + setupFunc func() string + wantValid bool + wantError bool + }{ + { + name: "valid token file", + setupFunc: func() string { + dir := filepath.Join(tmpDir, "valid") + os.MkdirAll(dir, 0755) + os.WriteFile(filepath.Join(dir, TokenFile), []byte(validToken), 0644) + return dir + }, + wantValid: true, + wantError: false, + }, + { + name: "token file does not exist", + setupFunc: func() string { + return filepath.Join(tmpDir, "nonexistent") + }, + wantValid: false, + wantError: false, + }, + { + name: "expired token", + setupFunc: func() string { + dir := filepath.Join(tmpDir, "expired") + os.MkdirAll(dir, 0755) + os.WriteFile(filepath.Join(dir, TokenFile), []byte(expiredToken), 0644) + return dir + }, + wantValid: false, + wantError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + hubKubeconfigDir := tt.setupFunc() + + secretOption := register.SecretOption{ + HubKubeconfigDir: hubKubeconfigDir, + } + + valid, err := driver.IsHubKubeConfigValid(context.Background(), secretOption) + if (err != nil) != tt.wantError { + t.Errorf("IsHubKubeConfigValid() error = %v, wantError %v", err, tt.wantError) + return + } + + if valid != tt.wantValid { + t.Errorf("IsHubKubeConfigValid() = %v, want %v", valid, tt.wantValid) + } + }) + } +} + +func TestParseToken(t *testing.T) { + now := time.Now() + iat := now.Unix() + exp := now.Add(1 * time.Hour).Unix() + expectedUID := "test-uid-12345" + + tests := []struct { + name string + token string + wantIat int64 + wantExp int64 + wantUID string + wantError bool + }{ + { + name: "valid token", + token: createMockJWTWithUID(t, exp, iat, expectedUID), + wantIat: iat, + wantExp: exp, + wantUID: expectedUID, + wantError: false, + }, + { + name: "invalid format - only 2 parts", + token: "header.payload", + wantError: true, + }, + { + name: "invalid format - 4 parts", + token: "header.payload.signature.extra", + wantError: true, + }, + { + name: "invalid base64 payload", + token: "header.!!!invalid!!!.signature", + wantError: true, + }, + { + name: "missing exp claim", + token: createMockJWTWithoutClaim(t, "exp", iat), + wantError: true, + }, + { + name: "missing iat claim", + token: createMockJWTWithoutClaim(t, "iat", exp), + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issueTime, expirationTime, uid, err := parseToken([]byte(tt.token)) + if (err != nil) != tt.wantError { + t.Errorf("parseToken() error = %v, wantError %v", err, tt.wantError) + return + } + + if !tt.wantError { + if issueTime.Unix() != tt.wantIat { + t.Errorf("parseToken() issueTime = %v, want %v", issueTime.Unix(), tt.wantIat) + } + if expirationTime.Unix() != tt.wantExp { + t.Errorf("parseToken() expirationTime = %v, want %v", expirationTime.Unix(), tt.wantExp) + } + if uid != tt.wantUID { + t.Errorf("parseToken() uid = %v, want %v", uid, tt.wantUID) + } + } + }) + } +} + +func TestIsTokenValid(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + tokenData []byte + desiredUID string + wantValid bool + reason string + }{ + { + name: "empty token", + tokenData: []byte(""), + wantValid: false, + reason: "token is empty", + }, + { + name: "valid token with plenty of life remaining", + tokenData: []byte(createMockJWTWithUID(t, now.Add(1*time.Hour).Unix(), now.Unix(), "test-uid")), + wantValid: true, + }, + { + name: "token near expiration (within refresh threshold)", + tokenData: []byte(createMockJWTWithUID(t, now.Add(10*time.Minute).Unix(), now.Add(-50*time.Minute).Unix(), "test-uid")), + wantValid: false, + }, + { + name: "expired token", + tokenData: []byte(createMockJWTWithUID(t, now.Add(-1*time.Hour).Unix(), now.Add(-2*time.Hour).Unix(), "test-uid")), + wantValid: false, + }, + { + name: "UID mismatch", + tokenData: []byte(createMockJWTWithUID(t, now.Add(1*time.Hour).Unix(), now.Unix(), "wrong-uid")), + desiredUID: "expected-uid", + wantValid: false, + }, + { + name: "non-positive lifetime (exp == iat)", + tokenData: []byte(createMockJWTWithUID(t, now.Unix(), now.Unix(), "test-uid")), + wantValid: false, + }, + { + name: "non-positive lifetime (exp < iat)", + tokenData: []byte(createMockJWTWithUID(t, now.Add(-1*time.Hour).Unix(), now.Unix(), "test-uid")), + wantValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + valid, reason := isTokenValid(tt.tokenData, tt.desiredUID) + if valid != tt.wantValid { + t.Errorf("isTokenValid() = %v, want %v (reason: %s)", valid, tt.wantValid, reason) + } + }) + } +} + +// createMockJWT creates a simplified mock JWT token for testing +// Note: This is not a real signed JWT, just enough structure for parsing tests +func createMockJWT(t *testing.T, exp, iat int64) string { + t.Helper() + return createMockJWTWithUID(t, exp, iat, "12345678-1234-1234-1234-123456789abc") +} + +// createMockJWTWithUID creates a mock JWT token with a specific UID +func createMockJWTWithUID(t *testing.T, exp, iat int64, uid string) string { + t.Helper() + + // JWT header (base64url encoded) + header := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9" // {"alg":"RS256","typ":"JWT"} + + // Create payload with exp, iat, and kubernetes metadata + payload := map[string]interface{}{ + "exp": exp, + "iat": iat, + "sub": "system:serviceaccount:default:test-sa", + "aud": "https://kubernetes.default.svc", + "kubernetes.io": map[string]interface{}{ + "namespace": "default", + "serviceaccount": map[string]interface{}{ + "name": "test-sa", + "uid": uid, + }, + }, + } + + payloadBytes, err := json.Marshal(payload) + if err != nil { + t.Fatalf("Failed to marshal payload: %v", err) + } + + payloadEncoded := base64.RawURLEncoding.EncodeToString(payloadBytes) + + // Mock signature (base64url encoded) + signature := "mock-signature" + + return header + "." + payloadEncoded + "." + signature +} + +// createMockJWTWithoutClaim creates a mock JWT token missing a specific claim +func createMockJWTWithoutClaim(t *testing.T, missingClaim string, value int64) string { + t.Helper() + + header := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9" + + payload := map[string]interface{}{ + "sub": "system:serviceaccount:default:test-sa", + "aud": "https://kubernetes.default.svc", + "kubernetes.io": map[string]interface{}{ + "namespace": "default", + "serviceaccount": map[string]interface{}{ + "name": "test-sa", + "uid": "test-uid", + }, + }, + } + + // Add the claims except the one we want to omit + if missingClaim != "exp" { + payload["exp"] = value + } + if missingClaim != "iat" { + payload["iat"] = value + } + + payloadBytes, err := json.Marshal(payload) + if err != nil { + t.Fatalf("Failed to marshal payload: %v", err) + } + + payloadEncoded := base64.RawURLEncoding.EncodeToString(payloadBytes) + signature := "mock-signature" + + return header + "." + payloadEncoded + "." + signature +} + +// TestTokenDriver_NilGuards tests nil pointer guards for critical components +func TestTokenDriver_NilGuards(t *testing.T) { + tests := []struct { + name string + setupDriver func() *TokenDriver + testFunc func(t *testing.T, driver *TokenDriver) + expectError bool + expectedErrorMsg string + }{ + { + name: "createToken with nil tokenControl", + setupDriver: func() *TokenDriver { + addonClients := newTestAddonClients() + return NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + }, + testFunc: func(t *testing.T, driver *TokenDriver) { + _, _, err := driver.createToken(context.Background()) + if err == nil { + t.Error("Expected error when tokenControl is nil") + } + if err != nil && err.Error() != "token control not initialized" { + t.Errorf("Expected 'token control not initialized' error, got: %v", err) + } + }, + expectError: true, + expectedErrorMsg: "token control not initialized", + }, + { + name: "InformerHandler returns correct informer and filter", + setupDriver: func() *TokenDriver { + addonClients := newTestAddonClients() + return NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + }, + testFunc: func(t *testing.T, driver *TokenDriver) { + informer, filter := driver.InformerHandler() + if informer == nil { + t.Error("Expected non-nil informer") + } + if filter == nil { + t.Error("Expected non-nil filter") + } + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + driver := tt.setupDriver() + tt.testFunc(t, driver) + }) + } +} + +// TestTokenDriver_EnsureTokenInfrastructureReady tests infrastructure readiness checks +func TestTokenDriver_EnsureTokenInfrastructureReady(t *testing.T) { + tests := []struct { + name string + addon *addonv1alpha1.ManagedClusterAddOn + expectedUID string + expectedReady bool + expectedError bool + }{ + { + name: "no TokenInfrastructureReady condition", + addon: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-addon", + }, + Status: addonv1alpha1.ManagedClusterAddOnStatus{ + Conditions: []metav1.Condition{}, + }, + }, + expectedUID: "", + expectedReady: false, + expectedError: false, + }, + { + name: "TokenInfrastructureReady condition is False", + addon: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-addon", + }, + Status: addonv1alpha1.ManagedClusterAddOnStatus{ + Conditions: []metav1.Condition{ + { + Type: "TokenInfrastructureReady", + Status: metav1.ConditionFalse, + Reason: "ServiceAccountNotFound", + }, + }, + }, + }, + expectedUID: "", + expectedReady: false, + expectedError: false, + }, + { + name: "TokenInfrastructureReady is True with valid UID", + addon: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-addon", + }, + Status: addonv1alpha1.ManagedClusterAddOnStatus{ + Conditions: []metav1.Condition{ + { + Type: "TokenInfrastructureReady", + Status: metav1.ConditionTrue, + Reason: "ServiceAccountReady", + Message: "ServiceAccount default/test-addon-agent (UID: 12345678-1234-1234-1234-123456789abc) is ready", + }, + }, + }, + }, + expectedUID: "12345678-1234-1234-1234-123456789abc", + expectedReady: true, + expectedError: false, + }, + { + name: "TokenInfrastructureReady is True but invalid message format", + addon: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-addon", + }, + Status: addonv1alpha1.ManagedClusterAddOnStatus{ + Conditions: []metav1.Condition{ + { + Type: "TokenInfrastructureReady", + Status: metav1.ConditionTrue, + Reason: "ServiceAccountReady", + Message: "Invalid message without UID", + }, + }, + }, + }, + expectedUID: "", + expectedReady: false, + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + uid, ready, err := driver.ensureTokenInfrastructureReady(context.Background(), tt.addon) + + if (err != nil) != tt.expectedError { + t.Errorf("Expected error=%v, got error=%v", tt.expectedError, err) + } + if ready != tt.expectedReady { + t.Errorf("Expected ready=%v, got ready=%v", tt.expectedReady, ready) + } + if uid != tt.expectedUID { + t.Errorf("Expected UID=%q, got UID=%q", tt.expectedUID, uid) + } + }) + } +} + +// TestTokenDriver_ParseServiceAccountUIDFromMessage tests UID parsing from condition messages +func TestTokenDriver_ParseServiceAccountUIDFromMessage(t *testing.T) { + tests := []struct { + name string + message string + expectedUID string + wantError bool + }{ + { + name: "valid message with UID", + message: "ServiceAccount default/test-addon-agent (UID: 12345678-1234-1234-1234-123456789abc) is ready", + expectedUID: "12345678-1234-1234-1234-123456789abc", + wantError: false, + }, + { + name: "message without UID prefix", + message: "ServiceAccount is ready", + expectedUID: "", + wantError: true, + }, + { + name: "message with UID prefix but no closing paren", + message: "ServiceAccount (UID: 12345678-1234-1234-1234-123456789abc", + expectedUID: "", + wantError: true, + }, + { + name: "empty message", + message: "", + expectedUID: "", + wantError: true, + }, + { + name: "message with empty UID", + message: "ServiceAccount (UID: ) is ready", + expectedUID: "", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + uid, err := driver.parseServiceAccountUIDFromMessage(tt.message) + + if (err != nil) != tt.wantError { + t.Errorf("Expected error=%v, got error=%v", tt.wantError, err) + } + if uid != tt.expectedUID { + t.Errorf("Expected UID=%q, got UID=%q", tt.expectedUID, uid) + } + }) + } +} + +// TestTokenDriver_InformerHandlerFilter tests the event filter function +func TestTokenDriver_InformerHandlerFilter(t *testing.T) { + addonClients := newTestAddonClients() + driver := NewTokenDriverForAddOn("test-addon", "test-cluster", NewTokenOption(), nil, addonClients) + + _, filter := driver.InformerHandler() + if filter == nil { + t.Fatal("Expected non-nil filter") + } + + tests := []struct { + name string + obj interface{} + expectedResult bool + }{ + { + name: "matching addon name", + obj: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-addon", + }, + }, + expectedResult: true, + }, + { + name: "non-matching addon name", + obj: &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-addon", + }, + }, + expectedResult: false, + }, + { + name: "invalid object type", + obj: "not-a-valid-object", + expectedResult: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := filter(tt.obj) + if result != tt.expectedResult { + t.Errorf("Expected filter result=%v, got=%v", tt.expectedResult, result) + } + }) + } +} diff --git a/pkg/registration/register/token/token_validation.go b/pkg/registration/register/token/token_validation.go new file mode 100644 index 000000000..884136fa9 --- /dev/null +++ b/pkg/registration/register/token/token_validation.go @@ -0,0 +1,123 @@ +package token + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "strings" + "time" +) + +const ( + // RefreshThreshold is the percentage of token lifetime remaining before refresh + // When the token has 20% or less of its lifetime remaining, it will be refreshed + RefreshThreshold = 0.2 +) + +// jwtClaims represents the claims section of a JWT token +type jwtClaims struct { + Exp int64 `json:"exp"` // Expiration time (Unix timestamp) + Iat int64 `json:"iat"` // Issued at time (Unix timestamp) + Kubernetes kubernetesMetadata `json:"kubernetes.io,omitempty"` +} + +// kubernetesMetadata represents Kubernetes-specific claims in the token +type kubernetesMetadata struct { + ServiceAccount serviceAccountMetadata `json:"serviceaccount,omitempty"` +} + +// serviceAccountMetadata represents service account information in the token +type serviceAccountMetadata struct { + UID string `json:"uid,omitempty"` +} + +// parseToken parses a JWT token and extracts issue time, expiration time, and service account UID +func parseToken(tokenData []byte) (issueTime, expirationTime time.Time, uid string, err error) { + token := string(tokenData) + + // JWT tokens have three parts separated by dots: header.payload.signature + parts := strings.Split(token, ".") + if len(parts) != 3 { + err = fmt.Errorf("invalid JWT token format: expected 3 parts, got %d", len(parts)) + return + } + + // Decode the payload (second part) + payloadEncoded := parts[1] + // JWT uses base64url encoding, which may need padding + if l := len(payloadEncoded) % 4; l > 0 { + payloadEncoded += strings.Repeat("=", 4-l) + } + + payloadBytes, decodeErr := base64.URLEncoding.DecodeString(payloadEncoded) + if decodeErr != nil { + err = fmt.Errorf("failed to decode JWT payload: %w", decodeErr) + return + } + + // Parse the claims + var claims jwtClaims + if unmarshalErr := json.Unmarshal(payloadBytes, &claims); unmarshalErr != nil { + err = fmt.Errorf("failed to unmarshal JWT claims: %w", unmarshalErr) + return + } + + if claims.Exp == 0 { + err = fmt.Errorf("token does not have expiration claim") + return + } + + if claims.Iat == 0 { + err = fmt.Errorf("token does not have issued-at claim") + return + } + + issueTime = time.Unix(claims.Iat, 0) + expirationTime = time.Unix(claims.Exp, 0) + uid = claims.Kubernetes.ServiceAccount.UID + + return +} + +// isTokenValid checks if a token is still valid based on its expiration and RefreshThreshold (20%) +// Returns (valid, reason) where reason describes why the token is invalid +// desiredUID is the expected service account UID; if empty, UID check is skipped +func isTokenValid(tokenData []byte, desiredUID string) (bool, string) { + if len(tokenData) == 0 { + return false, "token is empty" + } + + issueTime, expirationTime, uid, err := parseToken(tokenData) + if err != nil { + return false, fmt.Sprintf("failed to parse token: %v", err) + } + + // Check if the token's service account UID matches the desired UID + if desiredUID != "" && uid != desiredUID { + return false, fmt.Sprintf("token service account UID mismatch (expected %s, got %s)", desiredUID, uid) + } + + now := time.Now() + if expirationTime.Before(now) { + return false, fmt.Sprintf("token has expired at %s", expirationTime.Format(time.RFC3339)) + } + + totalLifetime := expirationTime.Sub(issueTime) + // Guard against non-positive lifetime (exp == iat or exp < iat) + if totalLifetime <= 0 { + return false, "token has non-positive lifetime" + } + + remaining := time.Until(expirationTime) + + // Calculate the percentage of lifetime remaining + remainingPercentage := remaining.Seconds() / totalLifetime.Seconds() + + // Token is valid if it has more than the threshold percentage of its lifetime remaining + if remainingPercentage <= RefreshThreshold { + return false, fmt.Sprintf("token is close to expiration (%.1f%% remaining, threshold %.1f%%)", + remainingPercentage*100, RefreshThreshold*100) + } + + return true, "" +} diff --git a/pkg/registration/spoke/addon/configuration.go b/pkg/registration/spoke/addon/configuration.go index 495da2736..81a6e6698 100644 --- a/pkg/registration/spoke/addon/configuration.go +++ b/pkg/registration/spoke/addon/configuration.go @@ -81,34 +81,37 @@ func isAddonRunningOutsideManagedCluster(addOn *addonv1alpha1.ManagedClusterAddO return false } -// getRegistrationConfigs reads annotations of a addon and returns a map of registrationConfig whose -// key is the hash of the registrationConfig -func getRegistrationConfigs(addOn *addonv1alpha1.ManagedClusterAddOn) (map[string]registrationConfig, error) { +// getRegistrationConfigs reads registrations and returns a map of registrationConfig whose +// key is the hash of the registrationConfig. +func getRegistrationConfigs( + addOnName string, + installOption addonInstallOption, + registrations []addonv1alpha1.RegistrationConfig, + kubeClientDriver string, +) (map[string]registrationConfig, error) { configs := map[string]registrationConfig{} - for _, registration := range addOn.Status.Registrations { + for _, registration := range registrations { config := registrationConfig{ - addOnName: addOn.Name, - addonInstallOption: addonInstallOption{ - AgentRunningOutsideManagedCluster: isAddonRunningOutsideManagedCluster(addOn), - InstallationNamespace: getAddOnInstallationNamespace(addOn), - }, - registration: registration, + addOnName: addOnName, + addonInstallOption: installOption, + registration: registration, } // set the secret name of client certificate switch registration.SignerName { case certificatesv1.KubeAPIServerClientSignerName: - config.secretName = fmt.Sprintf("%s-hub-kubeconfig", addOn.Name) + config.secretName = fmt.Sprintf("%s-hub-kubeconfig", addOnName) default: - config.secretName = fmt.Sprintf("%s-%s-client-cert", addOn.Name, strings.ReplaceAll(registration.SignerName, "/", "-")) + config.secretName = fmt.Sprintf("%s-%s-client-cert", addOnName, strings.ReplaceAll(registration.SignerName, "/", "-")) } // hash registration configuration, install namespace and addOnAgentRunningOutsideManagedCluster. Use the hash // value as the key of map to make sure each registration configuration and addon installation option is unique hash, err := getConfigHash( registration, - config.addonInstallOption) + config.addonInstallOption, + kubeClientDriver) if err != nil { return configs, err } @@ -119,8 +122,23 @@ func getRegistrationConfigs(addOn *addonv1alpha1.ManagedClusterAddOn) (map[strin return configs, nil } -func getConfigHash(registration addonv1alpha1.RegistrationConfig, installOption addonInstallOption) (string, error) { - data, err := json.Marshal(registration) +func getConfigHash(registration addonv1alpha1.RegistrationConfig, installOption addonInstallOption, kubeClientDriver string) (string, error) { + // Create a canonical config for hashing, excluding status fields set by the agent. + // Driver is always excluded (set by agent as status, not configuration) + // Subject is excluded only for token-based authentication (set by token driver) + // Subject is included for CSR-based and custom signer authentication (part of the configuration) + canonicalConfig := addonv1alpha1.RegistrationConfig{ + SignerName: registration.SignerName, + } + + // For KubeClient type registrations, check if driver is token + // For custom signers, always include subject + isKubeClientType := registration.SignerName == certificatesv1.KubeAPIServerClientSignerName + if !isKubeClientType || kubeClientDriver != "token" { + canonicalConfig.Subject = registration.Subject + } + + data, err := json.Marshal(canonicalConfig) if err != nil { return "", err } diff --git a/pkg/registration/spoke/addon/configuration_test.go b/pkg/registration/spoke/addon/configuration_test.go index 2a985e25b..71a132751 100644 --- a/pkg/registration/spoke/addon/configuration_test.go +++ b/pkg/registration/spoke/addon/configuration_test.go @@ -129,7 +129,11 @@ func TestGetRegistrationConfigs(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - configs, err := getRegistrationConfigs(c.addon) + installOption := addonInstallOption{ + AgentRunningOutsideManagedCluster: isAddonRunningOutsideManagedCluster(c.addon), + InstallationNamespace: getAddOnInstallationNamespace(c.addon), + } + configs, err := getRegistrationConfigs(c.addon.Name, installOption, c.addon.Status.Registrations, c.addon.Status.KubeClientDriver) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -165,8 +169,191 @@ func newRegistrationConfig(addOnName, addOnNamespace, signerName, commonName str registration: registration, } - hash, _ := getConfigHash(registration, config.addonInstallOption) + hash, _ := getConfigHash(registration, config.addonInstallOption, "") config.hash = hash return config } + +// TestConfigHash_StatusFieldsExcluded verifies that driver is always excluded from hash, +// and subject field is conditionally excluded based on driver type +func TestConfigHash_StatusFieldsExcluded(t *testing.T) { + installOption := addonInstallOption{ + InstallationNamespace: "test-ns", + AgentRunningOutsideManagedCluster: false, + } + + cases := []struct { + name string + config1 addonv1alpha1.RegistrationConfig + driver1 string + config2 addonv1alpha1.RegistrationConfig + driver2 string + expectEqual bool + }{ + { + name: "driver field always excluded - csr vs token with same subject", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver1: "csr", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver2: "token", + expectEqual: false, // Different because csr includes subject, token excludes it + }, + { + name: "token driver excludes subject - different subjects should have same hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user-1", + Groups: []string{"test-group-1"}, + }, + }, + driver1: "token", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user-2", + Groups: []string{"test-group-2"}, + }, + }, + driver2: "token", + expectEqual: true, // Same hash because subject is excluded for token driver + }, + { + name: "csr driver includes subject - different subjects should have different hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user-1", + Groups: []string{"test-group-1"}, + }, + }, + driver1: "csr", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user-2", + Groups: []string{"test-group-2"}, + }, + }, + driver2: "csr", + expectEqual: false, // Different hash because subject is included for csr driver + }, + { + name: "csr driver includes subject - same subjects should have same hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver1: "csr", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver2: "csr", + expectEqual: true, + }, + { + name: "custom signer includes subject - different subjects should have different hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: "custom.signer.io/custom", + Subject: addonv1alpha1.Subject{ + User: "test-user-1", + Groups: []string{"test-group-1"}, + }, + }, + driver1: "", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: "custom.signer.io/custom", + Subject: addonv1alpha1.Subject{ + User: "test-user-2", + Groups: []string{"test-group-2"}, + }, + }, + driver2: "", + expectEqual: false, // Different hash because subject is included for custom signer + }, + { + name: "custom signer includes subject - same subjects should have same hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: "custom.signer.io/custom", + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver1: "", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: "custom.signer.io/custom", + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver2: "", + expectEqual: true, + }, + { + name: "signer name changes hash", + config1: addonv1alpha1.RegistrationConfig{ + SignerName: certificates.KubeAPIServerClientSignerName, + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver1: "csr", + config2: addonv1alpha1.RegistrationConfig{ + SignerName: "custom.signer.io/custom", + Subject: addonv1alpha1.Subject{ + User: "test-user", + Groups: []string{"test-group"}, + }, + }, + driver2: "", + expectEqual: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + hash1, err := getConfigHash(c.config1, installOption, c.driver1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + hash2, err := getConfigHash(c.config2, installOption, c.driver2) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if c.expectEqual { + if hash1 != hash2 { + t.Errorf("expected hashes to be equal, got:\nhash1=%s\nhash2=%s", hash1, hash2) + } + } else { + if hash1 == hash2 { + t.Errorf("expected hashes to differ, got same hash: %s", hash1) + } + } + }) + } +} diff --git a/pkg/registration/spoke/addon/registration_controller.go b/pkg/registration/spoke/addon/registration_controller.go index 8734f8468..61252d18e 100644 --- a/pkg/registration/spoke/addon/registration_controller.go +++ b/pkg/registration/spoke/addon/registration_controller.go @@ -38,9 +38,12 @@ type addOnRegistrationController struct { hubAddOnLister addonlisterv1alpha1.ManagedClusterAddOnLister patcher patcher.Patcher[ *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus] - addonDriver register.AddonDriver + addonDriverFactory register.AddonDriverFactory + // addonAuthConfig holds the cluster-wide addon registration configuration + // that provides authentication method and access to driver options (CSR, Token) + addonAuthConfig register.AddonAuthConfig - startRegistrationFunc func(ctx context.Context, config registrationConfig) context.CancelFunc + startRegistrationFunc func(ctx context.Context, config registrationConfig) (context.CancelFunc, error) // registrationConfigs maps the addon name to a map of registrationConfigs whose key is the hash of // the registrationConfig @@ -55,7 +58,8 @@ func NewAddOnRegistrationController( addOnClient addonclient.Interface, managementKubeClient kubernetes.Interface, managedKubeClient kubernetes.Interface, - addonDriver register.AddonDriver, + addonDriverFactory register.AddonDriverFactory, + addonAuthConfig register.AddonAuthConfig, hubAddOnInformers addoninformerv1alpha1.ManagedClusterAddOnInformer, ) factory.Controller { c := &addOnRegistrationController{ @@ -65,7 +69,8 @@ func NewAddOnRegistrationController( managementKubeClient: managementKubeClient, spokeKubeClient: managedKubeClient, hubAddOnLister: hubAddOnInformers.Lister(), - addonDriver: addonDriver, + addonDriverFactory: addonDriverFactory, + addonAuthConfig: addonAuthConfig, patcher: patcher.NewPatcher[ *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus]( addOnClient.AddonV1alpha1().ManagedClusterAddOns(clusterName)), @@ -127,14 +132,24 @@ func (c *addOnRegistrationController) syncAddOn(ctx context.Context, syncCtx fac return nil } - cachedConfigs := c.addOnRegistrationConfigs[addOnName] - configs, err := getRegistrationConfigs(addOn) + // Ensure driver field is set for kubeClient type registrations + // If updated, return early to allow re-sync with the updated state + if updated, err := c.ensureDriver(ctx, addOn); err != nil || updated { + return err + } + + installOption := addonInstallOption{ + AgentRunningOutsideManagedCluster: isAddonRunningOutsideManagedCluster(addOn), + InstallationNamespace: getAddOnInstallationNamespace(addOn), + } + configs, err := getRegistrationConfigs(addOnName, installOption, addOn.Status.Registrations, addOn.Status.KubeClientDriver) if err != nil { return err } // stop registration for the stale registration configs var errs []error + cachedConfigs := c.addOnRegistrationConfigs[addOnName] for hash, cachedConfig := range cachedConfigs { if _, ok := configs[hash]; ok { continue @@ -152,27 +167,32 @@ func (c *addOnRegistrationController) syncAddOn(ctx context.Context, syncCtx fac for hash, config := range configs { // keep the unchanged configs if cachedConfig, ok := cachedConfigs[hash]; ok { + // Hash matches - keep the existing controller running + // Note: Driver is always excluded from hash, Subject is conditionally included based on driver type syncedConfigs[hash] = cachedConfig continue } // start registration for the new added configs - config.stopFunc = c.startRegistrationFunc(ctx, config) + stopFunc, err := c.startRegistrationFunc(ctx, config) + if err != nil { + errs = append(errs, err) + continue + } + config.stopFunc = stopFunc syncedConfigs[hash] = config } if len(syncedConfigs) == 0 { delete(c.addOnRegistrationConfigs, addOnName) - return nil + return operatorhelpers.NewMultiLineAggregate(errs) } c.addOnRegistrationConfigs[addOnName] = syncedConfigs - return nil + return operatorhelpers.NewMultiLineAggregate(errs) } // startRegistration starts a client certificate controller with the given config -func (c *addOnRegistrationController) startRegistration(ctx context.Context, config registrationConfig) context.CancelFunc { - ctx, stopFunc := context.WithCancel(ctx) - +func (c *addOnRegistrationController) startRegistration(ctx context.Context, config registrationConfig) (context.CancelFunc, error) { // the kubeClient here will be used to generate the hub kubeconfig secret for addon agents, it generates the secret // on the managed cluster by default, but if the addon agent is not running on the managed cluster(in Hosted mode // the addon agent runs outside the managed cluster, for more details see the hosted mode design docs for addon: @@ -197,7 +217,16 @@ func (c *addOnRegistrationController) startRegistration(ctx context.Context, con if config.registration.SignerName == certificatesv1.KubeAPIServerClientSignerName { secretOption.BootStrapKubeConfigFile = c.kubeconfigFile } - driver := c.addonDriver.Fork(config.addOnName, secretOption) + + // Pass AddonAuthConfig directly to Fork + // It provides authentication method and access to driver options (CSR, Token) + // Registration type (KubeClient vs CustomSigner) is determined by Fork implementation + // based on secretOption.Signer + driver, err := c.addonDriverFactory.Fork(config.addOnName, c.addonAuthConfig, secretOption) + if err != nil { + return nil, fmt.Errorf("failed to create driver for addon %s: %w", config.addOnName, err) + } + controllerName := fmt.Sprintf("ClientCertController@addon:%s:signer:%s", config.addOnName, config.registration.SignerName) statusUpdater := c.generateStatusUpdate(c.clusterName, config.addOnName) secretController := register.NewSecretController( @@ -206,10 +235,11 @@ func (c *addOnRegistrationController) startRegistration(ctx context.Context, con kubeInformerFactory.Core().V1().Secrets().Informer(), controllerName) + ctx, stopFunc := context.WithCancel(ctx) go kubeInformerFactory.Start(ctx.Done()) go secretController.Run(ctx, 1) - return stopFunc + return stopFunc, nil } func (c *addOnRegistrationController) generateStatusUpdate(clusterName, addonName string) register.StatusUpdateFunc { @@ -250,6 +280,41 @@ func (c *addOnRegistrationController) stopRegistration(ctx context.Context, conf return nil } +// ensureDriver sets Status.KubeClientDriver based on agent configuration to declare +// the authentication capability (csr or token) to the hub controller. +// Only sets the driver if the addon has a KubeClient type registration (KubeAPIServerClientSignerName). +// For custom signers, the driver is set to empty string. +func (c *addOnRegistrationController) ensureDriver(ctx context.Context, addon *addonv1alpha1.ManagedClusterAddOn) (bool, error) { + logger := klog.FromContext(ctx) + + // Check if addon has any KubeClient type registrations + hasKubeClientRegistration := false + for _, reg := range addon.Status.Registrations { + if reg.SignerName == certificatesv1.KubeAPIServerClientSignerName { + hasKubeClientRegistration = true + break + } + } + + addonCopy := addon.DeepCopy() + if hasKubeClientRegistration { + addonCopy.Status.KubeClientDriver = c.addonAuthConfig.GetKubeClientAuth() + } else { + addonCopy.Status.KubeClientDriver = "" + } + + updated, err := c.patcher.PatchStatus(ctx, addonCopy, addonCopy.Status, addon.Status) + if err != nil { + return false, err + } + + if updated { + logger.Info("Updated kubeClientDriver in status", "addon", addon.Name, "driver", addonCopy.Status.KubeClientDriver, "hasKubeClientRegistration", hasKubeClientRegistration) + } + + return updated, nil +} + // cleanup cleans both the registration configs and client certificate controllers for the addon func (c *addOnRegistrationController) cleanup(ctx context.Context, addOnName string) error { var errs []error diff --git a/pkg/registration/spoke/addon/registration_controller_test.go b/pkg/registration/spoke/addon/registration_controller_test.go index 0df03bbcf..87db4b771 100644 --- a/pkg/registration/spoke/addon/registration_controller_test.go +++ b/pkg/registration/spoke/addon/registration_controller_test.go @@ -14,10 +14,19 @@ import ( addonfake "open-cluster-management.io/api/client/addon/clientset/versioned/fake" addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" "open-cluster-management.io/sdk-go/pkg/basecontroller/factory" + "open-cluster-management.io/sdk-go/pkg/patcher" testingcommon "open-cluster-management.io/ocm/pkg/common/testing" + "open-cluster-management.io/ocm/pkg/registration/register" + registertesting "open-cluster-management.io/ocm/pkg/registration/register/testing" ) +type testDriverFactory struct{} + +func (f *testDriverFactory) Fork(addonName string, authConfig register.AddonAuthConfig, secretOption register.SecretOption) (register.RegisterDriver, error) { + return nil, nil +} + func TestRegistrationSync(t *testing.T) { clusterName := "cluster1" signerName := "signer1" @@ -327,18 +336,43 @@ func TestRegistrationSync(t *testing.T) { managementKubeClient: managementClient, spokeKubeClient: kubeClient, hubAddOnLister: addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns().Lister(), - startRegistrationFunc: func(ctx context.Context, config registrationConfig) context.CancelFunc { + patcher: patcher.NewPatcher[ + *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus]( + addonClient.AddonV1alpha1().ManagedClusterAddOns(clusterName)), + addonDriverFactory: &testDriverFactory{}, + startRegistrationFunc: func(ctx context.Context, config registrationConfig) (context.CancelFunc, error) { _, cancel := context.WithCancel(context.Background()) - return cancel + return cancel, nil }, addOnRegistrationConfigs: c.addOnRegistrationConfigs, + addonAuthConfig: ®istertesting.TestAddonAuthConfig{ + KubeClientAuth: "csr", + }, } + // First sync: sets the driver and returns early (status updated) err := controller.sync(context.Background(), testingcommon.NewFakeSyncContext(t, c.queueKey), c.queueKey) if err != nil { t.Errorf("unexpected error: %v", err) } + // Second sync: processes registrations (only if addon has registrations to process) + // The condition checks if there are registrations to test, not whether driver is set + if c.addOn != nil && len(c.addOn.Status.Registrations) > 0 { + // Update addon in store with driver set (simulating informer update) + updatedAddOn := c.addOn.DeepCopy() + updatedAddOn.Status.KubeClientDriver = "csr" + if err := addonStore.Update(updatedAddOn); err != nil { + t.Fatal(err) + } + + // Sync again to process registrations + err = controller.sync(context.Background(), testingcommon.NewFakeSyncContext(t, c.queueKey), c.queueKey) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + if len(c.expectedAddOnRegistrationConfigHashs) != len(controller.addOnRegistrationConfigs) { t.Errorf("expected %d addOns, but got %d", len(c.expectedAddOnRegistrationConfigHashs), len(controller.addOnRegistrationConfigs)) @@ -404,6 +438,6 @@ func hash(registration addonv1alpha1.RegistrationConfig, installNamespace string h, _ := getConfigHash(registration, addonInstallOption{ InstallationNamespace: installNamespace, AgentRunningOutsideManagedCluster: addOnAgentRunningOutsideManagedCluster, - }) + }, "") return h } diff --git a/pkg/registration/spoke/spokeagent.go b/pkg/registration/spoke/spokeagent.go index 4cdbceb1f..f88e4c702 100644 --- a/pkg/registration/spoke/spokeagent.go +++ b/pkg/registration/spoke/spokeagent.go @@ -414,8 +414,8 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, AddOnLeaseControllerSyncInterval, //TODO: this interval time should be allowed to change from outside ) - // addon registration only enabled when the registration driver is csr. - if addonDriver, ok := o.driver.(register.AddonDriver); ok { + // addon registration enabled when AddonDriverFactory is provided (supports CSR and token-based drivers) + if addonDriverFactory, ok := o.driver.(register.AddonDriverFactory); ok { addOnRegistrationController = addon.NewAddOnRegistrationController( o.agentOptions.SpokeClusterName, o.agentOptions.AgentID, @@ -423,7 +423,8 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, hubClient.AddonClient, managementKubeClient, spokeKubeClient, - addonDriver, + addonDriverFactory, + o.registrationOption.RegisterDriverOption, hubClient.AddonInformer, ) } @@ -475,8 +476,9 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, go managedClusterHealthCheckController.Run(ctx, 1) if features.SpokeMutableFeatureGate.Enabled(ocmfeature.AddonManagement) { go addOnLeaseController.Run(ctx, 1) - // addon controller will only run when the registration driver is csr. - if _, ok := o.driver.(register.AddonDriver); ok { + // addon registration controller runs when the driver implements AddonDriverFactory + // (supports CSR, GRPC, and AWS IRSA drivers with both CSR and token-based authentication) + if _, ok := o.driver.(register.AddonDriverFactory); ok { go addOnRegistrationController.Run(ctx, 1) } } diff --git a/test/integration/registration/addon_registration_test.go b/test/integration/registration/addon_registration_test.go index ed86033bc..35d84d139 100644 --- a/test/integration/registration/addon_registration_test.go +++ b/test/integration/registration/addon_registration_test.go @@ -237,11 +237,9 @@ var _ = ginkgo.Describe("Addon Registration", func() { return err } - addOn.Status = addonv1alpha1.ManagedClusterAddOnStatus{ - Registrations: []addonv1alpha1.RegistrationConfig{ - { - SignerName: signerName, - }, + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, }, } _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). @@ -335,17 +333,19 @@ var _ = ginkgo.Describe("Addon Registration", func() { _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Create(context.TODO(), addOn, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - created, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - created.Status = addonv1alpha1.ManagedClusterAddOnStatus{ - Registrations: []addonv1alpha1.RegistrationConfig{ + gomega.Eventually(func() error { + created, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + created.Status.Registrations = []addonv1alpha1.RegistrationConfig{ { SignerName: signerName, }, - }, - } - _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), created, metav1.UpdateOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), created, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) assertSuccessCSRApproval() @@ -392,18 +392,20 @@ var _ = ginkgo.Describe("Addon Registration", func() { assertSuccessAddOnBootstrap(signerName) // update registration config and change the signer - addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) newSignerName := "example.com/signer1" - addOn.Status = addonv1alpha1.ManagedClusterAddOnStatus{ - Registrations: []addonv1alpha1.RegistrationConfig{ + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ { SignerName: newSignerName, }, - }, - } - _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) assertSecretGone(addOnName, getSecretName(addOnName, signerName)) assertSuccessCSRApproval() @@ -437,20 +439,26 @@ var _ = ginkgo.Describe("Addon Registration", func() { // update subject for 15 times for i := 1; i <= 15; i++ { - addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - addOn.Status = addonv1alpha1.ManagedClusterAddOnStatus{ - Registrations: []addonv1alpha1.RegistrationConfig{ + currentIndex := i + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + if len(addOn.Status.Registrations) == 0 { + return fmt.Errorf("no registrations found") + } + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ { SignerName: addOn.Status.Registrations[0].SignerName, Subject: addonv1alpha1.Subject{ - User: fmt.Sprintf("test-%d", i), + User: fmt.Sprintf("test-%d", currentIndex), }, }, - }, - } - _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) // sleep 1 second to ensure controller issue a new csr. time.Sleep(1 * time.Second) } diff --git a/test/integration/registration/addon_token_registration_test.go b/test/integration/registration/addon_token_registration_test.go new file mode 100644 index 000000000..e419eba44 --- /dev/null +++ b/test/integration/registration/addon_token_registration_test.go @@ -0,0 +1,654 @@ +package registration_test + +import ( + "context" + "fmt" + "path" + "reflect" + "time" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + "github.com/openshift/library-go/pkg/controller/controllercmd" + certificates "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + clusterv1 "open-cluster-management.io/api/cluster/v1" + + "open-cluster-management.io/ocm/pkg/addon" + commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" + commonoptions "open-cluster-management.io/ocm/pkg/common/options" + "open-cluster-management.io/ocm/pkg/registration/register" + "open-cluster-management.io/ocm/pkg/registration/register/csr" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" + "open-cluster-management.io/ocm/pkg/registration/register/token" + "open-cluster-management.io/ocm/pkg/registration/spoke" + "open-cluster-management.io/ocm/test/integration/util" +) + +var _ = ginkgo.Describe("Addon Token Registration", func() { + var managedClusterName, hubKubeconfigSecret, hubKubeconfigDir, addOnName string + var err error + var cancel context.CancelFunc + var cancelAddonManager context.CancelFunc + var bootstrapKubeconfig string + var expectedProxyURL string + var signerName = certificates.KubeAPIServerClientSignerName + + ginkgo.BeforeEach(func() { + // Start the addon manager which includes the tokenInfrastructureController + // This controller creates ServiceAccounts for token-based authentication + addonMgrCtx, addonMgrCancel := context.WithCancel(context.Background()) + cancelAddonManager = addonMgrCancel + + go func() { + defer ginkgo.GinkgoRecover() + err := addon.RunManager(addonMgrCtx, &controllercmd.ControllerContext{ + KubeConfig: hubCfg, + EventRecorder: util.NewIntegrationTestEventRecorder("addon-manager"), + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }() + }) + + ginkgo.JustBeforeEach(func() { + suffix := rand.String(5) + managedClusterName = fmt.Sprintf("token-cluster-%s", suffix) + hubKubeconfigSecret = fmt.Sprintf("token-hub-kubeconfig-secret-%s", suffix) + hubKubeconfigDir = path.Join(util.TestDir, fmt.Sprintf("token-addontest-%s", suffix), "hub-kubeconfig") + addOnName = fmt.Sprintf("token-addon-%s", suffix) + + // Create agent options with token authentication for addons + driverOption := registerfactory.NewOptions() + driverOption.AddonKubeClientRegistrationAuth = "token" // Use token-based authentication for addons + + agentOptions := &spoke.SpokeAgentOptions{ + BootstrapKubeconfig: bootstrapKubeconfig, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: driverOption, + } + + commOptions := commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + // run registration agent + cancel = runAgent("token-addontest", agentOptions, commOptions, spokeCfg) + }) + + ginkgo.AfterEach(func() { + if cancel != nil { + cancel() + } + if cancelAddonManager != nil { + cancelAddonManager() + } + }) + + assertSuccessClusterBootstrap := func() { + // the spoke cluster and csr should be created after bootstrap + ginkgo.By("Check existence of ManagedCluster & CSR") + gomega.Eventually(func() error { + if _, err := util.GetManagedCluster(clusterClient, managedClusterName); err != nil { + return err + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + gomega.Eventually(func() error { + if _, err := util.FindUnapprovedSpokeCSR(kubeClient, managedClusterName); err != nil { + return err + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + // the spoke cluster should has finalizer that is added by hub controller + gomega.Eventually(func() bool { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + + if !commonhelpers.HasFinalizer(spokeCluster.Finalizers, clusterv1.ManagedClusterFinalizer) { + return false + } + + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + ginkgo.By("Accept and approve the ManagedCluster") + // simulate hub cluster admin to accept the managedcluster and approve the csr + err = util.AcceptManagedCluster(clusterClient, managedClusterName) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + err = authn.ApproveSpokeClusterCSR(kubeClient, managedClusterName, time.Hour*24) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // the managed cluster should have accepted condition after it is accepted + gomega.Eventually(func() bool { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + accepted := meta.FindStatusCondition(spokeCluster.Status.Conditions, clusterv1.ManagedClusterConditionHubAccepted) + return accepted != nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + // the hub kubeconfig secret should be filled after the csr is approved + gomega.Eventually(func() bool { + if _, err := util.GetFilledHubKubeConfigSecret(kubeClient, testNamespace, hubKubeconfigSecret); err != nil { + return false + } + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + ginkgo.By("ManagedCluster joins the hub") + // the spoke cluster should have joined condition finally + gomega.Eventually(func() bool { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + joined := meta.FindStatusCondition(spokeCluster.Status.Conditions, clusterv1.ManagedClusterConditionJoined) + return joined != nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + // ensure cluster namespace is in place + gomega.Eventually(func() bool { + _, err := kubeClient.CoreV1().Namespaces().Get(context.TODO(), managedClusterName, metav1.GetOptions{}) + return err == nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertValidTokenCredential := func(secretNamespace, secretName, expectedProxyURL string) { + ginkgo.By("Check token credential in secret") + gomega.Eventually(func() bool { + secret, err := kubeClient.CoreV1().Secrets(secretNamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) + if err != nil { + return false + } + // Verify token exists + if _, ok := secret.Data[token.TokenFile]; !ok { + return false + } + // Verify kubeconfig with token + kubeconfigData, ok := secret.Data[register.KubeconfigFile] + if !ok { + return false + } + + if expectedProxyURL != "" { + proxyURL, err := getProxyURLFromKubeconfigData(kubeconfigData) + if err != nil { + return false + } + return proxyURL == expectedProxyURL + } + + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertAddonLabel := func(clusterName, addonName, status string) { + ginkgo.By("Check addon status label on managed cluster") + gomega.Eventually(func() bool { + cluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + if len(cluster.Labels) == 0 { + return false + } + key := fmt.Sprintf("feature.open-cluster-management.io/addon-%s", addonName) + return cluster.Labels[key] == status + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertTokenRefreshedCondition := func(clusterName, addonName string) { + ginkgo.By("Check token refreshed addon status condition") + gomega.Eventually(func() bool { + addon, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(clusterName). + Get(context.TODO(), addonName, metav1.GetOptions{}) + if err != nil { + return false + } + return meta.IsStatusConditionTrue(addon.Status.Conditions, token.TokenRefreshedCondition) + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertSuccessCSRApproval := func() { + ginkgo.By("Approve bootstrap csr") + var csr *certificates.CertificateSigningRequest + gomega.Eventually(func() bool { + csr, err = util.FindUnapprovedAddOnCSR(kubeClient, managedClusterName, addOnName) + return err == nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + now := time.Now() + err = authn.ApproveCSR(kubeClient, csr, now.UTC(), now.Add(30*time.Second).UTC()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + + assertValidClientCertificate := func(secretNamespace, secretName, expectedProxyURL string) { + ginkgo.By("Check client certificate in secret") + gomega.Eventually(func() bool { + secret, err := kubeClient.CoreV1().Secrets(secretNamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) + if err != nil { + return false + } + if _, ok := secret.Data[csr.TLSKeyFile]; !ok { + return false + } + if _, ok := secret.Data[csr.TLSCertFile]; !ok { + return false + } + kubeconfigData, ok := secret.Data[register.KubeconfigFile] + if !ok { + return false + } + + if expectedProxyURL != "" { + proxyURL, err := getProxyURLFromKubeconfigData(kubeconfigData) + if err != nil { + return false + } + return proxyURL == expectedProxyURL + } + + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertHasNoAddonLabel := func(clusterName, addonName string) { + ginkgo.By("Check if addon status label on managed cluster deleted") + gomega.Eventually(func() bool { + cluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + if len(cluster.Labels) == 0 { + return true + } + key := fmt.Sprintf("feature.open-cluster-management.io/addon-%s", addonName) + _, ok := cluster.Labels[key] + return !ok + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertAddOnSignerUpdate := func(signerName string) { + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + } + + assertSuccessAddOnEnabling := func() { + ginkgo.By("Create ManagedClusterAddOn cr for token authentication") + // create addon namespace + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: addOnName, + }, + } + _, err = kubeClient.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // create addon + addOn := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: addOnName, + Namespace: managedClusterName, + }, + Spec: addonv1alpha1.ManagedClusterAddOnSpec{ + InstallNamespace: addOnName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Create(context.TODO(), addOn, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + _, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + + assertSuccessAddOnBootstrap := func(signerName string) { + assertSuccessAddOnEnabling() + assertAddOnSignerUpdate(signerName) + assertValidTokenCredential(addOnName, getSecretName(addOnName, signerName), expectedProxyURL) + assertAddonLabel(managedClusterName, addOnName, "unreachable") + assertTokenRefreshedCondition(managedClusterName, addOnName) + } + + assertSecretGone := func(secretNamespace, secretName string) { + gomega.Eventually(func() bool { + _, err = kubeClient.CoreV1().Secrets(secretNamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) + return errors.IsNotFound(err) + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + } + + assertRegistrationSucceed := func() { + ginkgo.It("should register addon with token successfully", func() { + assertSuccessClusterBootstrap() + assertSuccessAddOnBootstrap(signerName) + + ginkgo.By("Delete the addon and check if secret is gone") + err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Delete(context.TODO(), addOnName, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + assertSecretGone(addOnName, getSecretName(addOnName, signerName)) + + assertHasNoAddonLabel(managedClusterName, addOnName) + }) + } + + ginkgo.Context("without proxy", func() { + ginkgo.BeforeEach(func() { + bootstrapKubeconfig = bootstrapKubeConfigFile + expectedProxyURL = "" + }) + assertRegistrationSucceed() + + ginkgo.It("should register addon with token successfully even when the install namespace is not available at the beginning", func() { + assertSuccessClusterBootstrap() + + ginkgo.By("Create ManagedClusterAddOn cr for token authentication") + + // create addon + addOn := &addonv1alpha1.ManagedClusterAddOn{ + ObjectMeta: metav1.ObjectMeta{ + Name: addOnName, + Namespace: managedClusterName, + }, + Spec: addonv1alpha1.ManagedClusterAddOnSpec{ + InstallNamespace: addOnName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Create(context.TODO(), addOn, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + gomega.Eventually(func() error { + created, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + created.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), created, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + ginkgo.By("Wait for addon namespace - token should not be created yet") + gomega.Consistently(func() bool { + _, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), getSecretName(addOnName, signerName), metav1.GetOptions{}) + return errors.IsNotFound(err) + }, 10*time.Second, 2*time.Second).Should(gomega.BeTrue()) + + ginkgo.By("Create addon namespace") + // create addon namespace + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: addOnName, + }, + } + _, err = kubeClient.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + assertValidTokenCredential(addOnName, getSecretName(addOnName, signerName), expectedProxyURL) + + ginkgo.By("Delete the addon and check if secret is gone") + err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Delete(context.TODO(), addOnName, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + assertSecretGone(addOnName, getSecretName(addOnName, signerName)) + }) + + ginkgo.It("should rotate token successfully", func() { + assertSuccessClusterBootstrap() + assertSuccessAddOnBootstrap(signerName) + + secretName := getSecretName(addOnName, signerName) + secret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + originalToken := secret.Data[token.TokenFile] + gomega.Expect(originalToken).NotTo(gomega.BeNil()) + + ginkgo.By("Trigger token rotation by deleting the ServiceAccount") + // ServiceAccount naming convention: -agent + serviceAccountName := fmt.Sprintf("%s-agent", addOnName) + err = kubeClient.CoreV1().ServiceAccounts(managedClusterName).Delete(context.TODO(), serviceAccountName, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Wait for ServiceAccount to be recreated") + gomega.Eventually(func() bool { + _, err := kubeClient.CoreV1().ServiceAccounts(managedClusterName).Get(context.TODO(), serviceAccountName, metav1.GetOptions{}) + return err == nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + ginkgo.By("Wait for token rotation") + gomega.Eventually(func() bool { + newSecret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + if err != nil { + return false + } + + newToken := newSecret.Data[token.TokenFile] + if newToken == nil { + return false + } + + // Verify the token has changed + return !reflect.DeepEqual(originalToken, newToken) + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + }) + + ginkgo.It("should switch from token to CSR driver successfully", func() { + assertSuccessClusterBootstrap() + assertSuccessAddOnBootstrap(signerName) + + secretName := getSecretName(addOnName, signerName) + + ginkgo.By("Verify token-based secret exists") + secret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(secret.Data[token.TokenFile]).NotTo(gomega.BeNil()) + + ginkgo.By("Stop the agent and restart with CSR driver") + cancel() + + // Restart agent with CSR driver + driverOption := registerfactory.NewOptions() + driverOption.AddonKubeClientRegistrationAuth = "csr" // Switch to CSR-based authentication + + agentOptions := &spoke.SpokeAgentOptions{ + BootstrapKubeconfig: bootstrapKubeconfig, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: driverOption, + } + + commOptions := commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + cancel = runAgent("token-to-csr-test", agentOptions, commOptions, spokeCfg) + + ginkgo.By("Update addon registration with subject for CSR") + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + ginkgo.By("Approve CSR and verify client certificate") + assertSuccessCSRApproval() + assertValidClientCertificate(addOnName, secretName, expectedProxyURL) + + ginkgo.By("Verify CSR credentials exist in secret") + gomega.Eventually(func() bool { + secret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + if err != nil { + return false + } + _, hasCert := secret.Data[csr.TLSCertFile] + _, hasKey := secret.Data[csr.TLSKeyFile] + // Cert and key should exist (token may remain as leftover) + return hasCert && hasKey + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + }) + + ginkgo.It("should switch from CSR to token driver successfully", func() { + ginkgo.By("Start with CSR driver") + cancel() + + // Start agent with CSR driver + driverOption := registerfactory.NewOptions() + driverOption.AddonKubeClientRegistrationAuth = "csr" + + agentOptions := &spoke.SpokeAgentOptions{ + BootstrapKubeconfig: bootstrapKubeconfig, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: driverOption, + } + + commOptions := commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + cancel = runAgent("csr-to-token-test", agentOptions, commOptions, spokeCfg) + + assertSuccessClusterBootstrap() + + ginkgo.By("Create addon with CSR-based registration") + assertSuccessAddOnEnabling() + + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + secretName := getSecretName(addOnName, signerName) + + ginkgo.By("Approve CSR and verify client certificate") + assertSuccessCSRApproval() + assertValidClientCertificate(addOnName, secretName, expectedProxyURL) + + ginkgo.By("Verify CSR-based secret exists") + secret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(secret.Data[csr.TLSCertFile]).NotTo(gomega.BeNil()) + gomega.Expect(secret.Data[csr.TLSKeyFile]).NotTo(gomega.BeNil()) + + ginkgo.By("Stop the agent and restart with token driver") + cancel() + + // Restart agent with token driver + driverOption = registerfactory.NewOptions() + driverOption.AddonKubeClientRegistrationAuth = "token" + + agentOptions = &spoke.SpokeAgentOptions{ + BootstrapKubeconfig: bootstrapKubeconfig, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: driverOption, + } + + commOptions = commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + cancel = runAgent("csr-to-token-test", agentOptions, commOptions, spokeCfg) + + ginkgo.By("Update addon registration to remove subject (for token)") + gomega.Eventually(func() error { + addOn, err := addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + + addOn.Status.Registrations = []addonv1alpha1.RegistrationConfig{ + { + SignerName: signerName, + // No subject - will be set by token driver + }, + } + _, err = addOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName). + UpdateStatus(context.TODO(), addOn, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + ginkgo.By("Verify token credential is created") + assertValidTokenCredential(addOnName, secretName, expectedProxyURL) + assertTokenRefreshedCondition(managedClusterName, addOnName) + + ginkgo.By("Verify token credentials exist in secret") + gomega.Eventually(func() bool { + secret, err := kubeClient.CoreV1().Secrets(addOnName).Get(context.TODO(), secretName, metav1.GetOptions{}) + if err != nil { + return false + } + _, hasToken := secret.Data[token.TokenFile] + // Token should exist (cert and key may remain as leftover) + return hasToken + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + }) + }) + + ginkgo.Context("with http proxy", func() { + ginkgo.BeforeEach(func() { + bootstrapKubeconfig = bootstrapKubeConfigHTTPProxyFile + expectedProxyURL = httpProxyURL + }) + assertRegistrationSucceed() + }) + + ginkgo.Context("with https proxy", func() { + ginkgo.BeforeEach(func() { + bootstrapKubeconfig = bootstrapKubeConfigHTTPSProxyFile + expectedProxyURL = httpsProxyURL + }) + assertRegistrationSucceed() + }) +}) diff --git a/test/integration/registration/spokeagent_rebootstrap_test.go b/test/integration/registration/spokeagent_rebootstrap_test.go index ac549efa0..e4f8bab61 100644 --- a/test/integration/registration/spokeagent_rebootstrap_test.go +++ b/test/integration/registration/spokeagent_rebootstrap_test.go @@ -272,10 +272,12 @@ var _ = ginkgo.Describe("Rebootstrap", func() { _, err = hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Create(context.TODO(), addOn, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - created, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - created.Status = addonv1alpha1.ManagedClusterAddOnStatus{ - Registrations: []addonv1alpha1.RegistrationConfig{ + gomega.Eventually(func() error { + created, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.TODO(), addOnName, metav1.GetOptions{}) + if err != nil { + return err + } + created.Status.Registrations = []addonv1alpha1.RegistrationConfig{ { SignerName: signerName, Subject: addonv1alpha1.Subject{ @@ -285,10 +287,10 @@ var _ = ginkgo.Describe("Rebootstrap", func() { }, }, }, - }, - } - _, err = hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), created, metav1.UpdateOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + _, err = hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).UpdateStatus(context.TODO(), created, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) assertSuccessCSRApproval(managedClusterName, addOnName, hubKubeClient) assertValidClientCertificate(addOnName, getSecretName(addOnName, signerName), signerName, spokeKubeClient) diff --git a/test/integration/registration/spokeagent_restart_test.go b/test/integration/registration/spokeagent_restart_test.go index ac32abb63..55300dc8e 100644 --- a/test/integration/registration/spokeagent_restart_test.go +++ b/test/integration/registration/spokeagent_restart_test.go @@ -101,18 +101,22 @@ var _ = ginkgo.Describe("Agent Restart", func() { // remove the join condition. A new join condition will be added once the registration agent // is restarted successfully - spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - var conditions []metav1.Condition - for _, condition := range spokeCluster.Status.Conditions { - if condition.Type == clusterv1.ManagedClusterConditionJoined { - continue + gomega.Eventually(func() error { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err } - conditions = append(conditions, condition) - } - spokeCluster.Status.Conditions = conditions - _, err = clusterClient.ClusterV1().ManagedClusters().UpdateStatus(context.TODO(), spokeCluster, metav1.UpdateOptions{}) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + var conditions []metav1.Condition + for _, condition := range spokeCluster.Status.Conditions { + if condition.Type == clusterv1.ManagedClusterConditionJoined { + continue + } + conditions = append(conditions, condition) + } + spokeCluster.Status.Conditions = conditions + _, err = clusterClient.ClusterV1().ManagedClusters().UpdateStatus(context.TODO(), spokeCluster, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) ginkgo.By("Restart registration agent") agentOptions = &spoke.SpokeAgentOptions{