diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index f2e1127db..aa3ace88f 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -48,13 +48,12 @@ import ( const ( controllerName = "jwtcachefiller-controller" - typeReady = "Ready" - typeTLSConfigurationValid = "TLSConfigurationValid" - typeIssuerURLValid = "IssuerURLValid" - typeDiscoveryValid = "DiscoveryURLValid" - typeJWKSURLValid = "JWKSURLValid" - typeJWKSFetchValid = "JWKSFetchValid" - typeAuthenticatorValid = "AuthenticatorValid" + typeReady = "Ready" + typeIssuerURLValid = "IssuerURLValid" + typeDiscoveryValid = "DiscoveryURLValid" + typeJWKSURLValid = "JWKSURLValid" + typeJWKSFetchValid = "JWKSFetchValid" + typeAuthenticatorValid = "AuthenticatorValid" reasonSuccess = "Success" reasonNotReady = "NotReady" @@ -66,7 +65,6 @@ const ( reasonInvalidIssuerURLContainsWellKnownEndpoint = "InvalidIssuerURLContainsWellKnownEndpoint" reasonInvalidProviderJWKSURL = "InvalidProviderJWKSURL" reasonInvalidProviderJWKSURLScheme = "InvalidProviderJWKSURLScheme" - reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidDiscoveryProbe = "InvalidDiscoveryProbe" reasonInvalidAuthenticator = "InvalidAuthenticator" reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS" @@ -117,7 +115,8 @@ type tokenAuthenticatorCloser interface { type cachedJWTAuthenticator struct { authenticator.Token - spec *authenticationv1alpha1.JWTAuthenticatorSpec + spec *authenticationv1alpha1.JWTAuthenticatorSpec + // TODO: maybe also keep track of the bytes of the CA bundle itself (or a hash of those bytes) that were used to validate previously?? cancel context.CancelFunc } @@ -220,6 +219,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { var jwtAuthenticatorFromCache *cachedJWTAuthenticator if valueFromCache := c.cache.Get(cacheKey); valueFromCache != nil { jwtAuthenticatorFromCache = c.cacheValueAsJWTAuthenticator(valueFromCache) + // TODO: this is only comparing the specs of the old/new JWTAuthenticator.... BUT the CA bundle from the ConfigMap or Secret could have also changed, so check that somehow if jwtAuthenticatorFromCache != nil && reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &obj.Spec) { c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer). Info("actual jwt authenticator and desired jwt authenticator are the same") @@ -231,7 +231,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { conditions := make([]*metav1.Condition, 0) var errs []error - rootCAs, conditions, tlsOk := c.validateTLSBundle(obj.Spec.TLS, c.namespace, conditions) + rootCAs, conditions, tlsOk := c.validateTLSBundle(obj.Spec.TLS, conditions) _, conditions, issuerOk := c.validateIssuer(obj.Spec.Issuer, conditions) okSoFar := tlsOk && issuerOk @@ -299,11 +299,11 @@ func (c *jwtCacheFillerController) cacheValueAsJWTAuthenticator(value authncache return jwtAuthenticator } -func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, namespace string, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) { - condition, _, rootCAs, _ := tlsconfigutil.ValidateTLSConfig( +func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) { + condition, _, rootCAs := tlsconfigutil.ValidateTLSConfig( tlsconfigutil.TlsSpecForConcierge(tlsSpec), "spec.tls", - namespace, + c.namespace, c.secretInformer, c.configMapInformer) diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index 5601b8b85..40d7c66bc 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -44,7 +44,6 @@ import ( const ( controllerName = "webhookcachefiller-controller" typeReady = "Ready" - typeTLSConfigurationValid = "TLSConfigurationValid" typeWebhookConnectionValid = "WebhookConnectionValid" typeEndpointURLValid = "EndpointURLValid" typeAuthenticatorValid = "AuthenticatorValid" @@ -53,7 +52,6 @@ const ( reasonUnableToValidate = "UnableToValidate" reasonUnableToCreateClient = "UnableToCreateClient" reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook" - reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidEndpointURL = "InvalidEndpointURL" reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme" reasonUnableToDialServer = "UnableToDialServer" @@ -342,7 +340,7 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo } func (c *webhookCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []byte, []*metav1.Condition, bool) { - condition, pemBytes, rootCAs, _ := tlsconfigutil.ValidateTLSConfig( + condition, pemBytes, rootCAs := tlsconfigutil.ValidateTLSConfig( tlsconfigutil.TlsSpecForConcierge(tlsSpec), "spec.tls", c.namespace, diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index 383ba20dd..fd667ca10 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/internal/controller/conditionsutil/conditions_util.go @@ -13,6 +13,7 @@ import ( ) const ( + // TODO: why only move one here, why not more? ReasonSuccess = "Success" ) diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index 48e812d99..9a58f7e80 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -282,6 +282,7 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro tlsConfigCondition, certPool := c.validateTLSConfiguration(upstream.Spec.GitHubAPI.TLS) conditions = append(conditions, tlsConfigCondition) + // TODO: skip this if it is already validated for this same spec and CA bundle (or perhaps hash of CA bundle) githubConnectionCondition, hostURL, httpClient, githubConnectionErr := c.validateGitHubConnection( hostPort, certPool, @@ -373,7 +374,7 @@ func validateHost(gitHubAPIConfig idpv1alpha1.GitHubAPIConfig) (*metav1.Conditio } func (c *gitHubWatcherController) validateTLSConfiguration(tlsSpec *idpv1alpha1.TLSSpec) (*metav1.Condition, *x509.CertPool) { - tlsCondition, _, certPool, _ := tlsconfigutil.ValidateTLSConfig( + tlsCondition, _, certPool := tlsconfigutil.ValidateTLSConfig( tlsconfigutil.TLSSpecForSupervisor(tlsSpec), "spec.githubAPI.tls", c.namespace, diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 0248fa3b0..d076246d7 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -2347,9 +2347,8 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - secretInformer := kubeInformers.Core().V1().Secrets() - configMapInformer := kubeInformers.Core().V1().ConfigMaps() observableInformers := testutil.NewObservableWithInformerOption() + secretInformer := kubeInformers.Core().V1().Secrets() _ = New( "some-namespace", @@ -2357,7 +2356,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { supervisorfake.NewSimpleClientset(), supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), secretInformer, - configMapInformer, + kubeInformers.Core().V1().ConfigMaps(), logger, observableInformers.WithInformer, clock.RealClock{}, @@ -2390,7 +2389,7 @@ func TestGitHubUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { wantDelete bool }{ { - name: "a configMap in the right namespace", + name: "any ConfigMap", cm: goodCM, wantAdd: true, wantUpdate: true, @@ -2404,16 +2403,14 @@ func TestGitHubUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - gitHubIdentityProviderInformer := supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() observableInformers := testutil.NewObservableWithInformerOption() - configMapInformer := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().ConfigMaps() _ = New( namespace, dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), supervisorfake.NewSimpleClientset(), - gitHubIdentityProviderInformer, + supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().Secrets(), configMapInformer, logger, @@ -2448,7 +2445,7 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { wantDelete bool }{ { - name: "an IDP in the right namespace", + name: "any GitHubIdentityProvider", idp: goodIDP, wantAdd: true, wantUpdate: true, @@ -2462,8 +2459,8 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - gitHubIdentityProviderInformer := supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() observableInformers := testutil.NewObservableWithInformerOption() + gitHubIdentityProviderInformer := supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() _ = New( namespace, diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index e49cc04f9..ea7503253 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -324,15 +324,17 @@ func (c *oidcWatcherController) validateSecret(upstream *idpv1alpha1.OIDCIdentit // validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition. func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *idpv1alpha1.OIDCIdentityProvider, result *upstreamoidc.ProviderConfig) []*metav1.Condition { - // Get the provider and HTTP Client from cache if possible. - discoveredProvider, httpClient := c.validatorCache.getProvider(&upstream.Spec) - tlsCondition, _, certPool, _ := tlsconfigutil.ValidateTLSConfig( + tlsCondition, _, certPool := tlsconfigutil.ValidateTLSConfig( tlsconfigutil.TLSSpecForSupervisor(upstream.Spec.TLS), "spec.tls", upstream.Namespace, c.secretInformer, c.configMapInformer) + // TODO: If either the spec or the CA bundle has changed, then we need to redo the validations below. So maybe the cache key should be the combination of spec and bundle (or hash of bundle)? + // Get the provider and HTTP Client from cache if possible. + discoveredProvider, httpClient := c.validatorCache.getProvider(&upstream.Spec) + // If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache. if discoveredProvider == nil { var err error diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 5d949e011..81d791d80 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -254,7 +254,7 @@ func ValidateGenericLDAP( conditions.Append(secretValidCondition, true) tlsSpec := tlsconfigutil.TLSSpecForSupervisor(upstream.Spec().TLSSpec()) - tlsValidCondition, caBundle, _, _ := tlsconfigutil.ValidateTLSConfig(tlsSpec, "spec.tls", upstream.Namespace(), secretInformer, configMapInformer) + tlsValidCondition, caBundle, _ := tlsconfigutil.ValidateTLSConfig(tlsSpec, "spec.tls", upstream.Namespace(), secretInformer, configMapInformer) conditions.Append(tlsValidCondition, true) config.CABundle = caBundle @@ -277,6 +277,7 @@ func validateAndSetLDAPServerConnectivityAndSearchBase( config *upstreamldap.ProviderConfig, currentSecretVersion string, ) (*metav1.Condition, *metav1.Condition) { + // TODO: if the CA bundle has changed, then we should redo the below connection probes. So maybe this cache should also include the CA bundle (or the hash of the bundle) as part of the lookup? validatedSettings, hasPreviousValidatedSettings := validatedSettingsCache.Get(upstream.Name(), currentSecretVersion, upstream.Generation()) var ldapConnectionValidCondition, searchBaseFoundCondition *metav1.Condition diff --git a/internal/controller/tlsconfigutil/tls_config_util.go b/internal/controller/tlsconfigutil/tls_config_util.go index 74278a608..cf7261fac 100644 --- a/internal/controller/tlsconfigutil/tls_config_util.go +++ b/internal/controller/tlsconfigutil/tls_config_util.go @@ -149,33 +149,31 @@ func getCertPool( // 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. +// - 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 +// TODO: should this show the resource version of the Secret/ConfigMap to the user on all conditions? func ValidateTLSConfig( tlsSpec *TLSSpec, conditionPrefix string, namespace string, secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, -) (*metav1.Condition, []byte, *x509.CertPool, error) { +) (*metav1.Condition, []byte, *x509.CertPool) { // try to build a x509 cert pool using the ca data specified in the tlsSpec. certPool, bundle, err := getCertPool(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 + return invalidTLSCondition(err.Error()), nil, nil } // 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, noTLSConfigurationMessage)), bundle, certPool } - return validTLSCondition(fmt.Sprintf("%s is valid: %s", conditionPrefix, loadedTLSConfigurationMessage)), bundle, certPool, err + return validTLSCondition(fmt.Sprintf("%s is valid: %s", conditionPrefix, loadedTLSConfigurationMessage)), bundle, certPool } func readCABundleFromSource(source *caBundleSource, namespace string, secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer) (string, error) { @@ -190,33 +188,37 @@ func readCABundleFromSource(source *caBundleSource, namespace string, secretInfo } func readCABundleFromK8sSecret(namespace string, name string, key string, secretInformer corev1informers.SecretInformer) (string, error) { + namespacedName := fmt.Sprintf("%s/%s", namespace, name) + s, err := secretInformer.Lister().Secrets(namespace).Get(name) if err != nil { - return "", errors.Wrapf(err, "failed to get secret %s/%s", namespace, name) + return "", errors.Wrapf(err, "failed to get secret %q", namespacedName) } // for kubernetes secrets to be used as a certificate authority data source, the secret should be of type // kubernetes.io/tls or Opaque. It is an error to use a secret that is of any other type. if s.Type != corev1.SecretTypeTLS && s.Type != corev1.SecretTypeOpaque { - return "", fmt.Errorf("secret %s/%s of type %s cannot be used as a certificate authority data source", namespace, name, s.Type) + return "", fmt.Errorf("secret %q of type %q cannot be used as a certificate authority data source", namespacedName, s.Type) } // 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) + return "", fmt.Errorf("key %q not found in secret %q", key, namespacedName) } func readCABundleFromK8sConfigMap(namespace string, name string, key string, configMapInformer corev1informers.ConfigMapInformer) (string, error) { + namespacedName := fmt.Sprintf("%s/%s", namespace, name) + c, err := configMapInformer.Lister().ConfigMaps(namespace).Get(name) if err != nil { - return "", errors.Wrapf(err, "failed to get configmap %s/%s", namespace, name) + return "", errors.Wrapf(err, "failed to get configmap %q", namespacedName) } // 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) + return "", fmt.Errorf("key %q not found in configmap %q", key, namespacedName) } func validTLSCondition(message string) *metav1.Condition { diff --git a/internal/controller/tlsconfigutil/tls_config_util_test.go b/internal/controller/tlsconfigutil/tls_config_util_test.go index 96e02aa1a..89d7490f7 100644 --- a/internal/controller/tlsconfigutil/tls_config_util_test.go +++ b/internal/controller/tlsconfigutil/tls_config_util_test.go @@ -193,7 +193,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityDataSource is invalid: secret awesome-namespace/awesome-secret-ba of type kubernetes.io/basic-auth cannot be used as a certificate authority data source", + Message: `tls.certificateAuthorityDataSource is invalid: secret "awesome-namespace/awesome-secret-ba" of type "kubernetes.io/basic-auth" cannot be used as a certificate authority data source`, }, }, @@ -250,7 +250,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityDataSource is invalid: failed to get secret awesome-namespace/does-not-exist: secret \"does-not-exist\" not found", + Message: `tls.certificateAuthorityDataSource is invalid: failed to get secret "awesome-namespace/does-not-exist": secret "does-not-exist" not found`, }, }, { @@ -278,7 +278,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityDataSource is invalid: failed to get configmap awesome-namespace/does-not-exist: configmap \"does-not-exist\" not found", + Message: `tls.certificateAuthorityDataSource is invalid: failed to get configmap "awesome-namespace/does-not-exist": configmap "does-not-exist" not found`, }, }, { @@ -333,7 +333,7 @@ func TestValidateTLSConfig(t *testing.T) { sharedInformers.Start(ctx.Done()) sharedInformers.WaitForCacheSync(ctx.Done()) } - actualCondition, _, _, _ := ValidateTLSConfig(tt.tlsSpec, "tls", tt.namespace, secretsInformer, configMapInformer) + actualCondition, _, _ := ValidateTLSConfig(tt.tlsSpec, "tls", tt.namespace, secretsInformer, configMapInformer) require.Equal(t, tt.expectedCondition, actualCondition) }) } @@ -347,46 +347,8 @@ func TestReadCABundleFromK8sSecret(t *testing.T) { secretKey string k8sObjects []runtime.Object expectedData string - expectError bool + expectError string }{ - { - 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 tls secret and existing key", secretNamespace: "awesome-namespace", @@ -405,7 +367,7 @@ func TestReadCABundleFromK8sSecret(t *testing.T) { }, }, expectedData: "pinniped-is-awesome", - expectError: false, + expectError: "", }, { name: "should return data from existing opaque secret and existing key", @@ -425,7 +387,66 @@ func TestReadCABundleFromK8sSecret(t *testing.T) { }, }, expectedData: "pinniped-is-awesome", - expectError: false, + expectError: "", + }, + { + 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: `failed to get secret "awesome-namespace/does-not-exist": secret "does-not-exist" not found`, + }, + { + name: "should return error when secret has wrong type", + secretNamespace: "awesome-namespace", + secretName: "awesome-secret", + secretKey: "awesome", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Type: "other-type", + Data: map[string][]byte{ + "awesome": []byte("pinniped-is-awesome"), + }, + }, + }, + + expectError: `secret "awesome-namespace/awesome-secret" of type "other-type" cannot be used as a certificate authority data source`, + }, + { + 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", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "awesome": []byte("pinniped-is-awesome"), + }, + }, + }, + expectedData: "", + expectError: `key "something-else" not found in secret "awesome-namespace/awesome-secret"`, }, } @@ -446,8 +467,8 @@ func TestReadCABundleFromK8sSecret(t *testing.T) { sharedInformers.WaitForCacheSync(ctx.Done()) // 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) + if tt.expectError != "" { + require.ErrorContains(t, actualError, tt.expectError) } else { require.NoError(t, actualError) } @@ -464,8 +485,26 @@ func TestReadCABundleFromK8sConfigMap(t *testing.T) { configMapKey string k8sObjects []runtime.Object expectedData string - expectError bool + expectError string }{ + { + 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", + }, { name: "should return error reading a non-existent configMap", configMapNamespace: "awesome-namespace", @@ -483,7 +522,7 @@ func TestReadCABundleFromK8sConfigMap(t *testing.T) { }, }, expectedData: "", - expectError: true, + expectError: `failed to get configmap "awesome-namespace/does-not-exist": configmap "does-not-exist" not found`, }, { name: "should return error reading a non-existing key in an existing configMap", @@ -502,26 +541,7 @@ func TestReadCABundleFromK8sConfigMap(t *testing.T) { }, }, 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, + expectError: `key "does-not-exist" not found in configmap "awesome-namespace/awesome-configmap"`, }, } @@ -542,8 +562,8 @@ func TestReadCABundleFromK8sConfigMap(t *testing.T) { sharedInformers.Start(ctx.Done()) sharedInformers.WaitForCacheSync(ctx.Done()) actualData, actualError := readCABundleFromK8sConfigMap(tt.configMapNamespace, tt.configMapName, tt.configMapKey, configMapInformer) - if tt.expectError { - require.Error(t, actualError) + if tt.expectError != "" { + require.ErrorContains(t, actualError, tt.expectError) } else { require.NoError(t, actualError) } diff --git a/test/integration/concierge_tls_spec_test.go b/test/integration/concierge_tls_spec_test.go index 845f77a1f..5cdfcb4af 100644 --- a/test/integration/concierge_tls_spec_test.go +++ b/test/integration/concierge_tls_spec_test.go @@ -28,6 +28,8 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { customResourceName string expectedError string }{ + // TODO: these "spec.endpoint" could use the real URL of the local-user-authenticator + // TODO: should we repeat these tests using the JWTAuthenticator too? { name: "should disallow certificate authority data source with missing name", customResourceYaml: here.Doc(` diff --git a/test/integration/supervisor_tls_spec_test.go b/test/integration/supervisor_tls_spec_test.go index b7d80b608..d3724fbdd 100644 --- a/test/integration/supervisor_tls_spec_test.go +++ b/test/integration/supervisor_tls_spec_test.go @@ -28,6 +28,8 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { customResourceName string expectedError string }{ + // TODO: use the OIDC provider from env instead of bar.com + // TODO: make ths a loop to also run the same tests on LDAP, AD, GitHub?? { name: "should disallow certificate authority data source with missing name", customResourceYaml: here.Doc(`