From db2d7c8c50c7ec04565bb2389b1412c185aad37c Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Sat, 3 Aug 2024 16:35:44 -0700 Subject: [PATCH] assert on condition message in concierge_tls_spec_test.go and supervisor_tls_spec_test.go --- test/integration/concierge_tls_spec_test.go | 395 +++++++++++++++---- test/integration/supervisor_tls_spec_test.go | 332 +++++++++++----- 2 files changed, 535 insertions(+), 192 deletions(-) diff --git a/test/integration/concierge_tls_spec_test.go b/test/integration/concierge_tls_spec_test.go index 85d8103db..2aaefff1b 100644 --- a/test/integration/concierge_tls_spec_test.go +++ b/test/integration/concierge_tls_spec_test.go @@ -12,18 +12,26 @@ import ( "regexp" "strings" "testing" + "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/here" "go.pinniped.dev/test/testlib" ) // TestTLSSpecKubeBuilderValidationConcierge_Parallel tests kubebuilder validation on the TLSSpec // in Pinniped concierge CRDs for both WebhookAuthenticators and JWTAuthenticators. -func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { +func TestTLSSpecValidationConcierge_Parallel(t *testing.T) { env := testlib.IntegrationEnv(t) + ca, err := certauthority.New("pinniped-test", 24*time.Hour) + require.NoError(t, err) + indentedCAPEM := indentForHeredoc(string(ca.Bundle())) + webhookAuthenticatorYamlTemplate := here.Doc(` apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -46,110 +54,149 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { `) testCases := []struct { - name string - tlsYAML string - expectedErrorSnippets []string + name string + + tlsYAML func(secretOrConfigmapName string) string + + secretOrConfigmapKind string + secretType string + secretOrConfigmapDataYAML string + + wantErrorSnippets []string + wantTLSValidConditionMessage string }{ { name: "should disallow certificate authority data source with missing name", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`}, }, { name: "should disallow certificate authority data source with empty value for name", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: "" - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: "" + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, }, { name: "should disallow certificate authority data source with missing key", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`}, }, { name: "should disallow certificate authority data source with empty value for key", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - key: "" - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + key: "" + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, }, { name: "should disallow certificate authority data source with missing kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`}, }, { name: "should disallow certificate authority data source with empty value for kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: "" - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: "" + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`}, }, { name: "should disallow certificate authority data source with invalid kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: sorcery - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: sorcery + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`}, }, { - name: "should create a custom resource passing all validations using a Secret source", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - key: bar - `), - expectedErrorSnippets: nil, + name: "should create a custom resource passing all validations using a Secret source of type Opaque", + secretOrConfigmapKind: "Secret", + secretType: string(corev1.SecretTypeOpaque), + secretOrConfigmapDataYAML: here.Docf(` + bar: | + %s + `, indentedCAPEM), + tlsYAML: func(secretOrConfigmapName string) string { + return here.Docf(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: %s + key: bar + `, secretOrConfigmapName) + }, + wantErrorSnippets: nil, + wantTLSValidConditionMessage: `spec.tls is valid: using configured CA bundle`, }, { - name: "should create a custom resource passing all validations using a ConfigMap source", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: ConfigMap - name: foo - key: bar - `), - expectedErrorSnippets: nil, + name: "should create a custom resource passing all validations using a ConfigMap source", + secretOrConfigmapKind: "ConfigMap", + secretOrConfigmapDataYAML: here.Docf(` + bar: | + %s + `, indentedCAPEM), + tlsYAML: func(secretOrConfigmapName string) string { + return here.Docf(` + tls: + certificateAuthorityDataSource: + kind: ConfigMap + name: %s + key: bar + `, secretOrConfigmapName) + }, + wantErrorSnippets: nil, + wantTLSValidConditionMessage: `spec.tls is valid: using configured CA bundle`, }, { - name: "should create a custom resource without any tls spec", - tlsYAML: "", - expectedErrorSnippets: nil, + name: "should create a custom resource without any tls spec", + tlsYAML: func(secretOrConfigmapName string) string { return "" }, + wantErrorSnippets: nil, + wantTLSValidConditionMessage: "spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image", }, } @@ -157,44 +204,216 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - // Further indent every line except for the first line by four spaces. - // Use four spaces because that's what here.Doc uses. - // Do not indent the first line because the template already indents it. - indentedTLSYAML := strings.ReplaceAll(tc.tlsYAML, "\n", "\n ") - t.Run("apply webhook authenticator", func(t *testing.T) { resourceName := "test-webhook-authenticator-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.ConciergeNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + yamlBytes := []byte(fmt.Sprintf(webhookAuthenticatorYamlTemplate, - env.APIGroupSuffix, resourceName, env.TestWebhook.Endpoint, indentedTLSYAML)) + env.APIGroupSuffix, resourceName, env.TestWebhook.Endpoint, + indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName)))) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`webhookauthenticator.authentication.concierge.%s`, env.APIGroupSuffix), - tc.expectedErrorSnippets, + tc.wantErrorSnippets, "WebhookAuthenticator", resourceName, ) + + if tc.wantErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.ConciergeNamespace, + "WebhookAuthenticator", + tc.wantTLSValidConditionMessage, + ) + } }) t.Run("apply jwt authenticator", func(t *testing.T) { _, supervisorIssuer := env.InferSupervisorIssuerURL(t) resourceName := "test-jwt-authenticator-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.ConciergeNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + yamlBytes := []byte(fmt.Sprintf(jwtAuthenticatorYamlTemplate, - env.APIGroupSuffix, resourceName, supervisorIssuer, indentedTLSYAML)) + env.APIGroupSuffix, resourceName, supervisorIssuer, + indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName)))) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`jwtauthenticator.authentication.concierge.%s`, env.APIGroupSuffix), - tc.expectedErrorSnippets, + tc.wantErrorSnippets, "JWTAuthenticator", resourceName, ) + + if tc.wantErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.ConciergeNamespace, + "JWTAuthenticator", + tc.wantTLSValidConditionMessage, + ) + } }) }) } } +func indentForHeredoc(s string) string { + // Further indent every line except for the first line by four spaces. + // Use four spaces because that's what here.Doc uses. + // Do not indent the first line because the template already indents it. + return strings.ReplaceAll(s, "\n", "\n ") +} + +func requireTLSValidConditionMessageOnResource(t *testing.T, resourceName string, namespace string, resourceType string, wantMessage string) { + t.Helper() + + require.NotEmpty(t, resourceName, "bad test setup: empty resourceName") + require.NotEmpty(t, resourceType, "bad test setup: empty resourceType") + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + t.Cleanup(cancel) + + conciergeAuthClient := testlib.NewConciergeClientset(t).AuthenticationV1alpha1() + supervisorIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1() + + switch resourceType { + case "JWTAuthenticator": + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := conciergeAuthClient.JWTAuthenticators().Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + case "WebhookAuthenticator": + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := conciergeAuthClient.WebhookAuthenticators().Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + case "OIDCIdentityProvider": + require.NotEmpty(t, namespace, "bad test setup: empty namespace") + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := supervisorIDPClient.OIDCIdentityProviders(namespace).Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + case "LDAPIdentityProvider": + require.NotEmpty(t, namespace, "bad test setup: empty namespace") + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := supervisorIDPClient.LDAPIdentityProviders(namespace).Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + case "ActiveDirectoryIdentityProvider": + require.NotEmpty(t, namespace, "bad test setup: empty namespace") + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := supervisorIDPClient.ActiveDirectoryIdentityProviders(namespace).Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + case "GitHubIdentityProvider": + require.NotEmpty(t, namespace, "bad test setup: empty namespace") + testlib.RequireEventuallyf(t, func(requireEventually *require.Assertions) { + got, err := supervisorIDPClient.GitHubIdentityProviders(namespace).Get(ctx, resourceName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireConditionHasMessage(requireEventually, got.Status.Conditions, "TLSConfigurationValid", wantMessage) + }, 10*time.Second, 1*time.Second, "expected resource %s to have condition message %q", resourceName, wantMessage) + default: + require.Failf(t, "unexpected resource type", "type %q", resourceType) + } +} + +func requireConditionHasMessage(assertions *require.Assertions, actualConditions []metav1.Condition, conditionType string, wantMessage string) { + assertions.NotEmpty(actualConditions, "wanted to have conditions but was empty") + for _, c := range actualConditions { + if c.Type == conditionType { + assertions.Equal(wantMessage, c.Message) + return + } + } + assertions.Failf("did not find condition with expected type", + "type %q, actual conditions: %#v", conditionType, actualConditions) +} + +func createSecretOrConfigMapFromData( + t *testing.T, + resourceNameSuffix string, + namespace string, + kind string, + secretType string, + dataYAML string, +) string { + t.Helper() + + if kind == "" { + // Nothing to create. + return "" + } + + require.NotEmpty(t, resourceNameSuffix, "bad test setup: empty resourceNameSuffix") + require.NotEmpty(t, namespace, "bad test setup: empty namespace") + + var resourceYAML string + lowerKind := strings.ToLower(kind) + resourceName := lowerKind + "-" + resourceNameSuffix + + // Further indent every line except for the first line by four spaces. + // Use four spaces because that's what here.Doc uses. + // Do not indent the first line because the template already indents it. + indentedDataYAML := strings.ReplaceAll(dataYAML, "\n", "\n ") + + switch lowerKind { + case "secret": + require.NotEmpty(t, secretType, "bad test setup: empty secret type") + resourceYAML = here.Docf(` + apiVersion: v1 + kind: Secret + metadata: + name: %s + namespace: %s + type: %s + stringData: + %s + `, resourceName, namespace, secretType, indentedDataYAML) + case "configmap": + resourceYAML = here.Docf(` + apiVersion: v1 + kind: ConfigMap + metadata: + name: %s + namespace: %s + data: + %s + `, resourceName, namespace, indentedDataYAML) + default: + require.Failf(t, "unexpected kind in test setup", "kind was %q", kind) + } + + stdOut, stdErr, err := performKubectlApply(t, resourceName, []byte(resourceYAML)) + require.NoErrorf(t, err, + "expected kubectl apply to succeed but got: %s\nstdout: %s\nstderr: %s\nyaml:\n%s", + err, stdOut, stdErr, resourceYAML) + + return resourceName +} + func performKubectlApply(t *testing.T, resourceName string, yamlBytes []byte) (string, string, error) { t.Helper() @@ -231,6 +450,8 @@ func requireKubectlApplyResult( wantResourceType string, wantResourceName string, ) { + t.Helper() + if len(wantErrorSnippets) > 0 { require.Error(t, kubectlErr) actualErrorString := strings.TrimSuffix(kubectlStdErr, "\n") diff --git a/test/integration/supervisor_tls_spec_test.go b/test/integration/supervisor_tls_spec_test.go index 6eb978b63..687f15d9b 100644 --- a/test/integration/supervisor_tls_spec_test.go +++ b/test/integration/supervisor_tls_spec_test.go @@ -6,21 +6,31 @@ import ( "fmt" "strings" "testing" + "time" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + + "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/here" "go.pinniped.dev/test/testlib" ) // TestTLSSpecKubeBuilderValidationSupervisor_Parallel tests kubebuilder validation // on the TLSSpec in Pinniped supervisor CRDs using OIDCIdentityProvider as an example. -func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { +func TestTLSSpecValidationSupervisor_Parallel(t *testing.T) { env := testlib.IntegrationEnv(t) + ca, err := certauthority.New("pinniped-test", 24*time.Hour) + require.NoError(t, err) + indentedCAPEM := indentForHeredoc(string(ca.Bundle())) + oidcIDPTemplate := here.Doc(` apiVersion: idp.supervisor.%s/v1alpha1 kind: OIDCIdentityProvider metadata: name: %s + namespace: %s spec: issuer: %s authorizationConfig: @@ -36,6 +46,7 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { kind: LDAPIdentityProvider metadata: name: %s + namespace: %s spec: host: %s bind: @@ -53,6 +64,7 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { kind: ActiveDirectoryIdentityProvider metadata: name: %s + namespace: %s spec: host: %s bind: @@ -65,6 +77,7 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { kind: GitHubIdentityProvider metadata: name: %s + namespace: %s spec: allowAuthentication: organizations: @@ -76,136 +89,174 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { `) testCases := []struct { - name string - tlsYAML string - expectedErrorSnippets []string - expectedGitHubErrorSnippets []string + name string + + tlsYAML func(secretOrConfigmapName string) string + + secretOrConfigmapKind string + secretType string + secretOrConfigmapDataYAML string + + wantErrorSnippets []string + wantGitHubErrorSnippets []string + wantTLSValidConditionMessage string }{ { name: "should disallow certificate authority data source with missing name", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`}, - expectedGitHubErrorSnippets: []string{ + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`}, + wantGitHubErrorSnippets: []string{ `The %s "%s" is invalid:`, "spec.githubAPI.tls.certificateAuthorityDataSource.name: Required value", }, }, { name: "should disallow certificate authority data source with empty value for name", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: "" - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, - expectedGitHubErrorSnippets: []string{`The %s "%s" is invalid: spec.githubAPI.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.githubAPI.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: "" + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, + wantGitHubErrorSnippets: []string{`The %s "%s" is invalid: spec.githubAPI.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.githubAPI.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`}, }, { name: "should disallow certificate authority data source with missing key", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`}, - expectedGitHubErrorSnippets: []string{ + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`}, + wantGitHubErrorSnippets: []string{ `The %s "%s" is invalid:`, "spec.githubAPI.tls.certificateAuthorityDataSource.key: Required value", }, }, { name: "should disallow certificate authority data source with empty value for key", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - key: "" - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, - expectedGitHubErrorSnippets: []string{`The %s "%s" is invalid: spec.githubAPI.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.githubAPI.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + key: "" + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, + wantGitHubErrorSnippets: []string{`The %s "%s" is invalid: spec.githubAPI.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.githubAPI.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`}, }, { name: "should disallow certificate authority data source with missing kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`}, - expectedGitHubErrorSnippets: []string{ + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`}, + wantGitHubErrorSnippets: []string{ `The %s "%s" is invalid:`, "spec.githubAPI.tls.certificateAuthorityDataSource.kind: Required value", }, }, { name: "should disallow certificate authority data source with empty value for kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: "" - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`}, - expectedGitHubErrorSnippets: []string{ + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: "" + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`}, + wantGitHubErrorSnippets: []string{ `The %s "%s" is invalid:`, `spec.githubAPI.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`, }, }, { name: "should disallow certificate authority data source with invalid kind", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: sorcery - name: foo - key: bar - `), - expectedErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`}, - expectedGitHubErrorSnippets: []string{ + tlsYAML: func(secretOrConfigmapName string) string { + return here.Doc(` + tls: + certificateAuthorityDataSource: + kind: sorcery + name: foo + key: bar + `) + }, + wantErrorSnippets: []string{`The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`}, + wantGitHubErrorSnippets: []string{ `The %s "%s" is invalid:`, `spec.githubAPI.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`, }, }, { - name: "should create a custom resource passing all validations using a Secret source", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: Secret - name: foo - key: bar - `), - expectedErrorSnippets: nil, - expectedGitHubErrorSnippets: nil, + name: "should create a custom resource passing all validations using a Secret source of type Opaque", + secretOrConfigmapKind: "Secret", + secretType: string(corev1.SecretTypeOpaque), + secretOrConfigmapDataYAML: here.Docf(` + bar: | + %s + `, indentedCAPEM), + tlsYAML: func(secretOrConfigmapName string) string { + return here.Docf(` + tls: + certificateAuthorityDataSource: + kind: Secret + name: %s + key: bar + `, secretOrConfigmapName) + }, + wantErrorSnippets: nil, + wantGitHubErrorSnippets: nil, + wantTLSValidConditionMessage: "spec.tls is valid: using configured CA bundle", }, { - name: "should create a custom resource passing all validations using a ConfigMap source", - tlsYAML: here.Doc(` - tls: - certificateAuthorityDataSource: - kind: ConfigMap - name: foo - key: bar - `), - expectedErrorSnippets: nil, - expectedGitHubErrorSnippets: nil, + name: "should create a custom resource passing all validations using a ConfigMap source", + secretOrConfigmapKind: "ConfigMap", + secretOrConfigmapDataYAML: here.Docf(` + bar: | + %s + `, indentedCAPEM), + tlsYAML: func(secretOrConfigmapName string) string { + return here.Docf(` + tls: + certificateAuthorityDataSource: + kind: ConfigMap + name: %s + key: bar + `, secretOrConfigmapName) + }, + wantErrorSnippets: nil, + wantTLSValidConditionMessage: `spec.tls is valid: using configured CA bundle`, }, { - name: "should create a custom resource without any tls spec", - tlsYAML: "", - expectedErrorSnippets: nil, - expectedGitHubErrorSnippets: nil, + name: "should create a custom resource without any tls spec", + tlsYAML: func(secretOrConfigmapName string) string { return "" }, + wantErrorSnippets: nil, + wantGitHubErrorSnippets: nil, + wantTLSValidConditionMessage: "spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image", }, } @@ -213,68 +264,139 @@ func TestTLSSpecKubeBuilderValidationSupervisor_Parallel(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - // Further indent every line except for the first line by four spaces. - // Use four spaces because that's what here.Doc uses. - // Do not indent the first line because the template already indents it. - indentedTLSYAML := strings.ReplaceAll(tc.tlsYAML, "\n", "\n ") - t.Run("apply OIDC IDP", func(t *testing.T) { resourceName := "test-oidc-idp-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.SupervisorNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + yamlBytes := []byte(fmt.Sprintf(oidcIDPTemplate, - env.APIGroupSuffix, resourceName, env.SupervisorUpstreamOIDC.Issuer, indentedTLSYAML)) + env.APIGroupSuffix, resourceName, env.SupervisorNamespace, env.SupervisorUpstreamOIDC.Issuer, + indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName)))) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`oidcidentityprovider.idp.supervisor.%s`, env.APIGroupSuffix), - tc.expectedErrorSnippets, + tc.wantErrorSnippets, "OIDCIdentityProvider", resourceName, ) + + if tc.wantErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.SupervisorNamespace, + "OIDCIdentityProvider", + tc.wantTLSValidConditionMessage, + ) + } }) t.Run("apply LDAP IDP", func(t *testing.T) { resourceName := "test-ldap-idp-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.SupervisorNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + yamlBytes := []byte(fmt.Sprintf(ldapIDPTemplate, - env.APIGroupSuffix, resourceName, env.SupervisorUpstreamLDAP.Host, indentedTLSYAML)) + env.APIGroupSuffix, resourceName, env.SupervisorNamespace, env.SupervisorUpstreamLDAP.Host, + indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName)))) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`ldapidentityprovider.idp.supervisor.%s`, env.APIGroupSuffix), - tc.expectedErrorSnippets, + tc.wantErrorSnippets, "LDAPIdentityProvider", resourceName, ) + + if tc.wantErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.SupervisorNamespace, + "LDAPIdentityProvider", + tc.wantTLSValidConditionMessage, + ) + } }) t.Run("apply ActiveDirectory IDP", func(t *testing.T) { resourceName := "test-ad-idp-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.SupervisorNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + yamlBytes := []byte(fmt.Sprintf(activeDirectoryIDPTemplate, - env.APIGroupSuffix, resourceName, env.SupervisorUpstreamLDAP.Host, indentedTLSYAML)) + env.APIGroupSuffix, resourceName, env.SupervisorNamespace, env.SupervisorUpstreamLDAP.Host, + indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName)))) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`activedirectoryidentityprovider.idp.supervisor.%s`, env.APIGroupSuffix), - tc.expectedErrorSnippets, + tc.wantErrorSnippets, "ActiveDirectoryIdentityProvider", resourceName, ) + + if tc.wantErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.SupervisorNamespace, + "ActiveDirectoryIdentityProvider", + tc.wantTLSValidConditionMessage, + ) + } }) t.Run("apply GitHub IDP", func(t *testing.T) { - // GitHub is nested deeper - indentedTLSYAMLForGitHub := strings.ReplaceAll(indentedTLSYAML, "\n", "\n ") - resourceName := "test-github-idp-" + testlib.RandHex(t, 7) + + secretOrConfigmapResourceName := createSecretOrConfigMapFromData(t, + resourceName, + env.SupervisorNamespace, + tc.secretOrConfigmapKind, + tc.secretType, + tc.secretOrConfigmapDataYAML, + ) + + // GitHub is nested deeper. + indentedTLSYAMLForGitHub := indentForHeredoc(indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName))) + yamlBytes := []byte(fmt.Sprintf(githubIDPTemplate, - env.APIGroupSuffix, resourceName, indentedTLSYAMLForGitHub)) + env.APIGroupSuffix, resourceName, env.SupervisorNamespace, indentedTLSYAMLForGitHub)) stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes) requireKubectlApplyResult(t, stdOut, stdErr, err, fmt.Sprintf(`githubidentityprovider.idp.supervisor.%s`, env.APIGroupSuffix), - tc.expectedGitHubErrorSnippets, + tc.wantGitHubErrorSnippets, "GitHubIdentityProvider", resourceName, ) + + if tc.wantGitHubErrorSnippets == nil { + requireTLSValidConditionMessageOnResource(t, + resourceName, + env.SupervisorNamespace, + "GitHubIdentityProvider", + // The tls spec location is different for GitHubIdentityProvider, so adjust the expectation. + strings.Replace(tc.wantTLSValidConditionMessage, "spec.tls is ", "spec.githubAPI.tls is ", 1), + ) + } }) }) }