Refactor some variable names and update one comment

Change variable names to match previously renamed interface name.
This commit is contained in:
Ryan Richard 2022-01-14 10:06:00 -08:00
parent 438b58193d
commit 092a80f849
3 changed files with 27 additions and 14 deletions

View File

@ -226,7 +226,7 @@ type UpstreamActiveDirectoryIdentityProviderICache interface {
type activeDirectoryWatcherController struct { type activeDirectoryWatcherController struct {
cache UpstreamActiveDirectoryIdentityProviderICache cache UpstreamActiveDirectoryIdentityProviderICache
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI validatedSettingsCache upstreamwatchers.ValidatedSettingsCacheI
ldapDialer upstreamldap.LDAPDialer ldapDialer upstreamldap.LDAPDialer
client pinnipedclientset.Interface client pinnipedclientset.Interface
activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
@ -257,7 +257,7 @@ func New(
// For test dependency injection purposes. // For test dependency injection purposes.
func newInternal( func newInternal(
idpCache UpstreamActiveDirectoryIdentityProviderICache, idpCache UpstreamActiveDirectoryIdentityProviderICache,
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI, validatedSettingsCache upstreamwatchers.ValidatedSettingsCacheI,
ldapDialer upstreamldap.LDAPDialer, ldapDialer upstreamldap.LDAPDialer,
client pinnipedclientset.Interface, client pinnipedclientset.Interface,
activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,
@ -266,7 +266,7 @@ func newInternal(
) controllerlib.Controller { ) controllerlib.Controller {
c := activeDirectoryWatcherController{ c := activeDirectoryWatcherController{
cache: idpCache, cache: idpCache,
validatedSecretVersionsCache: validatedSecretVersionsCache, validatedSettingsCache: validatedSettingsCache,
ldapDialer: ldapDialer, ldapDialer: ldapDialer,
client: client, client: client,
activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer, activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer,
@ -351,7 +351,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
} }
} }
conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config) conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSettingsCache, config)
c.updateStatus(ctx, upstream, conditions.Conditions()) c.updateStatus(ctx, upstream, conditions.Conditions())

View File

@ -134,7 +134,7 @@ type UpstreamLDAPIdentityProviderICache interface {
type ldapWatcherController struct { type ldapWatcherController struct {
cache UpstreamLDAPIdentityProviderICache cache UpstreamLDAPIdentityProviderICache
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI validatedSettingsCache upstreamwatchers.ValidatedSettingsCacheI
ldapDialer upstreamldap.LDAPDialer ldapDialer upstreamldap.LDAPDialer
client pinnipedclientset.Interface client pinnipedclientset.Interface
ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer
@ -165,7 +165,7 @@ func New(
// For test dependency injection purposes. // For test dependency injection purposes.
func newInternal( func newInternal(
idpCache UpstreamLDAPIdentityProviderICache, idpCache UpstreamLDAPIdentityProviderICache,
validatedSecretVersionsCache upstreamwatchers.ValidatedSettingsCacheI, validatedSettingsCache upstreamwatchers.ValidatedSettingsCacheI,
ldapDialer upstreamldap.LDAPDialer, ldapDialer upstreamldap.LDAPDialer,
client pinnipedclientset.Interface, client pinnipedclientset.Interface,
ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer,
@ -174,7 +174,7 @@ func newInternal(
) controllerlib.Controller { ) controllerlib.Controller {
c := ldapWatcherController{ c := ldapWatcherController{
cache: idpCache, cache: idpCache,
validatedSecretVersionsCache: validatedSecretVersionsCache, validatedSettingsCache: validatedSettingsCache,
ldapDialer: ldapDialer, ldapDialer: ldapDialer,
client: client, client: client,
ldapIdentityProviderInformer: ldapIdentityProviderInformer, ldapIdentityProviderInformer: ldapIdentityProviderInformer,
@ -243,7 +243,7 @@ func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *
Dialer: c.ldapDialer, Dialer: c.ldapDialer,
} }
conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config) conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSettingsCache, config)
c.updateStatus(ctx, upstream, conditions.Conditions()) c.updateStatus(ctx, upstream, conditions.Conditions())

View File

@ -286,7 +286,13 @@ func (g *GradatedConditions) Append(condition *v1alpha1.Condition, isFatal bool)
g.gradatedConditions = append(g.gradatedConditions, gradatedCondition{condition: condition, isFatal: isFatal}) g.gradatedConditions = append(g.gradatedConditions, gradatedCondition{condition: condition, isFatal: isFatal})
} }
func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache ValidatedSettingsCacheI, config *upstreamldap.ProviderConfig) GradatedConditions { func ValidateGenericLDAP(
ctx context.Context,
upstream UpstreamGenericLDAPIDP,
secretInformer corev1informers.SecretInformer,
validatedSettingsCache ValidatedSettingsCacheI,
config *upstreamldap.ProviderConfig,
) GradatedConditions {
conditions := GradatedConditions{} conditions := GradatedConditions{}
secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config) secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config)
@ -298,7 +304,7 @@ func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, s
var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition
// No point in trying to connect to the server if the config was already determined to be invalid. // No point in trying to connect to the server if the config was already determined to be invalid.
if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue { if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue {
ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase(ctx, validatedSecretVersionsCache, upstream, config, currentSecretVersion) ldapConnectionValidCondition, searchBaseFoundCondition = validateAndSetLDAPServerConnectivityAndSearchBase(ctx, validatedSettingsCache, upstream, config, currentSecretVersion)
conditions.Append(ldapConnectionValidCondition, false) conditions.Append(ldapConnectionValidCondition, false)
if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil if searchBaseFoundCondition != nil { // currently, only used for AD, so may be nil
conditions.Append(searchBaseFoundCondition, true) conditions.Append(searchBaseFoundCondition, true)
@ -307,8 +313,14 @@ func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, s
return conditions return conditions
} }
func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, validatedSecretVersionsCache ValidatedSettingsCacheI, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) (*v1alpha1.Condition, *v1alpha1.Condition) { func validateAndSetLDAPServerConnectivityAndSearchBase(
validatedSettings, hasPreviousValidatedSettings := validatedSecretVersionsCache.Get(upstream.Name(), currentSecretVersion, upstream.Generation()) ctx context.Context,
validatedSettingsCache ValidatedSettingsCacheI,
upstream UpstreamGenericLDAPIDP,
config *upstreamldap.ProviderConfig,
currentSecretVersion string,
) (*v1alpha1.Condition, *v1alpha1.Condition) {
validatedSettings, hasPreviousValidatedSettings := validatedSettingsCache.Get(upstream.Name(), currentSecretVersion, upstream.Generation())
var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition
if hasPreviousValidatedSettings && validatedSettings.UserSearchBase != "" && validatedSettings.GroupSearchBase != "" { if hasPreviousValidatedSettings && validatedSettings.UserSearchBase != "" && validatedSettings.GroupSearchBase != "" {
@ -329,13 +341,14 @@ func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, vali
searchBaseFoundCondition = upstream.Spec().DetectAndSetSearchBase(searchBaseTimeout, config) searchBaseFoundCondition = upstream.Spec().DetectAndSetSearchBase(searchBaseTimeout, config)
// When there were no failures, write the newly validated settings to the cache. // When there were no failures, write the newly validated settings to the cache.
// It's okay for the search base condition to be nil, but if it exists make sure it was not a failure. // It's okay for the search base condition to be nil, since it's only used by Active Directory providers,
// but if it exists make sure it was not a failure.
if ldapConnectionValidCondition.Status == v1alpha1.ConditionTrue && if ldapConnectionValidCondition.Status == v1alpha1.ConditionTrue &&
(searchBaseFoundCondition == nil || (searchBaseFoundCondition.Status == v1alpha1.ConditionTrue)) { (searchBaseFoundCondition == nil || (searchBaseFoundCondition.Status == v1alpha1.ConditionTrue)) {
// Remember (in-memory for this pod) that the controller has successfully validated the LDAP or AD provider // Remember (in-memory for this pod) that the controller has successfully validated the LDAP or AD provider
// using this version of the Secret. This is for performance reasons, to avoid attempting to connect to // using this version of the Secret. This is for performance reasons, to avoid attempting to connect to
// the LDAP server more than is needed. If the pod restarts, it will attempt this validation again. // the LDAP server more than is needed. If the pod restarts, it will attempt this validation again.
validatedSecretVersionsCache.Set(upstream.Name(), ValidatedSettings{ validatedSettingsCache.Set(upstream.Name(), ValidatedSettings{
IDPSpecGeneration: upstream.Generation(), IDPSpecGeneration: upstream.Generation(),
BindSecretResourceVersion: currentSecretVersion, BindSecretResourceVersion: currentSecretVersion,
LDAPConnectionProtocol: config.ConnectionProtocol, LDAPConnectionProtocol: config.ConnectionProtocol,