Minor refactor

Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2024-07-25 13:40:21 -05:00
committed by Ryan Richard
parent 9a16dc28b7
commit e3ed722252
2 changed files with 25 additions and 23 deletions

View File

@@ -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

View File

@@ -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)