lookupEnv in oidclogin same as for static

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) {
+func SetLogLevel(lookupEnv func(string) (string, bool)) (*plog.PLogger, error) {
-	if os.Getenv("PINNIPED_DEBUG") == "true" {
+	debug, _ := lookupEnv("PINNIPED_DEBUG")
+	if debug == "true" {
 		err := plog.ValidateAndSetLogLevelGlobally(plog.LevelDebug)
 		if err != nil {
 			return nil, err

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)

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

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())
 		})
 	}
 }