fix(ldapgrouplink): fix typos, observe matching, and printcolumn

[henrysachs]

Summary

Follow-up fixes for #271 (LdapGroupLink feature).

Fixes

Category	Fix
Example	ldapProdider → ldapProvider
Observe	Match CN + Provider (prevents collision with multi-provider setups)
Update check	Add Provider to isLdapGroupLinkUpToDate
Printcolumn	Now shows external-name annotation
Comment	"deploy token" → "LDAP group link"
Comment	"Giltab" → "GitLab"
Constant	Remove unused errLdapGroupLinktNotFound
Test	unexpecedItem → unexpectedItem
Docs	Add comment explaining delete-then-create Update pattern

Composite External Name (NEW)

As suggested by @markussiebert, the external name now uses a composite format that includes both the LDAP provider and CN:

Format: <provider>/<cn>
Example: ldapmain/developers

This ensures uniqueness when the same CN exists across multiple LDAP providers within the same GitLab group.

Before:

$ kubectl get ldapgrouplinks
NAME         READY   SYNCED   AGE   EXTERNAL-NAME
cn-example   True    True     5m    developers      # ❌ Not unique across providers

After:

$ kubectl get ldapgrouplinks
NAME         READY   SYNCED   AGE   EXTERNAL-NAME
cn-example   True    True     5m    ldapmain/developers   # ✅ Unique per provider

Testing

make reviewable  # ✅ All tests pass, 0 lint errors

Signed-off-by: Henry Sachs henry.sachs@deutschebahn.com

[henrysachs]

@markussiebert @dariozachow @MisterMX based on the contribution from @stempher i made some small tweaks. Would be nice if someone could approve these. The e2e tests will fail until the gitlab client PR is merged.

[markussiebert]

External Name Should Be Composite Key

While reviewing the observe matching logic change, I noticed that the external name pattern may not be correctly implemented for LDAP group links.

Current Implementation Issue

The external name is set to only the CN:

meta.SetExternalName(cr, ldapGroupLink.CN)  // Only stores CN

However, according to the GitLab API documentation, an LDAP group link is uniquely identified by the combination of groupId + provider + (cn OR filter):

• DELETE endpoint: DELETE /groups/:id/ldap_group_links with body {"provider": "ldapmain", "cn": "group1"} or {"provider": "ldapmain", "filter": "(...)"}
• Note that group_access is NOT part of the identifier - it's just an attribute
• LDAP group links have no real "ID" field in the API
• The GitLab API supports filter as an alternative to cn, but this provider implementation only supports cn currently

The Problem

In multi-provider setups, you could have:

• ldapmain with cn=developers on group 123
• ldapsecondary with cn=developers on group 123

These are two different resources, but both would have the same external name (developers), violating the uniqueness constraint.

Current PR Workaround

This PR works around the issue by comparing both fields directly:

if gl.CN == cr.Spec.ForProvider.CN && gl.Provider == cr.Spec.ForProvider.LdapProvider

This fixes the immediate collision bug, but leaves ldapCN unused (it's extracted but never used in the comparison).

Observations

1. All fields are already immutable: groupId, cn, ldapProvider, and groupAccess are all marked +immutable
2. No update endpoint exists: GitLab API has no update operation - the current code does delete+recreate
3. Unique identifier is composite: groupId + provider + (cn OR filter)
4. Filter not implemented yet: This provider only supports cn currently, but filter support should be added in the future

Recommendation

The external name should be a composite key that includes the provider and is forward-compatible with filter support:

For current CN-only implementation:

meta.SetExternalName(cr, fmt.Sprintf("%s/cn/%s", ldapGroupLink.Provider, ldapGroupLink.CN))
// Example: "ldapmain/cn/developers"

When filter support is added:

if ldapGroupLink.CN != "" {
    meta.SetExternalName(cr, fmt.Sprintf("%s/cn/%s", ldapGroupLink.Provider, ldapGroupLink.CN))
} else {
    meta.SetExternalName(cr, fmt.Sprintf("%s/filter/%s", ldapGroupLink.Provider, ldapGroupLink.Filter))
}
// Examples: "ldapmain/cn/developers" or "ldapmain/filter/(objectClass=group)"

This pattern:

1. Makes the external name truly unique across providers within the same group
2. Distinguishes between CN-based and filter-based links
3. Aligns with how GitLab's API identifies these resources
4. Is forward-compatible with future filter support
5. Follows Crossplane conventions for resources without a single ID field

Note: We don't need groupId in the external name since the resource is already scoped to a specific group via spec.forProvider.groupId.

This PR can be merged as-is since it fixes the bug, but a follow-up to use composite external names would be cleaner and future-proof.

[dariozachow]

LGTM