New controller watches OIDCClients and updates validation Conditions

This commit is contained in:
Ryan Richard
2022-06-17 12:56:53 -04:00
committed by Monis Khan
parent f8183e0fab
commit 5aa0d91267
59 changed files with 3980 additions and 116 deletions

View File

@@ -0,0 +1,123 @@
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package conditionsutil
import (
"sort"
"k8s.io/apimachinery/pkg/api/equality"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
"go.pinniped.dev/internal/plog"
)
// MergeIDPConditions merges conditions into conditionsToUpdate. If returns true if it merged any error conditions.
func MergeIDPConditions(conditions []*idpv1alpha1.Condition, observedGeneration int64, conditionsToUpdate *[]idpv1alpha1.Condition, log plog.MinLogger) bool {
hadErrorCondition := false
for i := range conditions {
cond := conditions[i].DeepCopy()
cond.LastTransitionTime = v1.Now()
cond.ObservedGeneration = observedGeneration
if mergeIDPCondition(conditionsToUpdate, cond) {
log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message)
}
if cond.Status == idpv1alpha1.ConditionFalse {
hadErrorCondition = true
}
}
sort.SliceStable(*conditionsToUpdate, func(i, j int) bool {
return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type
})
return hadErrorCondition
}
// mergeIDPCondition merges a new idpv1alpha1.Condition into a slice of existing conditions. It returns true
// if the condition has meaningfully changed.
func mergeIDPCondition(existing *[]idpv1alpha1.Condition, new *idpv1alpha1.Condition) bool {
// Find any existing condition with a matching type.
var old *idpv1alpha1.Condition
for i := range *existing {
if (*existing)[i].Type == new.Type {
old = &(*existing)[i]
continue
}
}
// If there is no existing condition of this type, append this one and we're done.
if old == nil {
*existing = append(*existing, *new)
return true
}
// Set the LastTransitionTime depending on whether the status has changed.
new = new.DeepCopy()
if old.Status == new.Status {
new.LastTransitionTime = old.LastTransitionTime
}
// If anything has actually changed, update the entry and return true.
if !equality.Semantic.DeepEqual(old, new) {
*old = *new
return true
}
// Otherwise the entry is already up to date.
return false
}
// MergeConfigConditions merges conditions into conditionsToUpdate. If returns true if it merged any error conditions.
func MergeConfigConditions(conditions []*configv1alpha1.Condition, observedGeneration int64, conditionsToUpdate *[]configv1alpha1.Condition, log plog.MinLogger) bool {
hadErrorCondition := false
for i := range conditions {
cond := conditions[i].DeepCopy()
cond.LastTransitionTime = v1.Now()
cond.ObservedGeneration = observedGeneration
if mergeConfigCondition(conditionsToUpdate, cond) {
log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message)
}
if cond.Status == configv1alpha1.ConditionFalse {
hadErrorCondition = true
}
}
sort.SliceStable(*conditionsToUpdate, func(i, j int) bool {
return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type
})
return hadErrorCondition
}
// mergeConfigCondition merges a new idpv1alpha1.Condition into a slice of existing conditions. It returns true
// if the condition has meaningfully changed.
func mergeConfigCondition(existing *[]configv1alpha1.Condition, new *configv1alpha1.Condition) bool {
// Find any existing condition with a matching type.
var old *configv1alpha1.Condition
for i := range *existing {
if (*existing)[i].Type == new.Type {
old = &(*existing)[i]
continue
}
}
// If there is no existing condition of this type, append this one and we're done.
if old == nil {
*existing = append(*existing, *new)
return true
}
// Set the LastTransitionTime depending on whether the status has changed.
new = new.DeepCopy()
if old.Status == new.Status {
new.LastTransitionTime = old.LastTransitionTime
}
// If anything has actually changed, update the entry and return true.
if !equality.Semantic.DeepEqual(old, new) {
*old = *new
return true
}
// Otherwise the entry is already up to date.
return false
}

View File

@@ -1,68 +0,0 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package conditionsutil
import (
"sort"
"k8s.io/apimachinery/pkg/api/equality"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
"go.pinniped.dev/internal/plog"
)
// Merge merges conditions into conditionsToUpdate. If returns true if it merged any error conditions.
func Merge(conditions []*v1alpha1.Condition, observedGeneration int64, conditionsToUpdate *[]v1alpha1.Condition, log plog.MinLogger) bool {
hadErrorCondition := false
for i := range conditions {
cond := conditions[i].DeepCopy()
cond.LastTransitionTime = v1.Now()
cond.ObservedGeneration = observedGeneration
if mergeCondition(conditionsToUpdate, cond) {
log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message)
}
if cond.Status == v1alpha1.ConditionFalse {
hadErrorCondition = true
}
}
sort.SliceStable(*conditionsToUpdate, func(i, j int) bool {
return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type
})
return hadErrorCondition
}
// mergeCondition merges a new v1alpha1.Condition into a slice of existing conditions. It returns true
// if the condition has meaningfully changed.
func mergeCondition(existing *[]v1alpha1.Condition, new *v1alpha1.Condition) bool {
// Find any existing condition with a matching type.
var old *v1alpha1.Condition
for i := range *existing {
if (*existing)[i].Type == new.Type {
old = &(*existing)[i]
continue
}
}
// If there is no existing condition of this type, append this one and we're done.
if old == nil {
*existing = append(*existing, *new)
return true
}
// Set the LastTransitionTime depending on whether the status has changed.
new = new.DeepCopy()
if old.Status == new.Status {
new.LastTransitionTime = old.LastTransitionTime
}
// If anything has actually changed, update the entry and return true.
if !equality.Semantic.DeepEqual(old, new) {
*old = *new
return true
}
// Otherwise the entry is already up to date.
return false
}

View File

