diff --git a/pkg/registration/register/aws_irsa/aws_irsa.go b/pkg/registration/register/aws_irsa/aws_irsa.go index 42c0af683..8916905cc 100644 --- a/pkg/registration/register/aws_irsa/aws_irsa.go +++ b/pkg/registration/register/aws_irsa/aws_irsa.go @@ -8,7 +8,6 @@ import ( "github.com/openshift/library-go/pkg/operator/events" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -35,18 +34,15 @@ type AWSIRSADriver struct { managedClusterArn string hubClusterArn string managedClusterRoleSuffix string + + awsIRSAControl AWSIRSAControl } func (c *AWSIRSADriver) Process( ctx context.Context, controllerName string, secret *corev1.Secret, additionalSecretData map[string][]byte, - recorder events.Recorder, opt any) (*corev1.Secret, *metav1.Condition, error) { + recorder events.Recorder) (*corev1.Secret, *metav1.Condition, error) { - awsOption, ok := opt.(*AWSOption) - if !ok { - return nil, nil, fmt.Errorf("option type is not correct") - } - - isApproved, err := awsOption.AWSIRSAControl.isApproved(c.name) + isApproved, err := c.awsIRSAControl.isApproved(c.name) if err != nil { return nil, nil, err } @@ -82,12 +78,8 @@ func (c *AWSIRSADriver) BuildKubeConfigFromTemplate(kubeConfig *clientcmdapi.Con return kubeConfig } -func (c *AWSIRSADriver) InformerHandler(option any) (cache.SharedIndexInformer, factory.EventFilterFunc) { - awsOption, ok := option.(*AWSOption) - if !ok { - utilruntime.Must(fmt.Errorf("option type is not correct")) - } - return awsOption.AWSIRSAControl.Informer(), awsOption.EventFilterFunc +func (c *AWSIRSADriver) InformerHandler() (cache.SharedIndexInformer, factory.EventFilterFunc) { + return c.awsIRSAControl.Informer(), nil } func (c *AWSIRSADriver) IsHubKubeConfigValid(ctx context.Context, secretOption register.SecretOption) (bool, error) { @@ -104,11 +96,23 @@ func (c *AWSIRSADriver) ManagedClusterDecorator(cluster *clusterv1.ManagedCluste return cluster } -func NewAWSIRSADriver(managedClusterArn string, managedClusterRoleSuffix string, hubClusterArn string, name string) register.RegisterDriver { +func (c *AWSIRSADriver) BuildClients(_ context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { + clients, err := register.BuildClientsFromSecretOption(secretOption, bootstrap) + if err != nil { + return nil, err + } + c.awsIRSAControl, err = NewAWSIRSAControl(clients.ClusterInfomerFactory.Cluster(), clients.ClusterClient) + if err != nil { + return nil, fmt.Errorf("failed to create AWS IRSA control: %w", err) + } + return clients, nil +} + +func NewAWSIRSADriver(opt *AWSOption, secretOption register.SecretOption) register.RegisterDriver { return &AWSIRSADriver{ - managedClusterArn: managedClusterArn, - managedClusterRoleSuffix: managedClusterRoleSuffix, - hubClusterArn: hubClusterArn, - name: name, + managedClusterArn: opt.ManagedClusterArn, + managedClusterRoleSuffix: opt.ManagedClusterRoleSuffix, + hubClusterArn: opt.HubClusterArn, + name: secretOption.ClusterName, } } diff --git a/pkg/registration/register/aws_irsa/aws_irsa_test.go b/pkg/registration/register/aws_irsa/aws_irsa_test.go index 238a0ab14..0458c3769 100644 --- a/pkg/registration/register/aws_irsa/aws_irsa_test.go +++ b/pkg/registration/register/aws_irsa/aws_irsa_test.go @@ -13,7 +13,6 @@ import ( kubefake "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" testingcommon "open-cluster-management.io/ocm/pkg/common/testing" @@ -55,11 +54,11 @@ func TestProcess(t *testing.T) { register.AgentNameFile: []byte(testAgentName), } - awsOption := &AWSOption{ - AWSIRSAControl: ctrl, - } + awsOption := &AWSOption{} - driver := &AWSIRSADriver{} + driver := &AWSIRSADriver{ + awsIRSAControl: ctrl, + } if c.approvedIrsaRequest != nil { driver.name = testIrsaName @@ -230,13 +229,13 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - driver := NewAWSIRSADriver("", "", "", "") secretOption := register.SecretOption{ ClusterName: c.clusterName, AgentName: c.agentName, HubKubeconfigDir: tempDir, HubKubeconfigFile: path.Join(tempDir, "kubeconfig"), } + driver := NewAWSIRSADriver(NewAWSOption(), secretOption) if c.kubeconfig != nil { testinghelpers.WriteFile(path.Join(tempDir, "kubeconfig"), c.kubeconfig) } @@ -247,11 +246,8 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { testinghelpers.WriteFile(path.Join(tempDir, "tls.crt"), c.tlsCert) } if c.bootstapKubeconfig != nil { - bootstrapKubeconfig, err := clientcmd.Load(c.bootstapKubeconfig) - if err != nil { - t.Fatal(err) - } - secretOption.BootStrapKubeConfig = bootstrapKubeconfig + testinghelpers.WriteFile(path.Join(tempDir, "bootstrap-kubeconfig"), c.bootstapKubeconfig) + secretOption.BootStrapKubeConfigFile = path.Join(tempDir, "bootstrap-kubeconfig") } valid, err := register.IsHubKubeConfigValidFunc(driver, secretOption)(context.TODO()) diff --git a/pkg/registration/register/aws_irsa/options.go b/pkg/registration/register/aws_irsa/options.go index 6ac4fd897..ddd7c4251 100644 --- a/pkg/registration/register/aws_irsa/options.go +++ b/pkg/registration/register/aws_irsa/options.go @@ -1,52 +1,34 @@ package aws_irsa import ( - "fmt" + "errors" - "github.com/openshift/library-go/pkg/controller/factory" - "k8s.io/apimachinery/pkg/api/meta" - - addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" - hubclusterclientset "open-cluster-management.io/api/client/cluster/clientset/versioned" - managedclusterinformers "open-cluster-management.io/api/client/cluster/informers/externalversions/cluster" - - "open-cluster-management.io/ocm/pkg/registration/register" + "github.com/spf13/pflag" ) // AWSOption includes options that is used to monitor ManagedClusters type AWSOption struct { - EventFilterFunc factory.EventFilterFunc - AWSIRSAControl AWSIRSAControl + HubClusterArn string + ManagedClusterArn string + ManagedClusterRoleSuffix string } -func NewAWSOption( - secretOption register.SecretOption, - hubManagedClusterInformer managedclusterinformers.Interface, - hubClusterClientSet hubclusterclientset.Interface) (*AWSOption, error) { - awsIrsaControl, err := NewAWSIRSAControl(hubManagedClusterInformer, hubClusterClientSet) - if err != nil { - return nil, fmt.Errorf("failed to create AWS IRSA control: %w", err) - } - if err != nil { - return nil, err - } - return &AWSOption{ - EventFilterFunc: func(obj interface{}) bool { - accessor, err := meta.Accessor(obj) - if err != nil { - return false - } - labels := accessor.GetLabels() +func NewAWSOption() *AWSOption { + return &AWSOption{} +} - // should not contain addon key - _, ok := labels[addonv1alpha1.AddonLabelKey] - if ok { - return false - } +func (o *AWSOption) AddFlags(fs *pflag.FlagSet) { + fs.StringVar(&o.HubClusterArn, "hub-cluster-arn", o.HubClusterArn, + "The ARN of the EKS based hub cluster.") + fs.StringVar(&o.ManagedClusterArn, "managed-cluster-arn", o.ManagedClusterArn, + "The ARN of the EKS based managed cluster.") + fs.StringVar(&o.ManagedClusterRoleSuffix, "managed-cluster-role-suffix", o.ManagedClusterRoleSuffix, + "The suffix of the managed cluster IAM role.") +} - // only enqueue csr whose name starts with the cluster name - return accessor.GetName() == secretOption.ClusterName - }, - AWSIRSAControl: awsIrsaControl, - }, nil +func (o *AWSOption) Validate() error { + if o.HubClusterArn == "" { + return errors.New("EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa") + } + return nil } diff --git a/pkg/registration/register/common.go b/pkg/registration/register/common.go index 978efbdbb..17e98cb22 100644 --- a/pkg/registration/register/common.go +++ b/pkg/registration/register/common.go @@ -5,17 +5,26 @@ import ( "fmt" "os" "reflect" + "time" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/klog/v2" + addonclient "open-cluster-management.io/api/client/addon/clientset/versioned" + addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" + clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned" hubclusterclientset "open-cluster-management.io/api/client/cluster/clientset/versioned" + clusterv1informers "open-cluster-management.io/api/client/cluster/informers/externalversions" clusterv1listers "open-cluster-management.io/api/client/cluster/listers/cluster/v1" clusterv1 "open-cluster-management.io/api/cluster/v1" "open-cluster-management.io/sdk-go/pkg/patcher" @@ -118,8 +127,12 @@ func IsHubKubeConfigValidFunc(driver RegisterDriver, secretOption SecretOption) return false, err } - if secretOption.BootStrapKubeConfig != nil { - if valid, err := IsHubKubeconfigValid(secretOption.BootStrapKubeConfig, hubKubeconfig); !valid || err != nil { + if secretOption.BootStrapKubeConfigFile != "" { + bootStrapConfig, err := clientcmd.LoadFromFile(secretOption.BootStrapKubeConfigFile) + if err != nil { + return false, err + } + if valid, err := IsHubKubeconfigValid(bootStrapConfig, hubKubeconfig); !valid || err != nil { return valid, err } } @@ -227,3 +240,68 @@ func (a *AggregatedHubDriver) Cleanup(ctx context.Context, cluster *clusterv1.Ma } return errors.NewAggregate(errs) } + +// Clients hold all client needed to connect to hub +type Clients struct { + ClusterClient clusterv1client.Interface + KubeClient kubernetes.Interface + AddonClient addonclient.Interface + ClusterInfomerFactory clusterv1informers.SharedInformerFactory + KubeInformerFactory informers.SharedInformerFactory + AddonInformerFactory addoninformers.SharedInformerFactory +} + +func BuildClientsFromSecretOption(s SecretOption, bootstrap bool) (*Clients, error) { + var kubeConfig *rest.Config + var err error + if bootstrap { + if s.BootStrapKubeConfigFile == "" { + return nil, fmt.Errorf("no bootstrap kubeconfig found") + } + + kubeConfig, err = clientcmd.BuildConfigFromFlags("", s.BootStrapKubeConfigFile) + if err != nil { + return nil, fmt.Errorf("unable to load bootstrap kubeconfig: %w", err) + } + } else { + kubeConfig, err = clientcmd.BuildConfigFromFlags("", s.HubKubeconfigFile) + if err != nil { + return nil, fmt.Errorf("unable to load hub kubeconfig from file %q: %w", s.HubKubeconfigFile, err) + } + } + + clients := &Clients{} + clients.KubeClient, err = kubernetes.NewForConfig(kubeConfig) + if err != nil { + return nil, err + } + clients.ClusterClient, err = clusterv1client.NewForConfig(kubeConfig) + if err != nil { + return nil, err + } + clients.AddonClient, err = addonclient.NewForConfig(kubeConfig) + if err != nil { + return nil, err + } + + clients.KubeInformerFactory = informers.NewSharedInformerFactoryWithOptions( + clients.KubeClient, + 10*time.Minute, + informers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { + listOptions.LabelSelector = fmt.Sprintf("%s=%s", clusterv1.ClusterNameLabelKey, s.ClusterName) + }), + ) + clients.ClusterInfomerFactory = clusterv1informers.NewSharedInformerFactoryWithOptions( + clients.ClusterClient, + 10*time.Minute, + clusterv1informers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { + listOptions.FieldSelector = fields.OneTermEqualSelector("metadata.name", s.ClusterName).String() + }), + ) + clients.AddonInformerFactory = addoninformers.NewSharedInformerFactoryWithOptions( + clients.AddonClient, + 10*time.Minute, + addoninformers.WithNamespace(s.ClusterName), + ) + return clients, nil +} diff --git a/pkg/registration/register/common_test.go b/pkg/registration/register/common_test.go index a549ae4c5..5cd16885d 100644 --- a/pkg/registration/register/common_test.go +++ b/pkg/registration/register/common_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path" "reflect" "testing" "time" @@ -279,3 +280,72 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { }) } } + +func TestBuildClientFromSecretOptions(t *testing.T) { + tempDir, err := os.MkdirTemp("", "testvalidhubclientconfig") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + cert1 := testinghelpers.NewTestCert("system:open-cluster-management:cluster1:agent1", 60*time.Second) + defer os.RemoveAll(tempDir) + + kubeconfig := testinghelpers.NewKubeconfig( + "cluster1", "https://127.0.0.1:6001", "", "", nil, cert1.Key, cert1.Cert) + + cases := []struct { + name string + kubeconfig []byte + bootstrapKubeconfig []byte + bootstrap bool + expectErr bool + }{ + { + name: "bootstrap is not set", + kubeconfig: kubeconfig, + bootstrap: true, + expectErr: true, + }, + { + name: "bootstrap is set", + kubeconfig: nil, + bootstrapKubeconfig: kubeconfig, + bootstrap: true, + expectErr: false, + }, + { + name: "bootstrap is false", + kubeconfig: nil, + bootstrapKubeconfig: kubeconfig, + bootstrap: false, + expectErr: true, + }, + { + name: "bootstrap is false with correct kubeconfig", + kubeconfig: kubeconfig, + bootstrapKubeconfig: nil, + bootstrap: false, + expectErr: false, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + secretOpts := SecretOption{ + ClusterName: "cluster1", + AgentName: "agent1", + } + if tt.kubeconfig != nil { + testinghelpers.WriteFile(path.Join(tempDir, "kubeconfig"), tt.kubeconfig) + secretOpts.HubKubeconfigFile = path.Join(tempDir, "kubeconfig") + } + if tt.bootstrapKubeconfig != nil { + testinghelpers.WriteFile(path.Join(tempDir, "bootstrap-kubeconfig"), tt.bootstrapKubeconfig) + secretOpts.BootStrapKubeConfigFile = path.Join(tempDir, "bootstrap-kubeconfig") + } + _, err := BuildClientsFromSecretOption(secretOpts, tt.bootstrap) + if (err != nil) != tt.expectErr { + t.Errorf("expected error %v but got %v", tt.expectErr, err) + } + }) + } +} diff --git a/pkg/registration/register/csr/certficate_beta.go b/pkg/registration/register/csr/certficate_beta.go index 17917cc51..f858e85d2 100644 --- a/pkg/registration/register/csr/certficate_beta.go +++ b/pkg/registration/register/csr/certficate_beta.go @@ -21,7 +21,7 @@ type v1beta1CSRControl struct { hubCSRClient csrclient.CertificateSigningRequestInterface } -func (v *v1beta1CSRControl) isApproved(name string) (bool, error) { +func (v *v1beta1CSRControl) IsApproved(name string) (bool, error) { csr, err := v.get(name) if err != nil { return false, err @@ -38,7 +38,7 @@ func (v *v1beta1CSRControl) isApproved(name string) (bool, error) { return approved, nil } -func (v *v1beta1CSRControl) getIssuedCertificate(name string) ([]byte, error) { +func (v *v1beta1CSRControl) GetIssuedCertificate(name string) ([]byte, error) { csr, err := v.get(name) if err != nil { return nil, err @@ -51,7 +51,7 @@ func (v *v1beta1CSRControl) getIssuedCertificate(name string) ([]byte, error) { return v1beta1CSR.Status.Certificate, nil } -func (v *v1beta1CSRControl) create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, +func (v *v1beta1CSRControl) Create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, signerName string, expirationSeconds *int32) (string, error) { csr := &certificates.CertificateSigningRequest{ ObjectMeta: objMeta, diff --git a/pkg/registration/register/csr/certificate.go b/pkg/registration/register/csr/certificate.go index fa9b433e6..f44449eaf 100644 --- a/pkg/registration/register/csr/certificate.go +++ b/pkg/registration/register/csr/certificate.go @@ -148,10 +148,11 @@ func GetClusterAgentNamesFromCertificate(certData []byte) (clusterName, agentNam return "", "", nil } +// CSRControl is an interface that driver can optionally support for csr based registration for addons. type CSRControl interface { - create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, signerName string, expirationSeconds *int32) (string, error) - isApproved(name string) (bool, error) - getIssuedCertificate(name string) ([]byte, error) + Create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, signerName string, expirationSeconds *int32) (string, error) + IsApproved(name string) (bool, error) + GetIssuedCertificate(name string) ([]byte, error) // Informer is public so we can add indexer outside Informer() cache.SharedIndexInformer @@ -165,7 +166,7 @@ type v1CSRControl struct { hubCSRClient csrclient.CertificateSigningRequestInterface } -func (v *v1CSRControl) isApproved(name string) (bool, error) { +func (v *v1CSRControl) IsApproved(name string) (bool, error) { csr, err := v.get(name) if err != nil { return false, err @@ -182,7 +183,7 @@ func (v *v1CSRControl) isApproved(name string) (bool, error) { return approved, nil } -func (v *v1CSRControl) getIssuedCertificate(name string) ([]byte, error) { +func (v *v1CSRControl) GetIssuedCertificate(name string) ([]byte, error) { csr, err := v.get(name) if err != nil { return nil, err @@ -191,7 +192,7 @@ func (v *v1CSRControl) getIssuedCertificate(name string) ([]byte, error) { return v1CSR.Status.Certificate, nil } -func (v *v1CSRControl) create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, +func (v *v1CSRControl) Create(ctx context.Context, recorder events.Recorder, objMeta metav1.ObjectMeta, csrData []byte, signerName string, expirationSeconds *int32) (string, error) { csr := &certificates.CertificateSigningRequest{ ObjectMeta: objMeta, diff --git a/pkg/registration/register/csr/certificate_beta_test.go b/pkg/registration/register/csr/certificate_beta_test.go index 327d5eae2..fff61f7cf 100644 --- a/pkg/registration/register/csr/certificate_beta_test.go +++ b/pkg/registration/register/csr/certificate_beta_test.go @@ -63,11 +63,11 @@ func TestV1beta1CSRControlApprovedAndIssued(t *testing.T) { hubCSRClient: client.CertificatesV1beta1().CertificateSigningRequests(), } - actualApproved, err := ctrl.isApproved(c.csrName) + actualApproved, err := ctrl.IsApproved(c.csrName) assert.NoError(t, err) assert.Equal(t, c.isApproved, actualApproved) - issuedCertData, err := ctrl.getIssuedCertificate(c.csrName) + issuedCertData, err := ctrl.GetIssuedCertificate(c.csrName) assert.NoError(t, err) assert.Equal(t, c.isIssued, len(issuedCertData) > 0) }) diff --git a/pkg/registration/register/csr/certificate_test.go b/pkg/registration/register/csr/certificate_test.go index 2595cfc72..82282f60c 100644 --- a/pkg/registration/register/csr/certificate_test.go +++ b/pkg/registration/register/csr/certificate_test.go @@ -54,7 +54,7 @@ func TestIsCSRApproved(t *testing.T) { ctrl := &v1CSRControl{ hubCSRLister: lister, } - csrApproved, err := ctrl.isApproved(c.csr.Name) + csrApproved, err := ctrl.IsApproved(c.csr.Name) assert.NoError(t, err) if csrApproved != c.csrApproved { t.Errorf("expected %t, but got %t", c.csrApproved, csrApproved) diff --git a/pkg/registration/register/csr/csr.go b/pkg/registration/register/csr/csr.go index 4c3ac7239..05a599554 100644 --- a/pkg/registration/register/csr/csr.go +++ b/pkg/registration/register/csr/csr.go @@ -9,11 +9,14 @@ import ( "os" "path" "reflect" + "strings" "time" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" + certificates "k8s.io/api/certificates/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" @@ -21,9 +24,12 @@ import ( certutil "k8s.io/client-go/util/cert" "k8s.io/client-go/util/keyutil" "k8s.io/klog/v2" + "k8s.io/utils/pointer" + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" clusterv1 "open-cluster-management.io/api/cluster/v1" + "open-cluster-management.io/ocm/pkg/registration/hub/user" "open-cluster-management.io/ocm/pkg/registration/register" ) @@ -35,6 +41,13 @@ const ( // ClusterCertificateRotatedCondition is a condition type that client certificate is rotated ClusterCertificateRotatedCondition = "ClusterCertificateRotated" + + indexByCluster = "indexByCluster" + indexByAddon = "indexByAddon" + + // TODO(qiujian16) expose it if necessary in the future. + clusterCSRThreshold = 10 + addonCSRThreshold = 10 ) type CSRDriver struct { @@ -49,16 +62,21 @@ type CSRDriver struct { // 3. csrName set, keyData set: we are waiting for a new cert to be signed. // 4. csrName empty, keydata set: the CSR failed to create, this shouldn't happen, it's a bug. keyData []byte + + csrControl CSRControl + + // HaltCSRCreation halt the csr creation + haltCSRCreation func() bool + + opt *Option + + csrOption *CSROption } func (c *CSRDriver) Process( ctx context.Context, controllerName string, secret *corev1.Secret, additionalSecretData map[string][]byte, - recorder events.Recorder, opt any) (*corev1.Secret, *metav1.Condition, error) { + recorder events.Recorder) (*corev1.Secret, *metav1.Condition, error) { logger := klog.FromContext(ctx) - csrOption, ok := opt.(*CSROption) - if !ok { - return nil, nil, fmt.Errorf("option type is not correct") - } // reconcile pending csr if exists if len(c.csrName) > 0 { @@ -70,7 +88,7 @@ func (c *CSRDriver) Process( } // skip if csr is not approved yet - isApproved, err := csrOption.CSRControl.isApproved(c.csrName) + isApproved, err := c.csrControl.IsApproved(c.csrName) if err != nil { return nil, err } @@ -79,7 +97,7 @@ func (c *CSRDriver) Process( } // skip if csr is not issued - certData, err := csrOption.CSRControl.getIssuedCertificate(c.csrName) + certData, err := c.csrControl.GetIssuedCertificate(c.csrName) if err != nil { return nil, err } @@ -154,7 +172,7 @@ func (c *CSRDriver) Process( controllerName, secret, recorder, - csrOption.Subject, + c.csrOption.Subject, additionalSecretData) if err != nil { return secret, nil, err @@ -163,7 +181,7 @@ func (c *CSRDriver) Process( return nil, nil, nil } - shouldHalt := csrOption.HaltCSRCreation() + shouldHalt := c.haltCSRCreation() if shouldHalt { recorder.Eventf("ClientCertificateCreationHalted", "Stop creating csr since there are too many csr created already on hub", controllerName) @@ -186,12 +204,18 @@ func (c *CSRDriver) Process( if err != nil { return keyData, "", fmt.Errorf("invalid private key for certificate request: %w", err) } - csrData, err := certutil.MakeCSR(privateKey, csrOption.Subject, csrOption.DNSNames, nil) + csrData, err := certutil.MakeCSR(privateKey, c.csrOption.Subject, c.csrOption.DNSNames, nil) if err != nil { return keyData, "", fmt.Errorf("unable to generate certificate request: %w", err) } - createdCSRName, err := csrOption.CSRControl.create( - ctx, recorder, csrOption.ObjectMeta, csrData, csrOption.SignerName, csrOption.ExpirationSeconds) + + // do not set expiration second if it is 0 + expirationSeconds := pointer.Int32(c.opt.ExpirationSeconds) + if *expirationSeconds == 0 { + expirationSeconds = nil + } + createdCSRName, err := c.csrControl.Create( + ctx, recorder, c.csrOption.ObjectMeta, csrData, c.csrOption.SignerName, expirationSeconds) if err != nil { return keyData, "", err } @@ -225,12 +249,8 @@ func (c *CSRDriver) BuildKubeConfigFromTemplate(kubeConfig *clientcmdapi.Config) return kubeConfig } -func (c *CSRDriver) InformerHandler(option any) (cache.SharedIndexInformer, factory.EventFilterFunc) { - csrOption, ok := option.(*CSROption) - if !ok { - utilruntime.Must(fmt.Errorf("option type is not correct")) - } - return csrOption.CSRControl.Informer(), csrOption.EventFilterFunc +func (c *CSRDriver) InformerHandler() (cache.SharedIndexInformer, factory.EventFilterFunc) { + return c.csrControl.Informer(), c.csrOption.EventFilterFunc } func (c *CSRDriver) IsHubKubeConfigValid(ctx context.Context, secretOption register.SecretOption) (bool, error) { @@ -272,8 +292,108 @@ func (c *CSRDriver) ManagedClusterDecorator(cluster *clusterv1.ManagedCluster) * return cluster } -func NewCSRDriver() register.RegisterDriver { - return &CSRDriver{} +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), + } + + driver := &CSRDriver{ + csrOption: csrOption, + opt: c.opt, + csrControl: c.csrControl, + haltCSRCreation: haltAddonCSRCreationFunc(c.csrControl.Informer().GetIndexer(), secretOption.ClusterName, addonName), + } + + return driver +} + +func (c *CSRDriver) BuildClients(ctx context.Context, secretOption register.SecretOption, bootstrap bool) (*register.Clients, error) { + logger := klog.FromContext(ctx) + clients, err := register.BuildClientsFromSecretOption(secretOption, bootstrap) + if err != nil { + return nil, err + } + csrControl, err := NewCSRControl(logger, clients.KubeInformerFactory.Certificates(), clients.KubeClient) + if err != nil { + return nil, fmt.Errorf("failed to create CSR control: %w", err) + } + + err = csrControl.Informer().AddIndexers(cache.Indexers{ + indexByCluster: indexByClusterFunc, + }) + if err != nil { + return nil, err + } + + err = csrControl.Informer().AddIndexers(cache.Indexers{ + indexByAddon: indexByAddonFunc, + }) + if err != nil { + utilruntime.HandleError(err) + } + + c.csrControl = csrControl + c.haltCSRCreation = haltCSRCreationFunc(csrControl.Informer().GetIndexer(), secretOption.ClusterName) + return clients, nil +} + +var _ register.RegisterDriver = &CSRDriver{} +var _ register.AddonDriver = &CSRDriver{} + +func NewCSRDriver(opt *Option, secretOpts register.SecretOption) *CSRDriver { + driver := &CSRDriver{ + opt: opt, + } + driver.csrOption = &CSROption{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("%s-", secretOpts.ClusterName), + Labels: map[string]string{ + // the label is only an hint for cluster name. Anyone could set/modify it. + clusterv1.ClusterNameLabelKey: secretOpts.ClusterName, + }, + }, + Subject: &pkix.Name{ + Organization: []string{ + fmt.Sprintf("%s%s", user.SubjectPrefix, secretOpts.ClusterName), + user.ManagedClustersGroup, + }, + CommonName: fmt.Sprintf("%s%s:%s", user.SubjectPrefix, secretOpts.ClusterName, secretOpts.AgentName), + }, + SignerName: certificates.KubeAPIServerClientSignerName, + EventFilterFunc: func(obj interface{}) bool { + accessor, err := meta.Accessor(obj) + if err != nil { + return false + } + labels := accessor.GetLabels() + // only enqueue csr from a specific managed cluster + if labels[clusterv1.ClusterNameLabelKey] != secretOpts.ClusterName { + return false + } + + // should not contain addon key + _, ok := labels[addonv1alpha1.AddonLabelKey] + if ok { + return false + } + + // only enqueue csr whose name starts with the cluster name + return strings.HasPrefix(accessor.GetName(), fmt.Sprintf("%s-", secretOpts.ClusterName)) + }, + } + + return driver } func shouldCreateCSR( @@ -346,3 +466,72 @@ func jitter(percentage float64, maxFactor float64) float64 { newPercentage := percentage + percentage*rand.Float64()*maxFactor //#nosec G404 return newPercentage } + +func indexByAddonFunc(obj interface{}) ([]string, error) { + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + + cluster, ok := accessor.GetLabels()[clusterv1.ClusterNameLabelKey] + if !ok { + return []string{}, nil + } + + addon, ok := accessor.GetLabels()[addonv1alpha1.AddonLabelKey] + if !ok { + return []string{}, nil + } + + return []string{fmt.Sprintf("%s/%s", cluster, addon)}, nil +} + +func indexByClusterFunc(obj interface{}) ([]string, error) { + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + + cluster, ok := accessor.GetLabels()[clusterv1.ClusterNameLabelKey] + if !ok { + return []string{}, nil + } + + // should not contain addon key + if _, ok := accessor.GetLabels()[addonv1alpha1.AddonLabelKey]; ok { + return []string{}, nil + } + + return []string{cluster}, nil +} + +func createCSREventFilterFunc(clusterName, addOnName, signerName string) factory.EventFilterFunc { + return func(obj interface{}) bool { + accessor, err := meta.Accessor(obj) + if err != nil { + return false + } + labels := accessor.GetLabels() + // only enqueue csr from a specific managed cluster + if labels[clusterv1.ClusterNameLabelKey] != clusterName { + return false + } + // only enqueue csr created for a specific addon + if labels[addonv1alpha1.AddonLabelKey] != addOnName { + return false + } + + // only enqueue csr with a specific signer name + csr, ok := obj.(*certificates.CertificateSigningRequest) + if !ok { + return false + } + if len(csr.Spec.SignerName) == 0 { + return false + } + if csr.Spec.SignerName != signerName { + return false + } + return true + } +} diff --git a/pkg/registration/register/csr/csr_test.go b/pkg/registration/register/csr/csr_test.go index 6ce4a86eb..2cd95050c 100644 --- a/pkg/registration/register/csr/csr_test.go +++ b/pkg/registration/register/csr/csr_test.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path" + "reflect" "testing" "time" @@ -16,13 +17,18 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/client-go/informers" kubefake "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2/ktesting" + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" + clusterv1 "open-cluster-management.io/api/cluster/v1" + ocmfeature "open-cluster-management.io/api/feature" + testingcommon "open-cluster-management.io/ocm/pkg/common/testing" + "open-cluster-management.io/ocm/pkg/features" 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" @@ -150,7 +156,7 @@ func TestProcess(t *testing.T) { ctrl.approved = true ctrl.issuedCertData = c.approvedCSRCert.Cert } - hubKubeClient := kubefake.NewSimpleClientset(csrs...) + hubKubeClient := kubefake.NewClientset(csrs...) ctrl.csrClient = &hubKubeClient.Fake // GenerateName is not working for fake clent, we set the name with prepend reactor @@ -170,13 +176,16 @@ func TestProcess(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", }, - Subject: testSubject, - SignerName: certificates.KubeAPIServerClientSignerName, - HaltCSRCreation: func() bool { return false }, - CSRControl: ctrl, + Subject: testSubject, + SignerName: certificates.KubeAPIServerClientSignerName, } - driver := &CSRDriver{} + driver := &CSRDriver{ + csrControl: ctrl, + haltCSRCreation: func() bool { return false }, + csrOption: csrOption, + opt: NewCSROption(), + } if c.approvedCSRCert != nil { driver.csrName = testCSRName @@ -186,7 +195,7 @@ func TestProcess(t *testing.T) { syncCtx := testingcommon.NewFakeSyncContext(t, "test") secret, cond, err := driver.Process( - context.TODO(), "test", c.secret, additionalSecretData, syncCtx.Recorder(), csrOption) + context.TODO(), "test", c.secret, additionalSecretData, syncCtx.Recorder()) if err != nil { t.Errorf("unexpected error %v", err) } @@ -238,19 +247,20 @@ type mockCSRControl struct { csrClient *clienttesting.Fake } -func (m *mockCSRControl) create( +func (m *mockCSRControl) Create( _ context.Context, _ events.Recorder, objMeta metav1.ObjectMeta, _ []byte, _ string, _ *int32) (string, error) { mockCSR := &unstructured.Unstructured{} _, err := m.csrClient.Invokes(clienttesting.CreateActionImpl{ ActionImpl: clienttesting.ActionImpl{ - Verb: "create", + Verb: "create", + Resource: certificates.SchemeGroupVersion.WithResource("certificatesigningrequests"), }, Object: mockCSR, }, nil) return objMeta.Name + rand.String(4), err } -func (m *mockCSRControl) isApproved(name string) (bool, error) { +func (m *mockCSRControl) IsApproved(name string) (bool, error) { _, err := m.csrClient.Invokes(clienttesting.GetActionImpl{ ActionImpl: clienttesting.ActionImpl{ Verb: "get", @@ -262,7 +272,7 @@ func (m *mockCSRControl) isApproved(name string) (bool, error) { return m.approved, err } -func (m *mockCSRControl) getIssuedCertificate(name string) ([]byte, error) { +func (m *mockCSRControl) GetIssuedCertificate(name string) ([]byte, error) { _, err := m.csrClient.Invokes(clienttesting.GetActionImpl{ ActionImpl: clienttesting.ActionImpl{ Verb: "get", @@ -274,7 +284,9 @@ func (m *mockCSRControl) getIssuedCertificate(name string) ([]byte, error) { } func (m *mockCSRControl) Informer() cache.SharedIndexInformer { - panic("implement me") + client := kubefake.NewClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) + return informerFactory.Certificates().V1().CertificateSigningRequests().Informer() } func TestIsHubKubeConfigValidFunc(t *testing.T) { @@ -386,13 +398,16 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - driver := NewCSRDriver() secretOption := register.SecretOption{ ClusterName: c.clusterName, AgentName: c.agentName, HubKubeconfigDir: tempDir, HubKubeconfigFile: path.Join(tempDir, "kubeconfig"), } + driver := NewCSRDriver(NewCSROption(), secretOption) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if c.kubeconfig != nil { testinghelpers.WriteFile(path.Join(tempDir, "kubeconfig"), c.kubeconfig) } @@ -403,11 +418,11 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { testinghelpers.WriteFile(path.Join(tempDir, "tls.crt"), c.tlsCert) } if c.bootstapKubeconfig != nil { - bootstrapKubeconfig, err := clientcmd.Load(c.bootstapKubeconfig) + testinghelpers.WriteFile(path.Join(tempDir, "bootstrap-kubeconfig"), c.bootstapKubeconfig) if err != nil { t.Fatal(err) } - secretOption.BootStrapKubeConfig = bootstrapKubeconfig + secretOption.BootStrapKubeConfigFile = path.Join(tempDir, "bootstrap-kubeconfig") } valid, err := register.IsHubKubeConfigValidFunc(driver, secretOption)(context.TODO()) @@ -420,3 +435,330 @@ func TestIsHubKubeConfigValidFunc(t *testing.T) { }) } } + +func TestFilterCSREvents(t *testing.T) { + clusterName := "cluster1" + signerName := "signer1" + addOnName := "addon1" + + cases := []struct { + name string + csr *certificates.CertificateSigningRequest + expected bool + }{ + { + name: "csr not from the managed cluster", + csr: &certificates.CertificateSigningRequest{}, + }, + { + name: "csr not for the addon", + csr: &certificates.CertificateSigningRequest{}, + }, + { + name: "csr with different signer name", + csr: &certificates.CertificateSigningRequest{}, + }, + { + name: "valid csr", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + // the labels are only hints. Anyone could set/modify them. + clusterv1.ClusterNameLabelKey: clusterName, + addonv1alpha1.AddonLabelKey: addOnName, + }, + }, + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: signerName, + }, + }, + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + filterFunc := createCSREventFilterFunc(clusterName, addOnName, signerName) + actual := filterFunc(c.csr) + if actual != c.expected { + t.Errorf("Expected %v but got %v", c.expected, actual) + } + }) + } +} + +func TestIndexByClusterName(t *testing.T) { + testcases := []struct { + name string + csr *certificates.CertificateSigningRequest + expected []string + }{ + { + name: "no index", + csr: &certificates.CertificateSigningRequest{}, + expected: []string{}, + }, + { + name: "has cluster label", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{clusterv1.ClusterNameLabelKey: "cluster1"}, + }, + }, + expected: []string{"cluster1"}, + }, + { + name: "has cluster label", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster1", + addonv1alpha1.AddonLabelKey: "addon1", + }, + }, + }, + expected: []string{}, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + actual, err := indexByClusterFunc(tt.csr) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(actual, tt.expected) { + t.Errorf("Expected %v but got %v", tt.expected, actual) + } + }) + } +} + +func TestIndexByAddonFunc(t *testing.T) { + testcases := []struct { + name string + csr *certificates.CertificateSigningRequest + expected []string + }{ + { + name: "no index", + csr: &certificates.CertificateSigningRequest{}, + expected: []string{}, + }, + { + name: "has cluster label", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{clusterv1.ClusterNameLabelKey: "cluster1"}, + }, + }, + expected: []string{}, + }, + { + name: "has cluster label", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster1", + addonv1alpha1.AddonLabelKey: "addon1", + }, + }, + }, + expected: []string{"cluster1/addon1"}, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + actual, err := indexByAddonFunc(tt.csr) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(actual, tt.expected) { + t.Errorf("Expected %v but got %v", tt.expected, actual) + } + }) + } +} + +func TestNewCSRDriver(t *testing.T) { + secretOpts := register.SecretOption{ + ClusterName: "cluster1", + AgentName: "agent1", + } + driver := NewCSRDriver(NewCSROption(), secretOpts) + if driver.csrOption.Subject.CommonName != fmt.Sprintf("%scluster1:agent1", user.SubjectPrefix) { + t.Errorf("common name is not set correctly, got %s", driver.csrOption.Subject.CommonName) + } + ctrl := &mockCSRControl{} + hubKubeClient := kubefake.NewClientset() + ctrl.csrClient = &hubKubeClient.Fake + driver.csrControl = ctrl + + addonSecretOptions := register.SecretOption{ + ClusterName: "cluster1", + AgentName: "addonagent1", + Subject: &pkix.Name{ + CommonName: "addonagent1", + }, + } + addonDriver := driver.Fork("addon1", addonSecretOptions) + csrAddonDriver := addonDriver.(*CSRDriver) + if csrAddonDriver.csrOption.Subject.CommonName != "addonagent1" { + t.Errorf("common name is not set correctly") + } +} + +func TestCSREventFilterFunc(t *testing.T) { + filter := createCSREventFilterFunc("cluster1", "addon1", "signer1") + cases := []struct { + name string + csr *certificates.CertificateSigningRequest + expected bool + }{ + { + name: "incorrect cluster", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster2", + addonv1alpha1.AddonLabelKey: "addon1", + }, + }, + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "signer1", + }, + }, + expected: false, + }, + { + name: "incorrect addon", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster1", + addonv1alpha1.AddonLabelKey: "addon2", + }, + }, + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "signer1", + }, + }, + expected: false, + }, + { + name: "incorrect signer", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster1", + addonv1alpha1.AddonLabelKey: "addon1", + }, + }, + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "signer2", + }, + }, + expected: false, + }, + { + name: "all correct", + csr: &certificates.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.ClusterNameLabelKey: "cluster1", + addonv1alpha1.AddonLabelKey: "addon1", + }, + }, + Spec: certificates.CertificateSigningRequestSpec{ + SignerName: "signer1", + }, + }, + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + passed := filter(c.csr) + if passed != c.expected { + t.Errorf("Expected %v but got %v", c.expected, passed) + } + }) + } +} + +func TestBuildClient(t *testing.T) { + tempDir, err := os.MkdirTemp("", "testvalidhubclientconfig") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + cert1 := testinghelpers.NewTestCert("system:open-cluster-management:cluster1:agent1", 60*time.Second) + defer os.RemoveAll(tempDir) + + kubeconfig := testinghelpers.NewKubeconfig( + "cluster1", "https://127.0.0.1:6001", "", "", nil, cert1.Key, cert1.Cert) + + cases := []struct { + name string + kubeconfig []byte + bootstrapKubeconfig []byte + bootstrap bool + expectErr bool + }{ + { + name: "bootstrap is not set", + kubeconfig: kubeconfig, + bootstrap: true, + expectErr: true, + }, + { + name: "bootstrap is set", + kubeconfig: nil, + bootstrapKubeconfig: kubeconfig, + bootstrap: true, + expectErr: false, + }, + { + name: "bootstrap is false", + kubeconfig: nil, + bootstrapKubeconfig: kubeconfig, + bootstrap: false, + expectErr: true, + }, + { + name: "bootstrap is false with correct kubeconfig", + kubeconfig: kubeconfig, + bootstrapKubeconfig: nil, + bootstrap: false, + expectErr: false, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + err = features.SpokeMutableFeatureGate.Add(ocmfeature.DefaultSpokeRegistrationFeatureGates) + if err != nil { + t.Fatal(err) + } + + secretOpts := register.SecretOption{ + ClusterName: "cluster1", + AgentName: "agent1", + } + if tt.kubeconfig != nil { + testinghelpers.WriteFile(path.Join(tempDir, "kubeconfig"), tt.kubeconfig) + secretOpts.HubKubeconfigFile = path.Join(tempDir, "kubeconfig") + } + if tt.bootstrapKubeconfig != nil { + testinghelpers.WriteFile(path.Join(tempDir, "bootstrap-kubeconfig"), tt.bootstrapKubeconfig) + secretOpts.BootStrapKubeConfigFile = path.Join(tempDir, "bootstrap-kubeconfig") + } + driver := NewCSRDriver(NewCSROption(), secretOpts) + _, err := driver.BuildClients(context.TODO(), secretOpts, tt.bootstrap) + if (err != nil) != tt.expectErr { + t.Errorf("expected error %v but got %v", tt.expectErr, err) + } + }) + } +} diff --git a/pkg/registration/register/csr/options.go b/pkg/registration/register/csr/options.go index f54c9c469..8b24dcbf3 100644 --- a/pkg/registration/register/csr/options.go +++ b/pkg/registration/register/csr/options.go @@ -2,30 +2,13 @@ package csr import ( "crypto/x509/pkix" + "errors" "fmt" - "strings" "github.com/openshift/library-go/pkg/controller/factory" - certificates "k8s.io/api/certificates/v1" - "k8s.io/apimachinery/pkg/api/meta" + "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - certificatesinformers "k8s.io/client-go/informers/certificates" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" - "k8s.io/klog/v2" - - addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" - clusterv1 "open-cluster-management.io/api/cluster/v1" - - "open-cluster-management.io/ocm/pkg/registration/hub/user" - "open-cluster-management.io/ocm/pkg/registration/register" -) - -const ( - indexByCluster = "indexByCluster" - - // TODO(qiujian16) expose it if necessary in the future. - clusterCSRThreshold = 10 ) // CSROption includes options that is used to create and monitor csrs @@ -40,6 +23,12 @@ type CSROption struct { // SignerName is the name of the signer specified in the created csrs SignerName string + // EventFilterFunc matches csrs created with above options + EventFilterFunc factory.EventFilterFunc +} + +// Option is the option set from flag +type Option struct { // ExpirationSeconds is the requested duration of validity of the issued // certificate. // Certificate signers may not honor this field for various reasons: @@ -50,77 +39,24 @@ type CSROption struct { // 3. Signer whose configured minimum is longer than the requested duration // // The minimum valid value for expirationSeconds is 3600, i.e. 1 hour. - ExpirationSeconds *int32 - - // EventFilterFunc matches csrs created with above options - EventFilterFunc factory.EventFilterFunc - - CSRControl CSRControl - - // HaltCSRCreation halt the csr creation - HaltCSRCreation func() bool + ExpirationSeconds int32 } -func NewCSROption( - logger klog.Logger, - secretOption register.SecretOption, - csrExpirationSeconds int32, - hubCSRInformer certificatesinformers.Interface, - hubKubeClient kubernetes.Interface) (*CSROption, error) { - csrControl, err := NewCSRControl(logger, hubCSRInformer, hubKubeClient) - if err != nil { - return nil, fmt.Errorf("failed to create CSR control: %w", err) - } - var csrExpirationSecondsInCSROption *int32 - if csrExpirationSeconds != 0 { - csrExpirationSecondsInCSROption = &csrExpirationSeconds - } - err = csrControl.Informer().AddIndexers(cache.Indexers{ - indexByCluster: indexByClusterFunc, - }) - if err != nil { - return nil, err - } - return &CSROption{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-", secretOption.ClusterName), - Labels: map[string]string{ - // the label is only an hint for cluster name. Anyone could set/modify it. - clusterv1.ClusterNameLabelKey: secretOption.ClusterName, - }, - }, - Subject: &pkix.Name{ - Organization: []string{ - fmt.Sprintf("%s%s", user.SubjectPrefix, secretOption.ClusterName), - user.ManagedClustersGroup, - }, - CommonName: fmt.Sprintf("%s%s:%s", user.SubjectPrefix, secretOption.ClusterName, secretOption.AgentName), - }, - SignerName: certificates.KubeAPIServerClientSignerName, - EventFilterFunc: func(obj interface{}) bool { - accessor, err := meta.Accessor(obj) - if err != nil { - return false - } - labels := accessor.GetLabels() - // only enqueue csr from a specific managed cluster - if labels[clusterv1.ClusterNameLabelKey] != secretOption.ClusterName { - return false - } +func NewCSROption() *Option { + return &Option{} +} - // should not contain addon key - _, ok := labels[addonv1alpha1.AddonLabelKey] - if ok { - return false - } +func (o *Option) AddFlags(fs *pflag.FlagSet) { + fs.Int32Var(&o.ExpirationSeconds, "client-cert-expiration-seconds", o.ExpirationSeconds, + "The requested duration in seconds of validity of the issued client certificate. If this is not set, "+ + "the value of --cluster-signing-duration command-line flag of the kube-controller-manager will be used.") +} - // only enqueue csr whose name starts with the cluster name - return strings.HasPrefix(accessor.GetName(), fmt.Sprintf("%s-", secretOption.ClusterName)) - }, - HaltCSRCreation: haltCSRCreationFunc(csrControl.Informer().GetIndexer(), secretOption.ClusterName), - ExpirationSeconds: csrExpirationSecondsInCSROption, - CSRControl: csrControl, - }, nil +func (o *Option) Validate() error { + if o.ExpirationSeconds != 0 && o.ExpirationSeconds < 3600 { + return errors.New("client certificate expiration seconds must greater or qual to 3600") + } + return nil } func haltCSRCreationFunc(indexer cache.Indexer, clusterName string) func() bool { @@ -137,21 +73,17 @@ func haltCSRCreationFunc(indexer cache.Indexer, clusterName string) func() bool } } -func indexByClusterFunc(obj interface{}) ([]string, error) { - accessor, err := meta.Accessor(obj) - if err != nil { - return nil, err - } +func haltAddonCSRCreationFunc(indexer cache.Indexer, clusterName, addonName string) func() bool { + return func() bool { + items, err := indexer.ByIndex(indexByAddon, fmt.Sprintf("%s/%s", clusterName, addonName)) + if err != nil { + return false + } - cluster, ok := accessor.GetLabels()[clusterv1.ClusterNameLabelKey] - if !ok { - return []string{}, nil - } + if len(items) >= addonCSRThreshold { + return true + } - // should not contain addon key - if _, ok := accessor.GetLabels()[addonv1alpha1.AddonLabelKey]; ok { - return []string{}, nil + return false } - - return []string{cluster}, nil } diff --git a/pkg/registration/register/factory/options.go b/pkg/registration/register/factory/options.go new file mode 100644 index 000000000..43c8a3d4b --- /dev/null +++ b/pkg/registration/register/factory/options.go @@ -0,0 +1,48 @@ +package factory + +import ( + "github.com/spf13/pflag" + + "open-cluster-management.io/ocm/pkg/common/helpers" + "open-cluster-management.io/ocm/pkg/registration/register" + awsirsa "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" + "open-cluster-management.io/ocm/pkg/registration/register/csr" +) + +type Options struct { + RegistrationAuth string + CSROption *csr.Option + AWSISRAOption *awsirsa.AWSOption +} + +func NewOptions() *Options { + return &Options{ + CSROption: csr.NewCSROption(), + AWSISRAOption: awsirsa.NewAWSOption(), + } +} + +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.") + s.CSROption.AddFlags(fs) + s.AWSISRAOption.AddFlags(fs) +} + +func (s *Options) Validate() error { + switch s.RegistrationAuth { + case helpers.AwsIrsaAuthType: + return s.AWSISRAOption.Validate() + default: + return s.CSROption.Validate() + } +} + +func (s *Options) Driver(secretOption register.SecretOption) register.RegisterDriver { + switch s.RegistrationAuth { + case helpers.AwsIrsaAuthType: + return awsirsa.NewAWSIRSADriver(s.AWSISRAOption, secretOption) + default: + return csr.NewCSRDriver(s.CSROption, secretOption) + } +} diff --git a/pkg/registration/register/factory/options_test.go b/pkg/registration/register/factory/options_test.go new file mode 100644 index 000000000..9d5aa94b3 --- /dev/null +++ b/pkg/registration/register/factory/options_test.go @@ -0,0 +1,64 @@ +package factory + +import ( + "testing" + + awsirsa "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" + "open-cluster-management.io/ocm/pkg/registration/register/csr" +) + +func TestValidate(t *testing.T) { + tests := []struct { + name string + opt *Options + expectErr bool + }{ + { + name: "csr validate", + opt: &Options{ + RegistrationAuth: "csr", + CSROption: &csr.Option{ + ExpirationSeconds: 1200, + }, + }, + expectErr: true, + }, + { + name: "csr validate pass", + opt: &Options{ + RegistrationAuth: "csr", + CSROption: &csr.Option{ + ExpirationSeconds: 7200, + }, + }, + expectErr: false, + }, + { + name: "aws validate", + opt: &Options{ + RegistrationAuth: "awsirsa", + AWSISRAOption: &awsirsa.AWSOption{}, + }, + expectErr: true, + }, + { + name: "aws validate pass", + opt: &Options{ + RegistrationAuth: "awsirsa", + AWSISRAOption: &awsirsa.AWSOption{ + HubClusterArn: "arn:aws:iam::123456789012:role/aws-iam-authenticator", + }, + }, + expectErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.opt.Validate() + if tt.expectErr && err == nil { + t.Errorf("expect error but got nil") + } + }) + } +} diff --git a/pkg/registration/register/interface.go b/pkg/registration/register/interface.go index c060da5e4..6268a65bf 100644 --- a/pkg/registration/register/interface.go +++ b/pkg/registration/register/interface.go @@ -2,6 +2,7 @@ package register import ( "context" + "crypto/x509/pkix" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" @@ -33,7 +34,7 @@ type SecretOption struct { // BootStrapKubeConfig is the kubeconfig to generate hubkubeconfig, if set, create kubeconfig value // in the secret. - BootStrapKubeConfig *clientcmdapi.Config + BootStrapKubeConfigFile string // ClusterName is the cluster name, and it is set as a secret value if it is set. ClusterName string @@ -45,6 +46,11 @@ type SecretOption struct { ManagementSecretInformer cache.SharedIndexInformer ManagementCoreClient corev1client.CoreV1Interface + + // subject of the agent, only used for addon + Subject *pkix.Name + // csr signer for the addon + Signer string } // StatusUpdateFunc is A function to update the condition of the corresponding object. @@ -66,14 +72,22 @@ type RegisterDriver interface { name string, secret *corev1.Secret, additionalSecretData map[string][]byte, - recorder events.Recorder, opt any) (*corev1.Secret, *metav1.Condition, error) + recorder events.Recorder) (*corev1.Secret, *metav1.Condition, error) // InformerHandler returns informer of the related object. If no object needs to be watched, the func could // return nil, nil. - InformerHandler(option any) (cache.SharedIndexInformer, factory.EventFilterFunc) + InformerHandler() (cache.SharedIndexInformer, factory.EventFilterFunc) // ManagedClusterDecorator is to change managed cluster metadata or spec during registration process. ManagedClusterDecorator(cluster *clusterv1.ManagedCluster) *clusterv1.ManagedCluster + + // BuildClients setup clients for the driver based on the secretOption and return + 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 } // HubDriver interface is used to implement operations required to complete aws-irsa registration and csr registration. diff --git a/pkg/registration/register/secret_controller.go b/pkg/registration/register/secret_controller.go index 8127ed0a7..692d9f5c7 100644 --- a/pkg/registration/register/secret_controller.go +++ b/pkg/registration/register/secret_controller.go @@ -23,7 +23,6 @@ var ControllerResyncInterval = 5 * time.Minute // secretController run process in driver to get credential and keeps the defeined secret in secretOption update-to-date type secretController struct { SecretOption - option any driver RegisterDriver controllerName string statusUpdater StatusUpdateFunc @@ -34,15 +33,18 @@ type secretController struct { // NewSecretController return an instance of secretController func NewSecretController( secretOption SecretOption, - option any, driver RegisterDriver, statusUpdater StatusUpdateFunc, recorder events.Recorder, controllerName string, ) factory.Controller { additionalSecretData := map[string][]byte{} - if secretOption.BootStrapKubeConfig != nil { - kubeConfigTemplate, err := BaseKubeConfigFromBootStrap(secretOption.BootStrapKubeConfig) + if secretOption.BootStrapKubeConfigFile != "" { + bootstrapKubeCfg, err := clientcmd.LoadFromFile(secretOption.BootStrapKubeConfigFile) + if err != nil { + utilruntime.Must(err) + } + kubeConfigTemplate, err := BaseKubeConfigFromBootStrap(bootstrapKubeCfg) if err != nil { utilruntime.Must(err) } @@ -70,7 +72,6 @@ func NewSecretController( controllerName: controllerName, statusUpdater: statusUpdater, additionalSecretData: additionalSecretData, - option: option, } f := factory.New(). @@ -88,11 +89,13 @@ func NewSecretController( return false }, secretOption.ManagementSecretInformer) - driverInformer, driverFilter := driver.InformerHandler(option) + driverInformer, driverFilter := driver.InformerHandler() if driverInformer != nil && driverFilter != nil { f = f.WithFilteredEventsInformersQueueKeyFunc(func(obj runtime.Object) string { return factory.DefaultQueueKey }, driverFilter, driverInformer) + } else if driverInformer != nil { + f = f.WithInformers(driverInformer) } return f.WithSync(c.sync). @@ -120,7 +123,7 @@ func (c *secretController) sync(ctx context.Context, syncCtx factory.SyncContext } if c.secretToSave == nil { - secret, cond, err := c.driver.Process(ctx, c.controllerName, secret, c.additionalSecretData, syncCtx.Recorder(), c.option) + secret, cond, err := c.driver.Process(ctx, c.controllerName, secret, c.additionalSecretData, syncCtx.Recorder()) if cond != nil { if updateErr := c.statusUpdater(ctx, *cond); updateErr != nil { return updateErr diff --git a/pkg/registration/register/secret_controller_test.go b/pkg/registration/register/secret_controller_test.go index 3a31fe5ab..2a7278206 100644 --- a/pkg/registration/register/secret_controller_test.go +++ b/pkg/registration/register/secret_controller_test.go @@ -2,6 +2,8 @@ package register import ( "context" + "os" + "path" "testing" "time" @@ -15,6 +17,7 @@ import ( kubefake "k8s.io/client-go/kubernetes/fake" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" clusterv1 "open-cluster-management.io/api/cluster/v1" @@ -25,6 +28,31 @@ import ( func TestSync(t *testing.T) { commonName := "test" + tempDir, err := os.MkdirTemp("", "testvalidhubclientconfig") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + bootstrapKubeconfig := &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{"test-cluster": { + Server: "localhost", + InsecureSkipTLSVerify: true, + }}, + Contexts: map[string]*clientcmdapi.Context{"test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-user": { + Token: "test-token", + }, + }, + CurrentContext: "test-context", + } + err = clientcmd.WriteToFile(*bootstrapKubeconfig, path.Join(tempDir, "bootstrap-kubeconfig")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer os.RemoveAll(tempDir) testCases := []struct { name string option SecretOption @@ -88,26 +116,11 @@ func TestSync(t *testing.T) { { name: "addition secret data", option: SecretOption{ - SecretName: "test", - SecretNamespace: "test", - ClusterName: "cluster1", - AgentName: "agent1", - BootStrapKubeConfig: &clientcmdapi.Config{ - Clusters: map[string]*clientcmdapi.Cluster{"test-cluster": { - Server: "localhost", - InsecureSkipTLSVerify: true, - }}, - Contexts: map[string]*clientcmdapi.Context{"test-context": { - Cluster: "test-cluster", - AuthInfo: "test-user", - }}, - AuthInfos: map[string]*clientcmdapi.AuthInfo{ - "test-user": { - Token: "test-token", - }, - }, - CurrentContext: "test-context", - }, + SecretName: "test", + SecretNamespace: "test", + ClusterName: "cluster1", + AgentName: "agent1", + BootStrapKubeConfigFile: path.Join(tempDir, "bootstrap-kubeconfig"), }, secrets: []runtime.Object{}, driver: newFakeDriver(testinghelpers.NewHubKubeconfigSecret( @@ -141,7 +154,7 @@ func TestSync(t *testing.T) { c.option.ManagementSecretInformer = informerFactory.Core().V1().Secrets().Informer() updater := &fakeStatusUpdater{} ctrl := NewSecretController( - c.option, nil, c.driver, updater.update, syncCtx.Recorder(), "test") + c.option, c.driver, updater.update, syncCtx.Recorder(), "test") err := ctrl.Sync(context.Background(), syncCtx) if err != nil { t.Fatal(err) @@ -185,16 +198,20 @@ func (f *fakeDriver) BuildKubeConfigFromTemplate(config *clientcmdapi.Config) *c return config } +func (f *fakeDriver) BuildClients(ctx context.Context, secretOption SecretOption, bootstrap bool) (*Clients, error) { + return &Clients{}, nil +} + func (f *fakeDriver) Process( _ context.Context, _ string, _ *corev1.Secret, _ map[string][]byte, - _ events.Recorder, _ any) (*corev1.Secret, *metav1.Condition, error) { + _ events.Recorder) (*corev1.Secret, *metav1.Condition, error) { return f.secret, f.cond, f.err } -func (f *fakeDriver) InformerHandler(_ any) (cache.SharedIndexInformer, factory.EventFilterFunc) { +func (f *fakeDriver) InformerHandler() (cache.SharedIndexInformer, factory.EventFilterFunc) { return nil, nil } diff --git a/pkg/registration/spoke/addon/registration_controller.go b/pkg/registration/spoke/addon/registration_controller.go index 396c12567..fc551c6c5 100644 --- a/pkg/registration/spoke/addon/registration_controller.go +++ b/pkg/registration/spoke/addon/registration_controller.go @@ -12,30 +12,18 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" "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" addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" addonclient "open-cluster-management.io/api/client/addon/clientset/versioned" addoninformerv1alpha1 "open-cluster-management.io/api/client/addon/informers/externalversions/addon/v1alpha1" addonlisterv1alpha1 "open-cluster-management.io/api/client/addon/listers/addon/v1alpha1" - clusterv1 "open-cluster-management.io/api/cluster/v1" "open-cluster-management.io/sdk-go/pkg/patcher" "open-cluster-management.io/ocm/pkg/common/queue" "open-cluster-management.io/ocm/pkg/registration/register" - "open-cluster-management.io/ocm/pkg/registration/register/csr" -) - -const ( - indexByAddon = "indexByAddon" - - // TODO(qiujian16) expose it if necessary in the future. - addonCSRThreshold = 10 ) // addOnRegistrationController monitors ManagedClusterAddOns on hub and starts addOn registration @@ -45,15 +33,14 @@ const ( type addOnRegistrationController struct { clusterName string agentName string - kubeconfig *clientcmdapi.Config + kubeconfigFile string managementKubeClient kubernetes.Interface // in-cluster local management kubeClient spokeKubeClient kubernetes.Interface hubAddOnLister addonlisterv1alpha1.ManagedClusterAddOnLister patcher patcher.Patcher[ *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus] - csrControl csr.CSRControl - recorder events.Recorder - csrIndexer cache.Indexer + addonDriver register.AddonDriver + recorder events.Recorder startRegistrationFunc func(ctx context.Context, config registrationConfig) context.CancelFunc @@ -66,37 +53,29 @@ type addOnRegistrationController struct { func NewAddOnRegistrationController( clusterName string, agentName string, - kubeconfig *clientcmdapi.Config, + kubeconfigFile string, addOnClient addonclient.Interface, managementKubeClient kubernetes.Interface, managedKubeClient kubernetes.Interface, - csrControl csr.CSRControl, + addonDriver register.AddonDriver, hubAddOnInformers addoninformerv1alpha1.ManagedClusterAddOnInformer, recorder events.Recorder, ) factory.Controller { c := &addOnRegistrationController{ clusterName: clusterName, agentName: agentName, - kubeconfig: kubeconfig, + kubeconfigFile: kubeconfigFile, managementKubeClient: managementKubeClient, spokeKubeClient: managedKubeClient, hubAddOnLister: hubAddOnInformers.Lister(), - csrControl: csrControl, + addonDriver: addonDriver, patcher: patcher.NewPatcher[ *addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus]( addOnClient.AddonV1alpha1().ManagedClusterAddOns(clusterName)), recorder: recorder, - csrIndexer: csrControl.Informer().GetIndexer(), addOnRegistrationConfigs: map[string]map[string]registrationConfig{}, } - err := csrControl.Informer().AddIndexers(cache.Indexers{ - indexByAddon: indexByAddonFunc, - }) - if err != nil { - utilruntime.HandleError(err) - } - c.startRegistrationFunc = c.startRegistration return factory.New(). @@ -217,34 +196,18 @@ func (c *addOnRegistrationController) startRegistration(ctx context.Context, con SecretName: config.secretName, ManagementCoreClient: kubeClient.CoreV1(), ManagementSecretInformer: kubeInformerFactory.Core().V1().Secrets().Informer(), + Subject: config.x509Subject(c.clusterName, c.agentName), + Signer: config.registration.SignerName, + ClusterName: c.clusterName, } if config.registration.SignerName == certificatesv1.KubeAPIServerClientSignerName { - secretOption.BootStrapKubeConfig = c.kubeconfig + secretOption.BootStrapKubeConfigFile = c.kubeconfigFile } - - driver := csr.NewCSRDriver() - csrOption := &csr.CSROption{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("addon-%s-%s-", c.clusterName, config.addOnName), - Labels: map[string]string{ - // the labels are only hints. Anyone could set/modify them. - clusterv1.ClusterNameLabelKey: c.clusterName, - addonv1alpha1.AddonLabelKey: config.addOnName, - }, - }, - Subject: config.x509Subject(c.clusterName, c.agentName), - DNSNames: []string{fmt.Sprintf("%s.addon.open-cluster-management.io", config.addOnName)}, - SignerName: config.registration.SignerName, - EventFilterFunc: createCSREventFilterFunc(c.clusterName, config.addOnName, config.registration.SignerName), - HaltCSRCreation: c.haltCSRCreationFunc(config.addOnName), - CSRControl: c.csrControl, - } - + driver := c.addonDriver.Fork(config.addOnName, secretOption) controllerName := fmt.Sprintf("ClientCertController@addon:%s:signer:%s", config.addOnName, config.registration.SignerName) statusUpdater := c.generateStatusUpdate(c.clusterName, config.addOnName) - secretController := register.NewSecretController( - secretOption, csrOption, driver, statusUpdater, c.recorder, controllerName) + secretController := register.NewSecretController(secretOption, driver, statusUpdater, c.recorder, controllerName) go kubeInformerFactory.Start(ctx.Done()) go secretController.Run(ctx, 1) @@ -252,21 +215,6 @@ func (c *addOnRegistrationController) startRegistration(ctx context.Context, con return stopFunc } -func (c *addOnRegistrationController) haltCSRCreationFunc(addonName string) func() bool { - return func() bool { - items, err := c.csrIndexer.ByIndex(indexByAddon, fmt.Sprintf("%s/%s", c.clusterName, addonName)) - if err != nil { - return false - } - - if len(items) >= addonCSRThreshold { - return true - } - - return false - } -} - func (c *addOnRegistrationController) generateStatusUpdate(clusterName, addonName string) register.StatusUpdateFunc { return func(ctx context.Context, cond metav1.Condition) error { addon, err := c.hubAddOnLister.ManagedClusterAddOns(clusterName).Get(addonName) @@ -321,53 +269,3 @@ func (c *addOnRegistrationController) cleanup(ctx context.Context, addOnName str delete(c.addOnRegistrationConfigs, addOnName) return nil } - -func indexByAddonFunc(obj interface{}) ([]string, error) { - accessor, err := meta.Accessor(obj) - if err != nil { - return nil, err - } - - cluster, ok := accessor.GetLabels()[clusterv1.ClusterNameLabelKey] - if !ok { - return []string{}, nil - } - - addon, ok := accessor.GetLabels()[addonv1alpha1.AddonLabelKey] - if !ok { - return []string{}, nil - } - - return []string{fmt.Sprintf("%s/%s", cluster, addon)}, nil -} - -func createCSREventFilterFunc(clusterName, addOnName, signerName string) factory.EventFilterFunc { - return func(obj interface{}) bool { - accessor, err := meta.Accessor(obj) - if err != nil { - return false - } - labels := accessor.GetLabels() - // only enqueue csr from a specific managed cluster - if labels[clusterv1.ClusterNameLabelKey] != clusterName { - return false - } - // only enqueue csr created for a specific addon - if labels[addonv1alpha1.AddonLabelKey] != addOnName { - return false - } - - // only enqueue csr with a specific signer name - csr, ok := obj.(*certificatesv1.CertificateSigningRequest) - if !ok { - return false - } - if len(csr.Spec.SignerName) == 0 { - return false - } - if csr.Spec.SignerName != signerName { - return false - } - return true - } -} diff --git a/pkg/registration/spoke/addon/registration_controller_test.go b/pkg/registration/spoke/addon/registration_controller_test.go index da6249129..b72c0c5e3 100644 --- a/pkg/registration/spoke/addon/registration_controller_test.go +++ b/pkg/registration/spoke/addon/registration_controller_test.go @@ -7,7 +7,6 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events/eventstesting" - certificates "k8s.io/api/certificates/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" kubefake "k8s.io/client-go/kubernetes/fake" @@ -16,61 +15,10 @@ import ( 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" testingcommon "open-cluster-management.io/ocm/pkg/common/testing" ) -func TestFilterCSREvents(t *testing.T) { - clusterName := "cluster1" - signerName := "signer1" - - cases := []struct { - name string - csr *certificates.CertificateSigningRequest - expected bool - }{ - { - name: "csr not from the managed cluster", - csr: &certificates.CertificateSigningRequest{}, - }, - { - name: "csr not for the addon", - csr: &certificates.CertificateSigningRequest{}, - }, - { - name: "csr with different signer name", - csr: &certificates.CertificateSigningRequest{}, - }, - { - name: "valid csr", - csr: &certificates.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - // the labels are only hints. Anyone could set/modify them. - clusterv1.ClusterNameLabelKey: clusterName, - addonv1alpha1.AddonLabelKey: addOnName, - }, - }, - Spec: certificates.CertificateSigningRequestSpec{ - SignerName: signerName, - }, - }, - expected: true, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - filterFunc := createCSREventFilterFunc(clusterName, addOnName, signerName) - actual := filterFunc(c.csr) - if actual != c.expected { - t.Errorf("Expected %v but got %v", c.expected, actual) - } - }) - } -} - func TestRegistrationSync(t *testing.T) { clusterName := "cluster1" signerName := "signer1" @@ -356,8 +304,8 @@ func TestRegistrationSync(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - kubeClient := kubefake.NewSimpleClientset() - managementClient := kubefake.NewSimpleClientset() + kubeClient := kubefake.NewClientset() + managementClient := kubefake.NewClientset() var addons []runtime.Object if c.addOn != nil { addons = append(addons, c.addOn) diff --git a/pkg/registration/spoke/health_checker_test.go b/pkg/registration/spoke/health_checker_test.go index 80239136c..4cf935f3e 100644 --- a/pkg/registration/spoke/health_checker_test.go +++ b/pkg/registration/spoke/health_checker_test.go @@ -57,13 +57,13 @@ func TestHubKubeConfigHealthChecker(t *testing.T) { testinghelpers.WriteFile(path.Join(testDir, "tls.crt"), c.tlsCert) } - driver := csr.NewCSRDriver() secretOption := register.SecretOption{ ClusterName: "cluster1", AgentName: "agent1", HubKubeconfigDir: testDir, HubKubeconfigFile: path.Join(testDir, "kubeconfig"), } + driver := csr.NewCSRDriver(csr.NewCSROption(), secretOption) hc := &hubKubeConfigHealthChecker{ checkFunc: register.IsHubKubeConfigValidFunc(driver, secretOption), diff --git a/pkg/registration/spoke/options.go b/pkg/registration/spoke/options.go index 2d903fdf0..b59a49240 100644 --- a/pkg/registration/spoke/options.go +++ b/pkg/registration/spoke/options.go @@ -9,13 +9,11 @@ import ( ocmfeature "open-cluster-management.io/api/feature" - commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" "open-cluster-management.io/ocm/pkg/features" "open-cluster-management.io/ocm/pkg/registration/helpers" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" ) -var ClientCertHealthCheckInterval = 30 * time.Second - // SpokeAgentOptions holds configuration for spoke cluster agent type SpokeAgentOptions struct { // The differences among BootstrapKubeconfig, BootstrapKubeconfigSecret, BootstrapKubeconfigSecrets are: @@ -32,16 +30,13 @@ type SpokeAgentOptions struct { // See more details in: https://github.com/open-cluster-management-io/ocm/pull/443#discussion_r1610868646 HubConnectionTimeoutSeconds int32 - HubKubeconfigSecret string - SpokeExternalServerURLs []string - ClusterHealthCheckPeriod time.Duration - MaxCustomClusterClaims int - ClientCertExpirationSeconds int32 - ClusterAnnotations map[string]string - RegistrationAuth string - HubClusterArn string - ManagedClusterArn string - ManagedClusterRoleSuffix string + HubKubeconfigSecret string + SpokeExternalServerURLs []string + ClusterHealthCheckPeriod time.Duration + MaxCustomClusterClaims int + ClusterAnnotations map[string]string + + RegisterDriverOption *registerfactory.Options } func NewSpokeAgentOptions() *SpokeAgentOptions { @@ -51,6 +46,8 @@ func NewSpokeAgentOptions() *SpokeAgentOptions { ClusterHealthCheckPeriod: 1 * time.Minute, MaxCustomClusterClaims: 20, HubConnectionTimeoutSeconds: 600, // by default, the timeout is 10 minutes + + RegisterDriverOption: registerfactory.NewOptions(), } return options @@ -74,20 +71,10 @@ func (o *SpokeAgentOptions) AddFlags(fs *pflag.FlagSet) { "The period to check managed cluster kube-apiserver health") fs.IntVar(&o.MaxCustomClusterClaims, "max-custom-cluster-claims", o.MaxCustomClusterClaims, "The max number of custom cluster claims to expose.") - fs.Int32Var(&o.ClientCertExpirationSeconds, "client-cert-expiration-seconds", o.ClientCertExpirationSeconds, - "The requested duration in seconds of validity of the issued client certificate. If this is not set, "+ - "the value of --cluster-signing-duration command-line flag of the kube-controller-manager will be used.") fs.StringToStringVar(&o.ClusterAnnotations, "cluster-annotations", o.ClusterAnnotations, `the annotations with the reserve prefix "agent.open-cluster-management.io" set on ManagedCluster when creating only, other actors can update it afterwards.`) - //Consider grouping these flags for driverOption in a new Option struct and add the flags using function driverOptions.AddFlags(fs). - fs.StringVar(&o.RegistrationAuth, "registration-auth", o.RegistrationAuth, - "The type of authentication to use to authenticate with hub.") - fs.StringVar(&o.HubClusterArn, "hub-cluster-arn", o.HubClusterArn, - "The ARN of the EKS based hub cluster.") - fs.StringVar(&o.ManagedClusterArn, "managed-cluster-arn", o.ManagedClusterArn, - "The ARN of the EKS based managed cluster.") - fs.StringVar(&o.ManagedClusterRoleSuffix, "managed-cluster-role-suffix", o.ManagedClusterRoleSuffix, - "The suffix of the managed cluster IAM role.") + + o.RegisterDriverOption.AddFlags(fs) } // Validate verifies the inputs. @@ -116,12 +103,8 @@ func (o *SpokeAgentOptions) Validate() error { return errors.New("cluster healthcheck period must greater than zero") } - if o.ClientCertExpirationSeconds != 0 && o.ClientCertExpirationSeconds < 3600 { - return errors.New("client certificate expiration seconds must greater or qual to 3600") - } - - if (o.RegistrationAuth == commonhelpers.AwsIrsaAuthType) && (o.HubClusterArn == "") { - return errors.New("EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa") + if err := o.RegisterDriverOption.Validate(); err != nil { + return err } return nil diff --git a/pkg/registration/spoke/spokeagent.go b/pkg/registration/spoke/spokeagent.go index 9d79aed49..956e3bcd8 100644 --- a/pkg/registration/spoke/spokeagent.go +++ b/pkg/registration/spoke/spokeagent.go @@ -9,30 +9,22 @@ import ( "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/server/healthz" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" - addonclient "open-cluster-management.io/api/client/addon/clientset/versioned" - addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions" clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned" clusterscheme "open-cluster-management.io/api/client/cluster/clientset/versioned/scheme" clusterv1informers "open-cluster-management.io/api/client/cluster/informers/externalversions" - clusterv1 "open-cluster-management.io/api/cluster/v1" ocmfeature "open-cluster-management.io/api/feature" "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" "open-cluster-management.io/ocm/pkg/features" "open-cluster-management.io/ocm/pkg/registration/register" - 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/spoke/addon" "open-cluster-management.io/ocm/pkg/registration/spoke/lease" "open-cluster-management.io/ocm/pkg/registration/spoke/managedcluster" @@ -186,19 +178,6 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - // initiate registration driver - var registerDriver register.RegisterDriver - if o.registrationOption.RegistrationAuth == helpers.AwsIrsaAuthType { - registerDriver = awsIrsa.NewAWSIRSADriver(o.registrationOption.ManagedClusterArn, - o.registrationOption.ManagedClusterRoleSuffix, - o.registrationOption.HubClusterArn, - o.agentOptions.SpokeClusterName) - } else { - registerDriver = csr.NewCSRDriver() - } - - o.driver = registerDriver - // get spoke cluster CA bundle spokeClusterCABundle, err := o.getSpokeClusterCABundle(spokeClientConfig) if err != nil { @@ -226,32 +205,25 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, o.currentBootstrapKubeConfig = o.registrationOption.BootstrapKubeconfig } - // load bootstrap client config and create bootstrap clients - bootstrapClientConfig, err := clientcmd.BuildConfigFromFlags("", o.currentBootstrapKubeConfig) - if err != nil { - return fmt.Errorf("unable to load bootstrap kubeconfig from file %q: %w", o.currentBootstrapKubeConfig, err) - } - bootstrapKubeClient, err := kubernetes.NewForConfig(bootstrapClientConfig) - if err != nil { - return err - } - bootstrapClusterClient, err := clusterv1client.NewForConfig(bootstrapClientConfig) - if err != nil { - return err + // build up the secretOption + secretOption := register.SecretOption{ + SecretNamespace: o.agentOptions.ComponentNamespace, + SecretName: o.registrationOption.HubKubeconfigSecret, + ClusterName: o.agentOptions.SpokeClusterName, + AgentName: o.agentOptions.AgentID, + ManagementSecretInformer: namespacedManagementKubeInformerFactory.Core().V1().Secrets().Informer(), + ManagementCoreClient: managementKubeClient.CoreV1(), + HubKubeconfigFile: o.agentOptions.HubKubeconfigFile, + HubKubeconfigDir: o.agentOptions.HubKubeconfigDir, + BootStrapKubeConfigFile: o.currentBootstrapKubeConfig, } - // start a SpokeClusterCreatingController to make sure there is a spoke cluster on hub cluster - spokeClusterCreatingController := registration.NewManagedClusterCreatingController( - o.agentOptions.SpokeClusterName, - []registration.ManagedClusterDecorator{ - registration.AnnotationDecorator(o.registrationOption.ClusterAnnotations), - registration.ClientConfigDecorator(o.registrationOption.SpokeExternalServerURLs, spokeClusterCABundle), - o.driver.ManagedClusterDecorator, - }, - bootstrapClusterClient, - recorder, - ) - go spokeClusterCreatingController.Run(ctx, 1) + // initiate registration driver + o.driver = o.registrationOption.RegisterDriverOption.Driver(secretOption) + bootstrapClients, err := o.driver.BuildClients(ctx, secretOption, true) + if err != nil { + return err + } secretInformer := namespacedManagementKubeInformerFactory.Core().V1().Secrets() // Register BootstrapKubeconfigEventHandler as an event handler of secret informer, @@ -279,21 +251,6 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, go namespacedManagementKubeInformerFactory.Start(ctx.Done()) // check if there already exists a valid client config for hub - kubeconfig, err := clientcmd.LoadFromFile(o.currentBootstrapKubeConfig) - if err != nil { - return err - } - secretOption := register.SecretOption{ - SecretNamespace: o.agentOptions.ComponentNamespace, - SecretName: o.registrationOption.HubKubeconfigSecret, - ClusterName: o.agentOptions.SpokeClusterName, - AgentName: o.agentOptions.AgentID, - ManagementSecretInformer: namespacedManagementKubeInformerFactory.Core().V1().Secrets().Informer(), - ManagementCoreClient: managementKubeClient.CoreV1(), - HubKubeconfigFile: o.agentOptions.HubKubeconfigFile, - HubKubeconfigDir: o.agentOptions.HubKubeconfigDir, - BootStrapKubeConfig: kubeconfig, - } o.internalHubConfigValidFunc = register.IsHubKubeConfigValidFunc(o.driver, secretOption) ok, err := o.internalHubConfigValidFunc(ctx) if err != nil { @@ -308,30 +265,26 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, if !ok { // create a ClientCertForHubController for spoke agent bootstrap // the bootstrap informers are supposed to be terminated after completing the bootstrap process. - bootstrapInformerFactory := informers.NewSharedInformerFactory(bootstrapKubeClient, 10*time.Minute) - - bootstrapClusterInformerFactory := clusterv1informers.NewSharedInformerFactory(bootstrapClusterClient, 10*time.Minute) - - // TODO: Generate csrOption or awsOption based on the value of --registration-auth may be move it under registerdriver as well - registrationAuthOption, err := o.newRestirationAuthOption( - logger, - secretOption, - bootstrapInformerFactory, - bootstrapKubeClient, - bootstrapClusterInformerFactory, - bootstrapClusterClient, + bootstrapCtx, stopBootstrap := context.WithCancel(ctx) + // start a SpokeClusterCreatingController to make sure there is a spoke cluster on hub cluster + spokeClusterCreatingController := registration.NewManagedClusterCreatingController( + o.agentOptions.SpokeClusterName, + []registration.ManagedClusterDecorator{ + registration.AnnotationDecorator(o.registrationOption.ClusterAnnotations), + registration.ClientConfigDecorator(o.registrationOption.SpokeExternalServerURLs, spokeClusterCABundle), + o.driver.ManagedClusterDecorator, + }, + bootstrapClients.ClusterClient, + recorder, ) - if err != nil { - return err - } controllerName := fmt.Sprintf("BootstrapController@cluster:%s", o.agentOptions.SpokeClusterName) - bootstrapCtx, stopBootstrap := context.WithCancel(ctx) secretController := register.NewSecretController( - secretOption, registrationAuthOption, o.driver, register.GenerateBootstrapStatusUpdater(), recorder, controllerName) + secretOption, o.driver, register.GenerateBootstrapStatusUpdater(), recorder, controllerName) - go bootstrapInformerFactory.Start(bootstrapCtx.Done()) - go bootstrapClusterInformerFactory.Start(bootstrapCtx.Done()) + go bootstrapClients.ClusterInfomerFactory.Start(bootstrapCtx.Done()) + go bootstrapClients.KubeInformerFactory.Start(bootstrapCtx.Done()) + go spokeClusterCreatingController.Run(bootstrapCtx, 1) go secretController.Run(bootstrapCtx, 1) // Wait for the hub client config is ready. @@ -357,87 +310,38 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, stopBootstrap() } - // create hub clients and shared informer factories from hub kube config - hubClientConfig, err := clientcmd.BuildConfigFromFlags("", o.agentOptions.HubKubeconfigFile) - if err != nil { - return fmt.Errorf("unable to load hub kubeconfig from file %q: %w", o.agentOptions.HubKubeconfigFile, err) - } - - hubKubeClient, err := kubernetes.NewForConfig(hubClientConfig) - if err != nil { - return fmt.Errorf("failed to create hub kube client: %w", err) - } - - hubClusterClient, err := clusterv1client.NewForConfig(hubClientConfig) - if err != nil { - return fmt.Errorf("failed to create hub cluster client: %w", err) - } - - addOnClient, err := addonclient.NewForConfig(hubClientConfig) - if err != nil { - return fmt.Errorf("failed to create addon client: %w", err) - } - - hubKubeInformerFactory := informers.NewSharedInformerFactoryWithOptions( - hubKubeClient, - 10*time.Minute, - informers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { - listOptions.LabelSelector = fmt.Sprintf("%s=%s", clusterv1.ClusterNameLabelKey, o.agentOptions.SpokeClusterName) - }), - ) - addOnInformerFactory := addoninformers.NewSharedInformerFactoryWithOptions( - addOnClient, - 10*time.Minute, - addoninformers.WithNamespace(o.agentOptions.SpokeClusterName), - ) - // create a cluster informer factory with name field selector because we just need to handle the current spoke cluster - hubClusterInformerFactory := clusterv1informers.NewSharedInformerFactoryWithOptions( - hubClusterClient, - 10*time.Minute, - clusterv1informers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { - listOptions.FieldSelector = fields.OneTermEqualSelector("metadata.name", o.agentOptions.SpokeClusterName).String() - }), - ) - - recorder.Event("HubClientConfigReady", "Client config for hub is ready.") - - registrationAuthOption, err := o.newRestirationAuthOption( - logger, - secretOption, - hubKubeInformerFactory, - hubKubeClient, - hubClusterInformerFactory, - hubClusterClient, - ) + // reset clients from driver + hubClient, err := o.driver.BuildClients(ctx, secretOption, false) if err != nil { return err } + recorder.Event("HubClientConfigReady", "Client config for hub is ready.") // create another RegisterController for registration credential rotation controllerName := fmt.Sprintf("RegisterController@cluster:%s", o.agentOptions.SpokeClusterName) secretController := register.NewSecretController( - secretOption, registrationAuthOption, o.driver, register.GenerateStatusUpdater( - hubClusterClient, - hubClusterInformerFactory.Cluster().V1().ManagedClusters().Lister(), + secretOption, o.driver, register.GenerateStatusUpdater( + hubClient.ClusterClient, + hubClient.ClusterInfomerFactory.Cluster().V1().ManagedClusters().Lister(), o.agentOptions.SpokeClusterName), recorder, controllerName) // create ManagedClusterLeaseController to keep the spoke cluster heartbeat managedClusterLeaseController := lease.NewManagedClusterLeaseController( o.agentOptions.SpokeClusterName, - hubKubeClient, - hubClusterInformerFactory.Cluster().V1().ManagedClusters(), + hubClient.KubeClient, + hubClient.ClusterInfomerFactory.Cluster().V1().ManagedClusters(), recorder, ) - hubEventRecorder, err := helpers.NewEventRecorder(ctx, clusterscheme.Scheme, hubKubeClient, "klusterlet-agent") + hubEventRecorder, err := helpers.NewEventRecorder(ctx, clusterscheme.Scheme, hubClient.KubeClient, "klusterlet-agent") if err != nil { return fmt.Errorf("failed to create event recorder: %w", err) } // create NewManagedClusterStatusController to update the spoke cluster status managedClusterHealthCheckController := managedcluster.NewManagedClusterStatusController( o.agentOptions.SpokeClusterName, - hubClusterClient, - hubClusterInformerFactory.Cluster().V1().ManagedClusters(), + hubClient.ClusterClient, + hubClient.ClusterInfomerFactory.Cluster().V1().ManagedClusters(), spokeKubeClient.Discovery(), spokeClusterInformerFactory.Cluster().V1alpha1().ClusterClaims(), spokeKubeInformerFactory.Core().V1().Nodes(), @@ -452,8 +356,8 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, if features.SpokeMutableFeatureGate.Enabled(ocmfeature.AddonManagement) { addOnLeaseController = addon.NewManagedClusterAddOnLeaseController( o.agentOptions.SpokeClusterName, - addOnClient, - addOnInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(), + hubClient.AddonClient, + hubClient.AddonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(), managementKubeClient.CoordinationV1(), spokeKubeClient.CoordinationV1(), AddOnLeaseControllerSyncInterval, //TODO: this interval time should be allowed to change from outside @@ -461,16 +365,16 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, ) // addon registration only enabled when the registration driver is csr. - if csrOption, ok := registrationAuthOption.(*csr.CSROption); ok { + if addonDriver, ok := o.driver.(register.AddonDriver); ok { addOnRegistrationController = addon.NewAddOnRegistrationController( o.agentOptions.SpokeClusterName, o.agentOptions.AgentID, - kubeconfig, - addOnClient, + o.currentBootstrapKubeConfig, + hubClient.AddonClient, managementKubeClient, spokeKubeClient, - csrOption.CSRControl, - addOnInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(), + addonDriver, + hubClient.AddonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(), recorder, ) } @@ -480,7 +384,7 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, if features.SpokeMutableFeatureGate.Enabled(ocmfeature.MultipleHubs) { hubAcceptController = registration.NewHubAcceptController( o.agentOptions.SpokeClusterName, - hubClusterInformerFactory.Cluster().V1().ManagedClusters(), + hubClient.ClusterInfomerFactory.Cluster().V1().ManagedClusters(), func(ctx context.Context) error { logger.Info("Failed to connect to hub because of hubAcceptClient set to false, restart agent to reselect a new bootstrap kubeconfig") o.agentStopFunc() @@ -491,7 +395,7 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, hubTimeoutController = registration.NewHubTimeoutController( o.agentOptions.SpokeClusterName, - hubKubeClient, + hubClient.KubeClient, o.registrationOption.HubConnectionTimeoutSeconds, func(ctx context.Context) error { logger.Info("Failed to connect to hub because of lease out-of-date, restart agent to reselect a new bootstrap kubeconfig") @@ -502,10 +406,10 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, ) } - go hubKubeInformerFactory.Start(ctx.Done()) - go hubClusterInformerFactory.Start(ctx.Done()) + go hubClient.KubeInformerFactory.Start(ctx.Done()) + go hubClient.ClusterInfomerFactory.Start(ctx.Done()) go namespacedManagementKubeInformerFactory.Start(ctx.Done()) - go addOnInformerFactory.Start(ctx.Done()) + go hubClient.AddonInformerFactory.Start(ctx.Done()) go spokeKubeInformerFactory.Start(ctx.Done()) if features.SpokeMutableFeatureGate.Enabled(ocmfeature.ClusterClaim) { @@ -518,7 +422,7 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, if features.SpokeMutableFeatureGate.Enabled(ocmfeature.AddonManagement) { go addOnLeaseController.Run(ctx, 1) // addon controller will only run when the registration driver is csr. - if _, ok := registrationAuthOption.(*csr.CSROption); ok { + if _, ok := o.driver.(register.AddonDriver); ok { go addOnRegistrationController.Run(ctx, 1) } } @@ -558,29 +462,3 @@ func (o *SpokeAgentConfig) getSpokeClusterCABundle(kubeConfig *rest.Config) ([]b } return data, nil } - -func (o *SpokeAgentConfig) newRestirationAuthOption( - logger klog.Logger, - secretOption register.SecretOption, - kubeInformers informers.SharedInformerFactory, - kubeClient kubernetes.Interface, - clusterInformers clusterv1informers.SharedInformerFactory, - clusterClient clusterv1client.Interface, -) (any, error) { - if o.registrationOption.RegistrationAuth == helpers.AwsIrsaAuthType { - if o.registrationOption.HubClusterArn != "" { - return awsIrsa.NewAWSOption( - secretOption, - clusterInformers.Cluster(), - clusterClient) - } else { - return nil, fmt.Errorf("please provide EKS Hub Cluster ARN for the awsirsa based authentication") - } - } else { - return csr.NewCSROption(logger, - secretOption, - o.registrationOption.ClientCertExpirationSeconds, - kubeInformers.Certificates(), - kubeClient) - } -} diff --git a/pkg/registration/spoke/spokeagent_test.go b/pkg/registration/spoke/spokeagent_test.go index 850102bc7..ef578c1b5 100644 --- a/pkg/registration/spoke/spokeagent_test.go +++ b/pkg/registration/spoke/spokeagent_test.go @@ -25,6 +25,8 @@ import ( testingcommon "open-cluster-management.io/ocm/pkg/common/testing" "open-cluster-management.io/ocm/pkg/features" testinghelpers "open-cluster-management.io/ocm/pkg/registration/helpers/testing" + "open-cluster-management.io/ocm/pkg/registration/register/csr" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/test/integration/util" ) @@ -44,13 +46,6 @@ func init() { } func TestValidate(t *testing.T) { - defaultCompletedOptions := NewSpokeAgentOptions() - defaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig" - awsCompletedOptionsHubArnMissing := *defaultCompletedOptions - awsCompletedOptionsHubArnMissing.RegistrationAuth = commonhelpers.AwsIrsaAuthType - awsDefaultCompletedOptions := awsCompletedOptionsHubArnMissing - awsDefaultCompletedOptions.HubClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" - cases := []struct { name string options *SpokeAgentOptions @@ -79,50 +74,77 @@ func TestValidate(t *testing.T) { expectedErr: "cluster healthcheck period must greater than zero", }, { - name: "default completed options", - options: defaultCompletedOptions, + name: "default completed options", + options: func() *SpokeAgentOptions { + defaultCompletedOptions := NewSpokeAgentOptions() + defaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig" + return defaultCompletedOptions + }(), expectedErr: "", }, { - name: "default completed options for aws flow", - options: &awsDefaultCompletedOptions, + name: "default completed options for aws flow", + options: func() *SpokeAgentOptions { + awsDefaultCompletedOptions := NewSpokeAgentOptions() + awsDefaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig" + awsDefaultCompletedOptions.RegisterDriverOption.RegistrationAuth = commonhelpers.AwsIrsaAuthType + awsDefaultCompletedOptions.RegisterDriverOption.AWSISRAOption.HubClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" + return awsDefaultCompletedOptions + }(), expectedErr: "", }, { - name: "default completed options without HubClusterArn for aws flow", - options: &awsCompletedOptionsHubArnMissing, + name: "default completed options without HubClusterArn for aws flow", + options: func() *SpokeAgentOptions { + awsCompletedOptionsHubArnMissing := NewSpokeAgentOptions() + awsCompletedOptionsHubArnMissing.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig" + awsCompletedOptionsHubArnMissing.RegisterDriverOption.RegistrationAuth = commonhelpers.AwsIrsaAuthType + return awsCompletedOptionsHubArnMissing + }(), expectedErr: "EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa", }, { name: "default completed options", options: &SpokeAgentOptions{ - HubKubeconfigSecret: "hub-kubeconfig-secret", - ClusterHealthCheckPeriod: 1 * time.Minute, - MaxCustomClusterClaims: 20, - BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig", - ClientCertExpirationSeconds: 3599, + HubKubeconfigSecret: "hub-kubeconfig-secret", + ClusterHealthCheckPeriod: 1 * time.Minute, + MaxCustomClusterClaims: 20, + BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig", + RegisterDriverOption: ®isterfactory.Options{ + CSROption: &csr.Option{ + ExpirationSeconds: 3599, + }, + }, }, expectedErr: "client certificate expiration seconds must greater or qual to 3600", }, { name: "default completed options", options: &SpokeAgentOptions{ - HubKubeconfigSecret: "hub-kubeconfig-secret", - ClusterHealthCheckPeriod: 1 * time.Minute, - MaxCustomClusterClaims: 20, - BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig", - ClientCertExpirationSeconds: 3600, + HubKubeconfigSecret: "hub-kubeconfig-secret", + ClusterHealthCheckPeriod: 1 * time.Minute, + MaxCustomClusterClaims: 20, + BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig", + RegisterDriverOption: ®isterfactory.Options{ + CSROption: &csr.Option{ + ExpirationSeconds: 3600, + }, + }, }, expectedErr: "", }, { name: "MultipleHubs enabled, but bootstrapkubeconfigs is empty", options: &SpokeAgentOptions{ - HubKubeconfigSecret: "hub-kubeconfig-secret", - ClusterHealthCheckPeriod: 1 * time.Minute, - MaxCustomClusterClaims: 20, - BootstrapKubeconfigs: []string{}, - ClientCertExpirationSeconds: 3600, + HubKubeconfigSecret: "hub-kubeconfig-secret", + ClusterHealthCheckPeriod: 1 * time.Minute, + MaxCustomClusterClaims: 20, + BootstrapKubeconfigs: []string{}, + RegisterDriverOption: ®isterfactory.Options{ + CSROption: &csr.Option{ + ExpirationSeconds: 3600, + }, + }, }, pre: func() { _ = features.SpokeMutableFeatureGate.SetFromMap(map[string]bool{ @@ -141,7 +163,11 @@ func TestValidate(t *testing.T) { "/spoke/bootstrap/kubeconfig-hub1", "/spoke/bootstrap/kubeconfig-hub2", }, - ClientCertExpirationSeconds: 3600, + RegisterDriverOption: ®isterfactory.Options{ + CSROption: &csr.Option{ + ExpirationSeconds: 3600, + }, + }, }, pre: func() { _ = features.SpokeMutableFeatureGate.SetFromMap(map[string]bool{ diff --git a/test/integration/registration/addon_lease_test.go b/test/integration/registration/addon_lease_test.go index 936b84ad9..114b2798c 100644 --- a/test/integration/registration/addon_lease_test.go +++ b/test/integration/registration/addon_lease_test.go @@ -19,6 +19,7 @@ import ( commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -166,6 +167,7 @@ var _ = ginkgo.Describe("Addon Lease Resync", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() diff --git a/test/integration/registration/addon_registration_test.go b/test/integration/registration/addon_registration_test.go index 7c064ba59..ed86033bc 100644 --- a/test/integration/registration/addon_registration_test.go +++ b/test/integration/registration/addon_registration_test.go @@ -24,6 +24,7 @@ import ( 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/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -46,6 +47,7 @@ var _ = ginkgo.Describe("Addon Registration", func() { BootstrapKubeconfig: bootstrapKubeconfig, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() diff --git a/test/integration/registration/certificate_rotation_test.go b/test/integration/registration/certificate_rotation_test.go index aae2a481f..3b75b664c 100644 --- a/test/integration/registration/certificate_rotation_test.go +++ b/test/integration/registration/certificate_rotation_test.go @@ -8,6 +8,7 @@ import ( "github.com/onsi/gomega" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -25,6 +26,7 @@ var _ = ginkgo.Describe("Certificate Rotation", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() diff --git a/test/integration/registration/clusterannotations_aws_test.go b/test/integration/registration/clusterannotations_aws_test.go index f8c9222fb..cc89fa822 100644 --- a/test/integration/registration/clusterannotations_aws_test.go +++ b/test/integration/registration/clusterannotations_aws_test.go @@ -13,6 +13,7 @@ import ( 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/aws_irsa" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -28,10 +29,14 @@ var _ = ginkgo.Describe("Cluster Annotations for aws", func() { managedClusterRoleSuffix := "7f8141296c75f2871e3d030f85c35692" hubClusterArn := "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" agentOptions := &spoke.SpokeAgentOptions{ - RegistrationAuth: commonhelpers.AwsIrsaAuthType, - HubClusterArn: hubClusterArn, - ManagedClusterArn: managedClusterArn, - ManagedClusterRoleSuffix: managedClusterRoleSuffix, + RegisterDriverOption: ®isterfactory.Options{ + RegistrationAuth: commonhelpers.AwsIrsaAuthType, + AWSISRAOption: &aws_irsa.AWSOption{ + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + }, + }, BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, diff --git a/test/integration/registration/clusterannotations_test.go b/test/integration/registration/clusterannotations_test.go index 7d64c4f20..3911f3090 100644 --- a/test/integration/registration/clusterannotations_test.go +++ b/test/integration/registration/clusterannotations_test.go @@ -12,6 +12,7 @@ import ( commonoptions "open-cluster-management.io/ocm/pkg/common/options" "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -31,6 +32,7 @@ var _ = ginkgo.Describe("Cluster Annotations", func() { "agent.open-cluster-management.io/foo": "bar", "foo": "bar", // this annotation should be filtered out }, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() diff --git a/test/integration/registration/integration_suite_test.go b/test/integration/registration/integration_suite_test.go index 12e3c9869..cf01ad102 100644 --- a/test/integration/registration/integration_suite_test.go +++ b/test/integration/registration/integration_suite_test.go @@ -119,7 +119,6 @@ var _ = ginkgo.BeforeSuite(func() { // crank up the addon lease sync and udpate speed spoke.AddOnLeaseControllerSyncInterval = 5 * time.Second - spoke.ClientCertHealthCheckInterval = 5 * time.Second addon.AddOnLeaseControllerLeaseDurationSeconds = 1 // install cluster CRD and start a local kube-apiserver diff --git a/test/integration/registration/managedcluster_lease_test.go b/test/integration/registration/managedcluster_lease_test.go index 17709d366..eb96b08eb 100644 --- a/test/integration/registration/managedcluster_lease_test.go +++ b/test/integration/registration/managedcluster_lease_test.go @@ -16,6 +16,7 @@ import ( clusterv1 "open-cluster-management.io/api/cluster/v1" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -37,6 +38,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -56,6 +58,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -76,6 +79,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions = commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -94,6 +98,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -145,6 +150,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -172,6 +178,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokeagent_rebootstrap_test.go b/test/integration/registration/spokeagent_rebootstrap_test.go index 7c14aafb4..83b53dac1 100644 --- a/test/integration/registration/spokeagent_rebootstrap_test.go +++ b/test/integration/registration/spokeagent_rebootstrap_test.go @@ -462,14 +462,14 @@ var _ = ginkgo.Describe("Rebootstrap", func() { } }) - ginkgo.It("should join the hub once the bootstrap kubeconfig becomes vaid", func() { + ginkgo.It("should join the hub once the bootstrap kubeconfig becomes valid", func() { // the spoke cluster should not be created - gomega.Consistently(func() bool { + gomega.Consistently(func() error { if _, err := util.GetManagedCluster(clusterClient, managedClusterName); apierrors.IsNotFound(err) { - return true + return nil } - return false - }, 15, 3).Should(gomega.BeTrue()) + return fmt.Errorf("managed cluster should not be created") + }, 15, 3).Should(gomega.Succeed()) ginkgo.By("Replace the bootstrap kubeconfig with a valid one") err := authn.CreateBootstrapKubeConfigWithCertAge(bootstrapFile, serverCertFile, securePort, 10*time.Minute) diff --git a/test/integration/registration/spokeagent_recovery_test.go b/test/integration/registration/spokeagent_recovery_test.go index 02c34058b..a9e0f905c 100644 --- a/test/integration/registration/spokeagent_recovery_test.go +++ b/test/integration/registration/spokeagent_recovery_test.go @@ -15,6 +15,7 @@ import ( clusterv1 "open-cluster-management.io/api/cluster/v1" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -40,6 +41,7 @@ var _ = ginkgo.Describe("Agent Recovery", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -129,6 +131,7 @@ var _ = ginkgo.Describe("Agent Recovery", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokeagent_restart_test.go b/test/integration/registration/spokeagent_restart_test.go index 30195984a..ac32abb63 100644 --- a/test/integration/registration/spokeagent_restart_test.go +++ b/test/integration/registration/spokeagent_restart_test.go @@ -16,6 +16,7 @@ import ( clusterv1 "open-cluster-management.io/api/cluster/v1" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -40,6 +41,7 @@ var _ = ginkgo.Describe("Agent Restart", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -117,6 +119,7 @@ var _ = ginkgo.Describe("Agent Restart", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions = commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -170,6 +173,7 @@ var _ = ginkgo.Describe("Agent Restart", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -226,6 +230,7 @@ var _ = ginkgo.Describe("Agent Restart", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions = commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir @@ -293,6 +298,7 @@ var _ = ginkgo.Describe("Agent Restart", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokecluster_autoapproval_test.go b/test/integration/registration/spokecluster_autoapproval_test.go index 9c18a489e..2d07ac0aa 100644 --- a/test/integration/registration/spokecluster_autoapproval_test.go +++ b/test/integration/registration/spokecluster_autoapproval_test.go @@ -15,6 +15,7 @@ import ( clusterv1 "open-cluster-management.io/api/cluster/v1" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -38,6 +39,7 @@ var _ = ginkgo.Describe("Cluster Auto Approval", func() { BootstrapKubeconfig: bootstrapFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokecluster_aws_joining_test.go b/test/integration/registration/spokecluster_aws_joining_test.go index abf1292dc..3069a8e41 100644 --- a/test/integration/registration/spokecluster_aws_joining_test.go +++ b/test/integration/registration/spokecluster_aws_joining_test.go @@ -15,6 +15,8 @@ import ( "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" + awsirsa "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -42,10 +44,14 @@ var _ = ginkgo.Describe("Joining Process for aws flow", func() { // run registration agent agentOptions := &spoke.SpokeAgentOptions{ - RegistrationAuth: helpers.AwsIrsaAuthType, - HubClusterArn: hubClusterArn, - ManagedClusterArn: managedClusterArn, - ManagedClusterRoleSuffix: managedClusterRoleSuffix, + RegisterDriverOption: ®isterfactory.Options{ + RegistrationAuth: helpers.AwsIrsaAuthType, + AWSISRAOption: &awsirsa.AWSOption{ + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + }, + }, BootstrapKubeconfig: bootstrapKubeconfig, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, @@ -168,10 +174,14 @@ var _ = ginkgo.Describe("Joining Process for aws flow", func() { // run registration agent agentOptions := &spoke.SpokeAgentOptions{ - RegistrationAuth: helpers.AwsIrsaAuthType, - HubClusterArn: hubClusterArn, - ManagedClusterArn: managedClusterArn, - ManagedClusterRoleSuffix: managedClusterRoleSuffix, + RegisterDriverOption: ®isterfactory.Options{ + RegistrationAuth: helpers.AwsIrsaAuthType, + AWSISRAOption: &awsirsa.AWSOption{ + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + }, + }, BootstrapKubeconfig: bootstrapKubeconfig, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, @@ -201,10 +211,14 @@ var _ = ginkgo.Describe("Joining Process for aws flow", func() { // run registration agent agentOptions := &spoke.SpokeAgentOptions{ - RegistrationAuth: helpers.AwsIrsaAuthType, - HubClusterArn: hubClusterArn, - ManagedClusterArn: managedClusterArn, - ManagedClusterRoleSuffix: managedClusterRoleSuffix, + RegisterDriverOption: ®isterfactory.Options{ + RegistrationAuth: helpers.AwsIrsaAuthType, + AWSISRAOption: &awsirsa.AWSOption{ + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + }, + }, BootstrapKubeconfig: bootstrapKubeconfig, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, diff --git a/test/integration/registration/spokecluster_claim_test.go b/test/integration/registration/spokecluster_claim_test.go index 30c169c86..e8e2081ba 100644 --- a/test/integration/registration/spokecluster_claim_test.go +++ b/test/integration/registration/spokecluster_claim_test.go @@ -18,6 +18,7 @@ import ( commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -55,6 +56,7 @@ var _ = ginkgo.Describe("Cluster Claim", func() { HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, MaxCustomClusterClaims: maxCustomClusterClaims, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokecluster_joining_test.go b/test/integration/registration/spokecluster_joining_test.go index b5686acce..7a1978ace 100644 --- a/test/integration/registration/spokecluster_joining_test.go +++ b/test/integration/registration/spokecluster_joining_test.go @@ -15,6 +15,7 @@ import ( commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -42,6 +43,7 @@ var _ = ginkgo.Describe("Joining Process", func() { BootstrapKubeconfig: bootstrapKubeconfig, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/spokecluster_status_test.go b/test/integration/registration/spokecluster_status_test.go index e7b9394df..f2e12d4c7 100644 --- a/test/integration/registration/spokecluster_status_test.go +++ b/test/integration/registration/spokecluster_status_test.go @@ -13,6 +13,7 @@ import ( commonhelpers "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -37,6 +38,7 @@ var _ = ginkgo.Describe("Collecting Node Resource", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir diff --git a/test/integration/registration/taint_add_test.go b/test/integration/registration/taint_add_test.go index 7a56784d2..48f6aefae 100644 --- a/test/integration/registration/taint_add_test.go +++ b/test/integration/registration/taint_add_test.go @@ -18,6 +18,7 @@ import ( commonoptions "open-cluster-management.io/ocm/pkg/common/options" "open-cluster-management.io/ocm/pkg/registration/helpers" "open-cluster-management.io/ocm/pkg/registration/hub/taint" + registerfactory "open-cluster-management.io/ocm/pkg/registration/register/factory" "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) @@ -41,6 +42,7 @@ var _ = ginkgo.Describe("ManagedCluster Taints Update", func() { BootstrapKubeconfig: bootstrapKubeConfigFile, HubKubeconfigSecret: hubKubeconfigSecret, ClusterHealthCheckPeriod: 1 * time.Minute, + RegisterDriverOption: registerfactory.NewOptions(), } commOptions := commonoptions.NewAgentOptions() commOptions.HubKubeconfigDir = hubKubeconfigDir