djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix bug where LDAP or AD status conditions were not updated correctly

Browse Source 
When the LDAP and AD IDP watcher controllers encountered an update error
while trying to update the status conditions of the IDP resources, then
they would drop the computed desired new value of the condition on the
ground. Next time the controller ran it would not try to update the
condition again because it wants to use the cached settings and had
already forgotten the desired new value of the condition computed during
the previous run of the controller. This would leave the outdated value
of the condition on the IDP resource.

This bug would manifest in CI as random failures in which the expected
condition message and the actual condition message would refer to
different versions numbers of the bind secret. The actual condition
message would refer to an older version of the bind secret because the
update failed and then the new desired message got dropped on the
ground.

This commit changes the in-memory caching strategy to also cache the
computed condition messages, allowing the conditions to be updated
on the IDP resource during future calls to Sync() in the case of a
failed update.
This commit is contained in:
Ryan Richard 2022-01-07 17:19:13 -08:00
parent 287d5094ec
commit 7f99d78462
5 changed files with 436 additions and 157 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go
View File

@ -1,4 +1,4 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package activedirectoryupstreamwatcher implements a controller which watches ActiveDirectoryIdentityProviders.
@ -226,7 +226,7 @@ type UpstreamActiveDirectoryIdentityProviderICache interface {
type activeDirectoryWatcherController struct {
cache UpstreamActiveDirectoryIdentityProviderICache
validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI
ldapDialer upstreamldap.LDAPDialer
client pinnipedclientset.Interface
activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
@ -243,8 +243,8 @@ func New(
) controllerlib.Controller {
return newInternal(
idpCache,
// start with an empty secretVersionCache
upstreamwatchers.NewSecretVersionCache(),
// start with an empty cache
upstreamwatchers.NewValidatedSettingsCache(),
// nil means to use a real production dialer when creating objects to add to the cache
nil,
client,
@ -257,7 +257,7 @@ func New(
// For test dependency injection purposes.
func newInternal(
idpCache UpstreamActiveDirectoryIdentityProviderICache,
validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI,
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI,
ldapDialer upstreamldap.LDAPDialer,
client pinnipedclientset.Interface,
activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,

259
internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go
View File

@ -1,4 +1,4 @@
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package activedirectoryupstreamwatcher
@ -150,8 +150,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
const (
testNamespace = "test-namespace"
testResourceUID = "test-uid"
testName = "test-name"
testResourceUID = "test-uid"
testSecretName = "test-bind-secret"
testBindUsername = "test-bind-username"
testBindPassword = "test-bind-password"
@ -255,6 +255,19 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
ObservedGeneration: gen,
}
}
activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration := func(secretVersion string) v1alpha1.Condition {
c := activeDirectoryConnectionValidTrueCondition(0, secretVersion)
c.LastTransitionTime = metav1.Time{}
return c
}
condPtr := func(c v1alpha1.Condition) *v1alpha1.Condition {
return &c
}
withoutTime := func(c v1alpha1.Condition) v1alpha1.Condition {
c = *c.DeepCopy()
c.LastTransitionTime = metav1.Time{}
return c
}
tlsConfigurationValidLoadedTrueCondition := func(gen int64) v1alpha1.Condition {
return v1alpha1.Condition{
Type: "TLSConfigurationValid",
@ -377,7 +390,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
Conditions: allConditionsTrue(1234, "4242"),
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "missing secret",
@ -568,7 +589,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "sAMAccountName explicitly provided as group name attribute does not add an override",
@ -629,7 +658,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports",
@ -695,7 +732,22 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: &v1alpha1.Condition{
Type: "LDAPConnectionValid",
Status: "True",
Reason: "Success",
Message: fmt.Sprintf(
`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
"ldap.example.com", testBindUsername, testSecretName, "4242"),
},
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports",
@ -808,7 +860,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
Conditions: allConditionsTrue(1234, "4242"),
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream",
@ -852,7 +912,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway but not to validatedsettings (treated like a warning)",
@ -1004,7 +1072,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
},
@ -1016,7 +1092,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
Conditions: allConditionsTrue(1234, "4242"),
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "when the validated cache contains LDAP server info but the search base is empty, reload everything",
@ -1030,7 +1114,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
upstream.Spec.UserSearch.Base = ""
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, Generation: 1234}},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
IDPSpecGeneration: 1234,
}},
setupMocks: func(conn *mockldapconn.MockConn) {
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(2)
conn.EXPECT().Close().Times(2)
@ -1075,7 +1163,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: exampleDefaultNamingContext,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
{
name: "when the LDAP server connection was already validated using TLS, and the search base was found, load TLS and search base info into the cache",
@ -1088,7 +1184,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
upstream.Spec.UserSearch.Base = ""
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: exampleDefaultNamingContext,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
},
wantResultingCache: []*upstreamldap.ProviderConfig{
@ -1136,7 +1240,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: exampleDefaultNamingContext,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
{
@ -1149,7 +1255,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, Generation: 1234, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.StartTLS,
IDPSpecGeneration: 1234,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
},
@ -1166,7 +1280,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
@ -1183,7 +1299,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1233,
IDPSpecGeneration: 1233,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
@ -1203,7 +1321,49 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
name: "when the LDAP server connection condition failed to update previously, then write the cached condition from the previous connection validation",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) {
upstream.Generation = 1234 // current generation
upstream.Status.Conditions = []v1alpha1.Condition{
activeDirectoryConnectionValidTrueCondition(1234, "4200"), // old version of the condition, as if the previous update of conditions had failed
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
IDPSpecGeneration: 1234,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), // already previously validated with version 4242
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// The connection had already been validated previously and the result was cached, so don't probe the server again.
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
},
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, UID: testResourceUID, Generation: 1234},
Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{
Phase: "Ready",
Conditions: allConditionsTrue(1234, "4242"), // updated version of the condition using the cached condition value
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
@ -1222,7 +1382,6 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS, Generation: 1234}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@ -1241,7 +1400,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
@ -1258,7 +1419,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4241")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}}, // old version was validated
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
@ -1273,12 +1436,14 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
Conditions: allConditionsTrue(1234, "4242"),
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{
testName: {BindSecretResourceVersion: "4242",
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
@ -1336,7 +1501,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))),
}},
},
{
@ -1398,7 +1565,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: exampleDefaultNamingContext,
GroupSearchBase: exampleDefaultNamingContext,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
{
@ -1454,7 +1623,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: exampleDefaultNamingContext,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
{
name: "when the input activedirectoryidentityprovider leaves group search base blank but provides user search base, query for defaultNamingContext",
@ -1509,7 +1686,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
},
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: exampleDefaultNamingContext, Generation: 1234}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: exampleDefaultNamingContext,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
{
name: "when the input activedirectoryidentityprovider leaves group search base blank and query for defaultNamingContext fails",
@ -1662,7 +1847,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4241")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
@ -1715,7 +1902,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
GroupSearchBase: exampleDefaultNamingContext,
UserSearchBase: testUserSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))),
}},
},
}
@ -1752,20 +1941,20 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
return conn, nil
})}
var validatedSecretVersionCache *upstreamwatchers.SecretVersionCache
var validatedSettingsCache *upstreamwatchers.ValidatedSettingsCache
if tt.initialValidatedSettings != nil {
validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettingsByName: tt.initialValidatedSettings,
}
} else {
validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettingsByName: map[string]upstreamwatchers.ValidatedSettings{},
}
}
controller := newInternal(
cache,
validatedSecretVersionCache,
validatedSettingsCache,
dialer,
fakePinnipedClient,
pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(),
@ -1850,7 +2039,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
if tt.wantValidatedSettings == nil {
tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{}
}
require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName)
require.Equal(t, tt.wantValidatedSettings, validatedSettingsCache.ValidatedSettingsByName)
})
}
}

10
internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go
View File

@ -1,4 +1,4 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package ldapupstreamwatcher implements a controller which watches LDAPIdentityProviders.
@ -134,7 +134,7 @@ type UpstreamLDAPIdentityProviderICache interface {
type ldapWatcherController struct {
cache UpstreamLDAPIdentityProviderICache
validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI
ldapDialer upstreamldap.LDAPDialer
client pinnipedclientset.Interface
ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer
@ -151,8 +151,8 @@ func New(
) controllerlib.Controller {
return newInternal(
idpCache,
// start with an empty secretVersionCache
upstreamwatchers.NewSecretVersionCache(),
// start with an empty cache
upstreamwatchers.NewValidatedSettingsCache(),
// nil means to use a real production dialer when creating objects to add to the cache
nil,
client,
@ -165,7 +165,7 @@ func New(
// For test dependency injection purposes.
func newInternal(
idpCache UpstreamLDAPIdentityProviderICache,
validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI,
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI,
ldapDialer upstreamldap.LDAPDialer,
client pinnipedclientset.Interface,
ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer,

130
internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go
View File

@ -1,4 +1,4 @@
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package ldapupstreamwatcher
@ -253,6 +253,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
ObservedGeneration: gen,
}
}
ldapConnectionValidTrueConditionWithoutTimeOrGeneration := func(secretVersion string) v1alpha1.Condition {
c := ldapConnectionValidTrueCondition(0, secretVersion)
c.LastTransitionTime = metav1.Time{}
return c
}
condPtr := func(c v1alpha1.Condition) *v1alpha1.Condition {
return &c
}
tlsConfigurationValidLoadedTrueCondition := func(gen int64) v1alpha1.Condition {
return v1alpha1.Condition{
Type: "TLSConfigurationValid",
@ -317,7 +325,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
@ -507,8 +516,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -571,8 +582,17 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: &v1alpha1.Condition{
Type: "LDAPConnectionValid",
Status: "True",
Reason: "Success",
Message: fmt.Sprintf(
`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
"ldap.example.com", testBindUsername, testSecretName, "4242"),
},
}},
},
{
name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -676,7 +696,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
@ -726,8 +747,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning) but not the validated settings cache",
inputUpstreams: []runtime.Object{validUpstream},
@ -776,7 +799,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
@ -794,8 +818,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when the LDAP server connection was already validated using StartTLS for the current resource generation and secret version, then do not validate it again and keep using StartTLS",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -810,7 +836,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
@ -828,8 +855,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when the LDAP server connection was validated for an older resource generation, then try to validate it again",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -842,7 +871,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
Generation: 1233,
IDPSpecGeneration: 1233,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
}},
@ -864,8 +893,48 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}},
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when the LDAP server connection condition failed to update previously, then write the cached condition from the previous connection validation",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
upstream.Generation = 1234 // current generation
upstream.Status.Conditions = []v1alpha1.Condition{
ldapConnectionValidTrueCondition(1234, "4200"), // old version of the condition, as if the previous update of conditions had failed
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
IDPSpecGeneration: 1234,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), // already previously validated with version 4242
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// The connection had already been validated previously and the result was cached, so don't probe the server again.
// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
},
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: allConditionsTrue(1234, "4242"), // updated version of the condition using the cached condition value
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when the LDAP server connection validation previously failed for this resource generation, then try to validate it again",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -900,8 +969,11 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
}}}, {
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
name: "when the validated settings cache is incomplete, then try to validate it again",
// this shouldn't happen, but if it does, just throw it out and try again.
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
@ -940,7 +1012,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
@ -957,7 +1030,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
}}, // old version was validated
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
@ -977,7 +1050,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Generation: 1234,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}}},
}
@ -1013,20 +1087,20 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
return conn, nil
})}
var validatedSecretVersionCache *upstreamwatchers.SecretVersionCache
var validatedSettingsCache *upstreamwatchers.ValidatedSettingsCache
if tt.initialValidatedSettings != nil {
validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettingsByName: tt.initialValidatedSettings,
}
} else {
validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettingsByName: map[string]upstreamwatchers.ValidatedSettings{},
}
}
controller := newInternal(
cache,
validatedSecretVersionCache,
validatedSettingsCache,
dialer,
fakePinnipedClient,
pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders(),
@ -1075,7 +1149,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
if tt.wantValidatedSettings == nil {
tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{}
}
require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName)
require.Equal(t, tt.wantValidatedSettings, validatedSettingsCache.ValidatedSettingsByName)
})
}
}

