Skip to content

Commit b377040

Browse files
cfryanrcfryanr
authored
Merge pull request #2034 from vmware-tanzu/jtc/older-idps-should-use-unknown-condition-status
OIDC/LDAP/AD IDPs should use unknown condition status
2 parents fbbec50 + c1328d9 commit b377040

File tree

16 files changed

+591
-120
lines changed

16 files changed

+591
-120
lines changed

‎internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ const (
6464
reasonInvalidAuthenticator = "InvalidAuthenticator"
6565
reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS"
6666

67-
msgUnableToValidate = "unable to validate; see other conditions for details"
68-
6967
// These default values come from the way that the Supervisor issues and signs tokens. We make these
7068
// the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor.
7169
defaultUsernameClaim = oidcapi.IDTokenClaimUsername
@@ -462,7 +460,7 @@ func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context
462460
Type: typeDiscoveryValid,
463461
Status: metav1.ConditionUnknown,
464462
Reason: conditionsutil.ReasonUnableToValidate,
465-
Message: msgUnableToValidate,
463+
Message: conditionsutil.MessageUnableToValidate,
466464
})
467465
return nil, nil, conditions, nil
468466
}
@@ -500,7 +498,7 @@ func (c *jwtCacheFillerController) validateProviderJWKSURL(provider *coreosoidc.
500498
Type: typeJWKSURLValid,
501499
Status: metav1.ConditionUnknown,
502500
Reason: conditionsutil.ReasonUnableToValidate,
503-
Message: msgUnableToValidate,
501+
Message: conditionsutil.MessageUnableToValidate,
504502
})
505503
return "", conditions, nil
506504
}
@@ -567,7 +565,7 @@ func (c *jwtCacheFillerController) validateJWKSFetch(ctx context.Context, jwksUR
567565
Type: typeJWKSFetchValid,
568566
Status: metav1.ConditionUnknown,
569567
Reason: conditionsutil.ReasonUnableToValidate,
570-
Message: msgUnableToValidate,
568+
Message: conditionsutil.MessageUnableToValidate,
571569
})
572570
return nil, conditions, nil
573571
}
@@ -646,7 +644,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(
646644
Type: typeAuthenticatorValid,
647645
Status: metav1.ConditionUnknown,
648646
Reason: conditionsutil.ReasonUnableToValidate,
649-
Message: msgUnableToValidate,
647+
Message: conditionsutil.MessageUnableToValidate,
650648
})
651649
return nil, conditions, nil
652650
}

‎internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ const (
5454
reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook"
5555
reasonInvalidEndpointURL = "InvalidEndpointURL"
5656
reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme"
57-
58-
msgUnableToValidate = "unable to validate; see other conditions for details"
5957
)
6058

6159
type cachedWebhookAuthenticator struct {
@@ -344,7 +342,7 @@ func newWebhookAuthenticator(
344342
Type: typeAuthenticatorValid,
345343
Status: metav1.ConditionUnknown,
346344
Reason: conditionsutil.ReasonUnableToValidate,
347-
Message: msgUnableToValidate,
345+
Message: conditionsutil.MessageUnableToValidate,
348346
})
349347
return nil, conditions, nil
350348
}
@@ -425,7 +423,7 @@ func (c *webhookCacheFillerController) validateConnection(
425423
Type: typeWebhookConnectionValid,
426424
Status: metav1.ConditionUnknown,
427425
Reason: conditionsutil.ReasonUnableToValidate,
428-
Message: msgUnableToValidate,
426+
Message: conditionsutil.MessageUnableToValidate,
429427
})
430428
return conditions, nil
431429
}

‎internal/controller/conditionsutil/conditions_util.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ import (
1212
"go.pinniped.dev/internal/plog"
1313
)
1414

15-
// Some common reasons shared by conditions of various resources.
15+
// Some common reasons and messages shared by conditions of various resources.
1616
const (
1717
ReasonSuccess = "Success"
1818
ReasonNotReady = "NotReady"
1919
ReasonUnableToValidate = "UnableToValidate"
2020
ReasonUnableToDialServer = "UnableToDialServer"
2121
ReasonInvalidIssuerURL = "InvalidIssuerURL"
22+
23+
MessageUnableToValidate = "unable to validate; see other conditions for details"
2224
)
2325

