refactor tls spec validation into its own package

Signed-off-by: Ashish Amarnath <ashish.amarnath@broadcom.com>

internal/controller/conditionsutil/conditions_util.go

@@ -12,6 +12,10 @@ import (
 	"go.pinniped.dev/internal/plog"
 )
 
+const (
+	ReasonSuccess = "Success"
+)
+
 // MergeConditions merges conditions into conditionsToUpdate.
 // Note that LastTransitionTime refers to the time when the status changed,
 // but ObservedGeneration should be the current generation for all conditions, since Pinniped should always check every condition.

internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go

@@ -155,7 +155,7 @@ func (s *activeDirectoryUpstreamGenericLDAPSpec) DetectAndSetSearchBase(ctx cont
 	return &metav1.Condition{
 		Type:    upstreamwatchers.TypeSearchBaseFound,
 		Status:  metav1.ConditionTrue,
-		Reason:  upstreamwatchers.ReasonSuccess,
+		Reason:  conditionsutil.ReasonSuccess,
 		Message: "Successfully fetched defaultNamingContext to use as default search base from RootDSE.",
 	}
 }
@@ -235,6 +235,7 @@ type activeDirectoryWatcherController struct {
 	client                                  supervisorclientset.Interface
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
 	secretInformer                          corev1informers.SecretInformer
+	configMapInformer                       corev1informers.ConfigMapInformer
 }
 
 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamActiveDirectoryIdentityProviderICache.
@@ -243,6 +244,7 @@ func New(
 	client supervisorclientset.Interface,
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,
 	secretInformer corev1informers.SecretInformer,
+	configMapInformer corev1informers.ConfigMapInformer,
 	withInformer pinnipedcontroller.WithInformerOptionFunc,
 ) controllerlib.Controller {
 	return newInternal(
@@ -254,6 +256,7 @@ func New(
 		client,
 		activeDirectoryIdentityProviderInformer,
 		secretInformer,
+		configMapInformer,
 		withInformer,
 	)
 }
@@ -266,6 +269,7 @@ func newInternal(
 	client supervisorclientset.Interface,
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,
 	secretInformer corev1informers.SecretInformer,
+	configMapInformer corev1informers.ConfigMapInformer,
 	withInformer pinnipedcontroller.WithInformerOptionFunc,
 ) controllerlib.Controller {
 	c := activeDirectoryWatcherController{
@@ -275,6 +279,7 @@ func newInternal(
 		client:                                  client,
 		activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer,
 		secretInformer:                          secretInformer,
+		configMapInformer:                       configMapInformer,
 	}
 	return controllerlib.New(
 		controllerlib.Config{Name: activeDirectoryControllerName, Syncer: &c},
@@ -357,7 +362,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
 		}
 	}
 
-	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSettingsCache, config)
+	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.configMapInformer, c.validatedSettingsCache, config)
 
 	c.updateStatus(ctx, upstream, conditions.Conditions())
 

internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go

@@ -80,9 +80,10 @@ func TestActiveDirectoryUpstreamWatcherControllerFilterSecrets(t *testing.T) {
 			fakeKubeClient := fake.NewSimpleClientset()
 			kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0)
 			secretInformer := kubeInformers.Core().V1().Secrets()
+			configMapInformer := kubeInformers.Core().V1().ConfigMaps()
 			withInformer := testutil.NewObservableWithInformerOption()
 
-			New(nil, nil, activeDirectoryIDPInformer, secretInformer, withInformer.WithInformer)
+			New(nil, nil, activeDirectoryIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer)
 
 			unrelated := corev1.Secret{}
 			filter := withInformer.GetFilterForInformer(secretInformer)
@@ -124,9 +125,10 @@ func TestActiveDirectoryUpstreamWatcherControllerFilterActiveDirectoryIdentityPr
 			fakeKubeClient := fake.NewSimpleClientset()
 			kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0)
 			secretInformer := kubeInformers.Core().V1().Secrets()
+			configMapInformer := kubeInformers.Core().V1().ConfigMaps()
 			withInformer := testutil.NewObservableWithInformerOption()
 
-			New(nil, nil, activeDirectoryIDPInformer, secretInformer, withInformer.WithInformer)
+			New(nil, nil, activeDirectoryIDPInformer, secretInformer, configMapInformer, withInformer.WithInformer)
 
 			unrelated := corev1.Secret{}
 			filter := withInformer.GetFilterForInformer(activeDirectoryIDPInformer)
@@ -2048,6 +2050,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				fakePinnipedClient,
 				pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(),
 				kubeInformers.Core().V1().Secrets(),
+				kubeInformers.Core().V1().ConfigMaps(),
 				controllerlib.WithInformer,
 			)
 

internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go

@@ -143,6 +143,7 @@ type ldapWatcherController struct {
 	client                       supervisorclientset.Interface
 	ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer
 	secretInformer               corev1informers.SecretInformer
+	configMapInformer            corev1informers.ConfigMapInformer
 }
 
 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamLDAPIdentityProviderICache.
@@ -249,7 +250,7 @@ func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *
 		Dialer: c.ldapDialer,
 	}
 
-	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSettingsCache, config)
+	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.configMapInformer, c.validatedSettingsCache, config)
 
 	c.updateStatus(ctx, upstream, conditions.Conditions())
 

internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go

