Skip to content

Commit c1b8813

Browse files
pawelkucpawelkuc
committed
feat: nctl auth login in non-interactive env and without set token should not block
If you execute `nctl auth login` in a non-interactive environment (e.g. CI/CD) and forget to set `--api-token`, the browser should not try to open - you should see an error message instead.
1 parent b2bb0f2 commit c1b8813

15 files changed

+147
-66
lines changed

‎auth/login.go

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package auth
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net/url"
78
"os"
@@ -24,6 +25,8 @@ type LoginCmd struct {
2425
ClientID string `help:"Client ID is the OIDC client ID of the API." default:"nineapis.ch-f178254"`
2526
}
2627

28+
const ErrNonInteractiveEnvironmentEptyToken = "a static API token is required in non-interactive envirtonments"
29+
2730
func (l *LoginCmd) Run(ctx context.Context, command string, tk api.TokenGetter) error {
2831
apiURL, err := url.Parse(l.APIURL)
2932
if err != nil {
@@ -58,6 +61,10 @@ func (l *LoginCmd) Run(ctx context.Context, command string, tk api.TokenGetter)
5861
return login(ctx, cfg, loadingRules.GetDefaultFilename(), userInfo.User, "", project(l.Organization))
5962
}
6063

64+
if !format.IsInteractiveEnvironment(ctx, os.Stdout) {
65+
return errors.New(ErrNonInteractiveEnvironmentEptyToken)
66+
}
67+
6168
usePKCE := true
6269

6370
token, err := tk.GetTokenString(ctx, l.IssuerURL, l.ClientID, usePKCE)

‎auth/login_test.go

+88-36
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88
"path"
99
"testing"
1010

11+
"github.com/ninech/nctl/api"
12+
"github.com/ninech/nctl/internal/format"
1113
"github.com/ninech/nctl/internal/test"
14+
"github.com/stretchr/testify/require"
1215
"k8s.io/client-go/tools/clientcmd"
1316
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
1417
)
@@ -19,7 +22,19 @@ func (f *fakeTokenGetter) GetTokenString(ctx context.Context, issuerURL, clientI
1922
return test.FakeJWTToken, nil
2023
}
2124