2426
// MergeConditions merges conditions into conditionsToUpdate.

‎internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,6 @@ func (g *activeDirectoryUpstreamGenericLDAPImpl) Generation() int64 {
9494
return g.activeDirectoryIdentityProvider.Generation
9595
}
9696

97-
func (g *activeDirectoryUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus {
98-
return &activeDirectoryUpstreamGenericLDAPStatus{g.activeDirectoryIdentityProvider}
99-
}
100-
10197
type activeDirectoryUpstreamGenericLDAPSpec struct {
10298
activeDirectoryIdentityProvider idpv1alpha1.ActiveDirectoryIdentityProvider
10399
}
@@ -122,6 +118,15 @@ func (s *activeDirectoryUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.
122118
return &activeDirectoryUpstreamGenericLDAPGroupSearch{s.activeDirectoryIdentityProvider.Spec.GroupSearch}
123119
}
124120

121+
func (s *activeDirectoryUpstreamGenericLDAPSpec) UnknownSearchBaseCondition() *metav1.Condition {
122+
return &metav1.Condition{
123+
Type: upstreamwatchers.TypeSearchBaseFound,
124+
Status: metav1.ConditionUnknown,
125+
Reason: conditionsutil.ReasonUnableToValidate,
126+
Message: conditionsutil.MessageUnableToValidate,
127+
}
128+
}
129+
125130
func (s *activeDirectoryUpstreamGenericLDAPSpec) DetectAndSetSearchBase(ctx context.Context, config *upstreamldap.ProviderConfig) *metav1.Condition {
126131
config.GroupSearch.Base = s.activeDirectoryIdentityProvider.Spec.GroupSearch.Base
127132
config.UserSearch.Base = s.activeDirectoryIdentityProvider.Spec.UserSearch.Base
@@ -216,14 +221,6 @@ func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() str
216221
return g.groupSearch.Attributes.GroupName
217222
}
218223

219-
type activeDirectoryUpstreamGenericLDAPStatus struct {
220-
activeDirectoryIdentityProvider idpv1alpha1.ActiveDirectoryIdentityProvider
221-
}
222-
223-
func (s *activeDirectoryUpstreamGenericLDAPStatus) Conditions() []metav1.Condition {
224-
return s.activeDirectoryIdentityProvider.Status.Conditions
225-
}
226-
227224
// UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations.
228225
type UpstreamActiveDirectoryIdentityProviderICache interface {
229226
SetActiveDirectoryIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI)

‎internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
366366
ObservedGeneration: gen,
367367
}
368368
}
369+
369370
activeDirectoryConnectionValidTrueCondition := func(gen int64, secretVersion string) metav1.Condition {
370371
return metav1.Condition{
371372
Type: "LDAPConnectionValid",
@@ -383,6 +384,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
383384
c.LastTransitionTime = metav1.Time{}
384385
return c
385386
}
387+
ldapConnectionValidUnknownCondition := func(gen int64) metav1.Condition {
388+
return metav1.Condition{
389+
Type: "LDAPConnectionValid",
390+
Status: "Unknown",
391+
LastTransitionTime: now,
392+
Reason: "UnableToValidate",
393+
Message: "unable to validate; see other conditions for details",
394+
ObservedGeneration: gen,
395+
}
396+
}
397+
386398
condPtr := func(c metav1.Condition) *metav1.Condition {
387399
return &c
388400
}
@@ -391,6 +403,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
391403
c.LastTransitionTime = metav1.Time{}
392404
return c
393405
}
406+
394407
tlsConfigurationValidLoadedTrueCondition := func(gen int64, msg string) metav1.Condition {
395408
return metav1.Condition{
396409
Type: "TLSConfigurationValid",
@@ -435,6 +448,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
435448
}
436449
}
437450

