Skip to content

Commit

Permalink
fix: map RepoSync RoleBinding to RepoSync namespace (#989)
Browse files Browse the repository at this point in the history
The RoleBinding created for RepoSyncs is actually in the namespace of
the RepoSync, rather than the controller namespace. This change fixes
the watch configuration and mapping logic to correctly map the
RoleBinding.
  • Loading branch information
sdowell authored Nov 2, 2023
1 parent 7db8a68 commit 58c7cb4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
20 changes: 12 additions & 8 deletions pkg/reconcilermanager/controllers/reposync_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,9 @@ func (r *RepoSyncReconciler) SetupWithManager(mgr controllerruntime.Manager, wat
Watches(&source.Kind{Type: withNamespace(&corev1.ServiceAccount{}, configsync.ControllerNamespace)},
handler.EnqueueRequestsFromMapFunc(r.mapObjectToRepoSync),
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
Watches(&source.Kind{Type: withNamespace(&rbacv1.RoleBinding{}, configsync.ControllerNamespace)},
// Watch RoleBindings in all namespaces, because RoleBindings are created
// in the namespace of the RepoSync. Only maps to existing RepoSyncs.
Watches(&source.Kind{Type: &rbacv1.RoleBinding{}},
handler.EnqueueRequestsFromMapFunc(r.mapObjectToRepoSync),
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}))

Expand Down Expand Up @@ -759,12 +761,6 @@ func (r *RepoSyncReconciler) mapConfigMapToRepoSyncs(obj client.Object) []reconc
func (r *RepoSyncReconciler) mapObjectToRepoSync(obj client.Object) []reconcile.Request {
objRef := client.ObjectKeyFromObject(obj)

// Ignore changes from other namespaces because all the generated resources
// exist in the config-management-system namespace.
if objRef.Namespace != configsync.ControllerNamespace {
return nil
}

// Ignore changes from resources without the ns-reconciler prefix or configsync.gke.io:ns-reconciler
// because all the generated resources have the prefix.
nsRoleBindingName := RepoSyncPermissionsName()
Expand All @@ -778,6 +774,14 @@ func (r *RepoSyncReconciler) mapObjectToRepoSync(obj client.Object) []reconcile.
return nil
}

if objRef.Namespace != configsync.ControllerNamespace {
switch obj.(type) {
case *corev1.ServiceAccount, *appsv1.Deployment:
// All Deployments and ServiceAccounts are in config-management-system
return nil
}
}

allRepoSyncs := &v1beta1.RepoSyncList{}
if err := r.client.List(context.Background(), allRepoSyncs); err != nil {
klog.Errorf("failed to list all RepoSyncs for %s (%s): %v",
Expand All @@ -794,7 +798,7 @@ func (r *RepoSyncReconciler) mapObjectToRepoSync(obj client.Object) []reconcile.
reconcilerName := core.NsReconcilerName(rs.GetNamespace(), rs.GetName())
switch obj.(type) {
case *rbacv1.RoleBinding:
if objRef.Name == nsRoleBindingName {
if objRef.Name == nsRoleBindingName && objRef.Namespace == rs.Namespace {
requests = append(requests, reconcile.Request{
NamespacedName: client.ObjectKeyFromObject(&rs),
})
Expand Down
14 changes: 4 additions & 10 deletions pkg/reconcilermanager/controllers/reposync_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2821,26 +2821,20 @@ func TestMapObjectToRepoSync(t *testing.T) {
want: nil,
},
{
name: fmt.Sprintf("A rolebinding from the %s namespace, different from %s", nsReconcilerKey.Namespace, rsRoleBindingName),
object: fake.RoleBindingObject(core.Name("any"), core.Namespace(nsReconcilerKey.Namespace)),
name: fmt.Sprintf("A rolebinding from the %s namespace, different from %s", rs1.Namespace, rsRoleBindingName),
object: fake.RoleBindingObject(core.Name("any"), core.Namespace(rs1.Namespace)),
want: nil,
},
{
name: fmt.Sprintf("A rolebinding from the %s namespace, same as %s", nsReconcilerKey.Namespace, rsRoleBindingName),
object: fake.RoleBindingObject(core.Name(rsRoleBindingName), core.Namespace(nsReconcilerKey.Namespace)),
name: fmt.Sprintf("A rolebinding from the %s namespace, same as %s", rs1.Namespace, rsRoleBindingName),
object: fake.RoleBindingObject(core.Name(rsRoleBindingName), core.Namespace(rs1.Namespace)),
want: []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: "rs1",
Namespace: "ns1",
},
},
{
NamespacedName: types.NamespacedName{
Name: "rs2",
Namespace: "ns2",
},
},
},
},
}
Expand Down

0 comments on commit 58c7cb4

Please sign in to comment.