djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
b0ea7063c7
ContainerImage.Pinniped/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go

1181 lines
49 KiB
Go
Raw Normal View History

Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
// SPDX-License-Identifier: Apache-2.0
Split package upstreamwatchers into four packages
2021-05-12 21:00:39 +00:00
package ldapupstreamwatcher
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
import (
"context"
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
"encoding/base64"
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
"errors"
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
"fmt"
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
"sort"
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
"testing"
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
"time"
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
"github.com/go-ldap/ldap/v3"
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
"github.com/golang/mock/gomock"
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
"go.pinniped.dev/internal/certauthority"
Refactor active directory and ldap controllers to share almost everything Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-07-19 20:54:07 +00:00
"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers"
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
"go.pinniped.dev/internal/controllerlib"
Convert LDAP code to use endpointaddr package. Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 19:46:50 +00:00
"go.pinniped.dev/internal/endpointaddr"
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
"go.pinniped.dev/internal/mocks/mockldapconn"
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
"go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/internal/testutil"
"go.pinniped.dev/internal/upstreamldap"
)
func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) {
t.Parallel()
tests := []struct {
name string
secret metav1.Object
wantAdd bool
wantUpdate bool
wantDelete bool
}{
{
name: "a secret of the right type",
secret: &corev1.Secret{
Type: corev1.SecretTypeBasicAuth,
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
wantAdd: true,
wantUpdate: true,
wantDelete: true,
},
{
name: "a secret of the wrong type",
secret: &corev1.Secret{
Type: "this-is-the-wrong-type",
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
},
{
name: "resource of a data type which is not watched by this controller",
secret: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()
fakePinnipedClient := pinnipedfake.NewSimpleClientset()
pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0)
ldapIDPInformer := pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders()
fakeKubeClient := fake.NewSimpleClientset()
kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0)
secretInformer := kubeInformers.Core().V1().Secrets()
withInformer := testutil.NewObservableWithInformerOption()
Split package upstreamwatchers into four packages
2021-05-12 21:00:39 +00:00
New(nil, nil, ldapIDPInformer, secretInformer, withInformer.WithInformer)
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
unrelated := corev1.Secret{}
filter := withInformer.GetFilterForInformer(secretInformer)
require.Equal(t, test.wantAdd, filter.Add(test.secret))
require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.secret))
require.Equal(t, test.wantUpdate, filter.Update(test.secret, &unrelated))
require.Equal(t, test.wantDelete, filter.Delete(test.secret))
})
}
}
func TestLDAPUpstreamWatcherControllerFilterLDAPIdentityProviders(t *testing.T) {
t.Parallel()
tests := []struct {
name string
idp metav1.Object
wantAdd bool
wantUpdate bool
wantDelete bool
}{
{
name: "any LDAPIdentityProvider",
idp: &v1alpha1.LDAPIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
wantAdd: true,
wantUpdate: true,
wantDelete: true,
},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()
fakePinnipedClient := pinnipedfake.NewSimpleClientset()
pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0)
ldapIDPInformer := pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders()
fakeKubeClient := fake.NewSimpleClientset()
kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0)
secretInformer := kubeInformers.Core().V1().Secrets()
withInformer := testutil.NewObservableWithInformerOption()
Split package upstreamwatchers into four packages
2021-05-12 21:00:39 +00:00
New(nil, nil, ldapIDPInformer, secretInformer, withInformer.WithInformer)
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
unrelated := corev1.Secret{}
filter := withInformer.GetFilterForInformer(ldapIDPInformer)
require.Equal(t, test.wantAdd, filter.Add(test.idp))
require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.idp))
require.Equal(t, test.wantUpdate, filter.Update(test.idp, &unrelated))
require.Equal(t, test.wantDelete, filter.Delete(test.idp))
})
}
}
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
// Wrap the func into a struct so the test can do deep equal assertions on instances of upstreamldap.Provider.
type comparableDialer struct {
Insignificant change in ldap_upstream_watcher_test.go
2021-05-12 20:37:01 +00:00
upstreamldap.LDAPDialerFunc
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
}
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
t.Parallel()
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
now := metav1.NewTime(time.Now().UTC())
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
const (
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
testNamespace = "test-namespace"
testName = "test-name"
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
testResourceUID = "test-resource-uid"
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
testSecretName = "test-bind-secret"
testBindUsername = "test-bind-username"
testBindPassword = "test-bind-password"
testHost = "ldap.example.com:123"
testUserSearchBase = "test-user-search-base"
testUserSearchFilter = "test-user-search-filter"
testGroupSearchBase = "test-group-search-base"
testGroupSearchFilter = "test-group-search-filter"
testUsernameAttrName = "test-username-attr"
testGroupNameAttrName = "test-group-name-attr"
testUIDAttrName = "test-uid-attr"
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
)
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
testValidSecretData := map[string][]byte{"username": []byte(testBindUsername), "password": []byte(testBindPassword)}
testCA, err := certauthority.New("test CA", time.Minute)
require.NoError(t, err)
testCABundle := testCA.Bundle()
testCABundleBase64Encoded := base64.StdEncoding.EncodeToString(testCABundle)
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
validUpstream := &v1alpha1.LDAPIdentityProvider{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{
Name: testName,
Namespace: testNamespace,
Generation: 1234,
UID: testResourceUID,
},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Spec: v1alpha1.LDAPIdentityProviderSpec{
Host: testHost,
Some updates based on PR review
2021-04-27 19:43:09 +00:00
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testCABundleBase64Encoded},
Bind: v1alpha1.LDAPIdentityProviderBind{SecretName: testSecretName},
UserSearch: v1alpha1.LDAPIdentityProviderUserSearch{
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Base: testUserSearchBase,
Filter: testUserSearchFilter,
Some updates based on PR review
2021-04-27 19:43:09 +00:00
Attributes: v1alpha1.LDAPIdentityProviderUserSearchAttributes{
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Username: testUsernameAttrName,
Some updates based on PR review
2021-04-27 19:43:09 +00:00
UID: testUIDAttrName,
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
},
},
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
GroupSearch: v1alpha1.LDAPIdentityProviderGroupSearch{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
Attributes: v1alpha1.LDAPIdentityProviderGroupSearchAttributes{
GroupName: testGroupNameAttrName,
},
},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
},
}
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
editedValidUpstream := func(editFunc func(*v1alpha1.LDAPIdentityProvider)) *v1alpha1.LDAPIdentityProvider {
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
deepCopy := validUpstream.DeepCopy()
editFunc(deepCopy)
return deepCopy
}
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
providerConfigForValidUpstreamWithTLS := &upstreamldap.ProviderConfig{
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Name: testName,
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ResourceUID: testResourceUID,
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Host: testHost,
ConnectionProtocol: upstreamldap.TLS,
CABundle: testCABundle,
BindUsername: testBindUsername,
BindPassword: testBindPassword,
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
UserSearch: upstreamldap.UserSearchConfig{
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
GroupSearch: upstreamldap.GroupSearchConfig{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupNameAttrName,
},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
}
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
// Make a copy with targeted changes.
copyOfProviderConfigForValidUpstreamWithTLS := *providerConfigForValidUpstreamWithTLS
providerConfigForValidUpstreamWithStartTLS := &copyOfProviderConfigForValidUpstreamWithTLS
providerConfigForValidUpstreamWithStartTLS.ConnectionProtocol = upstreamldap.StartTLS
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
bindSecretValidTrueCondition := func(gen int64) v1alpha1.Condition {
return v1alpha1.Condition{
Type: "BindSecretValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded bind secret",
ObservedGeneration: gen,
}
}
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
ldapConnectionValidTrueCondition := func(gen int64, secretVersion string) v1alpha1.Condition {
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
return v1alpha1.Condition{
Type: "LDAPConnectionValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Message: fmt.Sprintf(
`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
testHost, testBindUsername, testSecretName, secretVersion),
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
ObservedGeneration: gen,
}
}
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
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
}
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition := func(gen int64) v1alpha1.Condition {
return v1alpha1.Condition{
Type: "TLSConfigurationValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded TLS configuration",
ObservedGeneration: gen,
}
}
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
allConditionsTrue := func(gen int64, secretVersion string) []v1alpha1.Condition {
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
return []v1alpha1.Condition{
bindSecretValidTrueCondition(gen),
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
ldapConnectionValidTrueCondition(gen, secretVersion),
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(gen),
}
}
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
validBindUserSecret := func(secretVersion string) *corev1.Secret {
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace, ResourceVersion: secretVersion},
Type: corev1.SecretTypeBasicAuth,
Data: testValidSecretData,
}
}
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
tests := []struct {
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
name string
Refactor active directory and ldap controllers to share almost everything Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-07-19 20:54:07 +00:00
initialValidatedSettings map[string]upstreamwatchers.ValidatedSettings
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
inputUpstreams []runtime.Object
inputSecrets []runtime.Object
setupMocks func(conn *mockldapconn.MockConn)
dialErrors map[string]error
wantErr string
wantResultingCache []*upstreamldap.ProviderConfig
wantResultingUpstreams []v1alpha1.LDAPIdentityProvider
Refactor active directory and ldap controllers to share almost everything Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-07-19 20:54:07 +00:00
wantValidatedSettings map[string]upstreamwatchers.ValidatedSettings
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
}{
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "no LDAPIdentityProvider upstreams clears the cache",
wantResultingCache: []*upstreamldap.ProviderConfig{},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
{
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
name: "one valid upstream updates the cache to include only that upstream",
inputUpstreams: []runtime.Object{validUpstream},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
}},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
},
{
name: "missing secret",
inputUpstreams: []runtime.Object{validUpstream},
inputSecrets: []runtime.Object{},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretNotFound",
Message: fmt.Sprintf(`secret "%s" not found`, testSecretName),
ObservedGeneration: 1234,
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(1234),
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
},
}},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
},
{
name: "secret has wrong type",
inputUpstreams: []runtime.Object{validUpstream},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Type: "some-other-type",
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
Data: testValidSecretData,
}},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretWrongType",
Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testSecretName),
ObservedGeneration: 1234,
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(1234),
Add a little more logic to ldap_upstream_watcher.go
2021-04-12 18:23:08 +00:00
},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
}},
},
{
name: "secret is missing key",
inputUpstreams: []runtime.Object{validUpstream},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace},
Type: corev1.SecretTypeBasicAuth,
}},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretMissingKeys",
Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testSecretName),
ObservedGeneration: 1234,
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(1234),
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
},
},
}},
},
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "CertificateAuthorityData is not base64 encoded",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
upstream.Spec.TLS.CertificateAuthorityData = "this-is-not-base64-encoded"
})},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("")},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
bindSecretValidTrueCondition(1234),
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
{
Type: "TLSConfigurationValid",
Status: "False",
LastTransitionTime: now,
Reason: "InvalidTLSConfig",
Message: "certificateAuthorityData is invalid: illegal base64 data at input byte 4",
ObservedGeneration: 1234,
},
},
},
}},
},
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "CertificateAuthorityData is not valid pem data",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
upstream.Spec.TLS.CertificateAuthorityData = base64.StdEncoding.EncodeToString([]byte("this is not pem data"))
})},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("")},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
bindSecretValidTrueCondition(1234),
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
{
Type: "TLSConfigurationValid",
Status: "False",
LastTransitionTime: now,
Reason: "InvalidTLSConfig",
Message: "certificateAuthorityData is invalid: no certificates found",
ObservedGeneration: 1234,
},
},
},
}},
},
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "nil TLS configuration is valid",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
upstream.Spec.TLS = nil
})},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
{
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Name: testName,
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ResourceUID: testResourceUID,
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Host: testHost,
ConnectionProtocol: upstreamldap.TLS,
CABundle: nil,
BindUsername: testBindUsername,
BindPassword: testBindPassword,
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
UserSearch: upstreamldap.UserSearchConfig{
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
GroupSearch: upstreamldap.GroupSearchConfig{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupNameAttrName,
},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
},
},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
bindSecretValidTrueCondition(1234),
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
ldapConnectionValidTrueCondition(1234, "4242"),
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
{
Type: "TLSConfigurationValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "no TLS configuration provided",
ObservedGeneration: 1234,
},
},
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
{
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) {
upstream.Spec.Host = "ldap.example.com" // when the port is not specified, automatically switch ports for StartTLS
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
dialErrors: map[string]error{
"ldap.example.com:" + ldap.DefaultLdapsPort: fmt.Errorf("some ldaps dial error"),
"ldap.example.com:" + ldap.DefaultLdapPort: nil, // no error on the regular ldap:// port
},
wantResultingCache: []*upstreamldap.ProviderConfig{
{
Name: testName,
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ResourceUID: testResourceUID,
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
Host: "ldap.example.com",
ConnectionProtocol: upstreamldap.StartTLS, // successfully fell back to using StartTLS
CABundle: testCABundle,
BindUsername: testBindUsername,
BindPassword: testBindPassword,
UserSearch: upstreamldap.UserSearchConfig{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
GroupSearch: upstreamldap.GroupSearchConfig{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupNameAttrName,
},
},
},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
bindSecretValidTrueCondition(1234),
{
Type: "LDAPConnectionValid",
Status: "True",
LastTransitionTime: now,
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"),
ObservedGeneration: 1234,
},
tlsConfigurationValidLoadedTrueCondition(1234),
},
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
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"),
},
}},
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
{
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) {
upstream.Spec.Host = "ldap.example.com:5678" // when the port is specified, do not automatically switch ports for StartTLS
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
setupMocks: func(conn *mockldapconn.MockConn) {
// Both dials fail, so there should be no bind.
},
dialErrors: map[string]error{
"ldap.example.com:5678": fmt.Errorf("some dial error"), // both TLS and StartTLS should try the same port and both fail
},
wantResultingCache: []*upstreamldap.ProviderConfig{
// even though the connection test failed, still loads into the cache because it is treated like a warning
{
Name: testName,
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ResourceUID: testResourceUID,
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
Host: "ldap.example.com:5678",
ConnectionProtocol: upstreamldap.TLS, // need to pick TLS or StartTLS to load into the cache when both fail, so choose TLS
CABundle: testCABundle,
BindUsername: testBindUsername,
BindPassword: testBindPassword,
UserSearch: upstreamldap.UserSearchConfig{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
GroupSearch: upstreamldap.GroupSearchConfig{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupNameAttrName,
},
},
},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
bindSecretValidTrueCondition(1234),
{
Type: "LDAPConnectionValid",
Status: "False",
LastTransitionTime: now,
Reason: "LDAPConnectionError",
Message: fmt.Sprintf(
`could not successfully connect to "%s" and bind as user "%s": error dialing host "%s": some dial error`,
"ldap.example.com:5678", testBindUsername, "ldap.example.com:5678"),
ObservedGeneration: 1234,
},
tlsConfigurationValidLoadedTrueCondition(1234),
},
},
}},
Test fix for ldap upstream watcher
2021-09-03 00:34:18 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
},
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "non-nil TLS configuration with empty CertificateAuthorityData is valid",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
upstream.Spec.TLS.CertificateAuthorityData = ""
})},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
{
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Name: testName,
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ResourceUID: testResourceUID,
upstreamldap.New() now supports a StartTLS config option - This enhances our LDAP client code to make it possible to optionally dial an LDAP server without TLS and then use StartTLS to upgrade the connection to TLS. - The controller for LDAPIdentityProviders is not using this option yet. That will come in a future commit.
2021-05-20 00:17:44 +00:00
Host: testHost,
ConnectionProtocol: upstreamldap.TLS,
CABundle: nil,
BindUsername: testBindUsername,
BindPassword: testBindPassword,
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
UserSearch: upstreamldap.UserSearchConfig{
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
Initial support for upstream LDAP group membership Reflect the upstream group membership into the Supervisor's downstream tokens, so they can be added to the user's identity on the workload clusters. LDAP group search is configurable on the LDAPIdentityProvider resource.
2021-05-17 18:10:26 +00:00
},
GroupSearch: upstreamldap.GroupSearchConfig{
Base: testGroupSearchBase,
Filter: testGroupSearchFilter,
GroupNameAttribute: testGroupNameAttrName,
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
},
},
},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ldap_upstream_watcher.go: decode and validate CertificateAuthorityData
2021-04-14 00:16:57 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
}},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
},
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
{
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{validUpstream, editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
upstream.Name = "other-upstream"
upstream.Generation = 42
upstream.Spec.Bind.SecretName = "non-existent-secret"
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
upstream.UID = "other-uid"
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
})},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind for the one valid upstream configuration.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{
{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42, UID: "other-uid"},
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretNotFound",
Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"),
ObservedGeneration: 42,
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(42),
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
},
},
},
{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
},
},
},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
{
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
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",
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
inputUpstreams: []runtime.Object{validUpstream},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("")},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
// Expect two calls to each of these: once for trying TLS and once for trying StartTLS.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(2).Return(errors.New("some bind error"))
conn.EXPECT().Close().Times(2)
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
bindSecretValidTrueCondition(1234),
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
{
Type: "LDAPConnectionValid",
Status: "False",
LastTransitionTime: now,
Reason: "LDAPConnectionError",
Message: fmt.Sprintf(
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
`could not successfully connect to "%s" and bind as user "%s": error binding as "%s": some bind error`,
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
testHost, testBindUsername, testBindUsername),
ObservedGeneration: 1234,
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
tlsConfigurationValidLoadedTrueCondition(1234),
},
},
}},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
},
{
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
name: "when the LDAP server connection was already validated using TLS for the current resource generation and secret version, then do not validate it again and keep using TLS",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
upstream.Generation = 1234
upstream.Status.Conditions = []v1alpha1.Condition{
ldapConnectionValidTrueCondition(1234, "4242"),
}
})},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{
testName: {BindSecretResourceVersion: "4242",
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
}},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
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.
},
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: allConditionsTrue(1234, "4242"),
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
{
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",
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
upstream.Generation = 1234
upstream.Status.Conditions = []v1alpha1.Condition{
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
ldapConnectionValidTrueCondition(1234, "4242"),
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
}
})},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
}},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
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.
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithStartTLS},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.StartTLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
{
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) {
upstream.Generation = 1234 // current generation
upstream.Status.Conditions = []v1alpha1.Condition{
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
ldapConnectionValidTrueCondition(1233, "4242"), // older spec generation!
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
}
})},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1233,
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
}},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
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")),
}},
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
{
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) {
upstream.Generation = 1234
upstream.Status.Conditions = []v1alpha1.Condition{
{
Type: "LDAPConnectionValid",
Status: "False", // failure!
LastTransitionTime: now,
Reason: "LDAPConnectionError",
Message: "some-error-message",
ObservedGeneration: 1234, // same (current) generation!
},
}
})},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: allConditionsTrue(1234, "4242"),
},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
}},
},
{
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
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) {
upstream.Generation = 1234
upstream.Status.Conditions = []v1alpha1.Condition{
{
Type: "LDAPConnectionValid",
Status: "False", // failure!
LastTransitionTime: now,
Reason: "LDAPConnectionError",
Message: "some-error-message",
ObservedGeneration: 1234, // same (current) generation!
},
}
})},
inputSecrets: []runtime.Object{validBindUserSecret("4242")},
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
}},
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Conditions: allConditionsTrue(1234, "4242"),
},
}},
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
}},
},
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
{
name: "when the LDAP server connection was already validated for this resource generation but the bind secret has changed, then try to validate it again",
inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) {
upstream.Generation = 1234
upstream.Status.Conditions = []v1alpha1.Condition{
ldapConnectionValidTrueCondition(1234, "4241"), // same spec generation, old secret version
}
})},
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
inputSecrets: []runtime.Object{validBindUserSecret("4242")}, // newer secret version!
initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4241",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
}}, // old version was validated
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
setupMocks: func(conn *mockldapconn.MockConn) {
// Should perform a test dial and bind.
conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
conn.EXPECT().Close().Times(1)
},
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
Require refresh tokens for upstream OIDC and save more session data - Requiring refresh tokens to be returned from upstream OIDC idps - Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication - Don't pass access=offline all the time
2021-10-08 22:48:21 +00:00
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testResourceUID},
Only test the server connection when the spec has changed This early version of the controller is not intended to act as an ongoing health check for your upstream LDAP server. It will connect to the LDAP server to essentially "lint" your configuration once. It will do it again only when you change your configuration. To account for transient errors, it will keep trying to connect to the server until it succeeds once. This commit does not include looking for changes in the associated bind user username/password Secret.
2021-04-15 23:46:27 +00:00
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready",
Retest the server connection when the bind Secret has changed Unfortunately, Secrets do not seem to have a Generation field, so we use the ResourceVersion field instead. This means that any change to the Secret will cause us to retry the connection to the LDAP server, even if the username and password fields in the Secret were not changed. Seems like an okay trade-off for this early draft of the controller compared to a more complex implementation.
2021-04-16 00:45:15 +00:00
Conditions: allConditionsTrue(1234, "4242"),
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
},
}},
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {
BindSecretResourceVersion: "4242",
LDAPConnectionProtocol: upstreamldap.TLS,
UserSearchBase: testUserSearchBase,
GroupSearchBase: testGroupSearchBase,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
IDPSpecGeneration: 1234,
ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")),
Fetch AD search base from defaultNamingContext when not specified
2021-07-21 20:24:54 +00:00
}}},
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
}
Add authentication dry run validation to LDAPIdentityProvider Also force the LDAP server pod to restart whenever the LDIF file changes, so whenever you redeploy the tools deployment with a new test user password the server will be updated.
2021-04-16 21:04:05 +00:00
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.inputUpstreams...)
pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0)
fakeKubeClient := fake.NewSimpleClientset(tt.inputSecrets...)
kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0)
cache := provider.NewDynamicUpstreamIDPProvider()
cache.SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{
Introduce upstreamldap.New to prevent changes to the underlying config Makes it easier to support using the same upstreamldap.Provider from multiple goroutines safely.
2021-04-15 17:25:35 +00:00
upstreamldap.New(upstreamldap.ProviderConfig{Name: "initial-entry"}),
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
})
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
conn := mockldapconn.NewMockConn(ctrl)
if tt.setupMocks != nil {
tt.setupMocks(conn)
}
Convert LDAP code to use endpointaddr package. Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 19:46:50 +00:00
dialer := &comparableDialer{upstreamldap.LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) {
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
if tt.dialErrors != nil {
Convert LDAP code to use endpointaddr package. Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 19:46:50 +00:00
dialErr := tt.dialErrors[addr.Endpoint()]
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
if dialErr != nil {
return nil, dialErr
}
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
}
return conn, nil
})}
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
var validatedSettingsCache *upstreamwatchers.ValidatedSettingsCache
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
if tt.initialValidatedSettings != nil {
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
ValidatedSettingsByName: tt.initialValidatedSettings,
}
} else {
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
validatedSettingsCache = &upstreamwatchers.ValidatedSettingsCache{
ValidatedSettings is all or nothing If either the search base or the tls settings is invalid, just recheck everything.
2021-09-07 18:45:32 +00:00
ValidatedSettingsByName: map[string]upstreamwatchers.ValidatedSettings{},
}
Cache ResourceVersion of the validated bind Secret in memory ...instead of caching it in the text of the Condition message
2021-05-13 22:22:36 +00:00
}
Some updates based on PR review
2021-04-27 19:43:09 +00:00
controller := newInternal(
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
cache,
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
validatedSettingsCache,
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
dialer,
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
fakePinnipedClient,
pinnipedInformers.IDP().V1alpha1().LDAPIdentityProviders(),
kubeInformers.Core().V1().Secrets(),
controllerlib.WithInformer,
)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
pinnipedInformers.Start(ctx.Done())
kubeInformers.Start(ctx.Done())
controllerlib.TestRunSynchronously(t, controller)
syncCtx := controllerlib.Context{Context: ctx, Key: controllerlib.Key{}}
if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
} else {
require.NoError(t, err)
}
actualIDPList := cache.GetLDAPIdentityProviders()
require.Equal(t, len(tt.wantResultingCache), len(actualIDPList))
for i := range actualIDPList {
actualIDP := actualIDPList[i].(*upstreamldap.Provider)
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
copyOfExpectedValueForResultingCache := *tt.wantResultingCache[i] // copy before edit to avoid race because these tests are run in parallel
Test the LDAP config by connecting to the server in the controller
2021-04-15 21:44:43 +00:00
// The dialer that was passed in to the controller's constructor should always have been
// passed through to the provider.
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
copyOfExpectedValueForResultingCache.Dialer = dialer
require.Equal(t, copyOfExpectedValueForResultingCache, actualIDP.GetConfig())
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
}
actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().LDAPIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})
require.NoError(t, err)
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
// Assert on the expected Status of the upstreams. Preprocess the upstreams a bit so that they're easier to assert against.
normalizedActualUpstreams := normalizeLDAPUpstreams(actualUpstreams.Items, now)
require.Equal(t, len(tt.wantResultingUpstreams), len(normalizedActualUpstreams))
for i := range tt.wantResultingUpstreams {
// Require each separately to get a nice diff when the test fails.
require.Equal(t, tt.wantResultingUpstreams[i], normalizedActualUpstreams[i])
}
Cache ResourceVersion of the validated bind Secret in memory ...instead of caching it in the text of the Condition message
2021-05-13 22:22:36 +00:00
// Check that the controller remembered which version of the secret it most recently validated successfully with.
LDAP upstream watcher controller tries using both TLS and StartTLS - Automatically try to fall back to using StartTLS when using TLS doesn't work. Only complain when both don't work. - Remember (in-memory) which one worked and keeping using that one in the future (unless the pod restarts).
2021-05-20 19:46:33 +00:00
if tt.wantValidatedSettings == nil {
Refactor active directory and ldap controllers to share almost everything Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-07-19 20:54:07 +00:00
tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{}
Cache ResourceVersion of the validated bind Secret in memory ...instead of caching it in the text of the Condition message
2021-05-13 22:22:36 +00:00
}
Fix bug where LDAP or AD status conditions were not updated correctly 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.
2022-01-08 01:19:13 +00:00
require.Equal(t, tt.wantValidatedSettings, validatedSettingsCache.ValidatedSettingsByName)
More LDAP WIP: started controller and LDAP server connection code Both are unfinished works in progress.
2021-04-10 01:49:43 +00:00
})
}
}
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
func normalizeLDAPUpstreams(upstreams []v1alpha1.LDAPIdentityProvider, now metav1.Time) []v1alpha1.LDAPIdentityProvider {
result := make([]v1alpha1.LDAPIdentityProvider, 0, len(upstreams))
for _, u := range upstreams {
normalized := u.DeepCopy()
// We're only interested in comparing the status, so zero out the spec.
normalized.Spec = v1alpha1.LDAPIdentityProviderSpec{}
// Round down the LastTransitionTime values to `now` if they were just updated. This makes
// it much easier to encode assertions about the expected timestamps.
for i := range normalized.Status.Conditions {
if time.Since(normalized.Status.Conditions[i].LastTransitionTime.Time) < 5*time.Second {
normalized.Status.Conditions[i].LastTransitionTime = now
}
}
result = append(result, *normalized)
}
ldap_upstream_watcher_test.go: add another unit test
2021-04-12 21:12:51 +00:00
sort.SliceStable(result, func(i, j int) bool {
return result[i].Name < result[j].Name
})
Add Conditions to LDAPIdentityProvider's Status and start to fill them - The ldap_upstream_watcher.go controller validates the bind secret and uses the Conditions to report errors. Shares some condition reporting logic with its sibling controller oidc_upstream_watcher.go, to the extent which is convenient without generics in golang.
2021-04-12 20:53:21 +00:00
return result
}
Reference in New Issue Copy Permalink