451+
searchBaseFoundUnknownCondition := func(gen int64) metav1.Condition {
452+
return metav1.Condition{
453+
Type: "SearchBaseFound",
454+
Status: "Unknown",
455+
LastTransitionTime: now,
456+
Reason: "UnableToValidate",
457+
Message: "unable to validate; see other conditions for details",
458+
ObservedGeneration: gen,
459+
}
460+
}
461+
438462
allConditionsTrue := func(gen int64, secretVersion string) []metav1.Condition {
439463
return []metav1.Condition{
440464
bindSecretValidTrueCondition(gen),
@@ -663,6 +687,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
663687
Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName),
664688
ObservedGeneration: 1234,
665689
},
690+
ldapConnectionValidUnknownCondition(1234),
691+
searchBaseFoundUnknownCondition(1234),
666692
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
667693
},
668694
},
@@ -691,6 +717,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
691717
Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName),
692718
ObservedGeneration: 1234,
693719
},
720+
ldapConnectionValidUnknownCondition(1234),
721+
searchBaseFoundUnknownCondition(1234),
694722
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
695723
},
696724
},
@@ -718,6 +746,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
718746
Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName),
719747
ObservedGeneration: 1234,
720748
},
749+
ldapConnectionValidUnknownCondition(1234),
750+
searchBaseFoundUnknownCondition(1234),
721751
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
722752
},
723753
},
@@ -737,6 +767,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
737767
Phase: "Error",
738768
Conditions: []metav1.Condition{
739769
bindSecretValidTrueCondition(1234),
770+
ldapConnectionValidUnknownCondition(1234),
771+
searchBaseFoundUnknownCondition(1234),
740772
{
741773
Type: "TLSConfigurationValid",
742774
Status: "False",
@@ -763,6 +795,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
763795
Phase: "Error",
764796
Conditions: []metav1.Condition{
765797
bindSecretValidTrueCondition(1234),
798+
ldapConnectionValidUnknownCondition(1234),
799+
searchBaseFoundUnknownCondition(1234),
766800
{
767801
Type: "TLSConfigurationValid",
768802
Status: "False",
@@ -1158,6 +1192,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
11581192
Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"),
11591193
ObservedGeneration: 42,
11601194
},
1195+
ldapConnectionValidUnknownCondition(42),
1196+
searchBaseFoundUnknownCondition(42),
11611197
tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"),
11621198
},
11631199
},

‎internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ func (c *gitHubWatcherController) validateGitHubConnection(
464464
Type: GitHubConnectionValid,
465465
Status: metav1.ConditionUnknown,
466466
Reason: conditionsutil.ReasonUnableToValidate,
467-
Message: "unable to validate; see other conditions for details",
467+
Message: conditionsutil.MessageUnableToValidate,
468468
}, nil, nil
469469
}
470470

‎internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ func (g *ldapUpstreamGenericLDAPImpl) Generation() int64 {
5050
return g.ldapIdentityProvider.Generation
5151
}
5252

