1. origin
  2. Commit 3446dceb8dae07187fc9baf033fed9eecfe3b3ed
Browse code

fixed group detection bug for LDAP prune

Steve Kuznetsov authored on 2015/12/16 00:18:04
Showing 4 changed files
pkg/cmd/experimental/syncgroups/groupdetector/groupdetector.go
History View file @ 3446dce
... ... 
@@ -18,7 +18,7 @@ type GroupBasedDetector struct {
18 18 
 }
19 19
20 20 
 func (l *GroupBasedDetector) Exists(ldapGroupUID string) (bool, error) {
21 
-	_, err := l.groupGetter.GroupEntryFor(ldapGroupUID)
21 
+	group, err := l.groupGetter.GroupEntryFor(ldapGroupUID)
22 22 
 	if ldaputil.IsQueryOutOfBoundsError(err) || ldaputil.IsEntryNotFoundError(err) || ldaputil.IsNoSuchObjectError(err) {
23 23 
 		return false, nil
24 24 
 	}
... ... 
@@ -26,6 +26,10 @@ func (l *GroupBasedDetector) Exists(ldapGroupUID string) (bool, error) {
26 26 
 		return false, err
27 27 
 	}
28 28
29 
+	if group == nil {
30 
+		return false, nil
31 
+	}
32 
+
29 33 
 	return true, nil
30 34 
 }
31 35
... ... 
@@ -76,13 +80,17 @@ type CompoundDetector struct {
76 76 
 }
77 77
78 78 
 func (l *CompoundDetector) Exists(ldapGrouUID string) (bool, error) {
79 
-	conclusion := false
79 
+	if len(l.locators) == 0 {
80 
+		return false, nil
81 
+	}
82 
+
83 
+	conclusion := true
80 84 
 	for _, locator := range l.locators {
81 85 
 		opinion, err := locator.Exists(ldapGrouUID)
82 86 
 		if err != nil {
83 87 
 			return false, err
84 88 
 		}
85 
-		conclusion = conclusion || opinion
89 
+		conclusion = conclusion && opinion
86 90 
 	}
87 91 
 	return conclusion, nil
88 92 
 }
pkg/cmd/experimental/syncgroups/groupdetector/groupdetector_test.go
History View file @ 3446dce
... ... 
@@ -43,9 +43,15 @@ func TestGroupBasedDetectorExists(t *testing.T) {
43 43 
 			expectedExists: false,
44 44 
 		},
45 45 
 		{
46 
-			name:           "no error",
46 
+			name:           "no error, no entries",
47 47 
 			groupGetter:    &puppetGetterExtractor{},
48 48 
 			expectedErr:    nil,
49 
+			expectedExists: false,
50 
+		},
51 
+		{
52 
+			name:           "no error, has entries",
53 
+			groupGetter:    &puppetGetterExtractor{returnVal: []*ldap.Entry{dummyEntry}},
54 
+			expectedErr:    nil,
49 55 
 			expectedExists: true,
50 56 
 		},
51 57 
 	}
