Skip to content

Commit

Permalink
added unit testing to existing LDAP sync code
Browse files Browse the repository at this point in the history
  • Loading branch information
stevekuznetsov committed Nov 3, 2015
1 parent 0c6c715 commit 63a542d
Show file tree
Hide file tree
Showing 10 changed files with 763 additions and 71 deletions.
15 changes: 7 additions & 8 deletions pkg/cmd/experimental/syncgroups/ad/augmented_ldapinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/openshift/origin/pkg/auth/ldaputil"
"github.com/openshift/origin/pkg/auth/ldaputil/ldapclient"
ldapinterfaces "github.com/openshift/origin/pkg/cmd/experimental/syncgroups/interfaces"
"github.com/openshift/origin/pkg/cmd/experimental/syncgroups/interfaces"
)

// NewLDAPInterface builds a new LDAPInterface using a schema-appropriate config
Expand All @@ -28,10 +28,6 @@ func NewAugmentedADLDAPInterface(clientConfig ldapclient.Config,

// LDAPInterface extracts the member list of an LDAP user entry from an LDAP server
// with first-class LDAP entries for users and group. Group membership is on the user. The LDAPInterface is *NOT* thread-safe.
// The LDAPInterface satisfies:
// - LDAPMemberExtractor
// - LDAPGroupGetter
// - LDAPGroupLister
type AugmentedADLDAPInterface struct {
*ADLDAPInterface

Expand All @@ -41,12 +37,15 @@ type AugmentedADLDAPInterface struct {
// groupNameAttributes defines which attributes on an LDAP group entry will be interpreted as its name to use for an OpenShift group
groupNameAttributes []string

// cachedGroups holds the result of group queries for later reference, indexed on group UID
// e.g. this will map an LDAP group UID to the LDAP entry returned from the query made using it
cachedGroups map[string]*ldap.Entry
}

var _ ldapinterfaces.LDAPMemberExtractor = &AugmentedADLDAPInterface{}
var _ ldapinterfaces.LDAPGroupGetter = &AugmentedADLDAPInterface{}
var _ ldapinterfaces.LDAPGroupLister = &AugmentedADLDAPInterface{}
var _ interfaces.LDAPMemberExtractor = &AugmentedADLDAPInterface{}
var _ interfaces.LDAPGroupGetter = &AugmentedADLDAPInterface{}
var _ interfaces.LDAPGroupLister = &AugmentedADLDAPInterface{}
var _ interfaces.LDAPGroupDetector = &AugmentedADLDAPInterface{}

// GroupFor returns an LDAP group entry for the given group UID by searching the internal cache
// of the LDAPInterface first, then sending an LDAP query if the cache did not contain the entry.
Expand Down
115 changes: 115 additions & 0 deletions pkg/cmd/experimental/syncgroups/ad/augmented_ldapinterface_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package ad

import (
"reflect"
"testing"

"github.com/go-ldap/ldap"
"github.com/openshift/origin/pkg/auth/ldaputil"
"github.com/openshift/origin/pkg/auth/ldaputil/ldapclient"
"github.com/openshift/origin/pkg/auth/ldaputil/testclient"
)

func TestGroupEntryFor(t *testing.T) {
var testCases = []struct {
name string
cacheSeed map[string]*ldap.Entry
client ldapclient.Client
baseDNOverride string
expectedError error
expectedEntry *ldap.Entry
}{
{
name: "cached entries",
cacheSeed: map[string]*ldap.Entry{
testGroupDN: testGroupEntry,
},
expectedError: nil,
expectedEntry: testGroupEntry,
},
{
name: "search request error",
baseDNOverride: "otherBaseDN",
expectedError: ldaputil.NewQueryOutOfBoundsError(testGroupDN, "otherBaseDN"),
expectedEntry: nil,
},
{
name: "search error",
client: testclient.NewMatchingSearchErrorClient(testclient.New(), testGroupDN, searchErr),
expectedError: searchErr,
expectedEntry: nil,
},
{
name: "no error",
client: testclient.NewDNMappingClient(
testclient.New(),
map[string][]*ldap.Entry{
testGroupDN: {testGroupEntry},
},
),
expectedError: nil,
expectedEntry: testGroupEntry,
},
}
for _, testCase := range testCases {
ldapInterface := newTestAugmentedADLDAPInterface(testCase.client)
if len(testCase.cacheSeed) > 0 {
ldapInterface.cachedGroups = testCase.cacheSeed
}
if len(testCase.baseDNOverride) > 0 {
ldapInterface.groupQuery.BaseDN = testCase.baseDNOverride
}
entry, err := ldapInterface.GroupEntryFor(testGroupDN)
if !reflect.DeepEqual(err, testCase.expectedError) {
t.Errorf("%s: incorrect error returned:\n\texpected:\n\t%v\n\tgot:\n\t%v\n", testCase.name, testCase.expectedError, err)
}
if !reflect.DeepEqual(entry, testCase.expectedEntry) {
t.Errorf("%s: incorrect members returned:\n\texpected:\n\t%v\n\tgot:\n\t%v\n", testCase.name, testCase.expectedEntry, entry)
}
}
}

const (
groupsBaseDN string = "ou=groups,dc=example,dc=com"
groupsBaseFilter string = "objectClass=groupOfNames"

testGroupDN string = "cn=testGroup," + groupsBaseDN
)

var testGroupEntry *ldap.Entry = &ldap.Entry{
DN: testGroupDN,
Attributes: []*ldap.EntryAttribute{
{
Name: "cn",
Values: []string{"testGroup"},
ByteValues: [][]byte{[]byte("testGroup")},
},
{
Name: "member",
Values: []string{testUserDN},
ByteValues: [][]byte{[]byte(testUserDN)},
},
},
}

func newTestAugmentedADLDAPInterface(client ldapclient.Client) *AugmentedADLDAPInterface {
return NewAugmentedADLDAPInterface(testclient.NewConfig(client),
testUserQuery,
testGroupMembershipAttributes,
testUserNameAttributes,
testGroupQuery,
testGroupNameAttributes)
}

// below are common test implementations of LDAPInterface fields
var testGroupQuery ldaputil.LDAPQueryOnAttribute = ldaputil.LDAPQueryOnAttribute{
LDAPQuery: ldaputil.LDAPQuery{
BaseDN: groupsBaseDN,
Scope: ldaputil.ScopeWholeSubtree,
DerefAliases: ldaputil.DerefAliasesAlways,
TimeLimit: 0,
Filter: groupsBaseFilter,
},
QueryAttribute: "dn",
}
var testGroupNameAttributes []string = []string{"cn"}
28 changes: 14 additions & 14 deletions pkg/cmd/experimental/syncgroups/ad/ldapinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/openshift/origin/pkg/auth/ldaputil"
"github.com/openshift/origin/pkg/auth/ldaputil/ldapclient"
ldapinterfaces "github.com/openshift/origin/pkg/cmd/experimental/syncgroups/interfaces"
"github.com/openshift/origin/pkg/cmd/experimental/syncgroups/interfaces"
)

// NewADLDAPInterface builds a new ADLDAPInterface using a schema-appropriate config
Expand All @@ -29,9 +29,6 @@ func NewADLDAPInterface(clientConfig ldapclient.Config,

// ADLDAPInterface extracts the member list of an LDAP group entry from an LDAP server
// with first-class LDAP entries for user only. The ADLDAPInterface is *NOT* thread-safe.
// The ADLDAPInterface satisfies:
// - LDAPMemberExtractor
// - LDAPGroupLister
type ADLDAPInterface struct {
// clientConfig holds LDAP connection information
clientConfig ldapclient.Config
Expand All @@ -43,12 +40,18 @@ type ADLDAPInterface struct {
// UserNameAttributes defines which attributes on an LDAP user entry will be interpreted as its name
userNameAttributes []string

cachePopulated bool
// cacheFullyPopulated determines if the cache has been fully populated
// populateCache() will populate it fully, specific calls to ExtractMembers() will not
cacheFullyPopulated bool
// ldapGroupToLDAPMembers holds the result of user queries for later reference, indexed on group UID
// e.g. this will map all LDAP users to the LDAP group UID whose entry returned them
ldapGroupToLDAPMembers map[string][]*ldap.Entry
}

var _ ldapinterfaces.LDAPMemberExtractor = &ADLDAPInterface{}
var _ ldapinterfaces.LDAPGroupLister = &ADLDAPInterface{}
// The LDAPInterface must conform to the following interfaces
var _ interfaces.LDAPMemberExtractor = &ADLDAPInterface{}
var _ interfaces.LDAPGroupLister = &ADLDAPInterface{}
var _ interfaces.LDAPGroupDetector = &ADLDAPInterface{}

// ExtractMembers returns the LDAP member entries for a group specified with a ldapGroupUID
func (e *ADLDAPInterface) ExtractMembers(ldapGroupUID string) ([]*ldap.Entry, error) {
Expand All @@ -74,9 +77,7 @@ func (e *ADLDAPInterface) ExtractMembers(ldapGroupUID string) ([]*ldap.Entry, er
return nil, err
}

for i := range currEntries {
currEntry := currEntries[i]

for _, currEntry := range currEntries {
if !isEntryPresent(usersInGroup, currEntry) {
usersInGroup = append(usersInGroup, currEntry)
}
Expand All @@ -101,7 +102,7 @@ func (e *ADLDAPInterface) ListGroups() ([]string, error) {
// populateCache queries all users to build a map of all the groups. If the cache has already been
// populated, this is a no-op.
func (e *ADLDAPInterface) populateCache() error {
if e.cachePopulated {
if e.cacheFullyPopulated {
return nil
}

Expand All @@ -112,8 +113,7 @@ func (e *ADLDAPInterface) populateCache() error {
return err
}

for i := range userEntries {
userEntry := userEntries[i]
for _, userEntry := range userEntries {
if userEntry == nil {
continue
}
Expand All @@ -130,7 +130,7 @@ func (e *ADLDAPInterface) populateCache() error {
}
}
}
e.cachePopulated = true
e.cacheFullyPopulated = true

return nil
}
Expand Down

0 comments on commit 63a542d

Please sign in to comment.