diff --git a/internal/controller/tlsconfigutil/tls_config_util.go b/internal/controller/tlsconfigutil/tls_config_util.go index 121609517..14d1e26dd 100644 --- a/internal/controller/tlsconfigutil/tls_config_util.go +++ b/internal/controller/tlsconfigutil/tls_config_util.go @@ -15,7 +15,6 @@ import ( authenticationv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" - "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/controller/conditionsutil" ) @@ -25,8 +24,6 @@ const ( noTLSConfigurationMessage = "no TLS configuration provided" loadedTLSConfigurationMessage = "loaded TLS configuration" typeTLSConfigurationValid = "TLSConfigurationValid" - - ErrNoCertificates = constable.Error("no certificates found") ) type caBundleSource struct { @@ -87,7 +84,6 @@ func TLSSpecForConcierge(source *authenticationv1alpha1.TLSSpec) *TLSSpec { // - 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, @@ -95,6 +91,12 @@ func ValidateTLSConfig( secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, ) (*metav1.Condition, []byte, *x509.CertPool) { + // TODO: This func should return a struct that abstracts away the internals of how a CA bundle is held in memory + // and can return the CA bundle as string PEM, []byte base64-encoded, CertPool, hash, etc, as well as compare itself + // to either a different struct instance or a hash. + // + // TODO: There could easily be a hash type struct alias for the specific hash value (e.g. "[32]byte") with an Equality function. + certPool, bundle, err := getCertPool(tlsSpec, conditionPrefix, namespace, secretInformer, configMapInformer) if err != nil { return invalidTLSCondition(err.Error()), nil, nil @@ -165,7 +167,7 @@ func getCertPool( ca := x509.NewCertPool() ok := ca.AppendCertsFromPEM(bundleBytes) if !ok { - return nil, nil, fmt.Errorf("%s is invalid: %s", field, ErrNoCertificates) + return nil, nil, fmt.Errorf("%s is invalid: no certificates found", field) } return ca, bundleBytes, nil diff --git a/internal/controller/tlsconfigutil/tls_config_util_test.go b/internal/controller/tlsconfigutil/tls_config_util_test.go index 25b11f1e8..cff871cb2 100644 --- a/internal/controller/tlsconfigutil/tls_config_util_test.go +++ b/internal/controller/tlsconfigutil/tls_config_util_test.go @@ -48,7 +48,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: " + noTLSConfigurationMessage, + Message: "spec.foo.tls is valid: " + noTLSConfigurationMessage, }, }, { @@ -58,7 +58,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: " + noTLSConfigurationMessage, + Message: "spec.foo.tls is valid: " + noTLSConfigurationMessage, }, }, { @@ -72,7 +72,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: " + loadedTLSConfigurationMessage, + Message: "spec.foo.tls is valid: " + loadedTLSConfigurationMessage, }, }, { @@ -84,7 +84,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityData is invalid: " + ErrNoCertificates.Error(), + Message: "spec.foo.tls.certificateAuthorityData is invalid: no certificates found", }, }, { @@ -96,7 +96,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityData is invalid: illegal base64 data at input byte 3", + Message: "spec.foo.tls.certificateAuthorityData is invalid: illegal base64 data at input byte 3", }, }, { @@ -113,7 +113,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls is invalid: both tls.certificateAuthorityDataSource and tls.certificateAuthorityData provided", + Message: "spec.foo.tls is invalid: both tls.certificateAuthorityDataSource and tls.certificateAuthorityData provided", }, }, { @@ -144,7 +144,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: loaded TLS configuration", + Message: "spec.foo.tls is valid: loaded TLS configuration", }, }, { @@ -175,7 +175,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: loaded TLS configuration", + Message: "spec.foo.tls is valid: loaded TLS configuration", }, }, { @@ -204,7 +204,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: `spec.foo.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`, }, }, { @@ -233,7 +233,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: `tls.certificateAuthorityDataSource is invalid: key "ca-bundle" not found in secret "awesome-namespace/awesome-secret"`, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" not found in secret "awesome-namespace/awesome-secret"`, }, }, { @@ -262,7 +262,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: `tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in secret "awesome-namespace/awesome-secret"`, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in secret "awesome-namespace/awesome-secret"`, }, }, { @@ -290,7 +290,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: `tls.certificateAuthorityDataSource is invalid: key "ca-bundle" not found in configmap "awesome-namespace/awesome-configmap"`, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" not found in configmap "awesome-namespace/awesome-configmap"`, }, }, { @@ -318,7 +318,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: `tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in configmap "awesome-namespace/awesome-configmap"`, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in configmap "awesome-namespace/awesome-configmap"`, }, }, { @@ -348,7 +348,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "tls is valid: loaded TLS configuration", + Message: "spec.foo.tls is valid: loaded TLS configuration", }, }, { @@ -366,7 +366,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: `spec.foo.tls.certificateAuthorityDataSource is invalid: failed to get secret "awesome-namespace/does-not-exist": secret "does-not-exist" not found`, }, }, { @@ -384,7 +384,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: `spec.foo.tls.certificateAuthorityDataSource is invalid: failed to get configmap "awesome-namespace/does-not-exist": configmap "does-not-exist" not found`, }, }, { @@ -412,7 +412,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "tls.certificateAuthorityDataSource is invalid: unsupported CA bundle source kind: SomethingElse", + Message: "spec.foo.tls.certificateAuthorityDataSource is invalid: unsupported CA bundle source kind: SomethingElse", }, }, } @@ -442,7 +442,7 @@ func TestValidateTLSConfig(t *testing.T) { // which would do this same call for us. sharedInformers.WaitForCacheSync(ctx.Done()) - actualCondition, actualBundle, actualCertPool := ValidateTLSConfig(tt.tlsSpec, "tls", tt.namespace, secretsInformer, configMapInformer) + actualCondition, actualBundle, actualCertPool := ValidateTLSConfig(tt.tlsSpec, "spec.foo.tls", tt.namespace, secretsInformer, configMapInformer) require.Equal(t, tt.expectedCondition, actualCondition) require.Equal(t, tt.expectedBundle, actualBundle)