Add JWTAuthenticators to the static validation checks for concierge TLS spec

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"))
+	}
+}