LDAP and AD IDPs now always report condition with type LDAPConnectionValid, even if the status is unknown

Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2024-08-06 13:03:00 -05:00
committed by Ryan Richard
parent 1c59a41cc5
commit afa3aa2232
10 changed files with 58 additions and 32 deletions

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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.

View File

@@ -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)

View File

@@ -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"),
},
},

View File

@@ -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
}

View File

@@ -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)

View File

@@ -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"),
},
},

View File

@@ -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,
}

View File

@@ -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
}