From 264778113d75ef6fb0014fa4d53fcd4eb6bdf92f Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Fri, 16 Apr 2021 14:38:05 -0700 Subject: [PATCH] lookupEnv in oidclogin same as for static --- cmd/pinniped/cmd/login_oidc.go | 11 +++++++---- cmd/pinniped/cmd/login_oidc_test.go | 20 +++++++------------- cmd/pinniped/cmd/login_static.go | 2 +- cmd/pinniped/cmd/login_static_test.go | 11 +++++++++++ 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 091827cc..37a19d58 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -38,13 +38,15 @@ func init() { } type oidcLoginCommandDeps struct { + lookupEnv func(string) (string, bool) login func(string, string, ...oidcclient.Option) (*oidctypes.Token, error) exchangeToken func(context.Context, *conciergeclient.Client, string) (*clientauthv1beta1.ExecCredential, error) } func oidcLoginCommandRealDeps() oidcLoginCommandDeps { return oidcLoginCommandDeps{ - login: oidcclient.Login, + lookupEnv: os.LookupEnv, + login: oidcclient.Login, exchangeToken: func(ctx context.Context, client *conciergeclient.Client, token string) (*clientauthv1beta1.ExecCredential, error) { return client.ExchangeToken(ctx, token) }, @@ -112,7 +114,7 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command { } func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLoginFlags) error { - pLogger, err := SetLogLevel() + pLogger, err := SetLogLevel(deps.lookupEnv) if err != nil { plog.WarningErr("Received error while setting log level", err) } @@ -264,8 +266,9 @@ func tokenCredential(token *oidctypes.Token) *clientauthv1beta1.ExecCredential { return &cred } -func SetLogLevel() (*plog.PLogger, error) { - if os.Getenv("PINNIPED_DEBUG") == "true" { +func SetLogLevel(lookupEnv func(string) (string, bool)) (*plog.PLogger, error) { + debug, _ := lookupEnv("PINNIPED_DEBUG") + if debug == "true" { err := plog.ValidateAndSetLogLevelGlobally(plog.LevelDebug) if err != nil { return nil, err diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 541c6a6d..32179fde 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -9,7 +9,6 @@ import ( "encoding/base64" "fmt" "io/ioutil" - "os" "path/filepath" "testing" "time" @@ -44,7 +43,7 @@ func TestLoginOIDCCommand(t *testing.T) { args []string loginErr error conciergeErr error - envVars map[string]string + env map[string]string wantError bool wantStdout string wantStderr string @@ -175,7 +174,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--client-id", "test-client-id", "--issuer", "test-issuer", }, - envVars: map[string]string{"PINNIPED_DEBUG": "true"}, + env: map[string]string{"PINNIPED_DEBUG": "true"}, wantOptionsCount: 3, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", wantLogs: []string{ @@ -202,7 +201,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--concierge-api-group-suffix", "some.suffix.com", "--credential-cache", testutil.TempDir(t) + "/credentials.yaml", }, - envVars: map[string]string{"PINNIPED_DEBUG": "true"}, + env: map[string]string{"PINNIPED_DEBUG": "true"}, wantOptionsCount: 7, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{},"status":{"token":"exchanged-token"}}` + "\n", wantLogs: []string{ @@ -214,21 +213,16 @@ func TestLoginOIDCCommand(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - for k, v := range tt.envVars { - kk := k - err := os.Setenv(kk, v) - require.NoError(t, err) - t.Cleanup(func() { - t.Log("cleaning up " + kk) - err = os.Unsetenv(kk) - }) - } testLogger := testlogger.New(t) klog.SetLogger(testLogger) var ( gotOptions []oidcclient.Option ) cmd := oidcLoginCommand(oidcLoginCommandDeps{ + lookupEnv: func(s string) (string, bool) { + v, ok := tt.env[s] + return v, ok + }, login: func(issuer string, clientID string, opts ...oidcclient.Option) (*oidctypes.Token, error) { require.Equal(t, "test-issuer", issuer) require.Equal(t, "test-client-id", clientID) diff --git a/cmd/pinniped/cmd/login_static.go b/cmd/pinniped/cmd/login_static.go index fb1df4d5..2b3fed1d 100644 --- a/cmd/pinniped/cmd/login_static.go +++ b/cmd/pinniped/cmd/login_static.go @@ -84,7 +84,7 @@ func staticLoginCommand(deps staticLoginDeps) *cobra.Command { } func runStaticLogin(out io.Writer, deps staticLoginDeps, flags staticLoginParams) error { - pLogger, err := SetLogLevel() + pLogger, err := SetLogLevel(deps.lookupEnv) if err != nil { plog.WarningErr("Received error while setting log level", err) } diff --git a/cmd/pinniped/cmd/login_static_test.go b/cmd/pinniped/cmd/login_static_test.go index c2e6d3ea..f391da48 100644 --- a/cmd/pinniped/cmd/login_static_test.go +++ b/cmd/pinniped/cmd/login_static_test.go @@ -12,6 +12,10 @@ import ( "testing" "time" + "k8s.io/klog/v2" + + "go.pinniped.dev/internal/testutil/testlogger" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" @@ -41,6 +45,7 @@ func TestLoginStaticCommand(t *testing.T) { wantStdout string wantStderr string wantOptionsCount int + wantLogs []string }{ { name: "help flag passed", @@ -126,10 +131,12 @@ func TestLoginStaticCommand(t *testing.T) { "--concierge-authenticator-name", "test-authenticator", }, conciergeErr: fmt.Errorf("some concierge error"), + env: map[string]string{"PINNIPED_DEBUG": "true"}, wantError: true, wantStderr: here.Doc(` Error: could not complete Concierge credential exchange: some concierge error `), + wantLogs: []string{"\"level\"=0 \"msg\"=\"Pinniped login: exchanging static token for cluster credential\" \"authenticator name\"=\"test-authenticator\" \"authenticator type\"=\"webhook\" \"endpoint\"=\"https://127.0.0.1/\""}, }, { name: "invalid API group suffix", @@ -157,6 +164,8 @@ func TestLoginStaticCommand(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { + testLogger := testlogger.New(t) + klog.SetLogger(testLogger) cmd := staticLoginCommand(staticLoginDeps{ lookupEnv: func(s string) (string, bool) { v, ok := tt.env[s] @@ -192,6 +201,8 @@ func TestLoginStaticCommand(t *testing.T) { } require.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") + + require.Equal(t, tt.wantLogs, testLogger.Lines()) }) } }