@@ -362,7 +362,7 @@ func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, ups
log := plog.WithValues("namespace", upstream.Namespace, "name", upstream.Name)
updated := upstream.DeepCopy()
hadErrorCondition := conditionsutil.Merge(conditions, upstream.Generation, &updated.Status.Conditions, log)
hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log)
updated.Status.Phase = v1alpha1.ActiveDirectoryPhaseReady
if hadErrorCondition {

View File

@@ -255,7 +255,7 @@ func (c *ldapWatcherController) updateStatus(ctx context.Context, upstream *v1al
log := plog.WithValues("namespace", upstream.Namespace, "name", upstream.Name)
updated := upstream.DeepCopy()
hadErrorCondition := conditionsutil.Merge(conditions, upstream.Generation, &updated.Status.Conditions, log)
hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log)
updated.Status.Phase = v1alpha1.LDAPPhaseReady
if hadErrorCondition {

View File

@@ -0,0 +1,317 @@
// Copyright 2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package oidcclientwatcher
import (
"context"
"fmt"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
corev1informers "k8s.io/client-go/informers/core/v1"
"go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
configInformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/config/v1alpha1"
pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controller/conditionsutil"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/oidcclientsecretstorage"
"go.pinniped.dev/internal/plog"
)
const (
clientSecretExists = "ClientSecretExists"
allowedGrantTypesValid = "AllowedGrantTypesValid"
allowedScopesValid = "AllowedScopesValid"
reasonSuccess = "Success"
reasonMissingRequiredValue = "MissingRequiredValue"
reasonNoClientSecretFound = "NoClientSecretFound"
authorizationCodeGrantTypeName = "authorization_code"
refreshTokenGrantTypeName = "refresh_token"
tokenExchangeGrantTypeName = "urn:ietf:params:oauth:grant-type:token-exchange" //nolint:gosec // this is not a credential
openidScopeName = "openid"
offlineAccessScopeName = "offline_access"
requestAudienceScopeName = "pinniped:request-audience"
usernameScopeName = "username"
groupsScopeName = "groups"
allowedGrantTypesFieldName = "allowedGrantTypes"
allowedScopesFieldName = "allowedScopes"
secretTypeToObserve = "storage.pinniped.dev/oidc-client-secret" //nolint:gosec // this is not a credential
)
type oidcClientWatcherController struct {
pinnipedClient pinnipedclientset.Interface
oidcClientInformer configInformers.OIDCClientInformer
secretInformer corev1informers.SecretInformer
}
// NewOIDCClientWatcherController returns a controllerlib.Controller that watches OIDCClients and updates
// their status with validation errors.
func NewOIDCClientWatcherController(
pinnipedClient pinnipedclientset.Interface,
secretInformer corev1informers.SecretInformer,
oidcClientInformer configInformers.OIDCClientInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc,
) controllerlib.Controller {
return controllerlib.New(
controllerlib.Config{
Name: "OIDCClientWatcherController",
Syncer: &oidcClientWatcherController{
pinnipedClient: pinnipedClient,
secretInformer: secretInformer,
oidcClientInformer: oidcClientInformer,
},
},
// We want to be notified when an OIDCClient's corresponding secret gets updated or deleted.
withInformer(
secretInformer,
pinnipedcontroller.MatchAnySecretOfTypeFilter(secretTypeToObserve, pinnipedcontroller.SingletonQueue()),
controllerlib.InformerOption{},
),
// We want to be notified when anything happens to an OIDCClient.
withInformer(
oidcClientInformer,
pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()),
controllerlib.InformerOption{},
),
)
}
// Sync implements controllerlib.Syncer.
func (c *oidcClientWatcherController) Sync(ctx controllerlib.Context) error {
// Sync could be called on either a Secret or an OIDCClient, so to keep it simple, revalidate
// all OIDCClients whenever anything changes.
oidcClients, err := c.oidcClientInformer.Lister().List(labels.Everything())
if err != nil {
return fmt.Errorf("failed to list OIDCClients: %w", err)
}
// We're only going to use storage to call GetName(), which happens to not need the constructor params.
// This is because we can read the Secrets from the informer cache here, instead of doing live reads.
storage := oidcclientsecretstorage.New(nil, nil)
for _, oidcClient := range oidcClients {
correspondingSecretName := storage.GetName(oidcClient.UID)
secret, err := c.secretInformer.Lister().Secrets(oidcClient.Namespace).Get(correspondingSecretName)
if err != nil {
if !k8serrors.IsNotFound(err) {
// Anything other than a NotFound error is unexpected when reading from an informer.
return fmt.Errorf("failed to get %s/%s secret: %w", oidcClient.Namespace, correspondingSecretName, err)
}
// Got a NotFound error, so continue. The Secret just doesn't exist yet, which is okay.
plog.DebugErr(
"OIDCClientWatcherController error getting storage Secret for OIDCClient's client secrets", err,
"oidcClientName", oidcClient.Name,
"oidcClientNamespace", oidcClient.Namespace,
"secretName", correspondingSecretName,
)
secret = nil
}
conditions := validateOIDCClient(oidcClient, secret)
if err := c.updateStatus(ctx.Context, oidcClient, conditions); err != nil {
return fmt.Errorf("cannot update OIDCClient '%s/%s': %w", oidcClient.Namespace, oidcClient.Name, err)
}
plog.Debug(
"OIDCClientWatcherController Sync updated an OIDCClient",
"oidcClientName", oidcClient.Name,
"oidcClientNamespace", oidcClient.Namespace,
"conditionsCount", len(conditions),
)
}
return nil
}
// validateOIDCClient validates the OIDCClient and its corresponding client secret storage Secret.
// When the corresponding client secret storage Secret was not found, pass nil to this function to
// get the validation error for that case.
func validateOIDCClient(oidcClient *v1alpha1.OIDCClient, secret *v1.Secret) []*v1alpha1.Condition {
c := validateSecret(secret, []*v1alpha1.Condition{})
c = validateAllowedGrantTypes(oidcClient, c)
c = validateAllowedScopes(oidcClient, c)
return c
}
// validateAllowedScopes checks if allowedScopes is valid on the OIDCClient.
func validateAllowedScopes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
switch {
case !allowedScopesContains(oidcClient, openidScopeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedScopesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must always be included in %q", openidScopeName, allowedScopesFieldName),
})
case allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName) && !allowedScopesContains(oidcClient, offlineAccessScopeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedScopesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must be included in %q when %q is included in %q",
offlineAccessScopeName, allowedScopesFieldName, refreshTokenGrantTypeName, allowedGrantTypesFieldName),
})
case allowedScopesContains(oidcClient, requestAudienceScopeName) &&
(!allowedScopesContains(oidcClient, usernameScopeName) || !allowedScopesContains(oidcClient, groupsScopeName)):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedScopesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q and %q must be included in %q when %q is included in %q",
usernameScopeName, groupsScopeName, allowedScopesFieldName, requestAudienceScopeName, allowedScopesFieldName),
})
case allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName) && !allowedScopesContains(oidcClient, requestAudienceScopeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedScopesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must be included in %q when %q is included in %q",
requestAudienceScopeName, allowedScopesFieldName, tokenExchangeGrantTypeName, allowedGrantTypesFieldName),
})
default:
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedScopesValid,
Status: v1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: fmt.Sprintf("%q is valid", allowedScopesFieldName),
})
}
return conditions
}
// validateAllowedGrantTypes checks if allowedGrantTypes is valid on the OIDCClient.
func validateAllowedGrantTypes(oidcClient *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
switch {
case !allowedGrantTypesContains(oidcClient, authorizationCodeGrantTypeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedGrantTypesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must always be included in %q",
authorizationCodeGrantTypeName, allowedGrantTypesFieldName),
})
case allowedScopesContains(oidcClient, offlineAccessScopeName) && !allowedGrantTypesContains(oidcClient, refreshTokenGrantTypeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedGrantTypesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must be included in %q when %q is included in %q",
refreshTokenGrantTypeName, allowedGrantTypesFieldName, offlineAccessScopeName, allowedScopesFieldName),
})
case allowedScopesContains(oidcClient, requestAudienceScopeName) && !allowedGrantTypesContains(oidcClient, tokenExchangeGrantTypeName):
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedGrantTypesValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingRequiredValue,
Message: fmt.Sprintf("%q must be included in %q when %q is included in %q",
tokenExchangeGrantTypeName, allowedGrantTypesFieldName, requestAudienceScopeName, allowedScopesFieldName),
})
default:
conditions = append(conditions, &v1alpha1.Condition{
Type: allowedGrantTypesValid,
Status: v1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: fmt.Sprintf("%q is valid", allowedGrantTypesFieldName),
})
}
return conditions
}
// validateSecret checks if the client secret storage Secret is valid and contains at least one client secret.
func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition) []*v1alpha1.Condition {
if secret == nil {
// Invalid: no storage Secret found.
conditions = append(conditions, &v1alpha1.Condition{
Type: clientSecretExists,
Status: v1alpha1.ConditionFalse,
Reason: reasonNoClientSecretFound,
Message: "no client secret found (no Secret storage found)",
})
return conditions
}
storedClientSecret, err := oidcclientsecretstorage.ReadFromSecret(secret)
if err != nil {
// Invalid: storage Secret exists but its data could not be parsed.
conditions = append(conditions, &v1alpha1.Condition{
Type: clientSecretExists,
Status: v1alpha1.ConditionFalse,
Reason: reasonNoClientSecretFound,
Message: fmt.Sprintf("error reading client secret storage: %s", err.Error()),
})
return conditions
}
// Successfully read the stored client secrets, so check if there are any stored in the list.
storedClientSecretsCount := len(storedClientSecret.SecretHashes)
if storedClientSecretsCount == 0 {
// Invalid: no client secrets stored.
conditions = append(conditions, &v1alpha1.Condition{
Type: clientSecretExists,
Status: v1alpha1.ConditionFalse,
Reason: reasonNoClientSecretFound,
Message: "no client secret found (empty list in storage)",
})
} else {
// Valid: has at least one client secret stored for this OIDC client.
conditions = append(conditions, &v1alpha1.Condition{
Type: clientSecretExists,
Status: v1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: fmt.Sprintf("%d client secret(s) found", storedClientSecretsCount),
})
}
return conditions
}
func allowedGrantTypesContains(haystack *v1alpha1.OIDCClient, needle string) bool {
for _, hay := range haystack.Spec.AllowedGrantTypes {
if hay == v1alpha1.GrantType(needle) {
return true
}
}
return false
}
func allowedScopesContains(haystack *v1alpha1.OIDCClient, needle string) bool {
for _, hay := range haystack.Spec.AllowedScopes {
if hay == v1alpha1.Scope(needle) {
return true
}
}
return false
}
func (c *oidcClientWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.OIDCClient, conditions []*v1alpha1.Condition) error {
updated := upstream.DeepCopy()
hadErrorCondition := conditionsutil.MergeConfigConditions(conditions, upstream.Generation, &updated.Status.Conditions, plog.New())
updated.Status.Phase = v1alpha1.PhaseReady
if hadErrorCondition {
updated.Status.Phase = v1alpha1.PhaseError
}
if equality.Semantic.DeepEqual(upstream, updated) {
return nil
}
_, err := c.pinnipedClient.
ConfigV1alpha1().
OIDCClients(upstream.Namespace).
UpdateStatus(ctx, updated, metav1.UpdateOptions{})
return err
}

