Merge pull request #16071 from enj/enj/i/ldap_prune_mapping/1484831

Automatic merge from submit-queue (batch tested with PRs 16150, 16284, 16296, 16071) Use groupUIDNameMapping for LDAP sync/prune with Openshift groups When syncing LDAP groups with --type=openshift or when pruning groups, the `LDAPGroupUIDToOpenShiftGroupNameMapping` should be taken into consideration since: 1. The system of truth in both flows is openshift groups 2. The mapping was probably used to name said openshift groups Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1484831 Signed-off-by: Monis Khan <[email protected]> @openshift/sig-security @stevekuznetsov
openshift · Sep 13, 2017 · 75e79bc · 75e79bc
2 parents 6c60ba4 + c3c59c2
commit 75e79bc
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 12 deletions.
diff --git a/pkg/cmd/server/api/validation/ldap.go b/pkg/cmd/server/api/validation/ldap.go
@@ -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 {

diff --git a/pkg/oc/admin/groups/sync/cli/prune.go b/pkg/oc/admin/groups/sync/cli/prune.go
@@ -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
 	}

diff --git a/pkg/oc/admin/groups/sync/cli/sync.go b/pkg/oc/admin/groups/sync/cli/sync.go
@@ -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
 		}
@@ -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
@@ -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
+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

diff --git a/test/extended/ldap_groups.sh b/test/extended/ldap_groups.sh
@@ -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}
@@ -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"
@@ -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