Refactor "get kubeconfig" a bit more to clean things up.

Signed-off-by: Matt Moyer <moyerm@vmware.com>

cmd/pinniped/cmd/flag_types.go

@@ -4,10 +4,15 @@
 package cmd
 
 import (
+	"bytes"
+	"crypto/x509"
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"strings"
 
+	"github.com/spf13/pflag"
+
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1"
 )
 
@@ -29,6 +34,8 @@ func (c *conciergeMode) String() string {
 		return "ImpersonationProxy"
 	case modeTokenCredentialRequestAPI:
 		return "TokenCredentialRequestAPI"
+	case modeUnknown:
+		fallthrough
 	default:
 		return "TokenCredentialRequestAPI"
 	}
@@ -61,7 +68,39 @@ func (c *conciergeMode) MatchesFrontend(frontend *configv1alpha1.CredentialIssue
 		return frontend.Type == configv1alpha1.ImpersonationProxyFrontendType
 	case modeTokenCredentialRequestAPI:
 		return frontend.Type == configv1alpha1.TokenCredentialRequestAPIFrontendType
+	case modeUnknown:
+		fallthrough
 	default:
 		return true
 	}
 }
+
+// caBundlePathsVar represents a list of CA bundle paths, which load from disk when the flag is populated.
+type caBundleVar []byte
+
+var _ pflag.Value = new(caBundleVar)
+
+func (c *caBundleVar) String() string {
+	return string(*c)
+}
+
+func (c *caBundleVar) Set(path string) error {
+	pem, err := ioutil.ReadFile(path)
+	if err != nil {
+		return fmt.Errorf("could not read CA bundle path: %w", err)
+	}
+	pool := x509.NewCertPool()
+	if !pool.AppendCertsFromPEM(pem) {
+		return fmt.Errorf("failed to load any CA certificates from %q", path)
+	}
+	if len(*c) == 0 {
+		*c = pem
+		return nil
+	}
+	*c = bytes.Join([][]byte{*c, pem}, []byte("\n"))
+	return nil
+}
+
+func (c *caBundleVar) Type() string {
+	return "path"
+}

cmd/pinniped/cmd/flag_types_test.go