... ... 
@@ -127,9 +133,15 @@ func TestCompoundDetectorExists(t *testing.T) {
127 127 
 		expectedExists bool
128 128 
 	}{
129 129 
 		{
130 
+			name:           "no detectors",
131 
+			locators:       []interfaces.LDAPGroupDetector{},
132 
+			expectedErr:    nil,
133 
+			expectedExists: false,
134 
+		},
135 
+		{
130 136 
 			name: "none fail to locate",
131 137 
 			locators: []interfaces.LDAPGroupDetector{
132 
-				NewGroupBasedDetector(&puppetGetterExtractor{}),
138 
+				NewGroupBasedDetector(&puppetGetterExtractor{returnVal: []*ldap.Entry{dummyEntry}}),
133 139 
 				NewMemberBasedDetector(&puppetGetterExtractor{returnVal: []*ldap.Entry{dummyEntry}}),
134 140 
 			},
135 141 
 			expectedErr:    nil,
... ... 
@@ -142,10 +154,10 @@ func TestCompoundDetectorExists(t *testing.T) {
142 142 
 				NewMemberBasedDetector(&puppetGetterExtractor{returnVal: []*ldap.Entry{dummyEntry}}),
143 143 
 			},
144 144 
 			expectedErr:    nil,
145 
-			expectedExists: true,
145 
+			expectedExists: false,
146 146 
 		},
147 147 
 		{
148 
-			name: "all fail to locate",
148 
+			name: "all fail to locate because of errors",
149 149 
 			locators: []interfaces.LDAPGroupDetector{
150 150 
 				NewGroupBasedDetector(&errEntryNotFoundGetterExtractor{}),
151 151 
 				NewMemberBasedDetector(&errOutOfBoundsGetterExtractor{}),
... ... 
@@ -154,6 +166,15 @@ func TestCompoundDetectorExists(t *testing.T) {
154 154 
 			expectedExists: false,
155 155 
 		},
156 156 
 		{
157 
+			name: "all locate no entries",
158 
+			locators: []interfaces.LDAPGroupDetector{
159 
+				NewGroupBasedDetector(&puppetGetterExtractor{}),
160 
+				NewMemberBasedDetector(&puppetGetterExtractor{}),
161 
+			},
162 
+			expectedErr:    nil,
163 
+			expectedExists: false,
164 
+		},
165 
+		{
157 166 
 			name: "one errors",
158 167 
 			locators: []interfaces.LDAPGroupDetector{
159 168 
 				NewGroupBasedDetector(&puppetGetterExtractor{}),
test/extended/authentication/ldap/augmented-ad/valid_all_ldap_sync_delete_prune.txt
History View file @ 3446dce
160 169 
new file mode 100644
... ... 
@@ -0,0 +1,27 @@
0 
+apiVersion: v1
1 
+kind: Group
2 
+metadata:
3 
+  annotations:
4 
+    openshift.io/ldap.uid: cn=group2,ou=groups,ou=adextended,dc=example,dc=com
5 
+    openshift.io/ldap.url: LDAP_SERVICE_IP:389
6 
+  creationTimestamp: null
7 
+  labels:
8 
+    openshift.io/ldap.host: LDAP_SERVICE_IP
9 
+  name: extended-group2
10 
+users:
11 
+- person1smith@example.com
12 
+- person2smith@example.com
13 
+- person3smith@example.com
14 
+apiVersion: v1
15 
+kind: Group
16 
+metadata:
17 
+  annotations:
18 
+    openshift.io/ldap.uid: cn=group3,ou=groups,ou=adextended,dc=example,dc=com
19 
+    openshift.io/ldap.url: LDAP_SERVICE_IP:389
20 
+  creationTimestamp: null
21 
+  labels:
22 
+    openshift.io/ldap.host: LDAP_SERVICE_IP
23 
+  name: extended-group3
24 
+users:
25 
+- person1smith@example.com
26 
+- person5smith@example.com
test/extended/ldap_groups.sh
History View file @ 3446dce
... ... 
@@ -216,12 +216,11 @@ for (( i=0; i<${#schema[@]}; i++ )); do
216 216 
 	openshift ex sync-groups --type=openshift --sync-config=sync-config.yaml --confirm
217 217 
 	compare_and_cleanup valid_all_ldap_sync_user_defined.txt
218 218
219 
-
220 
-
221 219 
 	echo -e "\tTEST: Sync all LDAP groups from LDAP server using DN as attribute whenever possible"
222 220 
     openshift ex sync-groups --sync-config=sync-config-dn-everywhere.yaml --confirm
223 221 
 	compare_and_cleanup valid_all_ldap_sync_dn_everywhere.txt
224 222
223 
+
225 224 
 	# PRUNING
226 225 
 	echo -e "\tTEST: Sync all LDAP groups from LDAP server, change LDAP UID, then prune OpenShift groups"
227 226 
 	openshift ex sync-groups --sync-config=sync-config.yaml --confirm
... ... 
@@ -231,3 +230,12 @@ for (( i=0; i<${#schema[@]}; i++ )); do
231 231
232 232 
     popd > /dev/null
233 233 
 done
234 
+
235 
+# special test for ad-extended
236 
+pushd ${BASETMPDIR}/augmented-ad > /dev/null
237 
+echo -e "\tTEST: Sync all LDAP groups from LDAP server, remove LDAP group metadata entry, then prune OpenShift groups"
238 
+openshift ex sync-groups --sync-config=sync-config.yaml --confirm
239 
+ldapdelete -x -h $LDAP_SERVICE_IP -p 389 -D cn=Manager,dc=example,dc=com -w admin "${group1_ldapuid}"
240 
+openshift ex prune-groups --sync-config=sync-config.yaml --confirm
241 
+compare_and_cleanup valid_all_ldap_sync_delete_prune.txt
242 
+popd > /dev/null
234 243 
\ No newline at end of file