| ... | ... |
@@ -27,8 +27,8 @@ func NewAugmentedADLDAPInterface(clientConfig ldapclient.Config, |
| 27 | 27 |
} |
| 28 | 28 |
} |
| 29 | 29 |
|
| 30 |
-// LDAPInterface extracts the member list of an LDAP user entry from an LDAP server |
|
| 31 |
-// with first-class LDAP entries for users and group. Group membership is on the user. The LDAPInterface is *NOT* thread-safe. |
|
| 30 |
+// AugmentedADLDAPInterface extracts the member list of an LDAP user entry from an LDAP server |
|
| 31 |
+// with first-class LDAP entries for users and group. Group membership is on the user. The AugmentedADLDAPInterface is *NOT* thread-safe. |
|
| 32 | 32 |
type AugmentedADLDAPInterface struct {
|
| 33 | 33 |
*ADLDAPInterface |
| 34 | 34 |
|
| ... | ... |
@@ -47,8 +47,8 @@ var _ interfaces.LDAPMemberExtractor = &AugmentedADLDAPInterface{}
|
| 47 | 47 |
var _ interfaces.LDAPGroupGetter = &AugmentedADLDAPInterface{}
|
| 48 | 48 |
var _ interfaces.LDAPGroupLister = &AugmentedADLDAPInterface{}
|
| 49 | 49 |
|
| 50 |
-// GroupFor returns an LDAP group entry for the given group UID by searching the internal cache |
|
| 51 |
-// of the LDAPInterface first, then sending an LDAP query if the cache did not contain the entry. |
|
| 50 |
+// GroupEntryFor returns an LDAP group entry for the given group UID by searching the internal cache |
|
| 51 |
+// of the AugmentedADLDAPInterface first, then sending an LDAP query if the cache did not contain the entry. |
|
| 52 | 52 |
// This also satisfies the LDAPGroupGetter interface |
| 53 | 53 |
func (e *AugmentedADLDAPInterface) GroupEntryFor(ldapGroupUID string) (*ldap.Entry, error) {
|
| 54 | 54 |
group, exists := e.cachedGroups[ldapGroupUID] |
| ... | ... |
@@ -6,6 +6,7 @@ import ( |
| 6 | 6 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync" |
| 7 | 7 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" |
| 8 | 8 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync/rfc2307" |
| 9 |
+ "github.com/openshift/origin/pkg/cmd/admin/groups/sync/syncerror" |
|
| 9 | 10 |
"github.com/openshift/origin/pkg/cmd/server/api" |
| 10 | 11 |
) |
| 11 | 12 |
|
| ... | ... |
@@ -17,6 +18,8 @@ type RFC2307Builder struct {
|
| 17 | 17 |
Config *api.RFC2307Config |
| 18 | 18 |
|
| 19 | 19 |
rfc2307LDAPInterface *rfc2307.LDAPInterface |
| 20 |
+ |
|
| 21 |
+ ErrorHandler syncerror.Handler |
|
| 20 | 22 |
} |
| 21 | 23 |
|
| 22 | 24 |
func (b *RFC2307Builder) GetGroupLister() (interfaces.LDAPGroupLister, error) {
|
| ... | ... |
@@ -58,7 +61,7 @@ func (b *RFC2307Builder) getRFC2307LDAPInterface() (*rfc2307.LDAPInterface, erro |
| 58 | 58 |
} |
| 59 | 59 |
b.rfc2307LDAPInterface = rfc2307.NewLDAPInterface(b.ClientConfig, |
| 60 | 60 |
groupQuery, b.Config.GroupNameAttributes, b.Config.GroupMembershipAttributes, |
| 61 |
- userQuery, b.Config.UserNameAttributes) |
|
| 61 |
+ userQuery, b.Config.UserNameAttributes, b.ErrorHandler) |
|
| 62 | 62 |
return b.rfc2307LDAPInterface, nil |
| 63 | 63 |
} |
| 64 | 64 |
|
| ... | ... |
@@ -23,6 +23,7 @@ import ( |
| 23 | 23 |
osclient "github.com/openshift/origin/pkg/client" |
| 24 | 24 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync" |
| 25 | 25 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" |
| 26 |
+ "github.com/openshift/origin/pkg/cmd/admin/groups/sync/syncerror" |
|
| 26 | 27 |
"github.com/openshift/origin/pkg/cmd/server/api" |
| 27 | 28 |
configapilatest "github.com/openshift/origin/pkg/cmd/server/api/latest" |
| 28 | 29 |
"github.com/openshift/origin/pkg/cmd/server/api/validation" |
| ... | ... |
@@ -74,8 +75,7 @@ const ( |
| 74 | 74 |
var AllowedSourceTypes = []string{string(GroupSyncSourceLDAP), string(GroupSyncSourceOpenShift)}
|
| 75 | 75 |
|
| 76 | 76 |
func ValidateSource(source GroupSyncSource) bool {
|
| 77 |
- knownSources := sets.NewString(string(GroupSyncSourceLDAP), string(GroupSyncSourceOpenShift)) |
|
| 78 |
- return knownSources.Has(string(source)) |
|
| 77 |
+ return sets.NewString(AllowedSourceTypes...).Has(string(source)) |
|
| 79 | 78 |
} |
| 80 | 79 |
|
| 81 | 80 |
type SyncOptions struct {
|
| ... | ... |
@@ -104,6 +104,21 @@ type SyncOptions struct {
|
| 104 | 104 |
Out io.Writer |
| 105 | 105 |
} |
| 106 | 106 |
|
| 107 |
+// CreateErrorHandler creates an error handler for the LDAP sync job |
|
| 108 |
+func (o *SyncOptions) CreateErrorHandler() syncerror.Handler {
|
|
| 109 |
+ components := []syncerror.Handler{}
|
|
| 110 |
+ if o.Config.RFC2307Config != nil {
|
|
| 111 |
+ if o.Config.RFC2307Config.TolerateOutOfScopeMembers {
|
|
| 112 |
+ components = append(components, syncerror.NewMemberLookupOutOfBoundsSuppressor(o.Stderr)) |
|
| 113 |
+ } |
|
| 114 |
+ if o.Config.RFC2307Config.TolerateMissingMembers {
|
|
| 115 |
+ components = append(components, syncerror.NewMemberLookupMemberNotFoundSuppressor(o.Stderr)) |
|
| 116 |
+ } |
|
| 117 |
+ } |
|
| 118 |
+ |
|
| 119 |
+ return syncerror.NewCompoundHandler(components...) |
|
| 120 |
+} |
|
| 121 |
+ |
|
| 107 | 122 |
func NewSyncOptions() *SyncOptions {
|
| 108 | 123 |
return &SyncOptions{
|
| 109 | 124 |
Stderr: os.Stderr, |
| ... | ... |
@@ -327,7 +342,9 @@ func (o *SyncOptions) Run(cmd *cobra.Command, f *clientcmd.Factory) error {
|
| 327 | 327 |
return fmt.Errorf("could not determine LDAP client configuration: %v", err)
|
| 328 | 328 |
} |
| 329 | 329 |
|
| 330 |
- syncBuilder, err := buildSyncBuilder(clientConfig, o.Config) |
|
| 330 |
+ errorHandler := o.CreateErrorHandler() |
|
| 331 |
+ |
|
| 332 |
+ syncBuilder, err := buildSyncBuilder(clientConfig, o.Config, errorHandler) |
|
| 331 | 333 |
if err != nil {
|
| 332 | 334 |
return err |
| 333 | 335 |
} |
| ... | ... |
@@ -399,10 +416,10 @@ func (o *SyncOptions) Run(cmd *cobra.Command, f *clientcmd.Factory) error {
|
| 399 | 399 |
return kerrs.NewAggregate(syncErrors) |
| 400 | 400 |
} |
| 401 | 401 |
|
| 402 |
-func buildSyncBuilder(clientConfig ldapclient.Config, syncConfig *api.LDAPSyncConfig) (SyncBuilder, error) {
|
|
| 402 |
+func buildSyncBuilder(clientConfig ldapclient.Config, syncConfig *api.LDAPSyncConfig, errorHandler syncerror.Handler) (SyncBuilder, error) {
|
|
| 403 | 403 |
switch {
|
| 404 | 404 |
case syncConfig.RFC2307Config != nil: |
| 405 |
- return &RFC2307Builder{ClientConfig: clientConfig, Config: syncConfig.RFC2307Config}, nil
|
|
| 405 |
+ return &RFC2307Builder{ClientConfig: clientConfig, Config: syncConfig.RFC2307Config, ErrorHandler: errorHandler}, nil
|
|
| 406 | 406 |
case syncConfig.ActiveDirectoryConfig != nil: |
| 407 | 407 |
return &ADBuilder{ClientConfig: clientConfig, Config: syncConfig.ActiveDirectoryConfig}, nil
|
| 408 | 408 |
case syncConfig.AugmentedActiveDirectoryConfig != nil: |
| 409 | 409 |
deleted file mode 100644 |
| ... | ... |
@@ -1,28 +0,0 @@ |
| 1 |
-package interfaces |
|
| 2 |
- |
|
| 3 |
-import ( |
|
| 4 |
- "fmt" |
|
| 5 |
-) |
|
| 6 |
- |
|
| 7 |
-func NewMemberLookupError(ldapGroupUID, ldapUserUID string, causedBy error) error {
|
|
| 8 |
- return &memberLookupError{ldapGroupUID: ldapGroupUID, ldapUserUID: ldapUserUID, causedBy: causedBy}
|
|
| 9 |
-} |
|
| 10 |
- |
|
| 11 |
-type memberLookupError struct {
|
|
| 12 |
- ldapGroupUID string |
|
| 13 |
- ldapUserUID string |
|
| 14 |
- causedBy error |
|
| 15 |
-} |
|
| 16 |
- |
|
| 17 |
-func (e *memberLookupError) Error() string {
|
|
| 18 |
- return fmt.Sprintf("membership lookup for user %q in group %q failed because of %q", e.ldapUserUID, e.ldapGroupUID, e.causedBy.Error())
|
|
| 19 |
-} |
|
| 20 |
- |
|
| 21 |
-func IsmemberLookupError(e error) bool {
|
|
| 22 |
- if e == nil {
|
|
| 23 |
- return false |
|
| 24 |
- } |
|
| 25 |
- |
|
| 26 |
- _, ok := e.(*memberLookupError) |
|
| 27 |
- return ok |
|
| 28 |
-} |
| ... | ... |
@@ -11,6 +11,7 @@ import ( |
| 11 | 11 |
"github.com/openshift/origin/pkg/auth/ldaputil/ldapclient" |
| 12 | 12 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync/groupdetector" |
| 13 | 13 |
"github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" |
| 14 |
+ "github.com/openshift/origin/pkg/cmd/admin/groups/sync/syncerror" |
|
| 14 | 15 |
) |
| 15 | 16 |
|
| 16 | 17 |
// NewLDAPInterface builds a new LDAPInterface using a schema-appropriate config |
| ... | ... |
@@ -19,7 +20,8 @@ func NewLDAPInterface(clientConfig ldapclient.Config, |
| 19 | 19 |
groupNameAttributes []string, |
| 20 | 20 |
groupMembershipAttributes []string, |
| 21 | 21 |
userQuery ldaputil.LDAPQueryOnAttribute, |
| 22 |
- userNameAttributes []string) *LDAPInterface {
|
|
| 22 |
+ userNameAttributes []string, |
|
| 23 |
+ errorHandler syncerror.Handler) *LDAPInterface {
|
|
| 23 | 24 |
|
| 24 | 25 |
return &LDAPInterface{
|
| 25 | 26 |
clientConfig: clientConfig, |
| ... | ... |
@@ -30,6 +32,7 @@ func NewLDAPInterface(clientConfig ldapclient.Config, |
| 30 | 30 |
userNameAttributes: userNameAttributes, |
| 31 | 31 |
cachedUsers: map[string]*ldap.Entry{},
|
| 32 | 32 |
cachedGroups: map[string]*ldap.Entry{},
|
| 33 |
+ errorHandler: errorHandler, |
|
| 33 | 34 |
} |
| 34 | 35 |
} |
| 35 | 36 |
|
| ... | ... |
@@ -57,6 +60,9 @@ type LDAPInterface struct {
|
| 57 | 57 |
// cachedUsers holds the result of user queries for later reference, indexed on user UID |
| 58 | 58 |
// e.g. this will map an LDAP user UID to the LDAP entry returned from the query made using it |
| 59 | 59 |
cachedUsers map[string]*ldap.Entry |
| 60 |
+ |
|
| 61 |
+ // errorHandler handles errors that occur |
|
| 62 |
+ errorHandler syncerror.Handler |
|
| 60 | 63 |
} |
| 61 | 64 |
|
| 62 | 65 |
// The LDAPInterface must conform to the following interfaces |
| ... | ... |
@@ -82,15 +88,26 @@ func (e *LDAPInterface) ExtractMembers(ldapGroupUID string) ([]*ldap.Entry, erro |
| 82 | 82 |
// find members on LDAP server or in cache |
| 83 | 83 |
for _, ldapMemberUID := range ldapMemberUIDs {
|
| 84 | 84 |
memberEntry, err := e.userEntryFor(ldapMemberUID) |
| 85 |
- if err != nil {
|
|
| 86 |
- return nil, interfaces.NewMemberLookupError(ldapGroupUID, ldapMemberUID, err) |
|
| 85 |
+ if err == nil {
|
|
| 86 |
+ members = append(members, memberEntry) |
|
| 87 |
+ continue |
|
| 88 |
+ } |
|
| 89 |
+ |
|
| 90 |
+ err = syncerror.NewMemberLookupError(ldapGroupUID, ldapMemberUID, err) |
|
| 91 |
+ handled, fatalErr := e.errorHandler.HandleError(err) |
|
| 92 |
+ if fatalErr != nil {
|
|
| 93 |
+ return nil, fatalErr |
|
| 87 | 94 |
} |
| 88 |
- members = append(members, memberEntry) |
|
| 95 |
+ |
|
| 96 |
+ if !handled {
|
|
| 97 |
+ return nil, err |
|
| 98 |
+ } |
|
| 99 |
+ |
|
| 89 | 100 |
} |
| 90 | 101 |
return members, nil |
| 91 | 102 |
} |
| 92 | 103 |
|
| 93 |
-// GroupFor returns an LDAP group entry for the given group UID by searching the internal cache |
|
| 104 |
+// GroupEntryFor returns an LDAP group entry for the given group UID by searching the internal cache |
|
| 94 | 105 |
// of the LDAPInterface first, then sending an LDAP query if the cache did not contain the entry. |
| 95 | 106 |
// This also satisfies the LDAPGroupGetter interface |
| 96 | 107 |
func (e *LDAPInterface) GroupEntryFor(ldapGroupUID string) (*ldap.Entry, error) {
|
| ... | ... |
@@ -3,6 +3,7 @@ package rfc2307 |
| 3 | 3 |
import ( |
| 4 | 4 |
"errors" |
| 5 | 5 |
"fmt" |
| 6 |
+ "io/ioutil" |
|
| 6 | 7 |
"reflect" |
| 7 | 8 |
"testing" |
| 8 | 9 |
|
| ... | ... |
@@ -10,7 +11,7 @@ import ( |
| 10 | 10 |
|
| 11 | 11 |
"github.com/openshift/origin/pkg/auth/ldaputil" |
| 12 | 12 |
"github.com/openshift/origin/pkg/auth/ldaputil/testclient" |
| 13 |
- "github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" |
|
| 13 |
+ "github.com/openshift/origin/pkg/cmd/admin/groups/sync/syncerror" |
|
| 14 | 14 |
) |
| 15 | 15 |
|
| 16 | 16 |
func newTestLDAPInterface(client ldap.Client) *LDAPInterface {
|
| ... | ... |
@@ -39,12 +40,18 @@ func newTestLDAPInterface(client ldap.Client) *LDAPInterface {
|
| 39 | 39 |
} |
| 40 | 40 |
userNameAttributes := []string{"cn"}
|
| 41 | 41 |
|
| 42 |
+ errorHandler := syncerror.NewCompoundHandler( |
|
| 43 |
+ syncerror.NewMemberLookupOutOfBoundsSuppressor(ioutil.Discard), |
|
| 44 |
+ syncerror.NewMemberLookupMemberNotFoundSuppressor(ioutil.Discard), |
|
| 45 |
+ ) |
|
| 46 |
+ |
|
| 42 | 47 |
return NewLDAPInterface(testclient.NewConfig(client), |
| 43 | 48 |
groupQuery, |
| 44 | 49 |
groupNameAttributes, |
| 45 | 50 |
groupMembershipAttributes, |
| 46 | 51 |
userQuery, |
| 47 |
- userNameAttributes) |
|
| 52 |
+ userNameAttributes, |
|
| 53 |
+ errorHandler) |
|
| 48 | 54 |
} |
| 49 | 55 |
|
| 50 | 56 |
// newTestUser returns a new LDAP entry with the CN |
| ... | ... |
@@ -95,10 +102,55 @@ func TestExtractMembers(t *testing.T) {
|
| 95 | 95 |
"cn=testUser,ou=users,dc=example,dc=com", |
| 96 | 96 |
errors.New("generic search error"),
|
| 97 | 97 |
), |
| 98 |
- expectedError: interfaces.NewMemberLookupError("cn=testGroup,ou=groups,dc=example,dc=com", "cn=testUser,ou=users,dc=example,dc=com", errors.New("generic search error")),
|
|
| 98 |
+ expectedError: syncerror.NewMemberLookupError("cn=testGroup,ou=groups,dc=example,dc=com", "cn=testUser,ou=users,dc=example,dc=com", errors.New("generic search error")),
|
|
| 99 | 99 |
expectedMembers: nil, |
| 100 | 100 |
}, |
| 101 | 101 |
{
|
| 102 |
+ name: "out of scope member lookup suppressed", |
|
| 103 |
+ client: testclient.NewMatchingSearchErrorClient( |
|
| 104 |
+ testclient.NewDNMappingClient( |
|
| 105 |
+ testclient.New(), |
|
| 106 |
+ map[string][]*ldap.Entry{
|
|
| 107 |
+ "cn=testGroup,ou=groups,dc=example,dc=com": {newTestGroup("testGroup", "cn=testUser,ou=users,dc=other-example,dc=com")},
|
|
| 108 |
+ }, |
|
| 109 |
+ ), |
|
| 110 |
+ "cn=testUser,ou=users,dc=other-example,dc=com", |
|
| 111 |
+ ldaputil.NewQueryOutOfBoundsError("cn=testUser,ou=users,dc=other-example,dc=com", "cn=testGroup,ou=groups,dc=example,dc=com"),
|
|
| 112 |
+ ), |
|
| 113 |
+ expectedError: nil, |
|
| 114 |
+ expectedMembers: []*ldap.Entry{},
|
|
| 115 |
+ }, |
|
| 116 |
+ {
|
|
| 117 |
+ name: "no such object member lookup error suppressed", |
|
| 118 |
+ client: testclient.NewMatchingSearchErrorClient( |
|
| 119 |
+ testclient.NewDNMappingClient( |
|
| 120 |
+ testclient.New(), |
|
| 121 |
+ map[string][]*ldap.Entry{
|
|
| 122 |
+ "cn=testGroup,ou=groups,dc=example,dc=com": {newTestGroup("testGroup", "cn=testUser,ou=users,dc=other-example,dc=com")},
|
|
| 123 |
+ }, |
|
| 124 |
+ ), |
|
| 125 |
+ "cn=testUser,ou=users,dc=other-example,dc=com", |
|
| 126 |
+ ldaputil.NewNoSuchObjectError("cn=testUser,ou=users,dc=other-example,dc=com"),
|
|
| 127 |
+ ), |
|
| 128 |
+ expectedError: nil, |
|
| 129 |
+ expectedMembers: []*ldap.Entry{},
|
|
| 130 |
+ }, |
|
| 131 |
+ {
|
|
| 132 |
+ name: "member not found member lookup error suppressed", |
|
| 133 |
+ client: testclient.NewMatchingSearchErrorClient( |
|
| 134 |
+ testclient.NewDNMappingClient( |
|
| 135 |
+ testclient.New(), |
|
| 136 |
+ map[string][]*ldap.Entry{
|
|
| 137 |
+ "cn=testGroup,ou=groups,dc=example,dc=com": {newTestGroup("testGroup", "cn=testUser,ou=users,dc=other-example,dc=com")},
|
|
| 138 |
+ }, |
|
| 139 |
+ ), |
|
| 140 |
+ "cn=testUser,ou=users,dc=other-example,dc=com", |
|
| 141 |
+ ldaputil.NewEntryNotFoundError("cn=testUser,ou=users,dc=other-example,dc=com", "objectClass=groupOfNames"),
|
|
| 142 |
+ ), |
|
| 143 |
+ expectedError: nil, |
|
| 144 |
+ expectedMembers: []*ldap.Entry{},
|
|
| 145 |
+ }, |
|
| 146 |
+ {
|
|
| 102 | 147 |
name: "no errors", |
| 103 | 148 |
client: testclient.NewDNMappingClient( |
| 104 | 149 |
testclient.New(), |
| 105 | 150 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,28 @@ |
| 0 |
+package syncerror |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "fmt" |
|
| 4 |
+) |
|
| 5 |
+ |
|
| 6 |
+func NewMemberLookupError(ldapGroupUID, ldapUserUID string, causedBy error) error {
|
|
| 7 |
+ return &memberLookupError{ldapGroupUID: ldapGroupUID, ldapUserUID: ldapUserUID, causedBy: causedBy}
|
|
| 8 |
+} |
|
| 9 |
+ |
|
| 10 |
+type memberLookupError struct {
|
|
| 11 |
+ ldapGroupUID string |
|
| 12 |
+ ldapUserUID string |
|
| 13 |
+ causedBy error |
|
| 14 |
+} |
|
| 15 |
+ |
|
| 16 |
+func (e *memberLookupError) Error() string {
|
|
| 17 |
+ return fmt.Sprintf("membership lookup for user %q in group %q failed because of %q", e.ldapUserUID, e.ldapGroupUID, e.causedBy.Error())
|
|
| 18 |
+} |
|
| 19 |
+ |
|
| 20 |
+func IsMemberLookupError(e error) bool {
|
|
| 21 |
+ if e == nil {
|
|
| 22 |
+ return false |
|
| 23 |
+ } |
|
| 24 |
+ |
|
| 25 |
+ _, ok := e.(*memberLookupError) |
|
| 26 |
+ return ok |
|
| 27 |
+} |
| 0 | 28 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,90 @@ |
| 0 |
+package syncerror |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "fmt" |
|
| 4 |
+ "io" |
|
| 5 |
+ |
|
| 6 |
+ "github.com/openshift/origin/pkg/auth/ldaputil" |
|
| 7 |
+) |
|
| 8 |
+ |
|
| 9 |
+// Handler knows how to handle errors |
|
| 10 |
+type Handler interface {
|
|
| 11 |
+ // HandleError processess an error without mutating it. If the error is determined to be fatal, |
|
| 12 |
+ // a non-nil error should be returned. |
|
| 13 |
+ HandleError(err error) (handled bool, fatalError error) |
|
| 14 |
+} |
|
| 15 |
+ |
|
| 16 |
+func NewCompoundHandler(handlers ...Handler) Handler {
|
|
| 17 |
+ return &compoundHandler{handlers: handlers}
|
|
| 18 |
+} |
|
| 19 |
+ |
|
| 20 |
+// compoundHandler chains other error handlers |
|
| 21 |
+type compoundHandler struct {
|
|
| 22 |
+ handlers []Handler |
|
| 23 |
+} |
|
| 24 |
+ |
|
| 25 |
+// HandleError asks each of the handlers to handle the error. If any handler decides the error is fatal or handles it, |
|
| 26 |
+// the chain is broken. |
|
| 27 |
+func (h *compoundHandler) HandleError(err error) (bool, error) {
|
|
| 28 |
+ for _, handler := range h.handlers {
|
|
| 29 |
+ handled, handleErr := handler.HandleError(err) |
|
| 30 |
+ if handled || handleErr != nil {
|
|
| 31 |
+ return handled, handleErr |
|
| 32 |
+ } |
|
| 33 |
+ } |
|
| 34 |
+ |
|
| 35 |
+ return false, nil |
|
| 36 |
+} |
|
| 37 |
+ |
|
| 38 |
+func NewMemberLookupOutOfBoundsSuppressor(err io.Writer) Handler {
|
|
| 39 |
+ return &memberLookupOutOfBoundsSuppressor{err: err}
|
|
| 40 |
+} |
|
| 41 |
+ |
|
| 42 |
+// memberLookupOutOfBoundsSuppressor suppresses member lookup errors caused by a search trying to go out of the base DN bounds |
|
| 43 |
+type memberLookupOutOfBoundsSuppressor struct {
|
|
| 44 |
+ // err determines where a log message will be printed |
|
| 45 |
+ err io.Writer |
|
| 46 |
+} |
|
| 47 |
+ |
|
| 48 |
+// HandleError suppresses member lookup errors caused by out-of-bounds queries, |
|
| 49 |
+func (h *memberLookupOutOfBoundsSuppressor) HandleError(err error) (bool, error) {
|
|
| 50 |
+ memberLookupError, isMemberLookupError := err.(*memberLookupError) |
|
| 51 |
+ if !isMemberLookupError {
|
|
| 52 |
+ return false, nil |
|
| 53 |
+ } |
|
| 54 |
+ |
|
| 55 |
+ if ldaputil.IsQueryOutOfBoundsError(memberLookupError.causedBy) {
|
|
| 56 |
+ fmt.Fprintf(h.err, "For group %q, ignoring member %q: %v\n", memberLookupError.ldapGroupUID, memberLookupError.ldapUserUID, memberLookupError.causedBy) |
|
| 57 |
+ return true, nil |
|
| 58 |
+ } |
|
| 59 |
+ |
|
| 60 |
+ return false, nil |
|
| 61 |
+} |
|
| 62 |
+ |
|
| 63 |
+func NewMemberLookupMemberNotFoundSuppressor(err io.Writer) Handler {
|
|
| 64 |
+ return &memberLookupMemberNotFoundSuppressor{err: err}
|
|
| 65 |
+} |
|
| 66 |
+ |
|
| 67 |
+// memberLookupMemberNotFoundSuppressor supresses member lookup errors caused by a search returning no valid entries, |
|
| 68 |
+// which can happen in two ways: |
|
| 69 |
+// - if the search is not by DN, an empty result list is returned |
|
| 70 |
+// - if the search is by DN, an error is returned from the LDAP server: no such object |
|
| 71 |
+type memberLookupMemberNotFoundSuppressor struct {
|
|
| 72 |
+ // err determines where a log message will be printed |
|
| 73 |
+ err io.Writer |
|
| 74 |
+} |
|
| 75 |
+ |
|
| 76 |
+// HandleError suppresses member lookup errors caused by no such object or entry not found errors, |
|
| 77 |
+func (h *memberLookupMemberNotFoundSuppressor) HandleError(err error) (bool, error) {
|
|
| 78 |
+ memberLookupError, isMemberLookupError := err.(*memberLookupError) |
|
| 79 |
+ if !isMemberLookupError {
|
|
| 80 |
+ return false, nil |
|
| 81 |
+ } |
|
| 82 |
+ |
|
| 83 |
+ if ldaputil.IsEntryNotFoundError(memberLookupError.causedBy) || ldaputil.IsNoSuchObjectError(memberLookupError.causedBy) {
|
|
| 84 |
+ fmt.Fprintf(h.err, "For group %q, ignoring member %q: %v\n", memberLookupError.ldapGroupUID, memberLookupError.ldapUserUID, memberLookupError.causedBy) |
|
| 85 |
+ return true, nil |
|
| 86 |
+ } |
|
| 87 |
+ |
|
| 88 |
+ return false, nil |
|
| 89 |
+} |
| 0 | 90 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,110 @@ |
| 0 |
+package syncerror |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "errors" |
|
| 4 |
+ "io/ioutil" |
|
| 5 |
+ "reflect" |
|
| 6 |
+ "testing" |
|
| 7 |
+ |
|
| 8 |
+ "github.com/openshift/origin/pkg/auth/ldaputil" |
|
| 9 |
+) |
|
| 10 |
+ |
|
| 11 |
+func TestSuppressMemberLookupErrorOutOfBounds(t *testing.T) {
|
|
| 12 |
+ var testCases = []struct {
|
|
| 13 |
+ name string |
|
| 14 |
+ err error |
|
| 15 |
+ expectedHandled bool |
|
| 16 |
+ expectedFatalError error |
|
| 17 |
+ }{
|
|
| 18 |
+ {
|
|
| 19 |
+ name: "nil error", |
|
| 20 |
+ err: nil, |
|
| 21 |
+ expectedHandled: false, |
|
| 22 |
+ expectedFatalError: nil, |
|
| 23 |
+ }, |
|
| 24 |
+ {
|
|
| 25 |
+ name: "other error", |
|
| 26 |
+ err: errors.New("generic error"),
|
|
| 27 |
+ expectedHandled: false, |
|
| 28 |
+ expectedFatalError: nil, |
|
| 29 |
+ }, |
|
| 30 |
+ {
|
|
| 31 |
+ name: "non-out-of-bounds member lookup error", |
|
| 32 |
+ err: NewMemberLookupError("", "", nil),
|
|
| 33 |
+ expectedHandled: false, |
|
| 34 |
+ expectedFatalError: nil, |
|
| 35 |
+ }, |
|
| 36 |
+ {
|
|
| 37 |
+ name: "out-of-bounds member lookup error", |
|
| 38 |
+ err: NewMemberLookupError("", "", ldaputil.NewQueryOutOfBoundsError("", "")),
|
|
| 39 |
+ expectedHandled: true, |
|
| 40 |
+ expectedFatalError: nil, |
|
| 41 |
+ }, |
|
| 42 |
+ } |
|
| 43 |
+ |
|
| 44 |
+ for _, testCase := range testCases {
|
|
| 45 |
+ handler := NewMemberLookupOutOfBoundsSuppressor(ioutil.Discard) |
|
| 46 |
+ |
|
| 47 |
+ actualHandled, actualFatalErr := handler.HandleError(testCase.err) |
|
| 48 |
+ if actualHandled != testCase.expectedHandled {
|
|
| 49 |
+ t.Errorf("%s: handler did not handle as expected: wanted handled=%t, got handled=%t", testCase.name, testCase.expectedHandled, actualHandled)
|
|
| 50 |
+ } |
|
| 51 |
+ |
|
| 52 |
+ if !reflect.DeepEqual(actualFatalErr, testCase.expectedFatalError) {
|
|
| 53 |
+ t.Errorf("%s: handler did not return correct error:\n\twanted\n\t%v,\n\tgot\n\t%v", testCase.name, testCase.expectedFatalError, actualFatalErr)
|
|
| 54 |
+ } |
|
| 55 |
+ } |
|
| 56 |
+} |
|
| 57 |
+ |
|
| 58 |
+func TestSuppressMemberLookupErrorMemberNotFound(t *testing.T) {
|
|
| 59 |
+ var testCases = []struct {
|
|
| 60 |
+ name string |
|
| 61 |
+ err error |
|
| 62 |
+ expectedHandled bool |
|
| 63 |
+ expectedFatalError error |
|
| 64 |
+ }{
|
|
| 65 |
+ {
|
|
| 66 |
+ name: "nil error", |
|
| 67 |
+ err: nil, |
|
| 68 |
+ expectedHandled: false, |
|
| 69 |
+ expectedFatalError: nil, |
|
| 70 |
+ }, |
|
| 71 |
+ {
|
|
| 72 |
+ name: "other error", |
|
| 73 |
+ err: errors.New("generic error"),
|
|
| 74 |
+ expectedHandled: false, |
|
| 75 |
+ expectedFatalError: nil, |
|
| 76 |
+ }, |
|
| 77 |
+ {
|
|
| 78 |
+ name: "non-member-not-found member lookup error", |
|
| 79 |
+ err: NewMemberLookupError("", "", nil),
|
|
| 80 |
+ expectedHandled: false, |
|
| 81 |
+ expectedFatalError: nil, |
|
| 82 |
+ }, |
|
| 83 |
+ {
|
|
| 84 |
+ name: "no such object member lookup error", |
|
| 85 |
+ err: NewMemberLookupError("", "", ldaputil.NewNoSuchObjectError("")),
|
|
| 86 |
+ expectedHandled: true, |
|
| 87 |
+ expectedFatalError: nil, |
|
| 88 |
+ }, |
|
| 89 |
+ {
|
|
| 90 |
+ name: "member not found member lookup error", |
|
| 91 |
+ err: NewMemberLookupError("", "", ldaputil.NewEntryNotFoundError("", "")),
|
|
| 92 |
+ expectedHandled: true, |
|
| 93 |
+ expectedFatalError: nil, |
|
| 94 |
+ }, |
|
| 95 |
+ } |
|
| 96 |
+ |
|
| 97 |
+ for _, testCase := range testCases {
|
|
| 98 |
+ handler := NewMemberLookupMemberNotFoundSuppressor(ioutil.Discard) |
|
| 99 |
+ |
|
| 100 |
+ actualHandled, actualFatalErr := handler.HandleError(testCase.err) |
|
| 101 |
+ if actualHandled != testCase.expectedHandled {
|
|
| 102 |
+ t.Errorf("%s: handler did not handle as expected: wanted handled=%t, got handled=%t", testCase.name, testCase.expectedHandled, actualHandled)
|
|
| 103 |
+ } |
|
| 104 |
+ |
|
| 105 |
+ if !reflect.DeepEqual(actualFatalErr, testCase.expectedFatalError) {
|
|
| 106 |
+ t.Errorf("%s: handler did not return correct error:\n\twanted\n\t%v,\n\tgot\n\t%v", testCase.name, testCase.expectedFatalError, actualFatalErr)
|
|
| 107 |
+ } |
|
| 108 |
+ } |
|
| 109 |
+} |
| ... | ... |
@@ -927,6 +927,21 @@ type RFC2307Config struct {
|
| 927 | 927 |
// UserNameAttributes defines which attributes on an LDAP user entry will be interpreted as its OpenShift user name. |
| 928 | 928 |
// This should match your PreferredUsername setting for your LDAPPasswordIdentityProvider |
| 929 | 929 |
UserNameAttributes []string |
| 930 |
+ |
|
| 931 |
+ // TolerateMissingMembers determines the behavior of the LDAP sync job when missing user entries are |
|
| 932 |
+ // encountered. If 'true', an LDAP query for users that doesn't find any will be tolerated and an only |
|
| 933 |
+ // and error will be logged. If 'false', the LDAP sync job will fail if a query for users doesn't find |
|
| 934 |
+ // any. The default value is 'false'. Misconfigured LDAP sync jobs with this flag set to 'true' can cause |
|
| 935 |
+ // group membership to be removed, so it is recommended to use this flag with caution. |
|
| 936 |
+ TolerateMissingMembers bool |
|
| 937 |
+ |
|
| 938 |
+ // TolerateOutOfScopeMembers determines the behavior of the LDAP sync job when out-of-scope user entries |
|
| 939 |
+ // are encountered. If 'true', an LDAP query for a user that falls outside of the base DN given for the all |
|
| 940 |
+ // user query will be tolerated and only an error will be logged. If 'false', the LDAP sync job will fail |
|
| 941 |
+ // if a user query would search outside of the base DN specified by the all user query. Misconfigured LDAP |
|
| 942 |
+ // sync jobs with this flag set to 'true' can result in groups missing users, so it is recommended to use |
|
| 943 |
+ // this flag with caution. |
|
| 944 |
+ TolerateOutOfScopeMembers bool |
|
| 930 | 945 |
} |
| 931 | 946 |
|
| 932 | 947 |
type ActiveDirectoryConfig struct {
|
| ... | ... |
@@ -879,6 +879,21 @@ type RFC2307Config struct {
|
| 879 | 879 |
// UserNameAttributes defines which attributes on an LDAP user entry will be used, in order, as its OpenShift user name. |
| 880 | 880 |
// The first attribute with a non-empty value is used. This should match your PreferredUsername setting for your LDAPPasswordIdentityProvider |
| 881 | 881 |
UserNameAttributes []string `json:"userNameAttributes"` |
| 882 |
+ |
|
| 883 |
+ // TolerateMissingMembers determines the behavior of the LDAP sync job when missing user entries are |
|
| 884 |
+ // encountered. If 'true', an LDAP query for users that doesn't find any will be tolerated and an only |
|
| 885 |
+ // and error will be logged. If 'false', the LDAP sync job will fail if a query for users doesn't find |
|
| 886 |
+ // any. The default value is 'false'. Misconfigured LDAP sync jobs with this flag set to 'true' can cause |
|
| 887 |
+ // group membership to be removed, so it is recommended to use this flag with caution. |
|
| 888 |
+ TolerateMissingMembers bool `json:"tolerateMissingMembers"` |
|
| 889 |
+ |
|
| 890 |
+ // TolerateOutOfScopeMembers determines the behavior of the LDAP sync job when out-of-scope user entries |
|
| 891 |
+ // are encountered. If 'true', an LDAP query for a user that falls outside of the base DN given for the all |
|
| 892 |
+ // user query will be tolerated and only an error will be logged. If 'false', the LDAP sync job will fail |
|
| 893 |
+ // if a user query would search outside of the base DN specified by the all user query. Misconfigured LDAP |
|
| 894 |
+ // sync jobs with this flag set to 'true' can result in groups missing users, so it is recommended to use |
|
| 895 |
+ // this flag with caution. |
|
| 896 |
+ TolerateOutOfScopeMembers bool `json:"tolerateOutOfScopeMembers"` |
|
| 882 | 897 |
} |
| 883 | 898 |
|
| 884 | 899 |
type ActiveDirectoryConfig struct {
|
| 885 | 900 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,22 @@ |
| 0 |
+kind: LDAPSyncConfig |
|
| 1 |
+apiVersion: v1 |
|
| 2 |
+url: ldap://LDAP_SERVICE_IP:389 |
|
| 3 |
+insecure: true |
|
| 4 |
+rfc2307: |
|
| 5 |
+ groupsQuery: |
|
| 6 |
+ baseDN: "ou=groups,ou=incomplete-rfc2307,dc=example,dc=com" |
|
| 7 |
+ scope: sub |
|
| 8 |
+ derefAliases: never |
|
| 9 |
+ filter: (objectclass=groupOfNames) |
|
| 10 |
+ groupUIDAttribute: dn |
|
| 11 |
+ groupNameAttributes: [ cn ] |
|
| 12 |
+ groupMembershipAttributes: [ member ] |
|
| 13 |
+ usersQuery: |
|
| 14 |
+ baseDN: "ou=people,ou=rfc2307,dc=example,dc=com" |
|
| 15 |
+ scope: sub |
|
| 16 |
+ derefAliases: never |
|
| 17 |
+ filter: (objectclass=inetOrgPerson) |
|
| 18 |
+ userUIDAttribute: dn |
|
| 19 |
+ userNameAttributes: [ mail ] |
|
| 20 |
+ tolerateMissingMembers: true |
|
| 21 |
+ tolerateOutOfScopeMembers: true |
|
| 0 | 22 |
\ No newline at end of file |
| 1 | 23 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,43 @@ |
| 0 |
+apiVersion: v1 |
|
| 1 |
+kind: Group |
|
| 2 |
+metadata: |
|
| 3 |
+ annotations: |
|
| 4 |
+ openshift.io/ldap.uid: cn=group1,ou=groups,ou=incomplete-rfc2307,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: group1 |
|
| 10 |
+users: |
|
| 11 |
+- person1smith@example.com |
|
| 12 |
+- person2smith@example.com |
|
| 13 |
+- person3smith@example.com |
|
| 14 |
+- person4smith@example.com |
|
| 15 |
+- person5smith@example.com |
|
| 16 |
+apiVersion: v1 |
|
| 17 |
+kind: Group |
|
| 18 |
+metadata: |
|
| 19 |
+ annotations: |
|
| 20 |
+ openshift.io/ldap.uid: cn=group2,ou=groups,ou=incomplete-rfc2307,dc=example,dc=com |
|
| 21 |
+ openshift.io/ldap.url: LDAP_SERVICE_IP:389 |
|
| 22 |
+ creationTimestamp: null |
|
| 23 |
+ labels: |
|
| 24 |
+ openshift.io/ldap.host: LDAP_SERVICE_IP |
|
| 25 |
+ name: group2 |
|
| 26 |
+users: |
|
| 27 |
+- person1smith@example.com |
|
| 28 |
+- person2smith@example.com |
|
| 29 |
+- person3smith@example.com |
|
| 30 |
+apiVersion: v1 |
|
| 31 |
+kind: Group |
|
| 32 |
+metadata: |
|
| 33 |
+ annotations: |
|
| 34 |
+ openshift.io/ldap.uid: cn=group3,ou=groups,ou=incomplete-rfc2307,dc=example,dc=com |
|
| 35 |
+ openshift.io/ldap.url: LDAP_SERVICE_IP:389 |
|
| 36 |
+ creationTimestamp: null |
|
| 37 |
+ labels: |
|
| 38 |
+ openshift.io/ldap.host: LDAP_SERVICE_IP |
|
| 39 |
+ name: group3 |
|
| 40 |
+users: |
|
| 41 |
+- person1smith@example.com |
|
| 42 |
+- person5smith@example.com |
| ... | ... |
@@ -229,6 +229,17 @@ for (( i=0; i<${#schema[@]}; i++ )); do
|
| 229 | 229 |
popd > /dev/null |
| 230 | 230 |
done |
| 231 | 231 |
|
| 232 |
+# special test for RFC2307 |
|
| 233 |
+pushd ${BASETMPDIR}/rfc2307 > /dev/null
|
|
| 234 |
+echo -e "\tTEST: Sync groups from LDAP server, tolerating errors" |
|
| 235 |
+oadm groups sync --sync-config=sync-config-tolerating.yaml --confirm 2>"${LOG_DIR}/tolerated-output.txt"
|
|
| 236 |
+grep 'For group "cn=group1,ou=groups,ou=incomplete\-rfc2307,dc=example,dc=com", ignoring member "cn=INVALID,ou=people,ou=rfc2307,dc=example,dc=com"' "${LOG_DIR}/tolerated-output.txt"
|
|
| 237 |
+grep 'For group "cn=group2,ou=groups,ou=incomplete\-rfc2307,dc=example,dc=com", ignoring member "cn=OUTOFSCOPE,ou=people,ou=OUTOFSCOPE,dc=example,dc=com"' "${LOG_DIR}/tolerated-output.txt"
|
|
| 238 |
+grep 'For group "cn=group3,ou=groups,ou=incomplete\-rfc2307,dc=example,dc=com", ignoring member "cn=INVALID,ou=people,ou=rfc2307,dc=example,dc=com"' "${LOG_DIR}/tolerated-output.txt"
|
|
| 239 |
+grep 'For group "cn=group3,ou=groups,ou=incomplete\-rfc2307,dc=example,dc=com", ignoring member "cn=OUTOFSCOPE,ou=people,ou=OUTOFSCOPE,dc=example,dc=com"' "${LOG_DIR}/tolerated-output.txt"
|
|
| 240 |
+compare_and_cleanup valid_all_ldap_sync_tolerating.yaml |
|
| 241 |
+popd > /dev/null |
|
| 242 |
+ |
|
| 232 | 243 |
# special test for augmented-ad |
| 233 | 244 |
pushd ${BASETMPDIR}/augmented-ad > /dev/null
|
| 234 | 245 |
echo -e "\tTEST: Sync all LDAP groups from LDAP server, remove LDAP group metadata entry, then prune OpenShift groups" |
| ... | ... |
@@ -236,4 +247,4 @@ oadm groups sync --sync-config=sync-config.yaml --confirm |
| 236 | 236 |
ldapdelete -x -h $LDAP_SERVICE_IP -p 389 -D cn=Manager,dc=example,dc=com -w admin "${group1_ldapuid}"
|
| 237 | 237 |
oadm groups prune --sync-config=sync-config.yaml --confirm |
| 238 | 238 |
compare_and_cleanup valid_all_ldap_sync_delete_prune.yaml |
| 239 |
-popd > /dev/null |
|
| 240 | 239 |
\ No newline at end of file |
| 240 |
+popd > /dev/null |