From 0626b22c705d5b1d4633fd5ef159c48b9bca4228 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 6 Aug 2024 09:05:14 -0500 Subject: [PATCH 1/6] OIDC Upstream Watcher now reports condition OIDCDiscoverySucceeded with status Unknown if TLS validation fails --- .../oidc_upstream_watcher.go | 36 ++++++++----------- .../oidc_upstream_watcher_test.go | 34 ++++-------------- 2 files changed, 21 insertions(+), 49 deletions(-) diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 02ef033e2..4ebcda244 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -263,24 +263,12 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst }) } - c.updateStatus(ctx.Context, upstream, conditions) + hadErrorCondition := c.updateStatus(ctx.Context, upstream, conditions) - valid := true - log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) - for _, condition := range conditions { - if condition.Status == metav1.ConditionFalse { - valid = false - log.WithValues( - "type", condition.Type, - "reason", condition.Reason, - "message", condition.Message, - ).Error("found failing condition", errOIDCFailureStatus) - } - } - if valid { - return &result + if hadErrorCondition { + return nil } - return nil + return &result } // validateSecret validates the .spec.client.secretName field and returns the appropriate ClientCredentialsSecretValid condition. @@ -345,9 +333,9 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id return []*metav1.Condition{ { Type: typeOIDCDiscoverySucceeded, - Status: metav1.ConditionFalse, - Reason: tlsconfigutil.ReasonInvalidTLSConfig, - Message: tlsCondition.Message, + Status: metav1.ConditionUnknown, + Reason: conditionsutil.ReasonUnableToValidate, + Message: "unable to validate; see other conditions for details", }, tlsCondition, } @@ -468,7 +456,11 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id } } -func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *idpv1alpha1.OIDCIdentityProvider, conditions []*metav1.Condition) { +func (c *oidcWatcherController) updateStatus( + ctx context.Context, + upstream *idpv1alpha1.OIDCIdentityProvider, + conditions []*metav1.Condition, +) bool { log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() @@ -480,7 +472,7 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *idpv } if equality.Semantic.DeepEqual(upstream, updated) { - return + return hadErrorCondition } _, err := c.client. @@ -490,6 +482,8 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *idpv if err != nil { log.Error("failed to update status", err) } + + return hadErrorCondition } func defaultClientShortTimeout(rootCAs *x509.CertPool) *http.Client { diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 659e25e8c..9beaab3b2 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -270,7 +270,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","reason":"SecretNotFound","message":"secret \"test-client-secret\" not found","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -310,7 +309,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","reason":"SecretWrongType","message":"referenced Secret \"test-client-secret\" has wrong type \"some-other-type\" (should be \"secrets.pinniped.dev/oidc-client\")","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -349,7 +347,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","reason":"SecretMissingKeys","message":"referenced Secret \"test-client-secret\" is missing required keys [\"clientID\" \"clientSecret\"]","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -388,11 +385,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`, - `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7"}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"Unknown","reason":"UnableToValidate","message":"unable to validate; see other conditions for details"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7","error":"OIDCIdentityProvider has a failing condition"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -403,8 +398,8 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { happyAdditionalAuthorizeParametersValidCondition, {Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: now, Reason: "Success", Message: "loaded client credentials"}, - {Type: "OIDCDiscoverySucceeded", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: `spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7`}, + {Type: "OIDCDiscoverySucceeded", Status: "Unknown", LastTransitionTime: now, Reason: "UnableToValidate", + Message: `unable to validate; see other conditions for details`}, {Type: "TLSConfigurationValid", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", Message: "spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 7"}, }, @@ -431,11 +426,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`, - `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"Unknown","reason":"UnableToValidate","message":"unable to validate; see other conditions for details"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -446,8 +439,8 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { happyAdditionalAuthorizeParametersValidCondition, {Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: now, Reason: "Success", Message: "loaded client credentials"}, - {Type: "OIDCDiscoverySucceeded", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`}, + {Type: "OIDCDiscoverySucceeded", Status: "Unknown", LastTransitionTime: now, Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details"}, {Type: "TLSConfigurationValid", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`}, }, @@ -474,7 +467,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"failed to parse issuer URL: parse \"%invalid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\": invalid URL escape \"%in\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"failed to parse issuer URL: parse \"%invalid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\": invalid URL escape \"%in\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -513,7 +505,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"issuer URL '` + strings.Replace(testIssuerURL, "https", "http", 1) + `' must have \"https\" scheme, not \"http\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"issuer URL '` + strings.Replace(testIssuerURL, "https", "http", 1) + `' must have \"https\" scheme, not \"http\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -552,7 +543,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"issuer URL '` + testIssuerURL + `?sub=foo' cannot contain query or fragment component"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"issuer URL '` + testIssuerURL + `?sub=foo' cannot contain query or fragment component","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -591,7 +581,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"issuer URL '` + testIssuerURL + `#fragment' cannot contain query or fragment component"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"issuer URL '` + testIssuerURL + `#fragment' cannot contain query or fragment component","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -632,7 +621,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": tls: failed to verify certificate: x509: certificate signed by unknown authority"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": tls: failed to verify certificate: x509: certificate signed by unknown authority","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -673,7 +661,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -713,7 +700,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"failed to parse revocation endpoint URL: parse \"%\": invalid URL escape \"%\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"failed to parse revocation endpoint URL: parse \"%\": invalid URL escape \"%\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -753,7 +739,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"authorization endpoint URL 'http://example.com/authorize' must have \"https\" scheme, not \"http\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"authorization endpoint URL 'http://example.com/authorize' must have \"https\" scheme, not \"http\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -793,7 +778,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"revocation endpoint URL 'http://example.com/revoke' must have \"https\" scheme, not \"http\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"revocation endpoint URL 'http://example.com/revoke' must have \"https\" scheme, not \"http\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -833,7 +817,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"token endpoint URL 'http://example.com/token' must have \"https\" scheme, not \"http\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"token endpoint URL 'http://example.com/token' must have \"https\" scheme, not \"http\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -873,7 +856,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"token endpoint URL '' must have \"https\" scheme, not \"\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"token endpoint URL '' must have \"https\" scheme, not \"\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -913,7 +895,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidResponse","message":"authorization endpoint URL '' must have \"https\" scheme, not \"\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidResponse","message":"authorization endpoint URL '' must have \"https\" scheme, not \"\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -1531,7 +1512,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"True","reason":"Success","message":"discovered issuer configuration"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"False","reason":"DisallowedParameterName","message":"the following additionalAuthorizeParameters are not allowed: response_type,scope,client_id,state,nonce,code_challenge,code_challenge_method,redirect_uri,hd"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","reason":"DisallowedParameterName","message":"the following additionalAuthorizeParameters are not allowed: response_type,scope,client_id,state,nonce,code_challenge,code_challenge_method,redirect_uri,hd","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -1574,7 +1554,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/ends-with-slash\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/ends-with-slash\" got \"` + testIssuerURL + `/ends-with-slash/\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/ends-with-slash\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/ends-with-slash\" got \"` + testIssuerURL + `/ends-with-slash/\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -1615,7 +1594,6 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/\" got \"` + testIssuerURL + `\""}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"True","reason":"Success","message":"spec.tls is valid: using configured CA bundle"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"Unreachable","message":"failed to perform OIDC discovery against \"` + testIssuerURL + `/\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/\" got \"` + testIssuerURL + `\"","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ From 1c59a41cc5a9b1bb035a2d2c72b45b83e4225b1a Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 6 Aug 2024 09:36:48 -0500 Subject: [PATCH 2/6] Remove some dead code from LDAP/AD controllers --- .../active_directory_upstream_watcher.go | 4 ---- .../ldapupstreamwatcher/ldap_upstream_watcher.go | 8 -------- .../upstreamwatchers/upstream_watchers.go | 1 - 3 files changed, 13 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 1a4865eed..cba7d0bd0 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -94,10 +94,6 @@ func (g *activeDirectoryUpstreamGenericLDAPImpl) Generation() int64 { return g.activeDirectoryIdentityProvider.Generation } -func (g *activeDirectoryUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus { - return &activeDirectoryUpstreamGenericLDAPStatus{g.activeDirectoryIdentityProvider} -} - type activeDirectoryUpstreamGenericLDAPSpec struct { activeDirectoryIdentityProvider idpv1alpha1.ActiveDirectoryIdentityProvider } diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index a34b6992b..c56f1f76f 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -50,10 +50,6 @@ func (g *ldapUpstreamGenericLDAPImpl) Generation() int64 { return g.ldapIdentityProvider.Generation } -func (g *ldapUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus { - return &ldapUpstreamGenericLDAPStatus{g.ldapIdentityProvider} -} - type ldapUpstreamGenericLDAPSpec struct { ldapIdentityProvider idpv1alpha1.LDAPIdentityProvider } @@ -128,10 +124,6 @@ type ldapUpstreamGenericLDAPStatus struct { ldapIdentityProvider idpv1alpha1.LDAPIdentityProvider } -func (s *ldapUpstreamGenericLDAPStatus) Conditions() []metav1.Condition { - return s.ldapIdentityProvider.Status.Conditions -} - // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamLDAPIdentityProviderICache interface { SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI) diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index ac1ed370e..32c42dfa6 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -99,7 +99,6 @@ type UpstreamGenericLDAPIDP interface { Name() string Namespace() string Generation() int64 - Status() UpstreamGenericLDAPStatus } type UpstreamGenericLDAPSpec interface { From afa3aa22324b24da18e60ed10005aeffad95ee37 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 6 Aug 2024 13:03:00 -0500 Subject: [PATCH 3/6] LDAP and AD IDPs now always report condition with type LDAPConnectionValid, even if the status is unknown Co-authored-by: Ryan Richard --- .../jwtcachefiller/jwtcachefiller.go | 10 ++++------ .../webhookcachefiller/webhookcachefiller.go | 6 ++---- .../conditionsutil/conditions_util.go | 4 +++- .../active_directory_upstream_watcher.go | 8 -------- .../active_directory_upstream_watcher_test.go | 17 +++++++++++++++++ .../github_upstream_watcher.go | 2 +- .../ldap_upstream_watcher.go | 4 ---- .../ldap_upstream_watcher_test.go | 18 ++++++++++++++++++ .../oidc_upstream_watcher.go | 6 +----- .../upstreamwatchers/upstream_watchers.go | 15 ++++++++++++--- 10 files changed, 58 insertions(+), 32 deletions(-) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 297c0fbd1..64ec4ffc1 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -64,8 +64,6 @@ const ( reasonInvalidAuthenticator = "InvalidAuthenticator" reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS" - msgUnableToValidate = "unable to validate; see other conditions for details" - // These default values come from the way that the Supervisor issues and signs tokens. We make these // the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor. defaultUsernameClaim = oidcapi.IDTokenClaimUsername @@ -462,7 +460,7 @@ func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context Type: typeDiscoveryValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return nil, nil, conditions, nil } @@ -500,7 +498,7 @@ func (c *jwtCacheFillerController) validateProviderJWKSURL(provider *coreosoidc. Type: typeJWKSURLValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return "", conditions, nil } @@ -567,7 +565,7 @@ func (c *jwtCacheFillerController) validateJWKSFetch(ctx context.Context, jwksUR Type: typeJWKSFetchValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return nil, conditions, nil } @@ -646,7 +644,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator( Type: typeAuthenticatorValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return nil, conditions, nil } diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index 2361381cc..f80dad113 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -54,8 +54,6 @@ const ( reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook" reasonInvalidEndpointURL = "InvalidEndpointURL" reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme" - - msgUnableToValidate = "unable to validate; see other conditions for details" ) type cachedWebhookAuthenticator struct { @@ -344,7 +342,7 @@ func newWebhookAuthenticator( Type: typeAuthenticatorValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return nil, conditions, nil } @@ -425,7 +423,7 @@ func (c *webhookCacheFillerController) validateConnection( Type: typeWebhookConnectionValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: msgUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, }) return conditions, nil } diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index 91bc24c63..fc90c3986 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/internal/controller/conditionsutil/conditions_util.go @@ -12,13 +12,15 @@ import ( "go.pinniped.dev/internal/plog" ) -// Some common reasons shared by conditions of various resources. +// Some common reasons and messages shared by conditions of various resources. const ( ReasonSuccess = "Success" ReasonNotReady = "NotReady" ReasonUnableToValidate = "UnableToValidate" ReasonUnableToDialServer = "UnableToDialServer" ReasonInvalidIssuerURL = "InvalidIssuerURL" + + MessageUnableToValidate = "unable to validate; see other conditions for details" ) // MergeConditions merges conditions into conditionsToUpdate. diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index cba7d0bd0..2c5dfffec 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -212,14 +212,6 @@ func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() str return g.groupSearch.Attributes.GroupName } -type activeDirectoryUpstreamGenericLDAPStatus struct { - activeDirectoryIdentityProvider idpv1alpha1.ActiveDirectoryIdentityProvider -} - -func (s *activeDirectoryUpstreamGenericLDAPStatus) Conditions() []metav1.Condition { - return s.activeDirectoryIdentityProvider.Status.Conditions -} - // UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamActiveDirectoryIdentityProviderICache interface { SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index 42cee8d20..f9537f034 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -402,6 +402,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } } + ldapConnectionValidUnknown := func(gen int64) metav1.Condition { + return metav1.Condition{ + Type: "LDAPConnectionValid", + Status: "Unknown", + LastTransitionTime: now, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + ObservedGeneration: gen, + } + } + searchBaseFoundInRootDSECondition := func(gen int64) metav1.Condition { return metav1.Condition{ Type: "SearchBaseFound", @@ -663,6 +674,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -691,6 +703,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -718,6 +731,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -737,6 +751,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), + ldapConnectionValidUnknown(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -763,6 +778,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), + ldapConnectionValidUnknown(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -1158,6 +1174,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"), ObservedGeneration: 42, }, + ldapConnectionValidUnknown(42), tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"), }, }, diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index c5b1a4ead..1f8236cac 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -464,7 +464,7 @@ func (c *gitHubWatcherController) validateGitHubConnection( Type: GitHubConnectionValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: "unable to validate; see other conditions for details", + Message: conditionsutil.MessageUnableToValidate, }, nil, nil } diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index c56f1f76f..213816ab5 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -120,10 +120,6 @@ func (g *ldapUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string { return g.groupSearch.Attributes.GroupName } -type ldapUpstreamGenericLDAPStatus struct { - ldapIdentityProvider idpv1alpha1.LDAPIdentityProvider -} - // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamLDAPIdentityProviderICache interface { SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI) diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index f529e9182..31647db6d 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -393,6 +393,18 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ObservedGeneration: gen, } } + + ldapConnectionValidUnknown := func(gen int64) metav1.Condition { + return metav1.Condition{ + Type: "LDAPConnectionValid", + Status: "Unknown", + LastTransitionTime: now, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + ObservedGeneration: gen, + } + } + allConditionsTrue := func(gen int64, secretVersion string) []metav1.Condition { return []metav1.Condition{ bindSecretValidTrueCondition(gen), @@ -588,6 +600,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -616,6 +629,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -643,6 +657,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName), ObservedGeneration: 1234, }, + ldapConnectionValidUnknown(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -662,6 +677,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), + ldapConnectionValidUnknown(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -688,6 +704,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), + ldapConnectionValidUnknown(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -981,6 +998,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"), ObservedGeneration: 42, }, + ldapConnectionValidUnknown(42), tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"), }, }, diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 4ebcda244..3c01188e8 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -27,7 +27,6 @@ import ( oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1" - "go.pinniped.dev/internal/constable" pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" @@ -61,9 +60,6 @@ const ( reasonInvalidResponse = "InvalidResponse" reasonDisallowedParameterName = "DisallowedParameterName" allParamNamesAllowedMsg = "additionalAuthorizeParameters parameter names are allowed" - - // Errors that are generated by our reconcile process. - errOIDCFailureStatus = constable.Error("OIDCIdentityProvider has a failing condition") ) var ( @@ -335,7 +331,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id Type: typeOIDCDiscoverySucceeded, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, - Message: "unable to validate; see other conditions for details", + Message: conditionsutil.MessageUnableToValidate, }, tlsCondition, } diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 32c42dfa6..c9ff9fbc4 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -229,9 +229,9 @@ type GradatedConditions struct { } func (g *GradatedConditions) Conditions() []*metav1.Condition { - conditions := []*metav1.Condition{} - for _, gc := range g.gradatedConditions { - conditions = append(conditions, gc.condition) + conditions := make([]*metav1.Condition, len(g.gradatedConditions)) + for i, gc := range g.gradatedConditions { + conditions[i] = gc.condition } return conditions } @@ -263,9 +263,18 @@ func ValidateGenericLDAP( if secretValidCondition.Status == metav1.ConditionTrue && tlsValidCondition.Status == metav1.ConditionTrue { ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase(ctx, validatedSettingsCache, upstream, config, currentSecretVersion) conditions.Append(ldapConnectionValidCondition, false) + // TODO: For AD, hould we add a condition of type SearchBaseFoundCondition when we can't validate the bind secret or TLS config??? if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil conditions.Append(searchBaseFoundCondition, true) } + } else { + connectionUnknownCondition := &metav1.Condition{ + Type: typeLDAPConnectionValid, + Status: metav1.ConditionUnknown, + Reason: conditionsutil.ReasonUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, + } + conditions.Append(connectionUnknownCondition, true) } return conditions } From 6b49cd7d286effcc551de1064387016d471809a2 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 6 Aug 2024 12:40:19 -0700 Subject: [PATCH 4/6] add Unknown SearchBaseFound status condition for AD only --- .../active_directory_upstream_watcher.go | 9 ++++ .../active_directory_upstream_watcher_test.go | 53 +++++++++++++------ .../ldap_upstream_watcher.go | 6 ++- .../ldap_upstream_watcher_test.go | 36 +++++++------ .../upstreamwatchers/upstream_watchers.go | 19 ++++--- 5 files changed, 80 insertions(+), 43 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 2c5dfffec..0026126be 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -118,6 +118,15 @@ func (s *activeDirectoryUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers. return &activeDirectoryUpstreamGenericLDAPGroupSearch{s.activeDirectoryIdentityProvider.Spec.GroupSearch} } +func (s *activeDirectoryUpstreamGenericLDAPSpec) UnknownSearchBaseCondition() *metav1.Condition { + return &metav1.Condition{ + Type: upstreamwatchers.TypeSearchBaseFound, + Status: metav1.ConditionUnknown, + Reason: conditionsutil.ReasonUnableToValidate, + Message: conditionsutil.MessageUnableToValidate, + } +} + func (s *activeDirectoryUpstreamGenericLDAPSpec) DetectAndSetSearchBase(ctx context.Context, config *upstreamldap.ProviderConfig) *metav1.Condition { config.GroupSearch.Base = s.activeDirectoryIdentityProvider.Spec.GroupSearch.Base config.UserSearch.Base = s.activeDirectoryIdentityProvider.Spec.UserSearch.Base diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index f9537f034..bc3b1c740 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -366,6 +366,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { ObservedGeneration: gen, } } + activeDirectoryConnectionValidTrueCondition := func(gen int64, secretVersion string) metav1.Condition { return metav1.Condition{ Type: "LDAPConnectionValid", @@ -383,6 +384,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { c.LastTransitionTime = metav1.Time{} return c } + ldapConnectionValidUnknownCondition := func(gen int64) metav1.Condition { + return metav1.Condition{ + Type: "LDAPConnectionValid", + Status: "Unknown", + LastTransitionTime: now, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + ObservedGeneration: gen, + } + } + condPtr := func(c metav1.Condition) *metav1.Condition { return &c } @@ -391,6 +403,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { c.LastTransitionTime = metav1.Time{} return c } + tlsConfigurationValidLoadedTrueCondition := func(gen int64, msg string) metav1.Condition { return metav1.Condition{ Type: "TLSConfigurationValid", @@ -402,17 +415,6 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } } - ldapConnectionValidUnknown := func(gen int64) metav1.Condition { - return metav1.Condition{ - Type: "LDAPConnectionValid", - Status: "Unknown", - LastTransitionTime: now, - Reason: "UnableToValidate", - Message: "unable to validate; see other conditions for details", - ObservedGeneration: gen, - } - } - searchBaseFoundInRootDSECondition := func(gen int64) metav1.Condition { return metav1.Condition{ Type: "SearchBaseFound", @@ -446,6 +448,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } } + searchBaseFoundUnknownCondition := func(gen int64) metav1.Condition { + return metav1.Condition{ + Type: "SearchBaseFound", + Status: "Unknown", + LastTransitionTime: now, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + ObservedGeneration: gen, + } + } + allConditionsTrue := func(gen int64, secretVersion string) []metav1.Condition { return []metav1.Condition{ bindSecretValidTrueCondition(gen), @@ -674,7 +687,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), + searchBaseFoundUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -703,7 +717,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), + searchBaseFoundUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -731,7 +746,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), + searchBaseFoundUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -751,7 +767,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), + searchBaseFoundUnknownCondition(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -778,7 +795,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), + searchBaseFoundUnknownCondition(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -1174,7 +1192,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"), ObservedGeneration: 42, }, - ldapConnectionValidUnknown(42), + ldapConnectionValidUnknownCondition(42), + searchBaseFoundUnknownCondition(42), tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"), }, }, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 213816ab5..f97fa77a9 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -74,10 +74,14 @@ func (s *ldapUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGen return &ldapUpstreamGenericLDAPGroupSearch{s.ldapIdentityProvider.Spec.GroupSearch} } +func (s *ldapUpstreamGenericLDAPSpec) UnknownSearchBaseCondition() *metav1.Condition { + return nil // currently, only AD returns a condition for this +} + func (s *ldapUpstreamGenericLDAPSpec) DetectAndSetSearchBase(_ context.Context, config *upstreamldap.ProviderConfig) *metav1.Condition { config.GroupSearch.Base = s.ldapIdentityProvider.Spec.GroupSearch.Base config.UserSearch.Base = s.ldapIdentityProvider.Spec.UserSearch.Base - return nil + return nil // currently, only AD returns a condition for this } type ldapUpstreamGenericLDAPUserSearch struct { diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index 31647db6d..a1fb3994d 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -363,6 +363,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ObservedGeneration: gen, } } + ldapConnectionValidTrueCondition := func(gen int64, secretVersion string) metav1.Condition { return metav1.Condition{ Type: "LDAPConnectionValid", @@ -380,9 +381,21 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { c.LastTransitionTime = metav1.Time{} return c } + ldapConnectionValidUnknownCondition := func(gen int64) metav1.Condition { + return metav1.Condition{ + Type: "LDAPConnectionValid", + Status: "Unknown", + LastTransitionTime: now, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + ObservedGeneration: gen, + } + } + condPtr := func(c metav1.Condition) *metav1.Condition { return &c } + tlsConfigurationValidLoadedTrueCondition := func(gen int64, msg string) metav1.Condition { return metav1.Condition{ Type: "TLSConfigurationValid", @@ -394,17 +407,6 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { } } - ldapConnectionValidUnknown := func(gen int64) metav1.Condition { - return metav1.Condition{ - Type: "LDAPConnectionValid", - Status: "Unknown", - LastTransitionTime: now, - Reason: "UnableToValidate", - Message: "unable to validate; see other conditions for details", - ObservedGeneration: gen, - } - } - allConditionsTrue := func(gen int64, secretVersion string) []metav1.Condition { return []metav1.Condition{ bindSecretValidTrueCondition(gen), @@ -600,7 +602,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -629,7 +631,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -657,7 +659,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName), ObservedGeneration: 1234, }, - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"), }, }, @@ -677,7 +679,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -704,7 +706,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Phase: "Error", Conditions: []metav1.Condition{ bindSecretValidTrueCondition(1234), - ldapConnectionValidUnknown(1234), + ldapConnectionValidUnknownCondition(1234), { Type: "TLSConfigurationValid", Status: "False", @@ -998,7 +1000,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"), ObservedGeneration: 42, }, - ldapConnectionValidUnknown(42), + ldapConnectionValidUnknownCondition(42), tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"), }, }, diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index c9ff9fbc4..5bfb3a67d 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -108,6 +108,7 @@ type UpstreamGenericLDAPSpec interface { UserSearch() UpstreamGenericLDAPUserSearch GroupSearch() UpstreamGenericLDAPGroupSearch DetectAndSetSearchBase(ctx context.Context, config *upstreamldap.ProviderConfig) *metav1.Condition + UnknownSearchBaseCondition() *metav1.Condition } type UpstreamGenericLDAPUserSearch interface { @@ -261,21 +262,23 @@ func ValidateGenericLDAP( var ldapConnectionValidCondition, searchBaseFoundCondition *metav1.Condition // No point in trying to connect to the server if the config was already determined to be invalid. if secretValidCondition.Status == metav1.ConditionTrue && tlsValidCondition.Status == metav1.ConditionTrue { - ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase(ctx, validatedSettingsCache, upstream, config, currentSecretVersion) - conditions.Append(ldapConnectionValidCondition, false) - // TODO: For AD, hould we add a condition of type SearchBaseFoundCondition when we can't validate the bind secret or TLS config??? - if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil - conditions.Append(searchBaseFoundCondition, true) - } + ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase( + ctx, validatedSettingsCache, upstream, config, currentSecretVersion) } else { - connectionUnknownCondition := &metav1.Condition{ + ldapConnectionValidCondition = &metav1.Condition{ Type: typeLDAPConnectionValid, Status: metav1.ConditionUnknown, Reason: conditionsutil.ReasonUnableToValidate, Message: conditionsutil.MessageUnableToValidate, } - conditions.Append(connectionUnknownCondition, true) + searchBaseFoundCondition = upstream.Spec().UnknownSearchBaseCondition() + } + // Append the conditions calculated by the if/else above. + conditions.Append(ldapConnectionValidCondition, false) + if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil + conditions.Append(searchBaseFoundCondition, true) } + return conditions } From f918edd846edaf1e66cc49a053c48d9153018201 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 6 Aug 2024 16:28:13 -0500 Subject: [PATCH 5/6] Add integration tests to ensure that LDAP/AD conditions with status Unknown if they cannot be validated Co-authored-by: Ryan Richard --- test/integration/ldap_client_test.go | 12 +- test/integration/supervisor_ad_idp_test.go | 193 ++++++++++++++++++ .../integration/supervisor_github_idp_test.go | 32 +-- test/integration/supervisor_ldap_idp_test.go | 174 ++++++++++++++++ test/testlib/client.go | 74 +++++++ 5 files changed, 463 insertions(+), 22 deletions(-) create mode 100644 test/integration/supervisor_ad_idp_test.go create mode 100644 test/integration/supervisor_ldap_idp_test.go diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index e6d97326d..69cd43e19 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -49,10 +49,10 @@ func TestLDAPSearch_Parallel(t *testing.T) { ldapsLocalhostPort := localhostPorts[1] unusedLocalhostPort := localhostPorts[2] - // Expose the the test LDAP server's TLS port on the localhost. + // Expose the test LDAP server's TLS port on the localhost. startKubectlPortForward(ctx, t, ldapsLocalhostPort, "ldaps", "ldap", env.ToolsNamespace) - // Expose the the test LDAP server's StartTLS port on the localhost. + // Expose the test LDAP server's StartTLS port on the localhost. startKubectlPortForward(ctx, t, ldapLocalhostPort, "ldap", "ldap", env.ToolsNamespace) providerConfig := func(editFunc func(p *upstreamldap.ProviderConfig)) *upstreamldap.ProviderConfig { @@ -853,11 +853,11 @@ func startKubectlPortForward(ctx context.Context, t *testing.T, hostPort, remote func findRecentlyUnusedLocalhostPorts(t *testing.T, howManyPorts int) []string { t.Helper() - listeners := []net.Listener{} - for range howManyPorts { - unusedPortGrabbingListener, err := net.Listen("tcp", "127.0.0.1:0") + listeners := make([]net.Listener, howManyPorts) + for i := range howManyPorts { + var err error + listeners[i], err = net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) - listeners = append(listeners, unusedPortGrabbingListener) } ports := make([]string, len(listeners)) diff --git a/test/integration/supervisor_ad_idp_test.go b/test/integration/supervisor_ad_idp_test.go new file mode 100644 index 000000000..91841590f --- /dev/null +++ b/test/integration/supervisor_ad_idp_test.go @@ -0,0 +1,193 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "encoding/base64" + "fmt" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/test/testlib" +) + +func TestActiveDirectoryIDPPhaseAndConditions_Parallel(t *testing.T) { + env := testlib.IntegrationEnv(t) + testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env) + + supervisorNamespace := testlib.IntegrationEnv(t).SupervisorNamespace + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + adIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().ActiveDirectoryIdentityProviders(supervisorNamespace) + + bindSecret := testlib.CreateTestSecret( + t, + env.SupervisorNamespace, + "ad-bind-secret", + corev1.SecretTypeBasicAuth, + map[string]string{ + corev1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + corev1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + + happySpec := idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: bindSecret.Name, + }, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + UserSearch: idpv1alpha1.ActiveDirectoryIdentityProviderUserSearch{ + Base: env.SupervisorUpstreamActiveDirectory.UserSearchBase, + Filter: "", + Attributes: idpv1alpha1.ActiveDirectoryIdentityProviderUserSearchAttributes{ + Username: env.SupervisorUpstreamActiveDirectory.TestUserMailAttributeName, + UID: env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeName, + }, + }, + GroupSearch: idpv1alpha1.ActiveDirectoryIdentityProviderGroupSearch{ + Base: env.SupervisorUpstreamActiveDirectory.GroupSearchBase, + Filter: "", // use the default value of "member={}" + Attributes: idpv1alpha1.ActiveDirectoryIdentityProviderGroupSearchAttributes{ + GroupName: "", // use the default value of "dn" + }, + }, + } + + tests := []struct { + name string + adSpec idpv1alpha1.ActiveDirectoryIdentityProviderSpec + wantPhase idpv1alpha1.ActiveDirectoryIdentityProviderPhase + wantConditions []*metav1.Condition + }{ + { + name: "Happy Path", + adSpec: happySpec, + wantPhase: idpv1alpha1.ActiveDirectoryPhaseReady, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "True", + Reason: "Success", + Message: "loaded bind secret", + }, + { + Type: "LDAPConnectionValid", + Status: "True", + Reason: "Success", + Message: fmt.Sprintf( + `successfully able to connect to %q and bind as user %q [validated with Secret %q at version %q]`, + env.SupervisorUpstreamActiveDirectory.Host, + env.SupervisorUpstreamActiveDirectory.BindUsername, + bindSecret.Name, + bindSecret.ResourceVersion), + }, + { + Type: "SearchBaseFound", + Status: "True", + Reason: "UsingConfigurationFromSpec", + Message: "Using search base from ActiveDirectoryIdentityProvider config.", + }, + { + Type: "TLSConfigurationValid", + Status: "True", + Reason: "Success", + Message: "spec.tls is valid: using configured CA bundle", + }, + }, + }, + { + name: "CA bundle is invalid yields conditions TLSConfigurationValid with status 'False' and LDAPConnectionValid with status 'Unknown'", + adSpec: func() idpv1alpha1.ActiveDirectoryIdentityProviderSpec { + temp := happySpec.DeepCopy() + temp.TLS.CertificateAuthorityData = "this-is-not-base64-encoded" + return *temp + }(), + wantPhase: idpv1alpha1.ActiveDirectoryPhaseError, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "True", + Reason: "Success", + Message: "loaded bind secret", + }, + { + Type: "LDAPConnectionValid", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "SearchBaseFound", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "TLSConfigurationValid", + Status: "False", + Reason: "InvalidTLSConfig", + Message: "spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 4", + }, + }, + }, + { + name: "Bind secret not found yields conditions BindSecretValid with status 'False' and LDAPConnectionValid with status 'Unknown'", + adSpec: func() idpv1alpha1.ActiveDirectoryIdentityProviderSpec { + temp := happySpec.DeepCopy() + temp.Bind.SecretName = "this-secret-does-not-exist" + return *temp + }(), + wantPhase: idpv1alpha1.ActiveDirectoryPhaseError, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + Reason: "SecretNotFound", + Message: `secret "this-secret-does-not-exist" not found`, + }, + { + Type: "LDAPConnectionValid", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "SearchBaseFound", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "TLSConfigurationValid", + Status: "True", + Reason: "Success", + Message: "spec.tls is valid: using configured CA bundle", + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + idp := testlib.CreateTestActiveDirectoryIdentityProvider(t, test.adSpec, test.wantPhase) + testlib.WaitForActiveDirectoryIdentityProviderStatusConditions( + ctx, + t, + adIDPClient, + idp.Name, + test.wantConditions, + ) + }) + } +} diff --git a/test/integration/supervisor_github_idp_test.go b/test/integration/supervisor_github_idp_test.go index 54f272aa0..bd713254a 100644 --- a/test/integration/supervisor_github_idp_test.go +++ b/test/integration/supervisor_github_idp_test.go @@ -24,7 +24,7 @@ import ( "go.pinniped.dev/test/testlib" ) -const generateNamePrefix = "integration-test-github-idp-" +const generateGitHubNamePrefix = "integration-test-github-idp-" func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { adminClient := testlib.NewKubernetesClientset(t) @@ -37,7 +37,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -240,7 +240,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { input := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, Spec: tt.inputSpec, } @@ -268,7 +268,7 @@ func TestGitHubIDPSetsDefaultsWithKubectl_Parallel(t *testing.T) { ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -278,7 +278,7 @@ func TestGitHubIDPSetsDefaultsWithKubectl_Parallel(t *testing.T) { }) t.Logf("Created namespace %s", ns.Name) - idpName := generateNamePrefix + testlib.RandHex(t, 16) + idpName := generateGitHubNamePrefix + testlib.RandHex(t, 16) githubIDPYaml := []byte(here.Doc(fmt.Sprintf(` --- @@ -336,8 +336,8 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { secretsClient := kubernetesClient.CoreV1().Secrets(supervisorNamespace) gitHubIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().GitHubIdentityProviders(supervisorNamespace) - happySecretName := generateNamePrefix + testlib.RandHex(t, 16) - invalidSecretName := generateNamePrefix + testlib.RandHex(t, 16) + happySecretName := generateGitHubNamePrefix + testlib.RandHex(t, 16) + invalidSecretName := generateGitHubNamePrefix + testlib.RandHex(t, 16) tests := []struct { name string @@ -490,7 +490,7 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { var secretName string for _, secret := range tt.secrets { - secret.GenerateName = generateNamePrefix + secret.GenerateName = generateGitHubNamePrefix created, err := secretsClient.Create(ctx, secret, metav1.CreateOptions{}) require.NoError(t, err) @@ -505,7 +505,7 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { for _, idp := range tt.idps { idp.Name = "" - idp.GenerateName = generateNamePrefix + idp.GenerateName = generateGitHubNamePrefix idp.Spec.Client.SecretName = secretName created, err := gitHubIDPClient.Create(ctx, idp, metav1.CreateOptions{}) @@ -532,7 +532,7 @@ func TestGitHubIDPInWrongNamespace_Parallel(t *testing.T) { namespaceClient := kubernetesClient.CoreV1().Namespaces() otherNamespace, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -545,7 +545,7 @@ func TestGitHubIDPInWrongNamespace_Parallel(t *testing.T) { idp := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, Namespace: otherNamespace.Name, }, Spec: idpv1alpha1.GitHubIdentityProviderSpec{ @@ -592,7 +592,7 @@ func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { namespaceClient := kubernetesClient.CoreV1().Namespaces() otherNamespace, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -605,7 +605,7 @@ func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, Namespace: otherNamespace.Name, }, Type: "secrets.pinniped.dev/github-client", @@ -621,7 +621,7 @@ func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { idp := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, Namespace: supervisorNamespace, }, Spec: idpv1alpha1.GitHubIdentityProviderSpec{ @@ -701,7 +701,7 @@ func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testi ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -714,7 +714,7 @@ func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testi input := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: generateNamePrefix, + GenerateName: generateGitHubNamePrefix, }, Spec: idpv1alpha1.GitHubIdentityProviderSpec{ AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ diff --git a/test/integration/supervisor_ldap_idp_test.go b/test/integration/supervisor_ldap_idp_test.go new file mode 100644 index 000000000..ad33865bf --- /dev/null +++ b/test/integration/supervisor_ldap_idp_test.go @@ -0,0 +1,174 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "encoding/base64" + "fmt" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/test/testlib" +) + +func TestLDAPIDPPhaseAndConditions_Parallel(t *testing.T) { + env := testlib.IntegrationEnv(t) + + supervisorNamespace := testlib.IntegrationEnv(t).SupervisorNamespace + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + ldapIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().LDAPIdentityProviders(supervisorNamespace) + + bindSecret := testlib.CreateTestSecret( + t, + env.SupervisorNamespace, + "ldap-bind-secret", + corev1.SecretTypeBasicAuth, + map[string]string{ + corev1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + corev1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + ) + + happySpec := idpv1alpha1.LDAPIdentityProviderSpec{ + Host: env.SupervisorUpstreamLDAP.Host, + Bind: idpv1alpha1.LDAPIdentityProviderBind{ + SecretName: bindSecret.Name, + }, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), + }, + UserSearch: idpv1alpha1.LDAPIdentityProviderUserSearch{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderUserSearchAttributes{ + Username: env.SupervisorUpstreamLDAP.TestUserMailAttributeName, + UID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, + }, + }, + GroupSearch: idpv1alpha1.LDAPIdentityProviderGroupSearch{ + Base: env.SupervisorUpstreamLDAP.GroupSearchBase, + Filter: "", // use the default value of "member={}" + Attributes: idpv1alpha1.LDAPIdentityProviderGroupSearchAttributes{ + GroupName: "", // use the default value of "dn" + }, + }, + } + + tests := []struct { + name string + ldapSpec idpv1alpha1.LDAPIdentityProviderSpec + wantPhase idpv1alpha1.LDAPIdentityProviderPhase + wantConditions []*metav1.Condition + }{ + { + name: "Happy Path", + ldapSpec: happySpec, + wantPhase: idpv1alpha1.LDAPPhaseReady, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "True", + Reason: "Success", + Message: "loaded bind secret", + }, + { + Type: "LDAPConnectionValid", + Status: "True", + Reason: "Success", + Message: fmt.Sprintf( + `successfully able to connect to %q and bind as user %q [validated with Secret %q at version %q]`, + env.SupervisorUpstreamLDAP.Host, + env.SupervisorUpstreamLDAP.BindUsername, + bindSecret.Name, + bindSecret.ResourceVersion), + }, + { + Type: "TLSConfigurationValid", + Status: "True", + Reason: "Success", + Message: "spec.tls is valid: using configured CA bundle", + }, + }, + }, + { + name: "CA bundle is invalid yields conditions TLSConfigurationValid with status 'False' and LDAPConnectionValid/SearchBaseFound with status 'Unknown'", + ldapSpec: func() idpv1alpha1.LDAPIdentityProviderSpec { + temp := happySpec.DeepCopy() + temp.TLS.CertificateAuthorityData = "this-is-not-base64-encoded" + return *temp + }(), + wantPhase: idpv1alpha1.LDAPPhaseError, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "True", + Reason: "Success", + Message: "loaded bind secret", + }, + { + Type: "LDAPConnectionValid", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "TLSConfigurationValid", + Status: "False", + Reason: "InvalidTLSConfig", + Message: "spec.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 4", + }, + }, + }, + { + name: "Bind secret not found yields conditions BindSecretValid with status 'False' and LDAPConnectionValid/SearchBaseFound with status 'Unknown'", + ldapSpec: func() idpv1alpha1.LDAPIdentityProviderSpec { + temp := happySpec.DeepCopy() + temp.Bind.SecretName = "this-secret-does-not-exist" + return *temp + }(), + wantPhase: idpv1alpha1.LDAPPhaseError, + wantConditions: []*metav1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + Reason: "SecretNotFound", + Message: `secret "this-secret-does-not-exist" not found`, + }, + { + Type: "LDAPConnectionValid", + Status: "Unknown", + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, + { + Type: "TLSConfigurationValid", + Status: "True", + Reason: "Success", + Message: "spec.tls is valid: using configured CA bundle", + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + idp := testlib.CreateTestLDAPIdentityProvider(t, test.ldapSpec, test.wantPhase) + testlib.WaitForLDAPIdentityProviderStatusConditions( + ctx, + t, + ldapIDPClient, + idp.Name, + test.wantConditions, + ) + }) + } +} diff --git a/test/testlib/client.go b/test/testlib/client.go index 36f455e82..41271f3e6 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -1000,6 +1000,80 @@ func WaitForGitHubIdentityProviderStatusConditions( }, 60*time.Second, 1*time.Second, "wanted conditions for GitHubIdentityProvider %q", gitHubIDPName) } +func WaitForLDAPIdentityProviderStatusConditions( + ctx context.Context, + t *testing.T, + client alpha1.LDAPIdentityProviderInterface, + ldapIDPName string, + expectConditions []*metav1.Condition, +) { + t.Helper() + + RequireEventuallyf(t, func(requireEventually *require.Assertions) { + idp, err := client.Get(ctx, ldapIDPName, metav1.GetOptions{}) + requireEventually.NoError(err) + + actualConditions := make([]*metav1.Condition, len(idp.Status.Conditions)) + for i, c := range idp.Status.Conditions { + actualConditions[i] = c.DeepCopy() + } + + requireEventually.Lenf(actualConditions, len(expectConditions), + "wanted status conditions: %#v", expectConditions) + + for i, wantCond := range expectConditions { + actualCond := actualConditions[i] + + // This is a cheat to avoid needing to make equality assertions on these fields. + requireEventually.NotZero(actualCond.LastTransitionTime) + wantCond.LastTransitionTime = actualCond.LastTransitionTime + requireEventually.NotZero(actualCond.ObservedGeneration) + wantCond.ObservedGeneration = actualCond.ObservedGeneration + + requireEventually.Equalf(wantCond, actualCond, + "wanted status conditions: %#v\nactual status conditions were: %#v\nnot equal at index %d", + expectConditions, &actualConditions, i) + } + }, 60*time.Second, 1*time.Second, "wanted conditions for LDAPIdentityProvider %q", ldapIDPName) +} + +func WaitForActiveDirectoryIdentityProviderStatusConditions( + ctx context.Context, + t *testing.T, + client alpha1.ActiveDirectoryIdentityProviderInterface, + activeDirectoryIDPName string, + expectConditions []*metav1.Condition, +) { + t.Helper() + + RequireEventuallyf(t, func(requireEventually *require.Assertions) { + idp, err := client.Get(ctx, activeDirectoryIDPName, metav1.GetOptions{}) + requireEventually.NoError(err) + + actualConditions := make([]*metav1.Condition, len(idp.Status.Conditions)) + for i, c := range idp.Status.Conditions { + actualConditions[i] = c.DeepCopy() + } + + requireEventually.Lenf(actualConditions, len(expectConditions), + "wanted status conditions: %#v", expectConditions) + + for i, wantCond := range expectConditions { + actualCond := actualConditions[i] + + // This is a cheat to avoid needing to make equality assertions on these fields. + requireEventually.NotZero(actualCond.LastTransitionTime) + wantCond.LastTransitionTime = actualCond.LastTransitionTime + requireEventually.NotZero(actualCond.ObservedGeneration) + wantCond.ObservedGeneration = actualCond.ObservedGeneration + + requireEventually.Equalf(wantCond, actualCond, + "wanted status conditions: %#v\nactual status conditions were: %#v\nnot equal at index %d", + expectConditions, &actualConditions, i) + } + }, 60*time.Second, 1*time.Second, "wanted conditions for ActiveDirectoryIdentityProvider %q", activeDirectoryIDPName) +} + func TestObjectMeta(t *testing.T, baseName string) metav1.ObjectMeta { return metav1.ObjectMeta{ GenerateName: fmt.Sprintf("test-%s-", baseName), From c1328d96193d57f156462e18100cce890bc0c6b0 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 6 Aug 2024 16:06:36 -0700 Subject: [PATCH 6/6] update expectation in supervisor_ldap_idp_test.go --- test/integration/supervisor_ldap_idp_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/integration/supervisor_ldap_idp_test.go b/test/integration/supervisor_ldap_idp_test.go index ad33865bf..bd80cba93 100644 --- a/test/integration/supervisor_ldap_idp_test.go +++ b/test/integration/supervisor_ldap_idp_test.go @@ -37,6 +37,14 @@ func TestLDAPIDPPhaseAndConditions_Parallel(t *testing.T) { }, ) + wantCABundleMessage := func(caBundleConfigured bool) string { + if caBundleConfigured { + return "spec.tls is valid: using configured CA bundle" + } else { + return "spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image" + } + } + happySpec := idpv1alpha1.LDAPIdentityProviderSpec{ Host: env.SupervisorUpstreamLDAP.Host, Bind: idpv1alpha1.LDAPIdentityProviderBind{ @@ -94,7 +102,7 @@ func TestLDAPIDPPhaseAndConditions_Parallel(t *testing.T) { Type: "TLSConfigurationValid", Status: "True", Reason: "Success", - Message: "spec.tls is valid: using configured CA bundle", + Message: wantCABundleMessage(len(happySpec.TLS.CertificateAuthorityData) != 0), }, }, }, @@ -152,7 +160,7 @@ func TestLDAPIDPPhaseAndConditions_Parallel(t *testing.T) { Type: "TLSConfigurationValid", Status: "True", Reason: "Success", - Message: "spec.tls is valid: using configured CA bundle", + Message: wantCABundleMessage(len(happySpec.TLS.CertificateAuthorityData) != 0), }, }, },