diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index 38004b907..383ba20dd 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/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. diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index efe64d055..b7813a9fb 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/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()) 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 80e47d7ca..78fd5380c 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/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, ) diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 820de4b48..bae4dc8b3 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/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()) diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 7d3d4f738..d9fa8cf62 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/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. diff --git a/internal/controller/tlsconfigutil/tls_config_util.go b/internal/controller/tlsconfigutil/tls_config_util.go new file mode 100644 index 000000000..8c380579d --- /dev/null +++ b/internal/controller/tlsconfigutil/tls_config_util.go @@ -0,0 +1,181 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tlsconfigutil + +import ( + "crypto/x509" + "encoding/base64" + "fmt" + + "github.com/pkg/errors" + v12 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers/core/v1" + + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/controller/conditionsutil" +) + +const ( + ReasonInvalidTLSConfig = "InvalidTLSConfig" + + noTLSConfigurationMessage = "no TLS configuration provided" + loadedTLSConfigurationMessage = "loaded TLS configuration" + typeTLSConfigurationValid = "TLSConfigurationValid" + + ErrNoCertificates = constable.Error("no certificates found") +) + +// BuildCertPoolIDP reads the tlsSpec of the IDP and returns an X509 cert pool with the CA data that is read either from +// the inline tls.certificateAuthorityData or from a kubernetes secret or a config map as specified in the +// tls.certificateAuthorityDataSource. +// If the provided tlsSpec is nil, a nil CA bundle will be returned. +// If the provided spec contains a CA bundle that is not properly encoded, an error will be returned. +// TODO: should this function be exposed outside this package? +func BuildCertPoolIDP( + tlsSpec *v1alpha1.TLSSpec, + conditionPrefix string, + namespace string, + secretInformer v1.SecretInformer, + configMapInformer v1.ConfigMapInformer, +) (*x509.CertPool, []byte, error) { + // if tlsSpec is nil, we return a nil cert pool and cert bundle. A nil error is also returned to indicate that + // a nil tlsSpec is nevertheless a valid one resulting in a valid TLS condition. + if tlsSpec == nil { + return nil, nil, nil + } + + // it is a configuration error to specify a ca bundle inline using the tls.certificateAuthorityDataSource field + // and also specifying a kubernetes secret or a config map to serve as the source for the ca bundle. + if len(tlsSpec.CertificateAuthorityData) > 0 && tlsSpec.CertificateAuthorityDataSource != nil { + return nil, nil, fmt.Errorf("%s is invalid: both tls.certificateAuthorityDataSource and tls.certificateAuthorityData provided", conditionPrefix) + } + + var err error + caBundle := tlsSpec.CertificateAuthorityData + field := fmt.Sprintf("%s.%s", conditionPrefix, "certificateAuthorityData") + // currently, the ca data supplied inline in the CRDs is expected to be base64 encoded. + // However, the ca data read from kubernetes secrets or config map will not be base64 encoded. + // For kubernetes secrets, secret data read using the client-go code automatically decodes base64 encoded values. + // So a base64 decode is required only when fetching ca bundle from the tls.certificateAuthorityData field. + decodeRequired := true + if tlsSpec.CertificateAuthorityDataSource != nil { + decodeRequired = false + // track the path of the field in the tlsSpec from which the CA data is sourced. + // this will be used to report in the condition status in case an invalid TLS condition is encountered. + field = fmt.Sprintf("%s.%s", conditionPrefix, "certificateAuthorityDataSource") + caBundle, err = readCABundleFromSource(tlsSpec.CertificateAuthorityDataSource, namespace, secretInformer, configMapInformer) + if err != nil { + return nil, nil, fmt.Errorf("%s is invalid: failed to read CA bundle from source %s/%s/%s: %s", + field, tlsSpec.CertificateAuthorityDataSource.Kind, tlsSpec.CertificateAuthorityDataSource.Name, + tlsSpec.CertificateAuthorityDataSource.Key, err.Error()) + } + } + + if len(caBundle) == 0 { + return nil, nil, nil + } + + bundleBytes := []byte(caBundle) + if decodeRequired { + bundleBytes, err = base64.StdEncoding.DecodeString(caBundle) + if err != nil { + return nil, nil, fmt.Errorf("%s is invalid: %s", conditionPrefix, err.Error()) + } + } + + // try to create a cert pool with the read ca data to determine validity of the ca bundle read from the tlsSpec. + ca := x509.NewCertPool() + ok := ca.AppendCertsFromPEM(bundleBytes) + if !ok { + return nil, nil, fmt.Errorf("%s is invalid: %s", field, ErrNoCertificates) + } + + return ca, bundleBytes, nil +} + +// ValidateTLSConfig reads ca bundle in the tlsSpec, supplied either inline using the CertificateAuthorityDate +// or as a reference to a kubernetes secret or configmap using the CertificateAuthorityDataSource, and returns +// a condition of type TLSConfigurationValid based on the validity of the ca bundle, +// a pem encoded ca bundle +// a X509 cert pool with the ca bundle +// any error encountered. +// TODO: it should suffice that this function returns a TLSConfigurationValid condition, and perhaps we could skip +// returning the error. This can be done once all controllers are able to use this function. +func ValidateTLSConfig( + tlsSpec *v1alpha1.TLSSpec, + conditionPrefix string, + namespace string, + secretInformer v1.SecretInformer, + configMapInformer v1.ConfigMapInformer, +) (*v12.Condition, []byte, *x509.CertPool, error) { + // try to build a x509 cert pool using the ca data specified in the tlsSpec. + certPool, bundle, err := BuildCertPoolIDP(tlsSpec, conditionPrefix, namespace, secretInformer, configMapInformer) + if err != nil { + // an error encountered during building a certpool using the ca data from the tlsSpec results in an invalid + // TLS condition. + return invalidTLSCondition(err.Error()), nil, nil, err + } + // for us, an empty or nil ca bundle read is results in a valid TLS condition, but we do want to convey that + // no ca data was supplied. + if bundle == nil { + return validTLSCondition(fmt.Sprintf("%s is valid: %s", conditionPrefix, noTLSConfigurationMessage)), bundle, certPool, err + } + + return validTLSCondition(fmt.Sprintf("%s is valid: %s", conditionPrefix, loadedTLSConfigurationMessage)), bundle, certPool, err +} + +func readCABundleFromSource(source *v1alpha1.CABundleSource, namespace string, secretInformer v1.SecretInformer, configMapInformer v1.ConfigMapInformer) (string, error) { + switch source.Kind { + case "Secret": + return readCABundleFromK8sSecret(namespace, source.Name, source.Key, secretInformer) + case "ConfigMap": + return readCABundleFromK8sConfigMap(namespace, source.Name, source.Key, configMapInformer) + default: + return "", fmt.Errorf("unsupported CA bundle source: %s", source.Kind) + } +} + +func readCABundleFromK8sSecret(namespace string, name string, key string, secretInformer v1.SecretInformer) (string, error) { + s, err := secretInformer.Lister().Secrets(namespace).Get(name) + if err != nil { + return "", errors.Wrapf(err, "failed to get secret %s/%s", namespace, name) + } + // ca bundle in the secret is expected to exist in a specific key, if that key does not exist, then it is an error + if val, exists := s.Data[key]; exists { + return string(val), nil + } + return "", fmt.Errorf("key %s not found in secret %s/%s", key, namespace, name) +} + +func readCABundleFromK8sConfigMap(namespace string, name string, key string, configMapInformer v1.ConfigMapInformer) (string, error) { + c, err := configMapInformer.Lister().ConfigMaps(namespace).Get(name) + if err != nil { + return "", errors.Wrapf(err, "failed to get configmap %s/%s", namespace, name) + } + + // ca bundle in the secret is expected to exist in a specific key, if that key does not exist, then it is an error + if val, exists := c.Data[key]; exists { + return val, nil + } + return "", fmt.Errorf("key %s not found in configmap %s/%s", key, namespace, name) +} + +func validTLSCondition(message string) *v12.Condition { + return &v12.Condition{ + Type: typeTLSConfigurationValid, + Status: v12.ConditionTrue, + Reason: conditionsutil.ReasonSuccess, + Message: message, + } +} + +func invalidTLSCondition(message string) *v12.Condition { + return &v12.Condition{ + Type: typeTLSConfigurationValid, + Status: v12.ConditionFalse, + Reason: ReasonInvalidTLSConfig, + Message: message, + } +} diff --git a/internal/controller/tlsconfigutil/tls_config_util_test.go b/internal/controller/tlsconfigutil/tls_config_util_test.go new file mode 100644 index 000000000..f776d1e65 --- /dev/null +++ b/internal/controller/tlsconfigutil/tls_config_util_test.go @@ -0,0 +1,432 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tlsconfigutil + +import ( + "encoding/base64" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/informers" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes/fake" + + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/internal/certauthority" +) + +func TestValidateTLSConfig(t *testing.T) { + testCA, err := certauthority.New("Test CA", 1*time.Hour) + require.NoError(t, err) + bundle := testCA.Bundle() + base64EncodedBundle := base64.StdEncoding.EncodeToString(bundle) + tests := []struct { + name string + tlsSpec *idpv1alpha1.TLSSpec + namespace string + k8sObjects []runtime.Object + expectedCondition *metav1.Condition + expectError bool + }{ + { + name: "nil TLSSpec should generate a noTLSConfigurationMessage condition", + tlsSpec: nil, + expectedCondition: validTLSCondition(noTLSConfigurationMessage), + expectError: false, + }, + { + name: "empty inline ca data should generate a loadedTLSConfigurationMessage condition", + tlsSpec: &idpv1alpha1.TLSSpec{}, + expectedCondition: validTLSCondition(loadedTLSConfigurationMessage), + expectError: false, + }, + { + name: "valid base64 encode ca data should generate a loadedTLSConfigurationMessage condition", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64EncodedBundle, + }, + expectedCondition: validTLSCondition(loadedTLSConfigurationMessage), + expectError: false, + }, + { + name: "valid base64 encoded non cert data should generate a invalidTLSCondition condition", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: "dGhpcyBpcyBzb21lIHRlc3QgZGF0YSB0aGF0IGlzIGJhc2U2NCBlbmNvZGVkIHRoYXQgaXMgbm90IGEgY2VydAo=", + }, + expectedCondition: invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", ErrNoCertificates)), + expectError: true, + }, + { + name: "non-base64 encoded string as ca data should generate an invalidTLSCondition condition", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: "non base64 encoded string", + }, + expectedCondition: invalidTLSCondition("certificateAuthorityData is invalid: illegal base64 data"), + expectError: true, + }, + { + name: "supplying certificateAuthorityDataSource and certificateAuthorityData should generate an invalid condition", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64EncodedBundle, + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "Secret", + Name: "super-secret", + Key: "ca-base64EncodedBundle", + }, + }, + expectedCondition: invalidTLSCondition("tls spec config error: both tls.certificateAuthorityDataSource and tls.certificateAuthorityData provided."), + expectError: true, + }, + { + name: "should return ca bundle from kubernetes secret", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "Secret", + Name: "awesome-secret", + Key: "ca-bundle", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Data: map[string][]byte{ + "ca-bundle": []byte(bundle), + }, + }, + }, + expectedCondition: validTLSCondition(fmt.Sprintf("tls is valid: %s", loadedTLSConfigurationMessage)), + expectError: false, + }, + { + name: "should return ca bundle from kubernetes configMap", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "ConfigMap", + Name: "awesome-cm", + Key: "ca-bundle", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-cm", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "ca-bundle": string(bundle), + }, + }, + }, + expectedCondition: validTLSCondition(fmt.Sprintf("tls is valid: %s", loadedTLSConfigurationMessage)), + expectError: false, + }, + { + name: "should return invalid condition when failing to read ca bundle from kubernetes secret that does not exist", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "Secret", + Name: "does-not-exist", + Key: "does-not-matter", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-cm", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "ca-bundle": string(bundle), + }, + }, + }, + expectedCondition: invalidTLSCondition("tls.certificateAuthorityDataSource is invalid: failed to read from source"), + expectError: true, + }, + { + name: "should return invalid condition when failing to read ca bundle from kubernetes configMap that does not exist", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "ConfigMap", + Name: "does-not-exist", + Key: "does-not-matter", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-cm", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "ca-bundle": string(bundle), + }, + }, + }, + expectedCondition: invalidTLSCondition("tls.certificateAuthorityDataSource is invalid: failed to read from source"), + expectError: true, + }, + { + name: "should return invalid condition when using an invalid certificate authority data source", + tlsSpec: &idpv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &idpv1alpha1.CABundleSource{ + Kind: "SomethingElse", + Name: "does-not-exist", + Key: "does-not-matter", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-cm", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "ca-bundle": string(bundle), + }, + }, + }, + expectedCondition: invalidTLSCondition("tls.certificateAuthorityDataSource is invalid: unsupported CA bundle source"), + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var secretsInformer corev1informers.SecretInformer + var configMapInformer corev1informers.ConfigMapInformer + + if len(tt.k8sObjects) > 0 { + stopSecretInformer := make(chan struct{}) + stopConfigMapInformer := make(chan struct{}) + fakeClient := fake.NewSimpleClientset(tt.k8sObjects...) + sharedInformers := informers.NewSharedInformerFactory(fakeClient, time.Second) + configMapInformer = sharedInformers.Core().V1().ConfigMaps() + secretsInformer = sharedInformers.Core().V1().Secrets() + + // Run the informer so that it can sync the objects from kubernetes into its cache. + // run as a go routine so that we can stop the informer and continue with our tests. + go secretsInformer.Informer().Run(stopSecretInformer) + // wait 1s before stopping the informer. 1s because, that's the resync duration of the informer. + time.Sleep(time.Second) + close(stopSecretInformer) + // TODO: can we avoid calling Run on both informers? + go configMapInformer.Informer().Run(stopConfigMapInformer) + time.Sleep(time.Second) + close(stopConfigMapInformer) + // now the objects from kubernetes should be sync'd into the informer cache. + } + actualCondition, _, _, err := ValidateTLSConfig(tt.tlsSpec, "tls", tt.namespace, secretsInformer, configMapInformer) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.expectedCondition.Type, actualCondition.Type) + require.Equal(t, tt.expectedCondition.Status, actualCondition.Status) + require.Equal(t, tt.expectedCondition.Reason, actualCondition.Reason) + } + }) + } +} + +func TestReadCABundleFromK8sSecret(t *testing.T) { + tests := []struct { + name string + secretNamespace string + secretName string + secretKey string + k8sObjects []runtime.Object + expectedData string + expectError bool + }{ + { + name: "should return error reading a non-existent secret", + secretNamespace: "awesome-namespace", + secretName: "does-not-exist", + secretKey: "does-not-matter", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Data: map[string][]byte{ + "awesome": []byte("pinniped-is-awesome"), + }, + }, + }, + expectedData: "", + expectError: true, + }, + { + name: "should return error reading a non-existing key in an existing secret", + secretNamespace: "awesome-namespace", + secretName: "awesome-secret", + secretKey: "something-else", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Data: map[string][]byte{ + "awesome": []byte("pinniped-is-awesome"), + }, + }, + }, + expectedData: "", + expectError: true, + }, + { + name: "should return data from existing secret and existing key", + secretNamespace: "awesome-namespace", + secretName: "awesome-secret", + secretKey: "awesome", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Data: map[string][]byte{ + "awesome": []byte("pinniped-is-awesome"), + }, + }, + }, + expectedData: "pinniped-is-awesome", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + stop := make(chan struct{}) + fakeClient := fake.NewSimpleClientset(tt.k8sObjects...) + secretsInformer := informers.NewSharedInformerFactory(fakeClient, time.Second).Core().V1().Secrets() + // Run the informer so that it can sync the objects from kubernetes into its cache. + // run as a go routine so that we can stop the informer and continue with our tests. + go secretsInformer.Informer().Run(stop) + // wait 1s before stopping the informer. 1s because, that's the resync duration of the informer. + time.Sleep(time.Second) + close(stop) + // now the objects from kubernetes should be sync'd into the informer cache. + actualData, actualError := readCABundleFromK8sSecret(tt.secretNamespace, tt.secretName, tt.secretKey, secretsInformer) + if tt.expectError { + require.Error(t, actualError) + } else { + require.NoError(t, actualError) + } + require.Equal(t, tt.expectedData, actualData) + }) + } +} + +func TestReadCABundleFromK8sConfigMap(t *testing.T) { + tests := []struct { + name string + configMapNamespace string + configMapName string + configMapKey string + k8sObjects []runtime.Object + expectedData string + expectError bool + }{ + { + name: "should return error reading a non-existent configMap", + configMapNamespace: "awesome-namespace", + configMapName: "does-not-exist", + configMapKey: "does-not-matter", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-configmap", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "awesome": "pinniped-is-awesome", + }, + }, + }, + expectedData: "", + expectError: true, + }, + { + name: "should return error reading a non-existing key in an existing configMap", + configMapNamespace: "awesome-namespace", + configMapName: "awesome-configmap", + configMapKey: "does-not-exist", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-configmap", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "awesome": "pinniped-is-awesome", + }, + }, + }, + expectedData: "", + expectError: true, + }, + { + name: "should return expected data from an existing key in an existing configMap", + configMapNamespace: "awesome-namespace", + configMapName: "awesome-configmap", + configMapKey: "awesome", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-configmap", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "awesome": "pinniped-is-awesome", + }, + }, + }, + expectedData: "pinniped-is-awesome", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + stop := make(chan struct{}) + fakeClient := fake.NewSimpleClientset(tt.k8sObjects...) + configMapInformer := informers.NewSharedInformerFactory(fakeClient, time.Second).Core().V1().ConfigMaps() + + // Run the informer so that it can sync the objects from kubernetes into its cache. + // run as a go routine so that we can stop the informer and continue with our tests. + go configMapInformer.Informer().Run(stop) + // wait 1s before stopping the informer. 1s because, that's the resync duration of the informer. + time.Sleep(time.Second) + close(stop) + // now the objects from kubernetes should be sync'd into the informer cache. + actualData, actualError := readCABundleFromK8sConfigMap(tt.configMapNamespace, tt.configMapName, tt.configMapKey, configMapInformer) + if tt.expectError { + require.Error(t, actualError) + } else { + require.NoError(t, actualError) + } + require.Equal(t, tt.expectedData, actualData) + }) + } +} diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index e3d796127..4dbf79245 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -154,6 +154,7 @@ func prepareControllers( federationDomainInformer := pinnipedInformers.Config().V1alpha1().FederationDomains() oidcClientInformer := pinnipedInformers.Config().V1alpha1().OIDCClients() secretInformer := kubeInformers.Core().V1().Secrets() + configMapInformer := kubeInformers.Core().V1().ConfigMaps() // Create controller manager. controllerManager := controllerlib. @@ -322,6 +323,7 @@ func prepareControllers( pinnipedClient, pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(), secretInformer, + configMapInformer, controllerlib.WithInformer, ), singletonWorker).