From a059d8dfce4e8c3a19bf56e239c634bc742eae8a Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 8 Mar 2021 14:31:13 -0600 Subject: [PATCH] Refactor "get kubeconfig" a bit more to clean things up. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/flag_types.go | 39 ++++++++++++ cmd/pinniped/cmd/flag_types_test.go | 31 +++++++++ cmd/pinniped/cmd/kubeconfig.go | 98 +++++++++++------------------ cmd/pinniped/cmd/kubeconfig_test.go | 61 +++++------------- 4 files changed, 122 insertions(+), 107 deletions(-) diff --git a/cmd/pinniped/cmd/flag_types.go b/cmd/pinniped/cmd/flag_types.go index c0e3624d..7b10e3a5 100644 --- a/cmd/pinniped/cmd/flag_types.go +++ b/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" +} diff --git a/cmd/pinniped/cmd/flag_types_test.go b/cmd/pinniped/cmd/flag_types_test.go index 8c0faeda..38295066 100644 --- a/cmd/pinniped/cmd/flag_types_test.go +++ b/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"))) +} diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 968e15d7..7ce12900 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/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 := configureConcierge(credentialIssuer, authenticator, &flags, cluster, &oidcCABundle, &execConfig, deps.log); err != nil { + if err := discoverConciergeParams(credentialIssuer, &flags, cluster, 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 != "" { - execConfig.Args = append(execConfig.Args, "--ca-bundle-data="+base64.StdEncoding.EncodeToString([]byte(oidcCABundle))) + if len(flags.oidc.caBundle) != 0 { + 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. - var conciergeCABundleData []byte - 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 { + // Auto-set --concierge-ca-bundle if it wasn't explicitly set.. + if len(flags.concierge.caBundle) == 0 { switch frontend.Type { case configv1alpha1.TokenCredentialRequestAPIFrontendType: - conciergeCABundleData = v1Cluster.CertificateAuthorityData + flags.concierge.caBundle = v1Cluster.CertificateAuthorityData case configv1alpha1.ImpersonationProxyFrontendType: - var err error - conciergeCABundleData, err = base64.StdEncoding.DecodeString(frontend.ImpersonationProxyInfo.CertificateAuthorityData) + data, 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{ diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 34c78ce3..3e84ca44 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/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{ - `"level"=0 "msg"="loaded Concierge certificate authority bundle" "roots"=1`, - }, + wantLogs: nil, wantStdout: here.Docf(` apiVersion: v1 clusters: