When reading CA bundle from a secret/configmap, return more specific err

When the bundle does not contain any certs, make the error more specific. Co-authored-by: Ryan Richard <richardry@vmware.com>
2026-01-07 14:05:50 +00:00 · 2024-07-25 14:19:17 -05:00
parent e3ed722252
commit 242fa8afb2
6 changed files with 80 additions and 13 deletions
--- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go
@@ -598,7 +598,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 							Status:             "False",
 							LastTransitionTime: now,
 							Reason:             "InvalidTLSConfig",
-							Message:            "spec.tls.certificateAuthorityData is invalid: no certificates found",
+							Message:            `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
 							ObservedGeneration: 1234,
 						},
 					},
--- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go
@@ -1394,7 +1394,7 @@ func TestController(t *testing.T) {
 							buildGitHubConnectionValidUnknown(t),
 							buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host),
 							buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy),
-							buildTLSConfigurationValidFalse(t, "spec.githubAPI.tls.certificateAuthorityData is invalid: no certificates found"),
+							buildTLSConfigurationValidFalse(t, `spec.githubAPI.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 4 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`),
 						},
 					},
 				},
@@ -1404,7 +1404,7 @@ func TestController(t *testing.T) {
 				buildLogForUpdatingClaimsValidTrue("some-idp-name"),
 				buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))),
 				buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host),
-				buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", "spec.githubAPI.tls.certificateAuthorityData is invalid: no certificates found"),
+				buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", `spec.githubAPI.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 4 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")`),
 				buildLogForUpdatingGitHubConnectionValidUnknown("some-idp-name"),
 				buildLogForUpdatingPhase("some-idp-name", "Error"),
 			},
--- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go
@@ -527,7 +527,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 							Status:             "False",
 							LastTransitionTime: now,
 							Reason:             "InvalidTLSConfig",
-							Message:            "spec.tls.certificateAuthorityData is invalid: no certificates found",
+							Message:            `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
 							ObservedGeneration: 1234,
 						},
 					},
--- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
@@ -431,11 +431,11 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 			wantErr: controllerlib.ErrSyntheticRequeue.Error(),
 			wantLogs: []string{
 				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`,
-				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found"}`,
-				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`,
+				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`,
 				`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:<line>$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`,
-				`{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:<line>$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found","error":"OIDCIdentityProvider has a failing condition"}`,
-				`{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:<line>$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found","error":"OIDCIdentityProvider has a failing condition"}`,
+				`{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:<line>$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`,
+				`{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:<line>$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`,
 			},
 			wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
 			wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{
@@ -447,9 +447,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 						{Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: now, Reason: "Success",
 							Message: "loaded client credentials"},
 						{Type: "OIDCDiscoverySucceeded", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig",
-							Message: `spec.tls.certificateAuthorityData is invalid: no certificates found`},
+							Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`},
 						{Type: "TLSConfigurationValid", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig",
-							Message: "spec.tls.certificateAuthorityData is invalid: no certificates found"},
+							Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`},
 					},
 				},
 			}},