25+
func checkErrorRequire(t *testing.T, err error, expectError bool, expectedErrMsg string) {
26+
t.Helper()
27+
if expectError {
28+
require.Error(t, err, "expected an error but got none")
29+
require.EqualError(t, err, expectedErrMsg, "unexpected error message")
30+
} else {
31+
require.NoError(t, err, "expected no error but got one")
32+
}
33+
}
34+
2235
func TestLoginCmd(t *testing.T) {
36+
ctx := format.WithForceInteractiveEnvironment(context.Background(), true)
37+
2338
// write our "existing" kubeconfig to a temp kubeconfig
2439
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
2540
if err != nil {
@@ -39,7 +54,7 @@ func TestLoginCmd(t *testing.T) {
3954
APIURL: "https://" + apiHost,
4055
IssuerURL: "https://auth.example.org",
4156
}
42-
if err := cmd.Run(context.Background(), "", tk); err != nil {
57+
if err := cmd.Run(ctx, "", tk); err != nil {
4358
t.Fatal(err)
4459
}
4560

@@ -58,46 +73,83 @@ func TestLoginCmd(t *testing.T) {
5873
}
5974

6075
func TestLoginStaticToken(t *testing.T) {
61-
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
62-
if err != nil {
63-
log.Fatal(err)
64-
}
65-
defer os.Remove(kubeconfig.Name())
66-
67-
os.Setenv(clientcmd.RecommendedConfigPathEnvVar, kubeconfig.Name())
68-
6976
apiHost := "api.example.org"
70-
token := test.FakeJWTToken
7177

72-
cmd := &LoginCmd{APIURL: "https://" + apiHost, APIToken: token, Organization: "test"}
73-
tk := &fakeTokenGetter{}
74-
if err := cmd.Run(context.Background(), "", tk); err != nil {
75-
t.Fatal(err)
76-
}
77-
78-
// read out the kubeconfig again to test the contents
79-
b, err := io.ReadAll(kubeconfig)
80-
if err != nil {
81-
t.Fatal(err)
82-
}
83-
84-
kc, err := clientcmd.Load(b)
85-
if err != nil {
86-
t.Fatal(err)
87-
}
88-
89-
checkConfig(t, kc, 1, "")
90-
91-
if token != kc.AuthInfos[apiHost].Token {
92-
t.Fatalf("expected token to be %s, got %s", token, kc.AuthInfos[apiHost].Token)
93-
}
94-
95-
if kc.AuthInfos[apiHost].Exec != nil {
96-
t.Fatalf("expected execConfig to be empty, got %v", kc.AuthInfos[apiHost].Exec)
78+
tests := []struct {
79+
name string
80+
ctx context.Context
81+
cmd *LoginCmd
82+
tk api.TokenGetter
83+
wantToken string
84+
wantErr bool
85+
wantErrMessage string
86+
}{
87+
{
88+
name: "interactive environment with token",
89+
ctx: format.WithForceInteractiveEnvironment(context.Background(), true),
90+
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: test.FakeJWTToken, Organization: "test"},
91+
tk: &fakeTokenGetter{},
92+
wantToken: test.FakeJWTToken,
93+
},
94+
{
95+
name: "non-interactive environment with token",
96+
ctx: context.Background(),
97+
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: test.FakeJWTToken, Organization: "test"},
98+
tk: &fakeTokenGetter{},
99+
wantToken: test.FakeJWTToken,
100+
},
101+
{
102+
name: "non-interactive environment with empty token",
103+
ctx: context.Background(),
104+
cmd: &LoginCmd{APIURL: "https://" + apiHost, APIToken: "", Organization: "test"},
105+
wantErr: true,
106+
wantErrMessage: ErrNonInteractiveEnvironmentEptyToken,
107+
},
108+
}
109+
for _, tt := range tests {
110+
t.Run(tt.name, func(t *testing.T) {
111+
t.Parallel()
112+
kubeconfig, err := os.CreateTemp("", "*-kubeconfig.yaml")
113+
if err != nil {
114+
log.Fatal(err)
115+
}
116+
defer os.Remove(kubeconfig.Name())
117+
os.Setenv(clientcmd.RecommendedConfigPathEnvVar, kubeconfig.Name())
118+
119+
err = tt.cmd.Run(tt.ctx, "", tt.tk)
120+
checkErrorRequire(t, err, tt.wantErr, tt.wantErrMessage)
121+
122+
if tt.wantErr {
123+
return
124+
}
125+
126+
// read out the kubeconfig again to test the contents
127+
b, err := io.ReadAll(kubeconfig)
128+
if err != nil {
129+
t.Fatal(err)
130+
}
131+
132+
kc, err := clientcmd.Load(b)
133+
if err != nil {
134+
t.Fatal(err)
135+
}
136+
137+
checkConfig(t, kc, 1, "")
138+
139+
if tt.wantToken != kc.AuthInfos[apiHost].Token {
140+
t.Fatalf("expected token to be %s, got %s", tt.wantToken, kc.AuthInfos[apiHost].Token)
141+
}
142+
143+
if kc.AuthInfos[apiHost].Exec != nil {
144+
t.Fatalf("expected execConfig to be empty, got %v", kc.AuthInfos[apiHost].Exec)
145+
}
146+
})
97147
}
98148
}
99149

100150
func TestLoginCmdWithoutExistingKubeconfig(t *testing.T) {
151+
ctx := format.WithForceInteractiveEnvironment(context.Background(), true)
152+
101153
dir, err := os.MkdirTemp("", "nctl-test-*")
102154
if err != nil {
103155
t.Fatal(err)
@@ -113,7 +165,7 @@ func TestLoginCmdWithoutExistingKubeconfig(t *testing.T) {
113165
IssuerURL: "https://auth.example.org",
114166
}
115167
tk := &fakeTokenGetter{}
116-
if err := cmd.Run(context.Background(), "", tk); err != nil {
168+
if err := cmd.Run(ctx, "", tk); err != nil {
117169
t.Fatal(err)
118170
}
119171

‎get/all.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (cmd *allCmd) Run(ctx context.Context, client *api.Client, get *Cmd) error
5858
case noHeader:
5959
return printItems(items, *get, defaultOut(cmd.out), false)
6060
case yamlOut:
61-
return format.PrettyPrintObjects(items, format.PrintOpts{Out: cmd.out})
61+
return format.PrettyPrintObjects(ctx, items, format.PrintOpts{Out: cmd.out})
6262
}
6363

6464
return nil

‎get/apiserviceaccount.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (asa *apiServiceAccountsCmd) Run(ctx context.Context, client *api.Client, g
5454
case noHeader:
5555
return asa.print(asaList.Items, get, false)
5656
case yamlOut:
57-
return format.PrettyPrintObjects(asaList.GetItems(), format.PrintOpts{})
57+
return format.PrettyPrintObjects(ctx, asaList.GetItems(), format.PrintOpts{})
5858
}
5959

6060
return nil

‎get/application.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@ func (cmd *applicationsCmd) Run(ctx context.Context, client *api.Client, get *Cm
4343
fmt.Fprintf(defaultOut(cmd.out), "no application with basic auth enabled found\n")
4444
return err
4545
}
46-
if printErr := printCredentials(creds, get, defaultOut(cmd.out)); printErr != nil {
46+
if printErr := printCredentials(ctx, creds, get, defaultOut(cmd.out)); printErr != nil {
4747
err = multierror.Append(err, printErr)
4848
}
4949
return err
5050
}
5151

5252
if cmd.DNS {
53-
return printDNSDetails(util.GatherDNSDetails(appList.Items), get, defaultOut(cmd.out))
53+
return printDNSDetails(ctx, util.GatherDNSDetails(appList.Items), get, defaultOut(cmd.out))
5454
}
5555

5656
switch get.Output {
@@ -59,7 +59,7 @@ func (cmd *applicationsCmd) Run(ctx context.Context, client *api.Client, get *Cm
5959
case noHeader:
6060
return printApplication(appList.Items, get, defaultOut(cmd.out), false)
6161
case yamlOut:
62-
return format.PrettyPrintObjects(appList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
62+
return format.PrettyPrintObjects(ctx, appList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
6363
case stats:
6464
return cmd.printStats(ctx, client, appList.Items, get, defaultOut(cmd.out))
6565
}
@@ -101,9 +101,9 @@ func printApplication(apps []apps.Application, get *Cmd, out io.Writer, header b
101101
return w.Flush()
102102
}
103103

104-
func printCredentials(creds []appCredentials, get *Cmd, out io.Writer) error {
104+
func printCredentials(ctx context.Context, creds []appCredentials, get *Cmd, out io.Writer) error {
105105
if get.Output == yamlOut {
106-
return format.PrettyPrintObjects(creds, format.PrintOpts{Out: out})
106+
return format.PrettyPrintObjects(ctx, creds, format.PrintOpts{Out: out})
107107
}
108108
return printCredentialsTabRow(creds, get, out)
109109
}
@@ -162,9 +162,9 @@ func join(list []string) string {
162162
return strings.Join(list, ",")
163163
}
164164

165-
func printDNSDetails(items []util.DNSDetail, get *Cmd, out io.Writer) error {
165+
func printDNSDetails(ctx context.Context, items []util.DNSDetail, get *Cmd, out io.Writer) error {
166166
if get.Output == yamlOut {
167-
return format.PrettyPrintObjects(items, format.PrintOpts{Out: out})
167+
return format.PrettyPrintObjects(ctx, items, format.PrintOpts{Out: out})
168168
}
169169
return printDNSDetailsTabRow(items, get, out)
170170
}

‎get/build.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (cmd *buildCmd) Run(ctx context.Context, client *api.Client, get *Cmd) erro
6464
case noHeader:
6565
return printBuild(buildList.Items, get, defaultOut(cmd.out), false)
6666
case yamlOut:
67-
return format.PrettyPrintObjects(buildList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
67+
return format.PrettyPrintObjects(ctx, buildList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
6868
}
6969

7070
return nil

‎get/cloudvm.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (cmd *cloudVMCmd) Run(ctx context.Context, client *api.Client, get *Cmd) er
3535
case noHeader:
3636
return cmd.printCloudVirtualMachineInstances(cloudVMList.Items, get, false)
3737
case yamlOut:
38-
return format.PrettyPrintObjects(cloudVMList.GetItems(), format.PrintOpts{})
38+
return format.PrettyPrintObjects(ctx, cloudVMList.GetItems(), format.PrintOpts{})
3939
}
4040

4141
return nil

‎get/clusters.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (l *clustersCmd) Run(ctx context.Context, client *api.Client, get *Cmd) err
3535
case noHeader:
3636
return printClusters(clusterList.Items, get, false)
3737
case yamlOut:
38-
return format.PrettyPrintObjects(clusterList.GetItems(), format.PrintOpts{})
38+
return format.PrettyPrintObjects(ctx, clusterList.GetItems(), format.PrintOpts{})
3939
case contexts:
4040
for _, cluster := range clusterList.Items {
4141
fmt.Printf("%s\n", config.ContextName(&cluster))

‎get/keyvaluestore.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (cmd *keyValueStoreCmd) Run(ctx context.Context, client *api.Client, get *C
4242
case noHeader:
4343
return cmd.printKeyValueStoreInstances(keyValueStoreList.Items, get, false)
4444
case yamlOut:
45-
return format.PrettyPrintObjects(keyValueStoreList.GetItems(), format.PrintOpts{})
45+
return format.PrettyPrintObjects(ctx, keyValueStoreList.GetItems(), format.PrintOpts{})
4646
}
4747

4848
return nil

‎get/mysql.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (cmd *mySQLCmd) Run(ctx context.Context, client *api.Client, get *Cmd) erro
5151
case noHeader:
5252
return cmd.printMySQLInstances(mysqlList.Items, get, false)
5353
case yamlOut:
54-
return format.PrettyPrintObjects(mysqlList.GetItems(), format.PrintOpts{})
54+
return format.PrettyPrintObjects(ctx, mysqlList.GetItems(), format.PrintOpts{})
5555
}
5656

5757
return nil

‎get/postgres.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (cmd *postgresCmd) Run(ctx context.Context, client *api.Client, get *Cmd) e
5151
case noHeader:
5252
return cmd.printPostgresInstances(postgresList.Items, get, false)
5353
case yamlOut:
54-
return format.PrettyPrintObjects(postgresList.GetItems(), format.PrintOpts{})
54+
return format.PrettyPrintObjects(ctx, postgresList.GetItems(), format.PrintOpts{})
5555
}
5656

5757
return nil

‎get/project.go

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func (proj *projectCmd) Run(ctx context.Context, client *api.Client, get *Cmd) e
4343
return printProject(projectList, *get, defaultOut(proj.out), false)
4444
case yamlOut:
4545
return format.PrettyPrintObjects(
46+
ctx,
4647
(&management.ProjectList{Items: projectList}).GetItems(),
4748
format.PrintOpts{
4849
Out: proj.out,

‎get/project_config.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (cmd *configsCmd) Run(ctx context.Context, client *api.Client, get *Cmd) er
3535
case noHeader:
3636
return printProjectConfigs(projectConfigList.Items, get, defaultOut(cmd.out), false)
3737
case yamlOut:
38-
return format.PrettyPrintObjects(projectConfigList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
38+
return format.PrettyPrintObjects(ctx, projectConfigList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
3939
}
4040

4141
return nil

‎get/releases.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (cmd *releasesCmd) Run(ctx context.Context, client *api.Client, get *Cmd) e
4646
case noHeader:
4747
return cmd.printReleases(releaseList.Items, get, false)
4848
case yamlOut:
49-
return format.PrettyPrintObjects(releaseList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
49+
return format.PrettyPrintObjects(ctx, releaseList.GetItems(), format.PrintOpts{Out: defaultOut(cmd.out)})
5050
}
5151

5252
return nil

0 commit comments

Comments
 (0)