diff --git a/pkg/reconcilermanager/controllers/reconciler_base.go b/pkg/reconcilermanager/controllers/reconciler_base.go index b6352d11a9..b8d9cb3c33 100644 --- a/pkg/reconcilermanager/controllers/reconciler_base.go +++ b/pkg/reconcilermanager/controllers/reconciler_base.go @@ -649,7 +649,7 @@ func ManagedObjectLabelMap(syncKind string, rsRef types.NamespacedName) map[stri } } -func (r *reconcilerBase) updateRBACBinding(ctx context.Context, reconcilerRef types.NamespacedName, binding client.Object) error { +func (r *reconcilerBase) updateRBACBinding(ctx context.Context, reconcilerRef, rsRef types.NamespacedName, binding client.Object) error { existingBinding := binding.DeepCopyObject() subjects := []rbacv1.Subject{r.serviceAccountSubject(reconcilerRef)} if crb, ok := binding.(*rbacv1.ClusterRoleBinding); ok { @@ -657,6 +657,7 @@ func (r *reconcilerBase) updateRBACBinding(ctx context.Context, reconcilerRef ty } else if rb, ok := binding.(*rbacv1.RoleBinding); ok { rb.Subjects = subjects } + core.AddLabels(binding, ManagedObjectLabelMap(r.syncKind, rsRef)) if equality.Semantic.DeepEqual(existingBinding, binding) { return nil } diff --git a/pkg/reconcilermanager/controllers/reposync_controller.go b/pkg/reconcilermanager/controllers/reposync_controller.go index 7a8c3be234..b60fe6bd8d 100644 --- a/pkg/reconcilermanager/controllers/reposync_controller.go +++ b/pkg/reconcilermanager/controllers/reposync_controller.go @@ -207,24 +207,20 @@ func (r *RepoSyncReconciler) upsertManagedObjects(ctx context.Context, reconcile rsRef := client.ObjectKeyFromObject(rs) r.logger(ctx).V(3).Info("Reconciling managed objects") + labelMap := ManagedObjectLabelMap(r.syncKind, rsRef) + // Create secret in config-management-system namespace using the // existing secret in the reposync.namespace. - if _, err := r.upsertAuthSecret(ctx, rs, reconcilerRef); err != nil { + if _, err := r.upsertAuthSecret(ctx, rs, reconcilerRef, labelMap); err != nil { return errors.Wrap(err, "upserting auth secret") } // Create secret in config-management-system namespace using the // existing secret in the reposync.namespace. - if _, err := r.upsertCACertSecret(ctx, rs, reconcilerRef); err != nil { + if _, err := r.upsertCACertSecret(ctx, rs, reconcilerRef, labelMap); err != nil { return errors.Wrap(err, "upserting CA cert secret") } - labelMap := map[string]string{ - metadata.SyncNamespaceLabel: rs.Namespace, - metadata.SyncNameLabel: rs.Name, - metadata.SyncKindLabel: r.syncKind, - } - // Overwrite reconciler pod ServiceAccount. var auth configsync.AuthType var gcpSAEmail string @@ -983,8 +979,12 @@ func (r *RepoSyncReconciler) upsertSharedRoleBinding(ctx context.Context, reconc childRB := &rbacv1.RoleBinding{} childRB.Name = rbRef.Name childRB.Namespace = rbRef.Namespace + labelMap := ManagedObjectLabelMap(r.syncKind, rsRef) + // Remove sync-name label since the RoleBinding may be shared + delete(labelMap, metadata.SyncNameLabel) op, err := CreateOrUpdate(ctx, r.client, childRB, func() error { + core.AddLabels(childRB, labelMap) childRB.RoleRef = rolereference(RepoSyncBaseClusterRoleName, "ClusterRole") childRB.Subjects = addSubject(childRB.Subjects, r.serviceAccountSubject(reconcilerRef)) return nil diff --git a/pkg/reconcilermanager/controllers/rootsync_controller.go b/pkg/reconcilermanager/controllers/rootsync_controller.go index 67f7a89fc4..c48d57ac29 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller.go @@ -899,7 +899,7 @@ func (r *RootSyncReconciler) manageRBACBindings(ctx context.Context, reconcilerR } if len(roleRefs) == 0 { // Backwards compatible behavior to default to cluster-admin - if err := r.upsertSharedClusterRoleBinding(ctx, RootSyncLegacyClusterRoleBindingName, "cluster-admin", reconcilerRef); err != nil { + if err := r.upsertSharedClusterRoleBinding(ctx, RootSyncLegacyClusterRoleBindingName, "cluster-admin", reconcilerRef, rsRef); err != nil { return err } // Clean up any RoleRefs created previously that are no longer declared @@ -914,7 +914,7 @@ func (r *RootSyncReconciler) manageRBACBindings(ctx context.Context, reconcilerR return nil } // Add the base ClusterRole for basic root reconciler functionality - if err := r.upsertSharedClusterRoleBinding(ctx, RootSyncBaseClusterRoleBindingName, RootSyncBaseClusterRoleName, reconcilerRef); err != nil { + if err := r.upsertSharedClusterRoleBinding(ctx, RootSyncBaseClusterRoleBindingName, RootSyncBaseClusterRoleName, reconcilerRef, rsRef); err != nil { return err } declaredRoleRefs := roleRefs @@ -937,7 +937,7 @@ func (r *RootSyncReconciler) manageRBACBindings(ctx context.Context, reconcilerR // - if they are no longer declared in roleRefs, delete for roleRef, binding := range currentRefMap { if _, ok := declaredRefMap[roleRef]; ok { // update - if err := r.updateRBACBinding(ctx, reconcilerRef, binding); err != nil { + if err := r.updateRBACBinding(ctx, reconcilerRef, rsRef, binding); err != nil { return errors.Wrap(err, "upserting RBAC Binding") } } else { // Clean up any RoleRefs created previously that are no longer declared @@ -956,12 +956,17 @@ func (r *RootSyncReconciler) manageRBACBindings(ctx context.Context, reconcilerR return nil } -func (r *RootSyncReconciler) upsertSharedClusterRoleBinding(ctx context.Context, name, clusterRole string, reconcilerRef types.NamespacedName) error { +func (r *RootSyncReconciler) upsertSharedClusterRoleBinding(ctx context.Context, name, clusterRole string, reconcilerRef, rsRef types.NamespacedName) error { crbRef := client.ObjectKey{Name: name} childCRB := &rbacv1.ClusterRoleBinding{} childCRB.Name = crbRef.Name + labelMap := ManagedObjectLabelMap(r.syncKind, rsRef) + // Remove sync-name label since the ClusterRoleBinding may be shared + delete(labelMap, metadata.SyncNameLabel) + op, err := CreateOrUpdate(ctx, r.client, childCRB, func() error { + core.AddLabels(childCRB, labelMap) childCRB.OwnerReferences = nil childCRB.RoleRef = rolereference(clusterRole, "ClusterRole") childCRB.Subjects = addSubject(childCRB.Subjects, r.serviceAccountSubject(reconcilerRef)) diff --git a/pkg/reconcilermanager/controllers/rootsync_controller_test.go b/pkg/reconcilermanager/controllers/rootsync_controller_test.go index 0f525e6717..6853c161e4 100644 --- a/pkg/reconcilermanager/controllers/rootsync_controller_test.go +++ b/pkg/reconcilermanager/controllers/rootsync_controller_test.go @@ -1579,11 +1579,10 @@ func validateReconcilerClusterRoleBindings(fakeClient *syncerFake.Client, rootSy opts := &client.ListOptions{} opts.LabelSelector = client.MatchingLabelsSelector{ - Selector: labels.SelectorFromSet(map[string]string{ - metadata.SyncKindLabel: configsync.RootSyncKind, - metadata.SyncNameLabel: rootSyncName, - metadata.SyncNamespaceLabel: configsync.ControllerNamespace, - }), + Selector: labels.SelectorFromSet(ManagedObjectLabelMap( + configsync.RootSyncKind, + types.NamespacedName{Name: rootSyncName, Namespace: configsync.ControllerNamespace}, + )), } err := fakeClient.List(ctx, got, opts) if err != nil { @@ -1619,11 +1618,7 @@ func validateReconcilerRoleBindings(fakeClient *syncerFake.Client, syncKind stri opts := &client.ListOptions{} opts.LabelSelector = client.MatchingLabelsSelector{ - Selector: labels.SelectorFromSet(map[string]string{ - metadata.SyncKindLabel: syncKind, - metadata.SyncNameLabel: rsRef.Name, - metadata.SyncNamespaceLabel: rsRef.Namespace, - }), + Selector: labels.SelectorFromSet(ManagedObjectLabelMap(syncKind, rsRef)), } err := fakeClient.List(ctx, got, opts) if err != nil { @@ -1710,6 +1705,10 @@ func TestRootSyncCreateWithOverrideRoleRefs(t *testing.T) { RootSyncBaseClusterRoleBindingName, RootSyncBaseClusterRoleName, core.UID("1"), core.ResourceVersion("1"), core.Generation(1), + core.Labels(map[string]string{ + metadata.SyncKindLabel: configsync.RootSyncKind, + metadata.SyncNamespaceLabel: configsync.ControllerNamespace, + }), ) defaultCrb.Subjects = addSubjectByName(nil, rootReconcilerName) @@ -1745,6 +1744,10 @@ func TestRootSyncCreateWithOverrideRoleRefs(t *testing.T) { RootSyncLegacyClusterRoleBindingName, "cluster-admin", core.UID("1"), core.ResourceVersion("1"), core.Generation(1), + core.Labels(map[string]string{ + metadata.SyncKindLabel: configsync.RootSyncKind, + metadata.SyncNamespaceLabel: configsync.ControllerNamespace, + }), ) clusterAdminCRB.Subjects = addSubjectByName(nil, rootReconcilerName) if err := validateClusterRoleBinding(clusterAdminCRB, fakeClient); err != nil { @@ -1770,6 +1773,10 @@ func TestMigrationToIndividualClusterRoleBindingsWhenDefaultRootSyncExists(t *te RootSyncLegacyClusterRoleBindingName, "cluster-admin", core.UID("1"), core.ResourceVersion("1"), core.Generation(1), + core.Labels(map[string]string{ + metadata.SyncKindLabel: configsync.RootSyncKind, + metadata.SyncNamespaceLabel: configsync.ControllerNamespace, + }), ) oldBinding.Subjects = addSubjectByName(nil, rs1ReconcilerName) oldBinding.Subjects = addSubjectByName(oldBinding.Subjects, rs2ReconcilerName) @@ -2049,6 +2056,10 @@ func TestMultipleRootSyncs(t *testing.T) { RootSyncLegacyClusterRoleBindingName, "cluster-admin", core.UID("1"), core.ResourceVersion("1"), core.Generation(1), + core.Labels(map[string]string{ + metadata.SyncKindLabel: configsync.RootSyncKind, + metadata.SyncNamespaceLabel: configsync.ControllerNamespace, + }), ) crb.Subjects = addSubjectByName(crb.Subjects, rootReconcilerName) rootContainerEnv1 := testReconciler.populateContainerEnvs(ctx, rs1, rootReconcilerName) diff --git a/pkg/reconcilermanager/controllers/secret.go b/pkg/reconcilermanager/controllers/secret.go index a018812bf3..7e7bc746d0 100644 --- a/pkg/reconcilermanager/controllers/secret.go +++ b/pkg/reconcilermanager/controllers/secret.go @@ -24,7 +24,6 @@ import ( "kpt.dev/configsync/pkg/api/configsync" "kpt.dev/configsync/pkg/api/configsync/v1beta1" "kpt.dev/configsync/pkg/core" - "kpt.dev/configsync/pkg/metadata" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -60,7 +59,7 @@ func shouldUpsertHelmSecret(rs *v1beta1.RepoSync) bool { // upsertAuthSecret creates or updates the auth secret in the // config-management-system namespace using an existing secret in the RepoSync // namespace. -func (r *reconcilerBase) upsertAuthSecret(ctx context.Context, rs *v1beta1.RepoSync, reconcilerRef types.NamespacedName) (client.ObjectKey, error) { +func (r *reconcilerBase) upsertAuthSecret(ctx context.Context, rs *v1beta1.RepoSync, reconcilerRef types.NamespacedName, labelMap map[string]string) (client.ObjectKey, error) { rsRef := client.ObjectKeyFromObject(rs) switch { case shouldUpsertGitSecret(rs): @@ -69,7 +68,7 @@ func (r *reconcilerBase) upsertAuthSecret(ctx context.Context, rs *v1beta1.RepoS if err != nil { return cmsSecretRef, errors.Wrap(err, "user secret required for git client authentication") } - _, err = r.upsertSecret(ctx, cmsSecretRef, rsRef, userSecret) + _, err = r.upsertSecret(ctx, cmsSecretRef, userSecret, labelMap) return cmsSecretRef, err case shouldUpsertHelmSecret(rs): nsSecretRef, cmsSecretRef := getSecretRefs(rsRef, reconcilerRef, v1beta1.GetSecretName(rs.Spec.Helm.SecretRef)) @@ -77,7 +76,7 @@ func (r *reconcilerBase) upsertAuthSecret(ctx context.Context, rs *v1beta1.RepoS if err != nil { return cmsSecretRef, errors.Wrap(err, "user secret required for helm client authentication") } - _, err = r.upsertSecret(ctx, cmsSecretRef, rsRef, userSecret) + _, err = r.upsertSecret(ctx, cmsSecretRef, userSecret, labelMap) return cmsSecretRef, err default: // No secret required @@ -88,7 +87,7 @@ func (r *reconcilerBase) upsertAuthSecret(ctx context.Context, rs *v1beta1.RepoS // upsertCACertSecret creates or updates the CA cert secret in the // config-management-system namespace using an existing secret in the RepoSync // namespace. -func (r *reconcilerBase) upsertCACertSecret(ctx context.Context, rs *v1beta1.RepoSync, reconcilerRef types.NamespacedName) (client.ObjectKey, error) { +func (r *reconcilerBase) upsertCACertSecret(ctx context.Context, rs *v1beta1.RepoSync, reconcilerRef types.NamespacedName, labelMap map[string]string) (client.ObjectKey, error) { rsRef := client.ObjectKeyFromObject(rs) if shouldUpsertCACertSecret(rs) { nsSecretRef, cmsSecretRef := getSecretRefs(rsRef, reconcilerRef, v1beta1.GetSecretName(rs.Spec.Git.CACertSecretRef)) @@ -96,7 +95,7 @@ func (r *reconcilerBase) upsertCACertSecret(ctx context.Context, rs *v1beta1.Rep if err != nil { return cmsSecretRef, errors.Wrap(err, "user secret required for git server validation") } - _, err = r.upsertSecret(ctx, cmsSecretRef, rsRef, userSecret) + _, err = r.upsertSecret(ctx, cmsSecretRef, userSecret, labelMap) return cmsSecretRef, err } // No secret required @@ -133,16 +132,13 @@ func getUserSecret(ctx context.Context, c client.Client, nsSecretRef client.Obje // upsertSecret creates or updates a secret in config-management-system // namespace using an existing user secret. -func (r *reconcilerBase) upsertSecret(ctx context.Context, cmsSecretRef, rsRef types.NamespacedName, userSecret *corev1.Secret) (controllerutil.OperationResult, error) { +func (r *reconcilerBase) upsertSecret(ctx context.Context, cmsSecretRef types.NamespacedName, userSecret *corev1.Secret, labelMap map[string]string) (controllerutil.OperationResult, error) { cmsSecret := &corev1.Secret{} cmsSecret.Name = cmsSecretRef.Name cmsSecret.Namespace = cmsSecretRef.Namespace op, err := CreateOrUpdate(ctx, r.client, cmsSecret, func() error { - core.AddLabels(cmsSecret, map[string]string{ - metadata.SyncNamespaceLabel: rsRef.Namespace, - metadata.SyncNameLabel: rsRef.Name, - }) + core.AddLabels(cmsSecret, labelMap) // Copy user secret data & type to managed secret cmsSecret.Data = userSecret.Data cmsSecret.Type = userSecret.Type diff --git a/pkg/reconcilermanager/controllers/secret_test.go b/pkg/reconcilermanager/controllers/secret_test.go index 3d228829be..2421b82dc2 100644 --- a/pkg/reconcilermanager/controllers/secret_test.go +++ b/pkg/reconcilermanager/controllers/secret_test.go @@ -75,6 +75,7 @@ func secret(t *testing.T, name, data string, auth configsync.AuthType, sourceTyp result.SetLabels(map[string]string{ metadata.SyncNamespaceLabel: reposyncNs, metadata.SyncNameLabel: reposyncName, + metadata.SyncKindLabel: configsync.RepoSyncKind, }) return result } @@ -197,7 +198,11 @@ func TestUpsertAuthSecret(t *testing.T) { }, client: tc.client, } - sKey, err := r.upsertAuthSecret(ctx, tc.reposync, nsReconcilerKey) + labelMap := ManagedObjectLabelMap(configsync.RepoSyncKind, types.NamespacedName{ + Name: tc.reposync.Name, + Namespace: tc.reposync.Namespace, + }) + sKey, err := r.upsertAuthSecret(ctx, tc.reposync, nsReconcilerKey, labelMap) assert.Equal(t, tc.wantKey, sKey, "unexpected secret key returned") if tc.wantError { assert.Error(t, err, "expected upsertAuthSecret to error")