148
internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go
View File

@ -1,4 +1,4 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package upstreamwatchers
@ -44,45 +44,61 @@ const (
ReasonErrorFetchingSearchBase = "ErrorFetchingSearchBase"
)
// An in-memory cache with an entry for each ActiveDirectoryIdentityProvider, to keep track of which ResourceVersion
// of the bind Secret, which TLS/StartTLS setting was used and which search base was found during the most recent successful validation.
type SecretVersionCacheI interface {
Get(upstreamName, resourceVersion string, generation int64) (ValidatedSettings, bool)
Set(upstreamName, resourceVersion string, generation int64, settings ValidatedSettings)
// ValidatedSettings is the struct which is cached by the ValidatedSettingsCacheI interface.
type ValidatedSettings struct {
IDPSpecGeneration int64 // which IDP spec was used during the validation
BindSecretResourceVersion string // which bind secret was used during the validation
// Cache the setting for TLS vs StartTLS. This is always auto-discovered by probing the server.
LDAPConnectionProtocol upstreamldap.LDAPConnectionProtocol
// Cache the settings for search bases. These could be configured by the IDP spec, or in the
// case of AD they can also be auto-discovered by probing the server.
UserSearchBase, GroupSearchBase string
// Cache copies of the conditions that were computed when the above settings were cached, so we
// can keep writing them to the status in the future. This matters most when the first attempt
// to write them to the IDP's status fails. In this case, future Syncs calls will be able to
// use these cached values to try writing them again.
ConnectionValidCondition, SearchBaseFoundCondition *v1alpha1.Condition
}
type SecretVersionCache struct {
// ValidatedSettingsCacheI is an interface for an in-memory cache with an entry for each upstream
// provider. It keeps track of settings that were already validated for a given IDP spec and bind
// secret for that upstream.
type ValidatedSettingsCacheI interface {
// Get the cached settings for a given upstream at a given generation which was previously
// validated using a given bind secret version. If no settings have been cached for the
// upstream, or if the settings were cached at a different generation of the upstream or
// using a different version of the bind secret, then return false to indicate that the
// desired settings were not cached yet for that combination of spec generation and secret version.
Get(upstreamName, resourceVersion string, idpSpecGeneration int64) (ValidatedSettings, bool)
// Set some settings into the cache for a given upstream.
Set(upstreamName string, settings ValidatedSettings)
}
type ValidatedSettingsCache struct {
ValidatedSettingsByName map[string]ValidatedSettings
}
func (s *SecretVersionCache) Get(upstreamName, resourceVersion string, generation int64) (ValidatedSettings, bool) {
validatedSettings := s.ValidatedSettingsByName[upstreamName]
if validatedSettings.BindSecretResourceVersion == resourceVersion &&
validatedSettings.Generation == generation && validatedSettings.UserSearchBase != "" &&
validatedSettings.GroupSearchBase != "" && validatedSettings.LDAPConnectionProtocol != "" {
func NewValidatedSettingsCache() ValidatedSettingsCacheI {
return &ValidatedSettingsCache{ValidatedSettingsByName: map[string]ValidatedSettings{}}
}
func (s *ValidatedSettingsCache) Get(upstreamName, resourceVersion string, idpSpecGeneration int64) (ValidatedSettings, bool) {
validatedSettings, found := s.ValidatedSettingsByName[upstreamName]
if found && validatedSettings.BindSecretResourceVersion == resourceVersion && validatedSettings.IDPSpecGeneration == idpSpecGeneration {
return validatedSettings, true
}
return ValidatedSettings{}, false
}
func (s *SecretVersionCache) Set(upstreamName, resourceVersion string, generation int64, settings ValidatedSettings) {
func (s *ValidatedSettingsCache) Set(upstreamName string, settings ValidatedSettings) {
s.ValidatedSettingsByName[upstreamName] = settings
}
type ValidatedSettings struct {
Generation int64
BindSecretResourceVersion string
LDAPConnectionProtocol upstreamldap.LDAPConnectionProtocol
UserSearchBase string
GroupSearchBase string
}
func NewSecretVersionCache() SecretVersionCacheI {
cache := SecretVersionCache{ValidatedSettingsByName: map[string]ValidatedSettings{}}
return &cache
}
// read only interface for sharing between ldap and active directory.
// UpstreamGenericLDAPIDP is a read-only interface for abstracting the differences between LDAP and Active Directory IDP types.
type UpstreamGenericLDAPIDP interface {
Spec() UpstreamGenericLDAPSpec
Name() string
@ -247,8 +263,15 @@ func ValidateSecret(secretInformer corev1informers.SecretInformer, secretName st
}, secret.ResourceVersion
}
// gradatedCondition is a condition and a boolean that tells you whether the condition is fatal or just a warning.
type gradatedCondition struct {
condition *v1alpha1.Condition
isFatal bool
}
// GradatedConditions is a list of conditions, where each condition can additionally be considered fatal or non-fatal.
type GradatedConditions struct {
gradatedConditions []GradatedCondition
gradatedConditions []gradatedCondition
}
func (g *GradatedConditions) Conditions() []*v1alpha1.Condition {
@ -260,76 +283,69 @@ func (g *GradatedConditions) Conditions() []*v1alpha1.Condition {
}
func (g *GradatedConditions) Append(condition *v1alpha1.Condition, isFatal bool) {
g.gradatedConditions = append(g.gradatedConditions, GradatedCondition{condition: condition, isFatal: isFatal})
g.gradatedConditions = append(g.gradatedConditions, gradatedCondition{condition: condition, isFatal: isFatal})
}
// A condition and a boolean that tells you whether it's fatal or just a warning.
type GradatedCondition struct {
condition *v1alpha1.Condition
isFatal bool
}
func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache SecretVersionCacheI, config *upstreamldap.ProviderConfig) GradatedConditions {
func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache ValidatedSettingsCacheI, config *upstreamldap.ProviderConfig) GradatedConditions {
conditions := GradatedConditions{}
secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config)
conditions.Append(secretValidCondition, true)
tlsValidCondition := ValidateTLSConfig(upstream.Spec().TLSSpec(), config)
conditions.Append(tlsValidCondition, true)
var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition
// No point in trying to connect to the server if the config was already determined to be invalid.
var ldapConnectionValidCondition *v1alpha1.Condition
var searchBaseFoundCondition *v1alpha1.Condition
if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue {
ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase(ctx, validatedSecretVersionsCache, upstream, config, currentSecretVersion)
if ldapConnectionValidCondition != nil {
conditions.Append(ldapConnectionValidCondition, false)
}
if searchBaseFoundCondition != nil {
if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil
conditions.Append(searchBaseFoundCondition, true)
}
}
return conditions
}
func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, validatedSecretVersionsCache SecretVersionCacheI, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) (*v1alpha1.Condition, *v1alpha1.Condition) {
// previouslyValidatedSecretVersion := validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()].BindSecretResourceVersion
// doesn't have an existing entry for ValidatedSettingsByName with this secret version ->
// lets double check tls connection
// if we can connect, put it in the secret cache
// also we KNOW we need to recheck the search base stuff too... so they should all be one function?
// but if tls validation fails no need to also try to get search base stuff?
func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, validatedSecretVersionsCache ValidatedSettingsCacheI, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) (*v1alpha1.Condition, *v1alpha1.Condition) {
validatedSettings, hasPreviousValidatedSettings := validatedSecretVersionsCache.Get(upstream.Name(), currentSecretVersion, upstream.Generation())
var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition
if !hasPreviousValidatedSettings {
if hasPreviousValidatedSettings && validatedSettings.UserSearchBase != "" && validatedSettings.GroupSearchBase != "" {
// Found previously validated settings in the cache (which is also not missing search base fields), so use them.
config.ConnectionProtocol = validatedSettings.LDAPConnectionProtocol
config.UserSearch.Base = validatedSettings.UserSearchBase
config.GroupSearch.Base = validatedSettings.GroupSearchBase
ldapConnectionValidCondition = validatedSettings.ConnectionValidCondition.DeepCopy()
searchBaseFoundCondition = validatedSettings.SearchBaseFoundCondition.DeepCopy()
} else {
// Did not find previously validated settings in the cache, so probe the LDAP server.
testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, probeLDAPTimeout)
defer cancelFunc()
ldapConnectionValidCondition = TestConnection(testConnectionTimeout, upstream.Spec().BindSecretName(), config, currentSecretVersion)
searchBaseTimeout, cancelFunc := context.WithTimeout(ctx, probeLDAPTimeout)
defer cancelFunc()
searchBaseFoundCondition = upstream.Spec().DetectAndSetSearchBase(searchBaseTimeout, config)
if ldapConnectionValidCondition.Status == v1alpha1.ConditionTrue {
// if it's nil, don't worry about the search base condition. But if it exists make sure the status is true.
if searchBaseFoundCondition == nil || (searchBaseFoundCondition.Status == v1alpha1.ConditionTrue) {
// Remember (in-memory for this pod) that the controller has successfully validated the LDAP provider
// When there were no failures, write the newly validated settings to the cache.
// It's okay for the search base condition to be nil, but if it exists make sure it was not a failure.
if ldapConnectionValidCondition.Status == v1alpha1.ConditionTrue &&
(searchBaseFoundCondition == nil || (searchBaseFoundCondition.Status == v1alpha1.ConditionTrue)) {
// Remember (in-memory for this pod) that the controller has successfully validated the LDAP or AD provider
// using this version of the Secret. This is for performance reasons, to avoid attempting to connect to
// the LDAP server more than is needed. If the pod restarts, it will attempt this validation again.
validatedSettings.LDAPConnectionProtocol = config.ConnectionProtocol
validatedSettings.BindSecretResourceVersion = currentSecretVersion
validatedSettings.Generation = upstream.Generation()
validatedSettings.UserSearchBase = config.UserSearch.Base
validatedSettings.GroupSearchBase = config.GroupSearch.Base
validatedSecretVersionsCache.Set(upstream.Name(), currentSecretVersion, upstream.Generation(), validatedSettings)
validatedSecretVersionsCache.Set(upstream.Name(), ValidatedSettings{
IDPSpecGeneration: upstream.Generation(),
BindSecretResourceVersion: currentSecretVersion,
LDAPConnectionProtocol: config.ConnectionProtocol,
UserSearchBase: config.UserSearch.Base,
GroupSearchBase: config.GroupSearch.Base,
ConnectionValidCondition: ldapConnectionValidCondition.DeepCopy(),
SearchBaseFoundCondition: searchBaseFoundCondition.DeepCopy(), // currently, only used for AD, so may be nil
})
}
}
} else {
config.ConnectionProtocol = validatedSettings.LDAPConnectionProtocol
config.UserSearch.Base = validatedSettings.UserSearchBase
config.GroupSearch.Base = validatedSettings.GroupSearchBase
}
return ldapConnectionValidCondition, searchBaseFoundCondition
}