53-
func (g *ldapUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus {
54-
return &ldapUpstreamGenericLDAPStatus{g.ldapIdentityProvider}
55-
}
56-
5753
type ldapUpstreamGenericLDAPSpec struct {
5854
ldapIdentityProvider idpv1alpha1.LDAPIdentityProvider
5955
}
@@ -78,10 +74,14 @@ func (s *ldapUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGen
7874
return &ldapUpstreamGenericLDAPGroupSearch{s.ldapIdentityProvider.Spec.GroupSearch}
7975
}
8076

77+
func (s *ldapUpstreamGenericLDAPSpec) UnknownSearchBaseCondition() *metav1.Condition {
78+
return nil // currently, only AD returns a condition for this
79+
}
80+
8181
func (s *ldapUpstreamGenericLDAPSpec) DetectAndSetSearchBase(_ context.Context, config *upstreamldap.ProviderConfig) *metav1.Condition {
8282
config.GroupSearch.Base = s.ldapIdentityProvider.Spec.GroupSearch.Base
8383
config.UserSearch.Base = s.ldapIdentityProvider.Spec.UserSearch.Base
84-
return nil
84+
return nil // currently, only AD returns a condition for this
8585
}
8686

8787
type ldapUpstreamGenericLDAPUserSearch struct {
@@ -124,14 +124,6 @@ func (g *ldapUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string {
124124
return g.groupSearch.Attributes.GroupName
125125
}
126126

127-
type ldapUpstreamGenericLDAPStatus struct {
128-
ldapIdentityProvider idpv1alpha1.LDAPIdentityProvider
129-
}
130-
131-
func (s *ldapUpstreamGenericLDAPStatus) Conditions() []metav1.Condition {
132-
return s.ldapIdentityProvider.Status.Conditions
133-
}
134-
135127
// UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations.
136128
type UpstreamLDAPIdentityProviderICache interface {
137129
SetLDAPIdentityProviders([]upstreamprovider.UpstreamLDAPIdentityProviderI)

‎internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
363363
ObservedGeneration: gen,
364364
}
365365
}
366+
366367
ldapConnectionValidTrueCondition := func(gen int64, secretVersion string) metav1.Condition {
367368
return metav1.Condition{
368369
Type: "LDAPConnectionValid",
@@ -380,9 +381,21 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
380381
c.LastTransitionTime = metav1.Time{}
381382
return c
382383
}
384+
ldapConnectionValidUnknownCondition := func(gen int64) metav1.Condition {
385+
return metav1.Condition{
386+
Type: "LDAPConnectionValid",
387+
Status: "Unknown",
388+
LastTransitionTime: now,
389+
Reason: "UnableToValidate",
390+
Message: "unable to validate; see other conditions for details",
391+
ObservedGeneration: gen,
392+
}
393+
}
394+
383395
condPtr := func(c metav1.Condition) *metav1.Condition {
384396
return &c
385397
}
398+
386399
tlsConfigurationValidLoadedTrueCondition := func(gen int64, msg string) metav1.Condition {
387400
return metav1.Condition{
388401
Type: "TLSConfigurationValid",
@@ -393,6 +406,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
393406
ObservedGeneration: gen,
394407
}
395408
}
409+
396410
allConditionsTrue := func(gen int64, secretVersion string) []metav1.Condition {
397411
return []metav1.Condition{
398412
bindSecretValidTrueCondition(gen),
@@ -588,6 +602,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
588602
Message: fmt.Sprintf(`secret "%s" not found`, testBindSecretName),
589603
ObservedGeneration: 1234,
590604
},
605+
ldapConnectionValidUnknownCondition(1234),
591606
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
592607
},
593608
},
@@ -616,6 +631,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
616631
Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testBindSecretName),
617632
ObservedGeneration: 1234,
618633
},
634+
ldapConnectionValidUnknownCondition(1234),
619635
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
620636
},
621637
},
@@ -643,6 +659,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
643659
Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testBindSecretName),
644660
ObservedGeneration: 1234,
645661
},
662+
ldapConnectionValidUnknownCondition(1234),
646663
tlsConfigurationValidLoadedTrueCondition(1234, "using configured CA bundle"),
647664
},
648665
},
@@ -662,6 +679,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
662679
Phase: "Error",
663680
Conditions: []metav1.Condition{
664681
bindSecretValidTrueCondition(1234),
682+
ldapConnectionValidUnknownCondition(1234),
665683
{
666684
Type: "TLSConfigurationValid",
667685
Status: "False",
@@ -688,6 +706,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
688706
Phase: "Error",
689707
Conditions: []metav1.Condition{
690708
bindSecretValidTrueCondition(1234),
709+
ldapConnectionValidUnknownCondition(1234),
691710
{
692711
Type: "TLSConfigurationValid",
693712
Status: "False",
@@ -981,6 +1000,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
9811000
Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"),
9821001
ObservedGeneration: 42,
9831002
},
1003+
ldapConnectionValidUnknownCondition(42),
9841004
tlsConfigurationValidLoadedTrueCondition(42, "using configured CA bundle"),
9851005
},
9861006
},

0 commit comments

Comments
 (0)