From afec420ce6f97a14b175f0348388312fb4c05563 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 23 Jul 2024 14:32:21 -0500 Subject: [PATCH] Add JWTAuthenticators to the static validation checks for concierge TLS spec --- test/integration/concierge_tls_spec_test.go | 292 +++++++++++++++----- 1 file changed, 230 insertions(+), 62 deletions(-) diff --git a/test/integration/concierge_tls_spec_test.go b/test/integration/concierge_tls_spec_test.go index 8ecca6416..00de622b0 100644 --- a/test/integration/concierge_tls_spec_test.go +++ b/test/integration/concierge_tls_spec_test.go @@ -6,13 +6,14 @@ import ( "bytes" "context" "fmt" + "net/url" "os" "os/exec" "path/filepath" + "regexp" "strings" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.pinniped.dev/internal/here" @@ -20,22 +21,20 @@ import ( ) // TestTLSSpecKubeBuilderValidationConcierge_Parallel tests kubebuilder validation on the TLSSpec -// in Pinniped concierge CRDs using WebhookAuthenticator as an example. +// in Pinniped concierge CRDs for both WebhookAuthenticators and JWTAuthenticators. func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { env := testlib.IntegrationEnv(t) - localUserAuthenticatorEndpoint := env.TestWebhook.Endpoint - testCases := []struct { - name string - customResourceYaml string - customResourceName string - expectedError string + name string + customWebhookAuthenticatorYaml string + customJWTAuthenticatorYaml string + resourceNamePrefix string + expectedError string }{ - // TODO: should we repeat these tests using the JWTAuthenticator too? { name: "should disallow certificate authority data source with missing name", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -48,12 +47,26 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { kind: Secret key: bar `), - customResourceName: "invalid-webhook-auth-missing-name", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: Secret + key: bar + `), + resourceNamePrefix: "invalid-tls-spec-missing-name", + expectedError: `The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Required value`, }, { name: "should disallow certificate authority data source with empty value for name", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -67,12 +80,27 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: "" key: bar `), - customResourceName: "invalid-webhook-auth-empty-name", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.name: Invalid value: "": spec.tls.certificateAuthorityDataSource.name in body should be at least 1 chars long`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: Secret + name: "" + key: bar + `), + resourceNamePrefix: "invalid-tls-spec-empty-name", + expectedError: `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", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -85,12 +113,26 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { kind: Secret name: foo `), - customResourceName: "invalid-webhook-auth-missing-key", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + `), + resourceNamePrefix: "invalid-tls-spec-missing-key", + expectedError: `The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Required value`, }, { name: "should disallow certificate authority data source with empty value for key", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -104,12 +146,27 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: foo key: "" `), - customResourceName: "invalid-webhook-auth-empty-kind", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.key: Invalid value: "": spec.tls.certificateAuthorityDataSource.key in body should be at least 1 chars long`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + key: "" + `), + resourceNamePrefix: "invalid-tls-spec-empty-kind", + expectedError: `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", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -122,12 +179,26 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: foo key: bar `), - customResourceName: "invalid-webhook-auth-missing-kind", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + name: foo + key: bar + `), + resourceNamePrefix: "invalid-tls-spec-missing-kind", + expectedError: `The %s "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Required value`, }, { name: "should disallow certificate authority data source with empty value for kind", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -141,12 +212,27 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: foo key: bar `), - customResourceName: "invalid-webhook-auth-invalid-kind", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "": supported values: "Secret", "ConfigMap"`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: "" + name: foo + key: bar + `), + resourceNamePrefix: "invalid-tls-spec-invalid-kind", + expectedError: `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", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -160,12 +246,27 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: foo key: bar `), - customResourceName: "invalid-webhook-auth-invalid-kind", - expectedError: `The WebhookAuthenticator "%s" is invalid: spec.tls.certificateAuthorityDataSource.kind: Unsupported value: "sorcery": supported values: "Secret", "ConfigMap"`, + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: sorcery + name: foo + key: bar + `), + resourceNamePrefix: "invalid-tls-spec-invalid-kind", + expectedError: `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", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -178,13 +279,28 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { kind: Secret name: foo key: bar - `), - customResourceName: "valid-webhook-auth-secret-kind", + `), + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: Secret + name: foo + key: bar + `), + resourceNamePrefix: "valid-webhook-auth-secret-kind", expectedError: "", }, { name: "should create a custom resource passing all validations using a ConfigMap source", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -198,12 +314,27 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { name: foo key: bar `), - customResourceName: "valid-webhook-auth-cm-kind", + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + tls: + certificateAuthorityDataSource: + kind: ConfigMap + name: foo + key: bar + `), + resourceNamePrefix: "valid-webhook-auth-cm-kind", expectedError: "", }, { name: "should create a custom resource without any tls spec", - customResourceYaml: here.Doc(` + customWebhookAuthenticatorYaml: here.Doc(` --- apiVersion: authentication.concierge.%s/v1alpha1 kind: WebhookAuthenticator @@ -212,7 +343,17 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { spec: endpoint: %s `), - customResourceName: "no-tls-spec", + customJWTAuthenticatorYaml: here.Doc(` + --- + apiVersion: authentication.concierge.%s/v1alpha1 + kind: JWTAuthenticator + metadata: + name: %s + spec: + issuer: %s + audience: some-audience + `), + resourceNamePrefix: "no-tls-spec", expectedError: "", }, } @@ -220,36 +361,63 @@ func TestTLSSpecKubeBuilderValidationConcierge_Parallel(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - yamlFilepath := filepath.Join(t.TempDir(), fmt.Sprintf("tls-spec-validation-%s.yaml", tc.customResourceName)) - resourceName := tc.customResourceName + "-" + testlib.RandHex(t, 7) - yamlBytes := []byte(fmt.Sprintf(tc.customResourceYaml, env.APIGroupSuffix, resourceName, localUserAuthenticatorEndpoint)) + t.Run("apply webhook authenticator", func(t *testing.T) { + webhookResourceName := tc.resourceNamePrefix + "-" + testlib.RandHex(t, 7) + webhookYamlBytes := []byte(fmt.Sprintf(tc.customWebhookAuthenticatorYaml, env.APIGroupSuffix, webhookResourceName, env.TestWebhook.Endpoint)) - require.NoError(t, os.WriteFile(yamlFilepath, yamlBytes, 0600)) - - // Use --validate=false to disable old client-side validations to avoid getting different error messages in Kube 1.24 and older. - // Note that this also disables validations of unknown and duplicate fields, but that's not what this test is about. - //nolint:gosec // this is test code. - cmd := exec.CommandContext(context.Background(), "kubectl", []string{"apply", "--validate=false", "-f", yamlFilepath}...) - - var stdOut, stdErr bytes.Buffer - cmd.Stdout = &stdOut - cmd.Stderr = &stdErr - err := cmd.Run() - - t.Cleanup(func() { - t.Helper() - //nolint:gosec // this is test code. - require.NoError(t, exec.Command("kubectl", []string{"delete", "--ignore-not-found", "-f", yamlFilepath}...).Run()) + performKubectlApply(t, webhookYamlBytes, tc.expectedError, "WebhookAuthenticator", webhookResourceName) }) - if tc.expectedError == "" { - assert.Empty(t, stdErr.String()) - assert.Equal(t, fmt.Sprintf("webhookauthenticator.authentication.concierge.pinniped.dev/%s created\n", resourceName), stdOut.String()) + t.Run("apply jwt authenticator", func(t *testing.T) { + issuerURL, err := url.Parse(env.SupervisorUpstreamOIDC.CallbackURL) require.NoError(t, err) - } else { - require.Equal(t, fmt.Sprintf(tc.expectedError, resourceName), strings.TrimSuffix(stdErr.String(), "\n")) - } + require.True(t, strings.HasSuffix(issuerURL.Path, "/callback")) + issuerURL.Path = strings.TrimSuffix(issuerURL.Path, "/callback") + + jwtAuthenticatorResourceName := tc.resourceNamePrefix + "-" + testlib.RandHex(t, 7) + jwtAuthenticatorYamlBytes := []byte(fmt.Sprintf(tc.customJWTAuthenticatorYaml, env.APIGroupSuffix, jwtAuthenticatorResourceName, issuerURL.String())) + + performKubectlApply(t, jwtAuthenticatorYamlBytes, tc.expectedError, "JWTAuthenticator", jwtAuthenticatorResourceName) + }) }) } } + +func performKubectlApply( + t *testing.T, + yamlBytes []byte, + expectedError string, + resourceType string, + resourceName string, +) { + t.Helper() + + yamlFilepath := filepath.Join(t.TempDir(), fmt.Sprintf("tls-spec-validation-%s.yaml", resourceName)) + + require.NoError(t, os.WriteFile(yamlFilepath, yamlBytes, 0600)) + + // Use --validate=false to disable old client-side validations to avoid getting different error messages in Kube 1.24 and older. + // Note that this also disables validations of unknown and duplicate fields, but that's not what this test is about. + //nolint:gosec // this is test code. + cmd := exec.CommandContext(context.Background(), "kubectl", []string{"apply", "--validate=false", "-f", yamlFilepath}...) + + var stdOut, stdErr bytes.Buffer + cmd.Stdout = &stdOut + cmd.Stderr = &stdErr + err := cmd.Run() + + t.Cleanup(func() { + t.Helper() + //nolint:gosec // this is test code. + require.NoError(t, exec.Command("kubectl", []string{"delete", "--ignore-not-found", "-f", yamlFilepath}...).Run()) + }) + + if expectedError == "" { + require.Empty(t, stdErr.String()) + require.Regexp(t, "^(webhookauthenticator|jwtauthenticator)"+regexp.QuoteMeta(fmt.Sprintf(".authentication.concierge.pinniped.dev/%s created\n", resourceName)), stdOut.String()) + require.NoError(t, err) + } else { + require.Equal(t, fmt.Sprintf(expectedError, resourceType, resourceName), strings.TrimSuffix(stdErr.String(), "\n")) + } +}