Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use groupUIDNameMapping for LDAP sync/prune with Openshift groups #16071

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/validation/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ func ValidateLDAPSyncConfig(config *api.LDAPSyncConfig) ValidationResults {
bindPassword, _ := api.ResolveStringValue(config.BindPassword)
validationResults.Append(ValidateLDAPClientConfig(config.URL, config.BindDN, bindPassword, config.CA, config.Insecure, nil))

for ldapGroupUID, openShiftGroupName := range config.LDAPGroupUIDToOpenShiftGroupNameMapping {
if len(ldapGroupUID) == 0 || len(openShiftGroupName) == 0 {
validationResults.AddErrors(field.Invalid(field.NewPath("groupUIDNameMapping").Key(ldapGroupUID), openShiftGroupName, "has empty key or value"))
}
}

schemaConfigsFound := []string{}

if config.RFC2307Config != nil {
Expand Down
7 changes: 4 additions & 3 deletions pkg/oc/admin/groups/sync/cli/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,18 @@ func NewCmdPrune(name, fullName string, f *clientcmd.Factory, out io.Writer) *co

func (o *PruneOptions) Complete(whitelistFile, blacklistFile, configFile string, args []string, f *clientcmd.Factory) error {
var err error
o.Whitelist, err = buildOpenShiftGroupNameList(args, whitelistFile)

o.Config, err = decodeSyncConfigFromFile(configFile)
if err != nil {
return err
}

o.Blacklist, err = buildOpenShiftGroupNameList([]string{}, blacklistFile)
o.Whitelist, err = buildOpenShiftGroupNameList(args, whitelistFile, o.Config.LDAPGroupUIDToOpenShiftGroupNameMapping)
if err != nil {
return err
}

o.Config, err = decodeSyncConfigFromFile(configFile)
o.Blacklist, err = buildOpenShiftGroupNameList([]string{}, blacklistFile, o.Config.LDAPGroupUIDToOpenShiftGroupNameMapping)
if err != nil {
return err
}
Expand Down
34 changes: 25 additions & 9 deletions pkg/oc/admin/groups/sync/cli/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,18 @@ func (o *SyncOptions) Complete(typeArg, whitelistFile, blacklistFile, configFile
}

var err error

o.Config, err = decodeSyncConfigFromFile(configFile)
if err != nil {
return err
}

if o.Source == GroupSyncSourceOpenShift {
o.Whitelist, err = buildOpenShiftGroupNameList(args, whitelistFile)
o.Whitelist, err = buildOpenShiftGroupNameList(args, whitelistFile, o.Config.LDAPGroupUIDToOpenShiftGroupNameMapping)
if err != nil {
return err
}
o.Blacklist, err = buildOpenShiftGroupNameList([]string{}, blacklistFile)
o.Blacklist, err = buildOpenShiftGroupNameList([]string{}, blacklistFile, o.Config.LDAPGroupUIDToOpenShiftGroupNameMapping)
if err != nil {
return err
}
Expand All @@ -215,11 +221,6 @@ func (o *SyncOptions) Complete(typeArg, whitelistFile, blacklistFile, configFile
}
}

o.Config, err = decodeSyncConfigFromFile(configFile)
if err != nil {
return err
}

osClient, _, err := f.Clients()
if err != nil {
return err
Expand All @@ -230,13 +231,28 @@ func (o *SyncOptions) Complete(typeArg, whitelistFile, blacklistFile, configFile
}

// buildOpenShiftGroupNameList builds a list of OpenShift names from file and args
func buildOpenShiftGroupNameList(args []string, file string) ([]string, error) {
// nameMapping is used to override the OpenShift names built from file and args
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad godoc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er what should I say instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't match func name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the second line of the go doc. The first line correctly states buildOpenShiftGroupNameList.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow am bad at reading diffs

func buildOpenShiftGroupNameList(args []string, file string, nameMapping map[string]string) ([]string, error) {
rawList, err := buildNameList(args, file)
if err != nil {
return nil, err
}

return openshiftGroupNamesOnlyList(rawList)
namesList, err := openshiftGroupNamesOnlyList(rawList)
if err != nil {
return nil, err
}

// override items in namesList if present in mapping
if len(nameMapping) > 0 {
for i, name := range namesList {
if nameOverride, ok := nameMapping[name]; ok {
namesList[i] = nameOverride
}
}
}

return namesList, nil
}

// buildNameLists builds a list from file and args
Expand Down
23 changes: 23 additions & 0 deletions test/extended/ldap_groups.sh
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ schema=('rfc2307' 'ad' 'augmented-ad')
for (( i=0; i<${#schema[@]}; i++ )); do
current_schema=${schema[$i]}
os::log::info "Testing schema: ${current_schema}"
os::test::junit::declare_suite_start "extended/ldap-groups/${current_schema}"

WORKINGDIR=${BASETMPDIR}/${current_schema}
mkdir ${WORKINGDIR}
Expand Down Expand Up @@ -209,6 +210,14 @@ for (( i=0; i<${#schema[@]}; i++ )); do
oc adm groups sync --sync-config=sync-config-dn-everywhere.yaml --confirm
compare_and_cleanup valid_all_ldap_sync_dn_everywhere.yaml

echo -e "\tTEST: Sync based on OpenShift groups respecting OpenShift mappings and whitelist file"
os::cmd::expect_success_and_text 'oc adm groups sync --whitelist=ldapgroupuids.txt --sync-config=sync-config-user-defined.yaml --confirm' 'group/'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name}' 'firstgroup secondgroup thirdgroup'
os::cmd::expect_success_and_text 'oc adm groups sync --type=openshift --whitelist=ldapgroupuids.txt --sync-config=sync-config-user-defined.yaml --confirm' 'group/'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name}' 'firstgroup secondgroup thirdgroup'
os::cmd::expect_success_and_text 'oc delete groups --all' 'deleted'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name} | wc -l' '0'


# PRUNING
echo -e "\tTEST: Sync all LDAP groups from LDAP server, change LDAP UID, then prune OpenShift groups"
Expand All @@ -217,11 +226,25 @@ for (( i=0; i<${#schema[@]}; i++ )); do
oc adm groups prune --sync-config=sync-config.yaml --confirm
compare_and_cleanup valid_all_ldap_sync_prune.yaml

echo -e "\tTEST: Sync all LDAP groups from LDAP server using whitelist file, then prune OpenShift groups using the same whitelist file"
os::cmd::expect_success_and_text 'oc adm groups sync --whitelist=ldapgroupuids.txt --sync-config=sync-config-user-defined.yaml --confirm' 'group/'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name}' 'firstgroup secondgroup thirdgroup'
os::cmd::expect_success_and_text 'oc adm groups prune --whitelist=ldapgroupuids.txt --sync-config=sync-config-user-defined.yaml --confirm | wc -l' '0'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name}' 'firstgroup secondgroup thirdgroup'
os::cmd::expect_success_and_text 'oc patch group secondgroup -p "{\"metadata\":{\"annotations\":{\"openshift.io/ldap.uid\":\"cn=garbage\"}}}"' 'group "secondgroup" patched'
os::cmd::expect_success_and_text 'oc adm groups prune --whitelist=ldapgroupuids.txt --sync-config=sync-config-user-defined.yaml --confirm' 'group/secondgroup'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name}' 'firstgroup thirdgroup'
os::cmd::expect_success_and_text 'oc delete groups --all' 'deleted'
os::cmd::expect_success_and_text 'oc get group -o jsonpath={.items[*].metadata.name} | wc -l' '0'


# PAGING
echo -e "\tTEST: Sync all LDAP groups from LDAP server using paged queries"
oc adm groups sync --sync-config=sync-config-paging.yaml --confirm
compare_and_cleanup valid_all_ldap_sync.yaml


os::test::junit::declare_suite_end
popd > /dev/null
done

Expand Down