✨ Support token-based authentication for template addons (#1363)

* ✨ Support token-based authentication for template addons

This change enables template type addons to work with both CSR-based
and token-based authentication through dynamic subject binding.

Changes:
- Modified createPermissionBinding() to extract dynamic subjects from
  addon.Status.Registrations instead of using hardcoded groups
- Added buildSubjectsFromRegistration() helper to extract user/groups
  from registration status
- Returns SubjectNotReadyError when subjects not ready (enables retry)
- Removed clusterAddonGroup() function (no longer needed)
- Updated addon-framework dependency to v1.2.0 for SubjectNotReadyError
- Added comprehensive tests for buildSubjectsFromRegistration
- Updated test helpers to include registration status with proper subjects

The implementation now supports:
- CSR-based authentication (existing)
- Token-based authentication (new)
- Any future authentication method that populates Status.Registrations

Related: 14af2a2eeb/enhancements/sig-architecture/167-token-based-addon-registration/README.md

🤖 Generated with Claude Code
https://claude.com/claude-code

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>

* test: add unit test for system:authenticated group filtering

Add a test case to verify that buildSubjectsFromRegistration correctly
filters out the system:authenticated group from the list of groups when
building RBAC subjects. This covers the filtering logic in
registration.go lines 560-562.

Also update the expected groups in TestTemplateCSRConfigurationsFunc
to match the implementation that includes both cluster-specific and
addon-wide groups for token-based authentication.

Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>

* feat: add addon-wide group and filter system:authenticated

Add support for addon-wide group in defaultGroups() to support
token-based authentication for template addons. This adds the
system:open-cluster-management:addon:{addonName} group in addition
to the cluster-specific group.

Also add filtering logic in buildSubjectsFromRegistration() to
exclude the system:authenticated group from RBAC subjects, as this
is a special Kubernetes group automatically added to all authenticated
users and should not be explicitly included in RoleBindings.

Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>

* refactor: implement custom CSR approver with flexible org validation

Replace addon-framework's DefaultCSRApprover with a custom implementation
that supports both legacy and new CSR organization structures.

Key changes:
- Implement defaultCSRApprover function that accepts 2 or 3 organization units
  - 3 orgs: legacy behavior including system:authenticated group in CSRs
  - 2 orgs: new behavior where system:authenticated is filtered out
- Add support for gRPC-based CSR requests by checking CSRUsernameAnnotation
- Validate all required default addon groups are present in CSR
- Add necessary imports: k8s.io/apimachinery/pkg/util/sets and operatorapiv1

This enables backward compatibility while supporting the new token-based
authentication flow where system:authenticated is excluded from CSR orgs
but included in registration configs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>

* refactor: use addon-framework's updated KubeClientSignerConfigurations

Remove custom implementations and use addon-framework's native functions
which now include system:authenticated group by default.

Changes:
- Remove custom kubeClientSignerConfigurations function
- Remove custom defaultGroups function
- Remove custom defaultCSRApprover function
- Use agent.KubeClientSignerConfigurations from addon-framework
- Use utils.DefaultCSRApprover from addon-framework
- Remove unused imports: k8s.io/apimachinery/pkg/util/sets and operatorapiv1

The addon-framework has been updated to include system:authenticated in
DefaultGroups(), eliminating the need for custom implementations.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>

---------

Signed-off-by: zhujian <jiazhu@redhat.com>
Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

pkg/addon/templateagent/registration.go

@@ -15,6 +15,7 @@ import (
 	certificatesv1 "k8s.io/api/certificates/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -353,7 +354,8 @@ func extractCAdata(caCertData, caKeyData []byte) ([]byte, []byte, error) {
 // that is deployed by addon template.
 // the returned func will create a rolebinding to bind the clusterRole/role which is
 // specified by the user, so the user is required to make sure the existence of the
-// clusterRole/role
+// clusterRole/role.
+// It uses dynamic subject binding which works with both CSR-based and token-based authentication.
 func (a *CRDTemplateAgentAddon) TemplatePermissionConfigFunc() agent.PermissionConfigFunc {
 
 	return func(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) error {
@@ -451,6 +453,22 @@ func (a *CRDTemplateAgentAddon) createKubeClientPermissions(
 func (a *CRDTemplateAgentAddon) createPermissionBinding(clusterName, addonName, namespace string,
 	roleRef rbacv1.RoleRef, owner *metav1.OwnerReference) error {
 
+	// Get the ManagedClusterAddOn to extract dynamic subjects from Status.Registrations
+	addon, err := a.addonLister.ManagedClusterAddOns(clusterName).Get(addonName)
+	if err != nil {
+		return fmt.Errorf("failed to get ManagedClusterAddOn %s/%s: %w", clusterName, addonName, err)
+	}
+
+	// Build subjects dynamically from addon.Status.Registrations for KubeClient signer
+	// This works with both CSR-based and token-based authentication
+	subjects := buildSubjectsFromRegistration(addon, certificatesv1.KubeAPIServerClientSignerName)
+
+	// If no subjects found, return pending error to retry later
+	// This can happen when the addon is first created and registrations are not yet populated
+	if len(subjects) == 0 {
+		return &agent.SubjectNotReadyError{}
+	}
+
 	binding := &rbacv1.RoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: fmt.Sprintf("open-cluster-management:%s:%s:agent",
@@ -461,14 +479,8 @@ func (a *CRDTemplateAgentAddon) createPermissionBinding(clusterName, addonName,
 				AddonTemplateLabelKey:          "",
 			},
 		},
-		RoleRef: roleRef,
-		Subjects: []rbacv1.Subject{
-			{
-				Kind:     rbacv1.GroupKind,
-				APIGroup: rbacv1.GroupName,
-				Name:     clusterAddonGroup(clusterName, addonName),
-			},
-		},
+		RoleRef:  roleRef,
+		Subjects: subjects,
 	}
 	if owner != nil {
 		binding.OwnerReferences = []metav1.OwnerReference{*owner}
@@ -484,12 +496,53 @@ func (a *CRDTemplateAgentAddon) createPermissionBinding(clusterName, addonName,
 		a.hubKubeClient.RbacV1(), recorderWrapper, binding)
 	if err == nil && modified {
 		a.logger.Info("Rolebinding for addon updated", "namespace", binding.Namespace, "name", binding.Name,
-			"clusterName", clusterName, "addonName", addonName)
+			"clusterName", clusterName, "addonName", addonName, "subjects", subjects)
 	}
 	return err
 }
 
-// clusterAddonGroup returns the group that represents the addon for the cluster
-func clusterAddonGroup(clusterName, addonName string) string {
-	return fmt.Sprintf("system:open-cluster-management:cluster:%s:addon:%s", clusterName, addonName)
+// buildSubjectsFromRegistration extracts and builds RBAC subjects from addon registration status.
+// Returns nil if no matching registration is found or if the subject is empty.
+// This function is based on the addon-framework implementation to support dynamic subject binding
+// for both CSR-based and token-based authentication.
+func buildSubjectsFromRegistration(addon *addonapiv1alpha1.ManagedClusterAddOn, signerName string) []rbacv1.Subject {
+	// Find the registration config for the specified signer
+	var subject *addonapiv1alpha1.Subject
+	for _, registration := range addon.Status.Registrations {
+		if registration.SignerName == signerName {
+			subject = &registration.Subject
+			break
+		}
+	}
+
+	// If no registration config found or subject is empty, return nil
+	if subject == nil || equality.Semantic.DeepEqual(*subject, addonapiv1alpha1.Subject{}) {
+		return nil
+	}
+
+	// Build subjects from the registration config subject
+	subjects := []rbacv1.Subject{}
+
+	// Include user if set
+	if subject.User != "" {
+		subjects = append(subjects, rbacv1.Subject{
+			Kind:     rbacv1.UserKind,
+			APIGroup: rbacv1.GroupName,
+			Name:     subject.User,
+		})
+	}
+
+	// Include groups
+	for _, group := range subject.Groups {
+		if group == "system:authenticated" {
+			continue
+		}
+		subjects = append(subjects, rbacv1.Subject{
+			Kind:     rbacv1.GroupKind,
+			APIGroup: rbacv1.GroupName,
+			Name:     group,
+		})
+	}
+
+	return subjects
 }

pkg/addon/templateagent/registration_test.go

@@ -458,9 +458,25 @@ func NewFakeTemplateManagedClusterAddon(name, clusterName, addonTemplateName, ad
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: clusterName,
+			UID:       "fakeuid",
+		},
+		Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{},
+		Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+			// Add default registration status with subjects for dynamic RBAC binding
+			Registrations: []addonapiv1alpha1.RegistrationConfig{
+				{
+					SignerName: certificatesv1.KubeAPIServerClientSignerName,
+					Subject: addonapiv1alpha1.Subject{
+						User: fmt.Sprintf("system:open-cluster-management:cluster:%s:addon:%s:agent:%s-agent",
+							clusterName, name, name),
+						Groups: []string{
+							fmt.Sprintf("system:open-cluster-management:cluster:%s:addon:%s", clusterName, name),
+							fmt.Sprintf("system:open-cluster-management:addon:%s", name),
+						},
+					},
+				},
+			},
 		},
-		Spec:   addonapiv1alpha1.ManagedClusterAddOnSpec{},
-		Status: addonapiv1alpha1.ManagedClusterAddOnStatus{},
 	}
 
 	if addonTemplateName != "" {
@@ -677,13 +693,25 @@ func TestTemplatePermissionConfigFunc(t *testing.T) {
 				if len(rb.OwnerReferences) != 0 {
 					t.Errorf("expected rolebinding to have 0 owner reference, got %d", len(rb.OwnerReferences))
 				}
-				if len(rb.Subjects) != 1 {
-					t.Errorf("expected rolebinding to have 1 subject, got %d", len(rb.Subjects))
+				// Dynamic subjects: 1 user + 2 groups
+				if len(rb.Subjects) != 3 {
+					t.Errorf("expected rolebinding to have 3 subjects, got %d", len(rb.Subjects))
 				}
-				if rb.Subjects[0].Name != "system:open-cluster-management:cluster:cluster1:addon:addon1" {
-					t.Errorf("expected rolebinding subject name to be system:open-cluster-management:cluster:cluster1:addon:addon1, got %s",
+				// Verify user subject
+				if rb.Subjects[0].Kind != rbacv1.UserKind {
+					t.Errorf("expected first subject to be User, got %s", rb.Subjects[0].Kind)
+				}
+				if rb.Subjects[0].Name != "system:open-cluster-management:cluster:cluster1:addon:addon1:agent:addon1-agent" {
+					t.Errorf("expected user subject name to be system:open-cluster-management:cluster:cluster1:addon:addon1:agent:addon1-agent, got %s",
 						rb.Subjects[0].Name)
 				}
+				// Verify group subjects
+				if rb.Subjects[1].Kind != rbacv1.GroupKind {
+					t.Errorf("expected second subject to be Group, got %s", rb.Subjects[1].Kind)
+				}
+				if rb.Subjects[2].Kind != rbacv1.GroupKind {
+					t.Errorf("expected third subject to be Group, got %s", rb.Subjects[2].Kind)
+				}
 			},
 		},
 		{
@@ -791,3 +819,254 @@ func TestAddonManagerNamespace(t *testing.T) {
 		os.Setenv("POD_NAMESPACE", "")
 	}
 }
+
+func TestBuildSubjectsFromRegistration(t *testing.T) {
+	cases := []struct {
+		name             string
+		addon            *addonapiv1alpha1.ManagedClusterAddOn
+		signerName       string
+		expectedSubjects []rbacv1.Subject
+	}{
+		{
+			name: "no registrations",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{},
+			},
+			signerName:       certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: nil,
+		},
+		{
+			name: "registration with user and groups",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject: addonapiv1alpha1.Subject{
+								User: "system:open-cluster-management:cluster:cluster1:addon:test-addon:agent:test-addon-agent",
+								Groups: []string{
+									"system:open-cluster-management:cluster:cluster1:addon:test-addon",
+									"system:open-cluster-management:addon:test-addon",
+								},
+							},
+						},
+					},
+				},
+			},
+			signerName: certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: []rbacv1.Subject{
+				{
+					Kind:     rbacv1.UserKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:open-cluster-management:cluster:cluster1:addon:test-addon:agent:test-addon-agent",
+				},
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:open-cluster-management:cluster:cluster1:addon:test-addon",
+				},
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:open-cluster-management:addon:test-addon",
+				},
+			},
+		},
+		{
+			name: "registration with only groups",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject: addonapiv1alpha1.Subject{
+								Groups: []string{
+									"system:open-cluster-management:cluster:cluster1:addon:test-addon",
+								},
+							},
+						},
+					},
+				},
+			},
+			signerName: certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: []rbacv1.Subject{
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:open-cluster-management:cluster:cluster1:addon:test-addon",
+				},
+			},
+		},
+		{
+			name: "registration with only user",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject: addonapiv1alpha1.Subject{
+								User: "system:serviceaccount:cluster1:test-addon-sa",
+							},
+						},
+					},
+				},
+			},
+			signerName: certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: []rbacv1.Subject{
+				{
+					Kind:     rbacv1.UserKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:serviceaccount:cluster1:test-addon-sa",
+				},
+			},
+		},
+		{
+			name: "empty subject",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject:    addonapiv1alpha1.Subject{},
+						},
+					},
+				},
+			},
+			signerName:       certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: nil,
+		},
+		{
+			name: "signer name not found",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: "custom.signer/name",
+							Subject: addonapiv1alpha1.Subject{
+								User: "test-user",
+							},
+						},
+					},
+				},
+			},
+			signerName:       certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: nil,
+		},
+		{
+			name: "multiple registrations - finds correct one",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: "custom.signer/name",
+							Subject: addonapiv1alpha1.Subject{
+								User: "custom-user",
+							},
+						},
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject: addonapiv1alpha1.Subject{
+								User: "kube-user",
+								Groups: []string{
+									"kube-group",
+								},
+							},
+						},
+					},
+				},
+			},
+			signerName: certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: []rbacv1.Subject{
+				{
+					Kind:     rbacv1.UserKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "kube-user",
+				},
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "kube-group",
+				},
+			},
+		},
+		{
+			name: "groups with system:authenticated should be filtered out",
+			addon: &addonapiv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-addon",
+					Namespace: "cluster1",
+				},
+				Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
+					Registrations: []addonapiv1alpha1.RegistrationConfig{
+						{
+							SignerName: certificatesv1.KubeAPIServerClientSignerName,
+							Subject: addonapiv1alpha1.Subject{
+								User: "system:serviceaccount:cluster1:test-addon-sa",
+								Groups: []string{
+									"system:authenticated",
+									"system:open-cluster-management:cluster:cluster1:addon:test-addon",
+									"system:serviceaccounts",
+								},
+							},
+						},
+					},
+				},
+			},
+			signerName: certificatesv1.KubeAPIServerClientSignerName,
+			expectedSubjects: []rbacv1.Subject{
+				{
+					Kind:     rbacv1.UserKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:serviceaccount:cluster1:test-addon-sa",
+				},
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:open-cluster-management:cluster:cluster1:addon:test-addon",
+				},
+				{
+					Kind:     rbacv1.GroupKind,
+					APIGroup: rbacv1.GroupName,
+					Name:     "system:serviceaccounts",
+				},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			subjects := buildSubjectsFromRegistration(c.addon, c.signerName)
+			if !equality.Semantic.DeepEqual(subjects, c.expectedSubjects) {
+				t.Errorf("expected subjects %v, got %v", c.expectedSubjects, subjects)
+			}
+		})
+	}
+}