--- a/internal/controller/tlsconfigutil/tls_config_util.go
+++ b/internal/controller/tlsconfigutil/tls_config_util.go
@@ -7,6 +7,7 @@ import (
 	"crypto/x509"
 	"encoding/base64"
 	"fmt"
+	"strings"

 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -83,7 +84,7 @@ func TLSSpecForConcierge(source *authenticationv1alpha1.TLSSpec) *TLSSpec {
 // 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
+// - a X509 cert pool with the ca bundle.
 func ValidateTLSConfig(
 	tlsSpec *TLSSpec,
 	conditionPrefix string,
@@ -134,6 +135,7 @@ func getCertPool(

 	var err error
 	caBundle := tlsSpec.CertificateAuthorityData
+	caBundleLength := len(caBundle)
 	field := fmt.Sprintf("%s.%s", conditionPrefix, "certificateAuthorityData")
 	// 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.
@@ -146,6 +148,7 @@ func getCertPool(
 		// 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)
+		caBundleLength = len(caBundle)
 		if err != nil {
 			return nil, nil, fmt.Errorf("%s is invalid: %s", field, err.Error())
 		}
@@ -167,7 +170,14 @@ func getCertPool(
 	ca := x509.NewCertPool()
 	ok := ca.AppendCertsFromPEM(bundleBytes)
 	if !ok {
-		return nil, nil, fmt.Errorf("%s is invalid: no certificates found", field)
+		if decodeRequired {
+			return nil, nil, fmt.Errorf("%s is invalid: no base64-encoded PEM certificates found in %d bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")",
+				field, caBundleLength)
+		}
+		namespacedName := fmt.Sprintf("%s/%s", namespace, tlsSpec.CertificateAuthorityDataSource.Name)
+
+		return nil, nil, fmt.Errorf(`%s is invalid: key %q with %d bytes of data in %s %q is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
+			field, tlsSpec.CertificateAuthorityDataSource.Key, caBundleLength, strings.ToLower(tlsSpec.CertificateAuthorityDataSource.Kind), namespacedName)
 	}

 	return ca, bundleBytes, nil
--- a/internal/controller/tlsconfigutil/tls_config_util_test.go
+++ b/internal/controller/tlsconfigutil/tls_config_util_test.go
@@ -84,7 +84,7 @@ func TestValidateTLSConfig(t *testing.T) {
 				Type:    typeTLSConfigurationValid,
 				Status:  metav1.ConditionFalse,
 				Reason:  ReasonInvalidTLSConfig,
-				Message: "spec.foo.tls.certificateAuthorityData is invalid: no certificates found",
+				Message: `spec.foo.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 88 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
 			},
 		},
 		{
@@ -265,6 +265,35 @@ func TestValidateTLSConfig(t *testing.T) {
 				Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in secret "awesome-namespace/awesome-secret"`,
 			},
 		},
+		{
+			name: "should return invalid condition when a secret has the configured key but the value is not a cert",
+			tlsSpec: &TLSSpec{
+				CertificateAuthorityDataSource: &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",
+					},
+					Type: corev1.SecretTypeOpaque,
+					Data: map[string][]byte{
+						"ca-bundle": []byte("this is not a certificate"),
+					},
+				},
+			},
+			expectedCondition: &metav1.Condition{
+				Type:    typeTLSConfigurationValid,
+				Status:  metav1.ConditionFalse,
+				Reason:  ReasonInvalidTLSConfig,
+				Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" with 25 bytes of data in secret "awesome-namespace/awesome-secret" is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
+			},
+		},
 		{
 			name: "should return invalid condition when a configmap does not have the configured key",
 			tlsSpec: &TLSSpec{
@@ -321,6 +350,34 @@ func TestValidateTLSConfig(t *testing.T) {
 				Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in configmap "awesome-namespace/awesome-configmap"`,
 			},
 		},
+		{
+			name: "should return invalid condition when a configmap has the configured key but its value not a cert",
+			tlsSpec: &TLSSpec{
+				CertificateAuthorityDataSource: &caBundleSource{
+					Kind: "ConfigMap",
+					Name: "awesome-configmap",
+					Key:  "ca-bundle",
+				},
+			},
+			namespace: "awesome-namespace",
+			k8sObjects: []runtime.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "awesome-configmap",
+						Namespace: "awesome-namespace",
+					},
+					Data: map[string]string{
+						"ca-bundle": "this is not a cert",
+					},
+				},
+			},
+			expectedCondition: &metav1.Condition{
+				Type:    typeTLSConfigurationValid,
+				Status:  metav1.ConditionFalse,
+				Reason:  ReasonInvalidTLSConfig,
+				Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" with 18 bytes of data in configmap "awesome-namespace/awesome-configmap" is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`,
+			},
+		},
 		{
 			name: "should return ca bundle from kubernetes configMap",
 			tlsSpec: &TLSSpec{