redhat-cop / redhat-cop/group-sync-operator

Renaming a GroupSync CR silently orphans every Group it had synced

Open
#466 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
132
Forks
71
PR merge metrics
No merged PRs in 30d

Description

Renaming a GroupSync CR permanently orphans every Group it had already synced: the Groups keep the old CR's name in their group-sync-operator.redhat-cop.io/sync-provider label, the new CR skips them, and the sync still reports success.

Observed on v0.0.36, and the responsible code is unchanged on main.

What happens

On a cluster where a GroupSync CR was renamed — delete the old CR, create a new one with an identical provider configuration — the Groups that the old CR had synced are never picked up again:

Groups still labelled with the old CR name 22 of 66
Errors reported none
status.lastSyncSuccessTime advances normally on every sync
group-sync-operator.redhat-cop.io/sync-time on those Groups frozen at the pre-rename value

Forcing a sync does not help. A .spec change bumps metadata.generation, the reconcile runs, lastSyncSuccessTime is updated — and those Groups are untouched.

Expected: the new CR adopts the Groups it is now responsible for and refreshes their metadata, which the write path already does correctly.

Root cause

internal/controller/groupsync_controller.go:107-108 builds the ownership label from the CR name:

// Provider Label
providerLabel := fmt.Sprintf("%s_%s", instance.Name, groupSyncer.GetProviderName())

internal/controller/groupsync_controller.go:155-161 then refuses any existing Group whose label does not equal that string:

} else {
    // Verify this group is not managed by another provider
    if groupProviderLabel, exists := ocpGroup.Labels[constants.SyncProvider]; !exists || (groupProviderLabel != providerLabel) {
        r.Log.Info("Group Provider Label Did Not Match Expected Provider Label", "Provider", groupSyncer.GetProviderName(), "Group Name", ocpGroup.Name, "Expected Label", providerLabel, "Found Label", groupProviderLabel)
        continue
    }
}

Because the label embeds the CR name, a rename makes every previously-synced Group look like it belongs to a different provider, so continue fires — three lines before the code that would have fixed it:

// Add Label for new resource
ocpGroup.Labels[constants.SyncProvider] = providerLabel        // line 178

// Add Gloabl Annotations/Labels
now := time.Now().UTC().Format(time.RFC3339)
ocpGroup.Annotations[constants.SyncTimestamp] = now            // line 182

The skip is invisible to the CR's status: it logs at Info and is never appended to syncErrors, so ManageSuccess still stamps LastSyncSuccessTime. That is why the sync looks healthy while doing nothing for those Groups. The guard dates to 241aa8b1 (2020-10-27); the Info line was added in 0656728c.

Evidence

The operator states the problem itself, once per skipped Group per sync (CR and group names redacted):

INFO  controllers.GroupSync  Group Provider Label Did Not Match Expected Provider Label
{"Provider": "ldap",
 "Group Name": "<group>",
 "Expected Label": "<new-cr-name>_ldap",
 "Found Label":    "<old-cr-name>_ldap"}

22 distinct Groups, 66 log lines across 3 syncs — one per Group per sync, and group_sync_error stayed at 0 throughout.

Reproduction

  1. Create a GroupSync CR named groupsync-a with any provider that yields at least one group, and let it sync.
  2. Confirm the label:
    $ oc get group <group> -o jsonpath='{.metadata.labels}'
    {"group-sync-operator.redhat-cop.io/sync-provider":"groupsync-a_ldap", ...}
    
  3. Delete groupsync-a and create groupsync-b with an identical provider configuration.
  4. Let it sync, or force one with a .spec change. status.lastSyncSuccessTime advances and no error is reported.
  5. The Group is unchanged:
    $ oc get group <group> -o jsonpath='{.metadata.labels}'
    {"group-sync-operator.redhat-cop.io/sync-provider":"groupsync-a_ldap", ...}   # still the old CR
    
    $ oc get groups -l group-sync-operator.redhat-cop.io/sync-provider=groupsync-b_ldap
    No resources found                                                            # invisible to the new CR
    
  6. The manager log contains the Group Provider Label Did Not Match Expected Provider Label line above.

Why it matters

The Group is not merely mislabelled — it is fully orphaned, and nothing in the system will ever collect or correct it:

  • pruneGroups cannot see it either. It selects on the current label (internal/controller/groupsync_controller.go:262-265), so the Group is neither updated nor pruned.
  • There are no ownerReferences to fall back on. r.CreateOrUpdateResource(context, nil, "", ocpGroup) at line 186 passes owner == nil, so garbage collection never removed these Groups when the old CR was deleted, and ownership is purely the label.
  • sync-time freezes, so "when was this Group last synced" is wrong for exactly the Groups that stopped being synced — the opposite of what an operator would want to be able to trust.
  • Anything keying on the label value mis-attributes those Groups. In our case per-CR group counts and a stale-group alert both went quiet for the 22, which is how we noticed.

The only remedy today is manual: oc label group <name> group-sync-operator.redhat-cop.io/sync-provider=<new>_<provider> --overwrite, or delete the Groups and let them be recreated — which revokes any RBAC bound to them until the next sync completes.

A fix

The write path already does the right thing once it is reached — lines 174-186 set the label, refresh
sync-time, and mergeMap preserves user-added metadata — so the question is only which existing Groups
the guard should let through.

I have a change against main for this and I am validating it with a unit test before proposing it; I will
follow up on this issue with the approach and open a PR. Happy to hear first if maintainers would prefer a
different direction, since the shape of the ownership test is a design call rather than an obvious one.

Not a duplicate

  • #319 — an LDAP-side group rename producing a provider lookup error that stops the sync. This issue is the opposite: no error, sync succeeds, and the CR was renamed rather than the LDAP group.
  • #3 — a feature request to carry labels/annotations from the identity provider onto Groups. This is about the operator's own provenance label not being refreshed on Groups it already manages.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with internal/controller/groupsync_controller.go, especially the provider-label guard, write path, and pruneGroups selection described in the issue. Run the controller's existing unit tests and add or inspect the unit test the reporter is validating. Done means a renamed CR adopts previously synced Groups, refreshes their label and sync-time metadata, and still reports status accurately.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
backend, devops
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.