View File

@@ -0,0 +1,903 @@
// Copyright 2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package oidcclientwatcher
import (
"context"
"encoding/base32"
"fmt"
"strings"
"testing"
"time"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
kubeinformers "k8s.io/client-go/informers"
kubernetesfake "k8s.io/client-go/kubernetes/fake"
configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/testutil"
)
func TestOIDCClientWatcherControllerFilterSecret(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: "storage.pinniped.dev/oidc-client-secret",
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: "secrets.pinniped.dev/some-other-type",
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
},
{
name: "resource of wrong data type",
secret: &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"},
},
},
}
for _, test := range tests {
tt := test
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
secretInformer := kubeinformers.NewSharedInformerFactory(
kubernetesfake.NewSimpleClientset(),
0,
).Core().V1().Secrets()
oidcClientsInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(),
0,
).Config().V1alpha1().OIDCClients()
withInformer := testutil.NewObservableWithInformerOption()
_ = NewOIDCClientWatcherController(
nil, // pinnipedClient, not needed
secretInformer,
oidcClientsInformer,
withInformer.WithInformer,
)
unrelated := corev1.Secret{}
filter := withInformer.GetFilterForInformer(secretInformer)
require.Equal(t, tt.wantAdd, filter.Add(tt.secret))
require.Equal(t, tt.wantUpdate, filter.Update(&unrelated, tt.secret))
require.Equal(t, tt.wantUpdate, filter.Update(tt.secret, &unrelated))
require.Equal(t, tt.wantDelete, filter.Delete(tt.secret))
})
}
}
func TestOIDCClientWatcherControllerFilterOIDCClient(t *testing.T) {
t.Parallel()
tests := []struct {
name string
oidcClient configv1alpha1.OIDCClient
wantAdd bool
wantUpdate bool
wantDelete bool
}{
{
name: "anything goes",
oidcClient: configv1alpha1.OIDCClient{},
wantAdd: true,
wantUpdate: true,
wantDelete: true,
},
}
for _, test := range tests {
tt := test
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
secretInformer := kubeinformers.NewSharedInformerFactory(
kubernetesfake.NewSimpleClientset(),
0,
).Core().V1().Secrets()
oidcClientsInformer := pinnipedinformers.NewSharedInformerFactory(
pinnipedfake.NewSimpleClientset(),
0,
).Config().V1alpha1().OIDCClients()
withInformer := testutil.NewObservableWithInformerOption()
_ = NewOIDCClientWatcherController(
nil, // pinnipedClient, not needed
secretInformer,
oidcClientsInformer,
withInformer.WithInformer,
)
unrelated := configv1alpha1.OIDCClient{}
filter := withInformer.GetFilterForInformer(oidcClientsInformer)
require.Equal(t, tt.wantAdd, filter.Add(&tt.oidcClient))
require.Equal(t, tt.wantUpdate, filter.Update(&unrelated, &tt.oidcClient))
require.Equal(t, tt.wantUpdate, filter.Update(&tt.oidcClient, &unrelated))
require.Equal(t, tt.wantDelete, filter.Delete(&tt.oidcClient))
})
}
}
func TestOIDCClientWatcherControllerSync(t *testing.T) {
t.Parallel()
const (
testName = "test-name"
testNamespace = "test-namespace"
testUID = "test-uid-123"
//nolint:gosec // this is not a credential
testBcryptSecret1 = "$2y$15$Kh7cRj0ScSD5QelE3ZNSl.nF04JDv7zb3SgGN.tSfLIX.4kt3UX7m" // bcrypt of "password1"
//nolint:gosec // this is not a credential
testBcryptSecret2 = "$2y$15$Kh7cRj0ScSD5QelE3ZNSl.nF04JDv7zb3SgGN.tSfLIX.4kt3UX7m" // bcrypt of "password2"
)
now := metav1.NewTime(time.Now().UTC())
earlier := metav1.NewTime(now.Add(-1 * time.Hour).UTC())
happyAllowedGrantTypesCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "AllowedGrantTypesValid",
Status: "True",
LastTransitionTime: time,
Reason: "Success",
Message: `"allowedGrantTypes" is valid`,
ObservedGeneration: observedGeneration,
}
}
sadAllowedGrantTypesCondition := func(time metav1.Time, observedGeneration int64, message string) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "AllowedGrantTypesValid",
Status: "False",
LastTransitionTime: time,
Reason: "MissingRequiredValue",
Message: message,
ObservedGeneration: observedGeneration,
}
}
happyClientSecretsCondition := func(howMany int, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "ClientSecretExists",
Status: "True",
LastTransitionTime: time,
Reason: "Success",
Message: fmt.Sprintf(`%d client secret(s) found`, howMany),
ObservedGeneration: observedGeneration,
}
}
sadClientSecretsCondition := func(time metav1.Time, observedGeneration int64, message string) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "ClientSecretExists",
Status: "False",
LastTransitionTime: time,
Reason: "NoClientSecretFound",
Message: message,
ObservedGeneration: observedGeneration,
}
}
happyAllowedScopesCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "AllowedScopesValid",
Status: "True",
LastTransitionTime: time,
Reason: "Success",
Message: `"allowedScopes" is valid`,
ObservedGeneration: observedGeneration,
}
}
sadAllowedScopesCondition := func(time metav1.Time, observedGeneration int64, message string) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "AllowedScopesValid",
Status: "False",
LastTransitionTime: time,
Reason: "MissingRequiredValue",
Message: message,
ObservedGeneration: observedGeneration,
}
}
secretNameForUID := func(uid string) string {
// See GetName() in OIDCClientSecretStorage for how the production code determines the Secret name.
// This test helper is intended to choose the same name.
return "pinniped-storage-oidc-client-secret-" +
strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString([]byte(uid)))
}
secretStringDataWithZeroClientSecrets := map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"1","hashes":[]}`),
"pinniped-storage-version": []byte("1"),
}
secretStringDataWithOneClientSecret := map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"1","hashes":["` + testBcryptSecret1 + `"]}`),
"pinniped-storage-version": []byte("1"),
}
secretStringDataWithTwoClientSecrets := map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"1","hashes":["` + testBcryptSecret1 + `","` + testBcryptSecret2 + `"]}`),
"pinniped-storage-version": []byte("1"),
}
secretStringDataWithWrongVersion := map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"wrong-version","hashes":[]}`),
"pinniped-storage-version": []byte("1"),
}
storageSecretForUIDWithData := func(uid string, data map[string][]byte) *corev1.Secret {
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: secretNameForUID(uid),
Labels: map[string]string{"storage.pinniped.dev/type": "oidc-client-secret"},
},
Type: "storage.pinniped.dev/oidc-client-secret",
Data: data,
}
}
tests := []struct {
name string
inputObjects []runtime.Object
inputSecrets []runtime.Object
wantErr string
wantResultingOIDCClients []configv1alpha1.OIDCClient
wantAPIActions int
}{
{
name: "no OIDCClients",
wantAPIActions: 0, // no updates
},
{
name: "successfully validate minimal OIDCClient and one client secret stored",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate minimal OIDCClient and two client secrets stored",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithTwoClientSecrets)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(2, now, 1234),
},
},
}},
},
{
name: "an already validated OIDCClient does not have its conditions updated when everything is still valid",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(earlier, 1234),
happyAllowedScopesCondition(earlier, 1234),
happyClientSecretsCondition(1, earlier, 1234),
},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 0, // no updates
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(earlier, 1234),
happyAllowedScopesCondition(earlier, 1234),
happyClientSecretsCondition(1, earlier, 1234),
},
},
}},
},
{
name: "missing required minimum settings and missing client secret storage",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{},
}},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
sadAllowedGrantTypesCondition(now, 1234, `"authorization_code" must always be included in "allowedGrantTypes"`),
sadAllowedScopesCondition(now, 1234, `"openid" must always be included in "allowedScopes"`),
sadClientSecretsCondition(now, 1234, "no client secret found (no Secret storage found)"),
},
},
}},
},
{
name: "client secret storage exists but cannot be read",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithWrongVersion)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
sadClientSecretsCondition(now, 1234, "error reading client secret storage: OIDC client secret storage data has wrong version: OIDC client secret storage has version wrong-version instead of 1"),
},
},
}},
},
{
name: "client secret storage exists but does not contain any client secrets",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithZeroClientSecrets)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
sadClientSecretsCondition(now, 1234, "no client secret found (empty list in storage)"),
},
},
}},
},
{
name: "can operate on multiple at a time, e.g. one is valid one another is missing required minimum settings",
inputObjects: []runtime.Object{
&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test1", Generation: 1234, UID: "uid1"},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
},
&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test2", Generation: 4567, UID: "uid2"},
Spec: configv1alpha1.OIDCClientSpec{},
},
},
inputSecrets: []runtime.Object{storageSecretForUIDWithData("uid1", secretStringDataWithOneClientSecret)},
wantAPIActions: 2, // one update for each OIDCClient
wantResultingOIDCClients: []configv1alpha1.OIDCClient{
{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test1", Generation: 1234, UID: "uid1"},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
},
{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test2", Generation: 4567, UID: "uid2"},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
sadAllowedGrantTypesCondition(now, 4567, `"authorization_code" must always be included in "allowedGrantTypes"`),
sadAllowedScopesCondition(now, 4567, `"openid" must always be included in "allowedScopes"`),
sadClientSecretsCondition(now, 4567, "no client secret found (no Secret storage found)"),
},
},
},
},
},
{
name: "a previously invalid OIDCClient has its spec changed to become valid so the conditions are updated",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 4567, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
// was invalid on previous run of controller which observed an old generation at an earlier time
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
sadAllowedGrantTypesCondition(earlier, 1234, `"authorization_code" must always be included in "allowedGrantTypes"`),
sadAllowedScopesCondition(earlier, 1234, `"openid" must always be included in "allowedScopes"`),
happyClientSecretsCondition(1, earlier, 1234),
},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 4567, UID: testUID},
// status was updated to reflect the current generation at the current time
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 4567),
happyAllowedScopesCondition(now, 4567),
happyClientSecretsCondition(1, earlier, 4567), // was already validated earlier
},
},
}},
},
{
name: "refresh_token must be included in allowedGrantTypes when offline_access is included in allowedScopes",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access"},
},
}},
wantAPIActions: 1, // one update
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
sadAllowedGrantTypesCondition(now, 1234, `"refresh_token" must be included in "allowedGrantTypes" when "offline_access" is included in "allowedScopes"`),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "urn:ietf:params:oauth:grant-type:token-exchange must be included in allowedGrantTypes when pinniped:request-audience is included in allowedScopes",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid", "pinniped:request-audience", "username", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
sadAllowedGrantTypesCondition(now, 1234, `"urn:ietf:params:oauth:grant-type:token-exchange" must be included in "allowedGrantTypes" when "pinniped:request-audience" is included in "allowedScopes"`),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "offline_access must be included in allowedScopes when refresh_token is included in allowedGrantTypes",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
sadAllowedScopesCondition(now, 1234, `"offline_access" must be included in "allowedScopes" when "refresh_token" is included in "allowedGrantTypes"`),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "username and groups must also be included in allowedScopes when pinniped:request-audience is included in allowedScopes: both missing",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"},
AllowedScopes: []configv1alpha1.Scope{"openid", "pinniped:request-audience"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
sadAllowedScopesCondition(now, 1234, `"username" and "groups" must be included in "allowedScopes" when "pinniped:request-audience" is included in "allowedScopes"`),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "username and groups must also be included in allowedScopes when pinniped:request-audience is included in allowedScopes: username missing",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"},
AllowedScopes: []configv1alpha1.Scope{"openid", "pinniped:request-audience", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
sadAllowedScopesCondition(now, 1234, `"username" and "groups" must be included in "allowedScopes" when "pinniped:request-audience" is included in "allowedScopes"`),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "username and groups must also be included in allowedScopes when pinniped:request-audience is included in allowedScopes: groups missing",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"},
AllowedScopes: []configv1alpha1.Scope{"openid", "pinniped:request-audience", "username"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
sadAllowedScopesCondition(now, 1234, `"username" and "groups" must be included in "allowedScopes" when "pinniped:request-audience" is included in "allowedScopes"`),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "pinniped:request-audience must be included in allowedScopes when urn:ietf:params:oauth:grant-type:token-exchange is included in allowedGrantTypes",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"},
AllowedScopes: []configv1alpha1.Scope{"openid"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Error",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
sadAllowedScopesCondition(now, 1234, `"pinniped:request-audience" must be included in "allowedScopes" when "urn:ietf:params:oauth:grant-type:token-exchange" is included in "allowedGrantTypes"`),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient with all allowedGrantTypes and all allowedScopes",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "pinniped:request-audience", "username", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient for offline access without kube API access without username/groups",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient for offline access without kube API access with username",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "username"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient for offline access without kube API access with groups",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient for offline access without kube API access with both username and groups",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code", "refresh_token"},
AllowedScopes: []configv1alpha1.Scope{"openid", "offline_access", "username", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient without offline access without kube API access with username",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid", "username"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient without offline access without kube API access with groups",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid", "username"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
{
name: "successfully validate an OIDCClient without offline access without kube API access with both username and groups",
inputObjects: []runtime.Object{&configv1alpha1.OIDCClient{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: configv1alpha1.OIDCClientSpec{
AllowedGrantTypes: []configv1alpha1.GrantType{"authorization_code"},
AllowedScopes: []configv1alpha1.Scope{"openid", "username", "groups"},
},
}},
inputSecrets: []runtime.Object{storageSecretForUIDWithData(testUID, secretStringDataWithOneClientSecret)},
wantAPIActions: 1, // one update
wantResultingOIDCClients: []configv1alpha1.OIDCClient{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: configv1alpha1.OIDCClientStatus{
Phase: "Ready",
Conditions: []configv1alpha1.Condition{
happyAllowedGrantTypesCondition(now, 1234),
happyAllowedScopesCondition(now, 1234),
happyClientSecretsCondition(1, now, 1234),
},
},
}},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.inputObjects...)
fakePinnipedClientForInformers := pinnipedfake.NewSimpleClientset(tt.inputObjects...)
pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClientForInformers, 0)
fakeKubeClient := kubernetesfake.NewSimpleClientset(tt.inputSecrets...)
kubeInformers := kubeinformers.NewSharedInformerFactoryWithOptions(fakeKubeClient, 0)
controller := NewOIDCClientWatcherController(
fakePinnipedClient,
kubeInformers.Core().V1().Secrets(),
pinnipedInformers.Config().V1alpha1().OIDCClients(),
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)
}
require.Len(t, fakePinnipedClient.Actions(), tt.wantAPIActions)
actualOIDCClients, err := fakePinnipedClient.ConfigV1alpha1().OIDCClients(testNamespace).List(ctx, metav1.ListOptions{})
require.NoError(t, err)
// Assert on the expected Status of the OIDCClients. Preprocess them a bit so that they're easier to assert against.
require.ElementsMatch(t, tt.wantResultingOIDCClients, normalizeOIDCClients(actualOIDCClients.Items, now))
})
}
}
func normalizeOIDCClients(oidcClients []configv1alpha1.OIDCClient, now metav1.Time) []configv1alpha1.OIDCClient {
result := make([]configv1alpha1.OIDCClient, 0, len(oidcClients))
for _, u := range oidcClients {
normalized := u.DeepCopy()
// We're only interested in comparing the status, so zero out the spec.
normalized.Spec = configv1alpha1.OIDCClientSpec{}
// 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)
}
return result
}

View File

@@ -410,7 +410,7 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *v1al
log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name)
updated := upstream.DeepCopy()
hadErrorCondition := conditionsutil.Merge(conditions, upstream.Generation, &updated.Status.Conditions, log)
hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log)
updated.Status.Phase = v1alpha1.PhaseReady
if hadErrorCondition {