@@ -5,8 +5,6 @@ package upstreamwatchers
 
 import (
 	"context"
-	"crypto/x509"
-	"encoding/base64"
 	"fmt"
 	"time"
 
@@ -15,32 +13,27 @@ import (
 	corev1informers "k8s.io/client-go/informers/core/v1"
 
 	idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
-	"go.pinniped.dev/internal/constable"
+	"go.pinniped.dev/internal/controller/conditionsutil"
+	"go.pinniped.dev/internal/controller/tlsconfigutil"
 	"go.pinniped.dev/internal/federationdomain/upstreamprovider"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/upstreamldap"
 )
 
 const (
-	ReasonNotFound         = "SecretNotFound"
-	ReasonWrongType        = "SecretWrongType"
-	ReasonMissingKeys      = "SecretMissingKeys"
-	ReasonSuccess          = "Success"
-	ReasonInvalidTLSConfig = "InvalidTLSConfig"
-
-	ErrNoCertificates = constable.Error("no certificates found")
+	ReasonNotFound    = "SecretNotFound"
+	ReasonWrongType   = "SecretWrongType"
+	ReasonMissingKeys = "SecretMissingKeys"
 
 	LDAPBindAccountSecretType = corev1.SecretTypeBasicAuth
 	probeLDAPTimeout          = 90 * time.Second
 
 	// Constants related to conditions.
-	typeBindSecretValid              = "BindSecretValid"
-	typeTLSConfigurationValid        = "TLSConfigurationValid"
-	typeLDAPConnectionValid          = "LDAPConnectionValid"
-	TypeSearchBaseFound              = "SearchBaseFound"
-	reasonLDAPConnectionError        = "LDAPConnectionError"
-	noTLSConfigurationMessage        = "no TLS configuration provided"
-	loadedTLSConfigurationMessage    = "loaded TLS configuration"
+	typeBindSecretValid       = "BindSecretValid"
+	typeLDAPConnectionValid   = "LDAPConnectionValid"
+	TypeSearchBaseFound       = "SearchBaseFound"
+	reasonLDAPConnectionError = "LDAPConnectionError"
+
 	ReasonUsingConfigurationFromSpec = "UsingConfigurationFromSpec"
 	ReasonErrorFetchingSearchBase    = "ErrorFetchingSearchBase"
 )
@@ -135,29 +128,6 @@ type UpstreamGenericLDAPStatus interface {
 	Conditions() []metav1.Condition
 }
 
-func ValidateTLSConfig(tlsSpec *idpv1alpha1.TLSSpec, config *upstreamldap.ProviderConfig) *metav1.Condition {
-	if tlsSpec == nil {
-		return validTLSCondition(noTLSConfigurationMessage)
-	}
-	if len(tlsSpec.CertificateAuthorityData) == 0 {
-		return validTLSCondition(loadedTLSConfigurationMessage)
-	}
-
-	bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData)
-	if err != nil {
-		return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error()))
-	}
-
-	ca := x509.NewCertPool()
-	ok := ca.AppendCertsFromPEM(bundle)
-	if !ok {
-		return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", ErrNoCertificates))
-	}
-
-	config.CABundle = bundle
-	return validTLSCondition(loadedTLSConfigurationMessage)
-}
-
 func TestConnection(
 	ctx context.Context,
 	bindSecretName string,
@@ -200,30 +170,12 @@ func TestConnection(
 	return &metav1.Condition{
 		Type:   typeLDAPConnectionValid,
 		Status: metav1.ConditionTrue,
-		Reason: ReasonSuccess,
+		Reason: conditionsutil.ReasonSuccess,
 		Message: fmt.Sprintf(`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 			config.Host, config.BindUsername, bindSecretName, currentSecretVersion),
 	}
 }
 
-func validTLSCondition(message string) *metav1.Condition {
-	return &metav1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  metav1.ConditionTrue,
-		Reason:  ReasonSuccess,
-		Message: message,
-	}
-}
-
-func invalidTLSCondition(message string) *metav1.Condition {
-	return &metav1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  metav1.ConditionFalse,
-		Reason:  ReasonInvalidTLSConfig,
-		Message: message,
-	}
-}
-
 func ValidateSecret(secretInformer corev1informers.SecretInformer, secretName string, secretNamespace string, config *upstreamldap.ProviderConfig) (*metav1.Condition, string) {
 	secret, err := secretInformer.Lister().Secrets(secretNamespace).Get(secretName)
 	if err != nil {
@@ -260,7 +212,7 @@ func ValidateSecret(secretInformer corev1informers.SecretInformer, secretName st
 	return &metav1.Condition{
 		Type:    typeBindSecretValid,
 		Status:  metav1.ConditionTrue,
-		Reason:  ReasonSuccess,
+		Reason:  conditionsutil.ReasonSuccess,
 		Message: "loaded bind secret",
 	}, secret.ResourceVersion
 }
@@ -292,6 +244,7 @@ func ValidateGenericLDAP(
 	ctx context.Context,
 	upstream UpstreamGenericLDAPIDP,
 	secretInformer corev1informers.SecretInformer,
+	configMapInformer corev1informers.ConfigMapInformer,
 	validatedSettingsCache ValidatedSettingsCacheI,
 	config *upstreamldap.ProviderConfig,
 ) GradatedConditions {
@@ -300,8 +253,9 @@ func ValidateGenericLDAP(
 	secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config)
 	conditions.Append(secretValidCondition, true)
 
-	tlsValidCondition := ValidateTLSConfig(upstream.Spec().TLSSpec(), config)
+	tlsValidCondition, caBundle, _, _ := tlsconfigutil.ValidateTLSConfig(upstream.Spec().TLSSpec(), "", upstream.Namespace(), secretInformer, configMapInformer)
 	conditions.Append(tlsValidCondition, true)
+	config.CABundle = caBundle
 
 	var ldapConnectionValidCondition, searchBaseFoundCondition *metav1.Condition
 	// No point in trying to connect to the server if the config was already determined to be invalid.