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

Added unit testing for LDAP group sync #5669

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 8 additions & 10 deletions pkg/cmd/experimental/syncgroups/ad/augmented_ldapinterface.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package ad

import (
"k8s.io/kubernetes/pkg/util/sets"

"gopkg.in/ldap.v2"

"k8s.io/kubernetes/pkg/util/sets"

"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,14 @@ 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{}

// 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
108 changes: 108 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,108 @@
package ad

import (
"errors"
"fmt"
"reflect"
"testing"

"gopkg.in/ldap.v2"

"github.com/openshift/origin/pkg/auth/ldaputil"
"github.com/openshift/origin/pkg/auth/ldaputil/testclient"
)

func newTestAugmentedADLDAPInterface(client ldap.Client) *AugmentedADLDAPInterface {
Copy link
Contributor

Choose a reason for hiding this comment

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

accept groupsBaseDN and groupsBaseFilter as args, so I don't have to know your harness when I try to use this.

// below are common test implementations of LDAPInterface fields
userQuery := ldaputil.LDAPQuery{
BaseDN: "ou=users,dc=example,dc=com",
Scope: ldaputil.ScopeWholeSubtree,
DerefAliases: ldaputil.DerefAliasesAlways,
TimeLimit: 0,
Filter: "objectClass=inetOrgPerson",
}
groupMembershipAttributes := []string{"memberOf"}
userNameAttributes := []string{"cn"}
groupQuery := ldaputil.LDAPQueryOnAttribute{
LDAPQuery: ldaputil.LDAPQuery{
BaseDN: "ou=groups,dc=example,dc=com",
Scope: ldaputil.ScopeWholeSubtree,
DerefAliases: ldaputil.DerefAliasesAlways,
TimeLimit: 0,
Filter: "objectClass=groupOfNames",
},
QueryAttribute: "dn",
}
groupNameAttributes := []string{"cn"}

return NewAugmentedADLDAPInterface(testclient.NewConfig(client),
userQuery,
groupMembershipAttributes,
userNameAttributes,
groupQuery,
groupNameAttributes)
}

// newDefaultTestGroup returns a new LDAP entry with the given CN
func newTestGroup(CN string) *ldap.Entry {
return ldap.NewEntry(fmt.Sprintf("cn=%s,ou=groups,dc=example,dc=com", CN), map[string][]string{"cn": {CN}})
}

func TestGroupEntryFor(t *testing.T) {
var testCases = []struct {
name string
cacheSeed map[string]*ldap.Entry
client ldap.Client
baseDNOverride string
expectedError error
expectedEntry *ldap.Entry
}{
{
name: "cached entries",
cacheSeed: map[string]*ldap.Entry{
"cn=testGroup,ou=groups,dc=example,dc=com": newTestGroup("testGroup"),
},
expectedError: nil,
expectedEntry: newTestGroup("testGroup"),
},
{
name: "search request error",
baseDNOverride: "otherBaseDN",
expectedError: ldaputil.NewQueryOutOfBoundsError("cn=testGroup,ou=groups,dc=example,dc=com", "otherBaseDN"),
expectedEntry: nil,
},
{
name: "search error",
client: testclient.NewMatchingSearchErrorClient(testclient.New(), "cn=testGroup,ou=groups,dc=example,dc=com", errors.New("generic search error")),
expectedError: errors.New("generic search error"),
expectedEntry: nil,
},
{
name: "no error",
client: testclient.NewDNMappingClient(
testclient.New(),
map[string][]*ldap.Entry{
"cn=testGroup,ou=groups,dc=example,dc=com": {newTestGroup("testGroup")},
},
),
expectedError: nil,
expectedEntry: newTestGroup("testGroup"),
},
}
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("cn=testGroup,ou=groups,dc=example,dc=com")
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)
}
}
}
31 changes: 15 additions & 16 deletions pkg/cmd/experimental/syncgroups/ad/ldapinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package ad
import (
"reflect"

"k8s.io/kubernetes/pkg/util/sets"

"gopkg.in/ldap.v2"

"k8s.io/kubernetes/pkg/util/sets"

"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,17 @@ 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{}

// 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 +76,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 +101,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 +112,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 +129,7 @@ func (e *ADLDAPInterface) populateCache() error {
}
}
}
e.cachePopulated = true
e.cacheFullyPopulated = true

return nil
}
Expand Down