@@ -4,11 +4,19 @@
 package cmd
 
 import (
+	"bytes"
+	"crypto/x509/pkix"
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1"
+	"go.pinniped.dev/internal/certauthority"
+	"go.pinniped.dev/internal/testutil"
 )
 
 func TestConciergeModeFlag(t *testing.T) {
@@ -41,3 +49,26 @@ func TestConciergeModeFlag(t *testing.T) {
 	require.Equal(t, modeImpersonationProxy, m)
 	require.Equal(t, "ImpersonationProxy", m.String())
 }
+
+func TestCABundleFlag(t *testing.T) {
+	testCA, err := certauthority.New(pkix.Name{CommonName: "Test CA"}, 1*time.Hour)
+	require.NoError(t, err)
+	tmpdir := testutil.TempDir(t)
+	emptyFilePath := filepath.Join(tmpdir, "empty")
+	require.NoError(t, ioutil.WriteFile(emptyFilePath, []byte{}, 0600))
+
+	testCAPath := filepath.Join(tmpdir, "testca.pem")
+	require.NoError(t, ioutil.WriteFile(testCAPath, testCA.Bundle(), 0600))
+
+	c := caBundleVar{}
+	require.Equal(t, "path", c.Type())
+	require.Equal(t, "", c.String())
+	require.EqualError(t, c.Set("./does/not/exist"), "could not read CA bundle path: open ./does/not/exist: no such file or directory")
+	require.EqualError(t, c.Set(emptyFilePath), fmt.Sprintf("failed to load any CA certificates from %q", emptyFilePath))
+
+	require.NoError(t, c.Set(testCAPath))
+	require.Equal(t, 1, bytes.Count(c, []byte("BEGIN CERTIFICATE")))
+
+	require.NoError(t, c.Set(testCAPath))
+	require.Equal(t, 2, bytes.Count(c, []byte("BEGIN CERTIFICATE")))
+}

cmd/pinniped/cmd/kubeconfig.go

@@ -4,14 +4,12 @@
 package cmd
 
 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
@@ -76,7 +74,7 @@ type getKubeconfigOIDCParams struct {
 	skipBrowser       bool
 	sessionCachePath  string
 	debugSessionCache bool
-	caBundlePaths     []string
+	caBundle          caBundleVar
 	requestAudience   string
 }
 
@@ -86,7 +84,7 @@ type getKubeconfigConciergeParams struct {
 	authenticatorName string
 	authenticatorType string
 	apiGroupSuffix    string
-	caBundlePath      string
+	caBundle          caBundleVar
 	endpoint          string
 	mode              conciergeMode
 }
@@ -126,7 +124,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
 	f.StringVar(&flags.concierge.authenticatorName, "concierge-authenticator-name", "", "Concierge authenticator name (default: autodiscover)")
 	f.StringVar(&flags.concierge.apiGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
 
-	f.StringVar(&flags.concierge.caBundlePath, "concierge-ca-bundle", "", "Path to TLS certificate authority bundle (PEM format, optional, can be repeated) to use when connecting to the Concierge")
+	f.Var(&flags.concierge.caBundle, "concierge-ca-bundle", "Path to TLS certificate authority bundle (PEM format, optional, can be repeated) to use when connecting to the Concierge")
 	f.StringVar(&flags.concierge.endpoint, "concierge-endpoint", "", "API base for the Concierge endpoint")
 	f.Var(&flags.concierge.mode, "concierge-mode", "Concierge mode of operation")
 
@@ -136,7 +134,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
 	f.StringSliceVar(&flags.oidc.scopes, "oidc-scopes", []string{oidc.ScopeOfflineAccess, oidc.ScopeOpenID, "pinniped:request-audience"}, "OpenID Connect scopes to request during login")
 	f.BoolVar(&flags.oidc.skipBrowser, "oidc-skip-browser", false, "During OpenID Connect login, skip opening the browser (just print the URL)")
 	f.StringVar(&flags.oidc.sessionCachePath, "oidc-session-cache", "", "Path to OpenID Connect session cache file")
-	f.StringSliceVar(&flags.oidc.caBundlePaths, "oidc-ca-bundle", nil, "Path to TLS certificate authority bundle (PEM format, optional, can be repeated)")
+	f.Var(&flags.oidc.caBundle, "oidc-ca-bundle", "Path to TLS certificate authority bundle (PEM format, optional, can be repeated)")
 	f.BoolVar(&flags.oidc.debugSessionCache, "oidc-debug-session-cache", false, "Print debug logs related to the OpenID Connect session cache")
 	f.StringVar(&flags.oidc.requestAudience, "oidc-request-audience", "", "Request a token with an alternate audience using RFC8693 token exchange")
 	f.StringVar(&flags.kubeconfigPath, "kubeconfig", os.Getenv("KUBECONFIG"), "Path to kubeconfig file")
@@ -187,11 +185,6 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 	}
 	execConfig.ProvideClusterInfo = true
 
-	oidcCABundle, err := loadCABundlePaths(flags.oidc.caBundlePaths)
-	if err != nil {
-		return fmt.Errorf("could not read --oidc-ca-bundle: %w", err)
-	}
-
 	clientConfig := newClientConfig(flags.kubeconfigPath, flags.kubeconfigContextOverride)
 	currentKubeConfig, err := clientConfig.RawConfig()
 	if err != nil {
@@ -221,10 +214,26 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 		if err != nil {
 			return err
 		}
-
+		if err := discoverConciergeParams(credentialIssuer, &flags, cluster, deps.log); err != nil {
-		if err := configureConcierge(credentialIssuer, authenticator, &flags, cluster, &oidcCABundle, &execConfig, deps.log); err != nil {
 			return err
 		}
+		if err := discoverAuthenticatorParams(authenticator, &flags, deps.log); err != nil {
+			return err
+		}
+		// Append the flags to configure the Concierge credential exchange at runtime.
+		execConfig.Args = append(execConfig.Args,
+			"--enable-concierge",
+			"--concierge-api-group-suffix="+flags.concierge.apiGroupSuffix,
+			"--concierge-authenticator-name="+flags.concierge.authenticatorName,
+			"--concierge-authenticator-type="+flags.concierge.authenticatorType,
+			"--concierge-endpoint="+flags.concierge.endpoint,
+			"--concierge-ca-bundle-data="+base64.StdEncoding.EncodeToString(flags.concierge.caBundle),
+			"--concierge-mode="+flags.concierge.mode.String(),
+		)
+
+		// Point kubectl at the concierge endpoint.
+		cluster.Server = flags.concierge.endpoint
+		cluster.CertificateAuthorityData = flags.concierge.caBundle
 	}
 
 	// If one of the --static-* flags was passed, output a config that runs `pinniped login static`.
@@ -263,8 +272,8 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 	if flags.oidc.listenPort != 0 {
 		execConfig.Args = append(execConfig.Args, "--listen-port="+strconv.Itoa(int(flags.oidc.listenPort)))
 	}
-	if oidcCABundle != "" {
+	if len(flags.oidc.caBundle) != 0 {
-		execConfig.Args = append(execConfig.Args, "--ca-bundle-data="+base64.StdEncoding.EncodeToString([]byte(oidcCABundle)))
+		execConfig.Args = append(execConfig.Args, "--ca-bundle-data="+base64.StdEncoding.EncodeToString(flags.oidc.caBundle))
 	}
 	if flags.oidc.sessionCachePath != "" {
 		execConfig.Args = append(execConfig.Args, "--session-cache="+flags.oidc.sessionCachePath)
@@ -282,7 +291,7 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 	return writeConfigAsYAML(out, kubeconfig)
 }
 
-func configureConcierge(credentialIssuer *configv1alpha1.CredentialIssuer, authenticator metav1.Object, flags *getKubeconfigParams, v1Cluster *clientcmdapi.Cluster, oidcCABundle *string, execConfig *clientcmdapi.ExecConfig, log logr.Logger) error {
+func discoverConciergeParams(credentialIssuer *configv1alpha1.CredentialIssuer, flags *getKubeconfigParams, v1Cluster *clientcmdapi.Cluster, log logr.Logger) error {
 	// Autodiscover the --concierge-mode.
 	frontend, err := getConciergeFrontend(credentialIssuer, flags.concierge.mode)
 	if err != nil {
@@ -312,29 +321,24 @@ func configureConcierge(credentialIssuer *configv1alpha1.CredentialIssuer, authe
 		log.Info("discovered Concierge endpoint", "endpoint", flags.concierge.endpoint)
 	}
 
-	// Load specified --concierge-ca-bundle or autodiscover a value.
+	// Auto-set --concierge-ca-bundle if it wasn't explicitly set..
-	var conciergeCABundleData []byte
+	if len(flags.concierge.caBundle) == 0 {
-	if flags.concierge.caBundlePath != "" {
-		caBundleString, err := loadCABundlePaths([]string{flags.concierge.caBundlePath})
-		if err != nil {
-			return fmt.Errorf("could not read --concierge-ca-bundle: %w", err)
-		}
-		conciergeCABundleData = []byte(caBundleString)
-		log.Info("loaded Concierge certificate authority bundle", "roots", countCACerts(conciergeCABundleData))
-	} else {
 		switch frontend.Type {
 		case configv1alpha1.TokenCredentialRequestAPIFrontendType:
-			conciergeCABundleData = v1Cluster.CertificateAuthorityData
+			flags.concierge.caBundle = v1Cluster.CertificateAuthorityData
 		case configv1alpha1.ImpersonationProxyFrontendType:
-			var err error
+			data, err := base64.StdEncoding.DecodeString(frontend.ImpersonationProxyInfo.CertificateAuthorityData)
-			conciergeCABundleData, err = base64.StdEncoding.DecodeString(frontend.ImpersonationProxyInfo.CertificateAuthorityData)
 			if err != nil {
 				return fmt.Errorf("autodiscovered Concierge CA bundle is invalid: %w", err)
 			}
+			flags.concierge.caBundle = data
 		}
-		log.Info("discovered Concierge certificate authority bundle", "roots", countCACerts(conciergeCABundleData))
+		log.Info("discovered Concierge certificate authority bundle", "roots", countCACerts(flags.concierge.caBundle))
 	}
+	return nil
+}
 
+func discoverAuthenticatorParams(authenticator metav1.Object, flags *getKubeconfigParams, log logr.Logger) error {
 	switch auth := authenticator.(type) {
 	case *conciergev1alpha1.WebhookAuthenticator:
 		// If the --concierge-authenticator-type/--concierge-authenticator-name flags were not set explicitly, set
@@ -367,30 +371,15 @@ func configureConcierge(credentialIssuer *configv1alpha1.CredentialIssuer, authe
 
 		// If the --oidc-ca-bundle flags was not set explicitly, default it to the
 		// spec.tls.certificateAuthorityData field of the JWTAuthenticator.
-		if *oidcCABundle == "" && auth.Spec.TLS != nil && auth.Spec.TLS.CertificateAuthorityData != "" {
+		if len(flags.oidc.caBundle) == 0 && auth.Spec.TLS != nil && auth.Spec.TLS.CertificateAuthorityData != "" {
 			decoded, err := base64.StdEncoding.DecodeString(auth.Spec.TLS.CertificateAuthorityData)
 			if err != nil {
 				return fmt.Errorf("tried to autodiscover --oidc-ca-bundle, but JWTAuthenticator %s has invalid spec.tls.certificateAuthorityData: %w", auth.Name, err)
 			}
 			log.Info("discovered OIDC CA bundle", "roots", countCACerts(decoded))
-			*oidcCABundle = string(decoded)
+			flags.oidc.caBundle = decoded
 		}
 	}
-
-	// Append the flags to configure the Concierge credential exchange at runtime.
-	execConfig.Args = append(execConfig.Args,
-		"--enable-concierge",
-		"--concierge-api-group-suffix="+flags.concierge.apiGroupSuffix,
-		"--concierge-authenticator-name="+flags.concierge.authenticatorName,
-		"--concierge-authenticator-type="+flags.concierge.authenticatorType,
-		"--concierge-endpoint="+flags.concierge.endpoint,
-		"--concierge-ca-bundle-data="+base64.StdEncoding.EncodeToString(conciergeCABundleData),
-		"--concierge-mode="+flags.concierge.mode.String(),
-	)
-
-	// Point kubectl at the concierge endpoint.
-	v1Cluster.Server = flags.concierge.endpoint
-	v1Cluster.CertificateAuthorityData = conciergeCABundleData
 	return nil
 }
 
@@ -437,21 +426,6 @@ func getConciergeFrontend(credentialIssuer *configv1alpha1.CredentialIssuer, mod
 	return nil, fmt.Errorf("could not find successful Concierge strategy matching --concierge-mode=%s", mode.String())
 }
 
-func loadCABundlePaths(paths []string) (string, error) {
-	if len(paths) == 0 {
-		return "", nil
-	}
-	blobs := make([][]byte, 0, len(paths))
-	for _, p := range paths {
-		pem, err := ioutil.ReadFile(p)
-		if err != nil {
-			return "", err
-		}
-		blobs = append(blobs, pem)
-	}
-	return string(bytes.Join(blobs, []byte("\n"))), nil
-}
-
 func newExecKubeconfig(cluster *clientcmdapi.Cluster, execConfig *clientcmdapi.ExecConfig) clientcmdapi.Config {
 	const name = "pinniped"
 	return clientcmdapi.Config{

cmd/pinniped/cmd/kubeconfig_test.go

@@ -69,7 +69,7 @@ func TestGetKubeconfig(t *testing.T) {
 				      --concierge-api-group-suffix string     Concierge API group suffix (default "pinniped.dev")
 				      --concierge-authenticator-name string   Concierge authenticator name (default: autodiscover)
 				      --concierge-authenticator-type string   Concierge authenticator type (e.g., 'webhook', 'jwt') (default: autodiscover)
-				      --concierge-ca-bundle string            Path to TLS certificate authority bundle (PEM format, optional, can be repeated) to use when connecting to the Concierge
+				      --concierge-ca-bundle path              Path to TLS certificate authority bundle (PEM format, optional, can be repeated) to use when connecting to the Concierge
 				      --concierge-credential-issuer string    Concierge CredentialIssuer object to use for autodiscovery (default: autodiscover)
 				      --concierge-endpoint string             API base for the Concierge endpoint
 				      --concierge-mode mode                   Concierge mode of operation (default TokenCredentialRequestAPI)
@@ -77,7 +77,7 @@ func TestGetKubeconfig(t *testing.T) {
 				      --kubeconfig string                     Path to kubeconfig file
 				      --kubeconfig-context string             Kubeconfig context name (default: current active context)
 				      --no-concierge                          Generate a configuration which does not use the Concierge, but sends the credential to the cluster directly
-				      --oidc-ca-bundle strings                Path to TLS certificate authority bundle (PEM format, optional, can be repeated)
+				      --oidc-ca-bundle path                   Path to TLS certificate authority bundle (PEM format, optional, can be repeated)
 				      --oidc-client-id string                 OpenID Connect client ID (default: autodiscover) (default "pinniped-cli")
 				      --oidc-issuer string                    OpenID Connect issuer URL (default: autodiscover)
 				      --oidc-listen-port uint16               TCP port for localhost listener (authorization code flow only)
@@ -102,13 +102,24 @@ func TestGetKubeconfig(t *testing.T) {
 			`),
 		},
 		{
-			name: "invalid CA bundle paths",
+			name: "invalid OIDC CA bundle path",
 			args: []string{
 				"--oidc-ca-bundle", "./does/not/exist",
 			},
 			wantError: true,
 			wantStderr: here.Doc(`
-				Error: could not read --oidc-ca-bundle: open ./does/not/exist: no such file or directory
+				Error: invalid argument "./does/not/exist" for "--oidc-ca-bundle" flag: could not read CA bundle path: open ./does/not/exist: no such file or directory
+			`),
+		},
+		{
+			name: "invalid Concierge CA bundle",
+			args: []string{
+				"--kubeconfig", "./testdata/kubeconfig.yaml",
+				"--concierge-ca-bundle", "./does/not/exist",
+			},
+			wantError: true,
+			wantStderr: here.Doc(`
+				Error: invalid argument "./does/not/exist" for "--concierge-ca-bundle" flag: could not read CA bundle path: open ./does/not/exist: no such file or directory
 			`),
 		},
 		{
@@ -473,44 +484,6 @@ func TestGetKubeconfig(t *testing.T) {
 				Error: tried to autodiscover --oidc-ca-bundle, but JWTAuthenticator test-authenticator has invalid spec.tls.certificateAuthorityData: illegal base64 data at input byte 7
 			`),
 		},
-		{
-			name: "invalid concierge ca bundle",
-			args: []string{
-				"--kubeconfig", "./testdata/kubeconfig.yaml",
-				"--concierge-ca-bundle", "./does/not/exist",
-				"--concierge-endpoint", "https://impersonation-proxy-endpoint.test",
-				"--concierge-authenticator-name", "test-authenticator",
-				"--concierge-authenticator-type", "webhook",
-				"--concierge-mode", "ImpersonationProxy",
-			},
-			conciergeObjects: []runtime.Object{
-				&configv1alpha1.CredentialIssuer{
-					ObjectMeta: metav1.ObjectMeta{Name: "test-credential-issuer"},
-					Status: configv1alpha1.CredentialIssuerStatus{
-						Strategies: []configv1alpha1.CredentialIssuerStrategy{{
-							Type:   configv1alpha1.ImpersonationProxyStrategyType,
-							Status: configv1alpha1.SuccessStrategyStatus,
-							Reason: configv1alpha1.ListeningStrategyReason,
-							Frontend: &configv1alpha1.CredentialIssuerFrontend{
-								Type: configv1alpha1.ImpersonationProxyFrontendType,
-								ImpersonationProxyInfo: &configv1alpha1.ImpersonationProxyInfo{
-									Endpoint:                 "https://impersonation-proxy-endpoint.example.com",
-									CertificateAuthorityData: base64.StdEncoding.EncodeToString(testConciergeCA.Bundle()),
-								},
-							},
-						}},
-					},
-				},
-				&conciergev1alpha1.WebhookAuthenticator{ObjectMeta: metav1.ObjectMeta{Name: "test-authenticator"}},
-			},
-			wantLogs: []string{
-				`"level"=0 "msg"="discovered CredentialIssuer"  "name"="test-credential-issuer"`,
-			},
-			wantError: true,
-			wantStderr: here.Doc(`
-				Error: could not read --concierge-ca-bundle: open ./does/not/exist: no such file or directory
-			`),
-		},
 		{
 			name: "invalid static token flags",
 			args: []string{
@@ -827,9 +800,7 @@ func TestGetKubeconfig(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{Name: "test-authenticator"},
 				},
 			},
-			wantLogs: []string{
+			wantLogs: nil,
-				`"level"=0 "msg"="loaded Concierge certificate authority bundle"  "roots"=1`,
-			},
 			wantStdout: here.Docf(`
         		apiVersion: v1
         		clusters: