Skip to content

Commit 69c5178

Browse files
moonmingmoonming
committed
fix(context): move HTTP timeout to validateContext, add unit tests
Remove global 30s timeout from NewAuthenticatedClient() that was killing long-running CI operations (config dump ~55s, config sync ~166s). Apply a dedicated 10s timeout only inside validateContext(). Add --skip-validation to E2E context tests using fake localhost:7443 since they test context management, not server connectivity. Add 10 unit tests for validateContext() and createRun() covering success, auth errors, unreachable servers, gateway group validation, and --skip-validation flag.
1 parent e5c73d4 commit 69c5178

File tree

4 files changed

+231
-9
lines changed

4 files changed

+231
-9
lines changed

‎pkg/api/client.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"net/http"
10-
"time"
1110
)
1211

1312
// Client is a thin wrapper around net/http for the API7 EE Admin API.
@@ -33,7 +32,6 @@ func NewAuthenticatedClient(apiKey string, tlsSkipVerify bool, caCert string) *h
3332
}
3433
return &http.Client{
3534
Transport: transport,
36-
Timeout: 30 * time.Second,
3735
}
3836
}
3937

‎pkg/cmd/context/create/create.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"net/http"
7+
"time"
78

89
"github.com/spf13/cobra"
910

@@ -58,6 +59,7 @@ func NewCmd(f *cmd.Factory) *cobra.Command {
5859

5960
func validateContext(ctx config.Context) error {
6061
httpClient := api.NewAuthenticatedClient(ctx.Token, ctx.TLSSkipVerify, ctx.CACert)
62+
httpClient.Timeout = 10 * time.Second
6163
client := api.NewClient(httpClient, ctx.Server)
6264

6365
// Test connectivity with lightweight API call

‎pkg/cmd/context/create/create_test.go‎

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
package create
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/api7/a7/internal/config"
13+
cmd "github.com/api7/a7/pkg/cmd"
14+
"github.com/api7/a7/pkg/iostreams"
15+
)
16+
17+
func TestValidateContext_Success(t *testing.T) {
18+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
19+
assert.Equal(t, "/api/gateway_groups", r.URL.Path)
20+
w.WriteHeader(http.StatusOK)
21+
_, _ = w.Write([]byte(`{"total":1,"list":[{"id":"default","name":"default"}]}`))
22+
}))
23+
defer srv.Close()
24+
25+
err := validateContext(config.Context{
26+
Server: srv.URL,
27+
Token: "valid-token",
28+
})
29+
require.NoError(t, err)
30+
}
31+
32+
func TestValidateContext_InvalidToken(t *testing.T) {
33+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
34+
w.WriteHeader(http.StatusUnauthorized)
35+
_, _ = w.Write([]byte(`{"error_msg":"invalid token"}`))
36+
}))
37+
defer srv.Close()
38+
39+
err := validateContext(config.Context{
40+
Server: srv.URL,
41+
Token: "bad-token",
42+
})
43+
require.Error(t, err)
44+
assert.Contains(t, err.Error(), "authentication failed: invalid token")
45+
}
46+
47+
func TestValidateContext_PermissionDenied(t *testing.T) {
48+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
49+
w.WriteHeader(http.StatusForbidden)
50+
_, _ = w.Write([]byte(`{"error_msg":"forbidden"}`))
51+
}))
52+
defer srv.Close()
53+
54+
err := validateContext(config.Context{
55+
Server: srv.URL,
56+
Token: "restricted-token",
57+
})
58+
require.Error(t, err)
59+
assert.Contains(t, err.Error(), "permission denied")
60+
}
61+
62+
func TestValidateContext_ServerError(t *testing.T) {
63+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
64+
w.WriteHeader(http.StatusInternalServerError)
65+
_, _ = w.Write([]byte(`{"error_msg":"internal error"}`))
66+
}))
67+
defer srv.Close()
68+
69+
err := validateContext(config.Context{
70+
Server: srv.URL,
71+
Token: "some-token",
72+
})
73+
require.Error(t, err)
74+
assert.Contains(t, err.Error(), "server error")
75+
}
76+
77+
func TestValidateContext_UnreachableServer(t *testing.T) {
78+
err := validateContext(config.Context{
79+
Server: "https://127.0.0.1:1",
80+
Token: "some-token",
81+
})
82+
require.Error(t, err)
83+
assert.Contains(t, err.Error(), "cannot connect to server")
84+
}
85+
86+
func TestValidateContext_GatewayGroupNotFound(t *testing.T) {
87+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
88+
switch r.URL.Path {
89+
case "/api/gateway_groups":
90+
w.WriteHeader(http.StatusOK)
91+
_, _ = w.Write([]byte(`{"total":0,"list":[]}`))
92+
case "/api/gateway_groups/nonexistent":
93+
w.WriteHeader(http.StatusNotFound)
94+
_, _ = w.Write([]byte(`{"error_msg":"not found"}`))
95+
default:
96+
w.WriteHeader(http.StatusNotFound)
97+
}
98+
}))
99+
defer srv.Close()
100+
101+
err := validateContext(config.Context{
102+
Server: srv.URL,
103+
Token: "valid-token",
104+
GatewayGroup: "nonexistent",
105+
})
106+
require.Error(t, err)
107+
assert.Contains(t, err.Error(), `gateway group "nonexistent" not found`)
108+
}
109+
110+
func TestValidateContext_GatewayGroupExists(t *testing.T) {
111+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
112+
switch r.URL.Path {
113+
case "/api/gateway_groups":
114+
w.WriteHeader(http.StatusOK)
115+
_, _ = w.Write([]byte(`{"total":1,"list":[{"id":"default","name":"default"}]}`))
116+
case "/api/gateway_groups/default":
117+
w.WriteHeader(http.StatusOK)
118+
_, _ = w.Write([]byte(`{"id":"default","name":"default"}`))
119+
default:
120+
w.WriteHeader(http.StatusNotFound)
121+
}
122+
}))
123+
defer srv.Close()
124+
125+
err := validateContext(config.Context{
126+
Server: srv.URL,
127+
Token: "valid-token",
128+
GatewayGroup: "default",
129+
})
130+
require.NoError(t, err)
131+
}
132+
133+
func TestValidateContext_VerifiesAPIKeyHeader(t *testing.T) {
134+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
135+
if r.Header.Get("X-API-KEY") != "expected-token" {
136+
w.WriteHeader(http.StatusUnauthorized)
137+
_, _ = w.Write([]byte(`{"error_msg":"invalid token"}`))
138+
return
139+
}
140+
w.WriteHeader(http.StatusOK)
141+
_, _ = w.Write([]byte(`{"total":0,"list":[]}`))
142+
}))
143+
defer srv.Close()
144+
145+
err := validateContext(config.Context{
146+
Server: srv.URL,
147+
Token: "expected-token",
148+
})
149+
require.NoError(t, err)
150+
151+
err = validateContext(config.Context{
152+
Server: srv.URL,
153+
Token: "wrong-token",
154+
})
155+
require.Error(t, err)
156+
assert.Contains(t, err.Error(), "authentication failed")
157+
}
158+
159+
func TestCreateRun_SkipValidation(t *testing.T) {
160+
cfgPath := fmt.Sprintf("%s/config.yaml", t.TempDir())
161+
cfg := config.NewFileConfigWithPath(cfgPath)
162+
163+
opts := &Options{
164+
Config: func() (config.Config, error) {
165+
return cfg, nil
166+
},
167+
Name: "test-ctx",
168+
Server: "https://127.0.0.1:1",
169+
Token: "fake-token",
170+
SkipValidation: true,
171+
}
172+
173+
ios, _, _, _ := iostreams.Test()
174+
f := &cmd.Factory{
175+
IOStreams: ios,
176+
Config: func() (config.Config, error) {
177+
return cfg, nil
178+
},
179+
}
180+
181+
err := createRun(opts, f)
182+
require.NoError(t, err)
183+
184+
saved, err := cfg.GetContext("test-ctx")
185+
require.NoError(t, err)
186+
assert.Equal(t, "https://127.0.0.1:1", saved.Server)
187+
assert.Equal(t, "fake-token", saved.Token)
188+
}
189+
190+
func TestCreateRun_ValidationFails(t *testing.T) {
191+
cfgPath := fmt.Sprintf("%s/config.yaml", t.TempDir())
192+
cfg := config.NewFileConfigWithPath(cfgPath)
193+
194+
opts := &Options{
195+
Config: func() (config.Config, error) {
196+
return cfg, nil
197+
},
198+
Name: "test-ctx",
199+
Server: "https://127.0.0.1:1",
200+
Token: "fake-token",
201+
SkipValidation: false,
202+
}
203+
204+
ios, _, _, _ := iostreams.Test()
205+
f := &cmd.Factory{
206+
IOStreams: ios,
207+
Config: func() (config.Config, error) {
208+
return cfg, nil
209+
},
210+
}
211+
212+
err := createRun(opts, f)
213+
require.Error(t, err)
214+
assert.Contains(t, err.Error(), "validation failed")
215+
assert.Contains(t, err.Error(), "cannot connect to server")
216+
217+
_, getErr := cfg.GetContext("test-ctx")
218+
assert.Error(t, getErr)
219+
}

‎test/e2e/context_test.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func TestContext_CreateAndUse(t *testing.T) {
2020
"--token", "test123",
2121
"--gateway-group", "default",
2222
"--tls-skip-verify",
23+
"--skip-validation",
2324
)
2425
require.NoError(t, err, stderr)
2526

@@ -33,6 +34,7 @@ func TestContext_CreateAndUse(t *testing.T) {
3334
"--token", "test123",
3435
"--gateway-group", "default",
3536
"--tls-skip-verify",
37+
"--skip-validation",
3638
)
3739
require.NoError(t, err, stderr)
3840

@@ -47,9 +49,9 @@ func TestContext_CreateAndUse(t *testing.T) {
4749
func TestContext_List(t *testing.T) {
4850
env := []string{"A7_CONFIG_DIR=" + t.TempDir()}
4951

50-
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443")
52+
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443", "--skip-validation")
5153
require.NoError(t, err, stderr)
52-
_, stderr, err = runA7WithEnv(env, "context", "create", "staging", "--server", "https://localhost:7443")
54+
_, stderr, err = runA7WithEnv(env, "context", "create", "staging", "--server", "https://localhost:7443", "--skip-validation")
5355
require.NoError(t, err, stderr)
5456

5557
stdout, stderr, err := runA7WithEnv(env, "context", "list")
@@ -66,9 +68,9 @@ func TestContext_List(t *testing.T) {
6668
func TestContext_Delete(t *testing.T) {
6769
env := []string{"A7_CONFIG_DIR=" + t.TempDir()}
6870

69-
_, stderr, err := runA7WithEnv(env, "context", "create", "ctx-to-delete", "--server", "https://localhost:7443")
71+
_, stderr, err := runA7WithEnv(env, "context", "create", "ctx-to-delete", "--server", "https://localhost:7443", "--skip-validation")
7072
require.NoError(t, err, stderr)
71-
_, stderr, err = runA7WithEnv(env, "context", "create", "staging", "--server", "https://localhost:7443")
73+
_, stderr, err = runA7WithEnv(env, "context", "create", "staging", "--server", "https://localhost:7443", "--skip-validation")
7274
require.NoError(t, err, stderr)
7375

7476
_, stderr, err = runA7WithEnv(env, "context", "delete", "ctx-to-delete")
@@ -83,10 +85,10 @@ func TestContext_Delete(t *testing.T) {
8385
func TestContext_CreateDuplicate(t *testing.T) {
8486
env := []string{"A7_CONFIG_DIR=" + t.TempDir()}
8587

86-
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443")
88+
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443", "--skip-validation")
8789
require.NoError(t, err, stderr)
8890

89-
_, stderr, err = runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443")
91+
_, stderr, err = runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443", "--skip-validation")
9092
require.Error(t, err)
9193
assert.Contains(t, stderr, "already exists")
9294
}
@@ -102,7 +104,7 @@ func TestContext_UseNonExistent(t *testing.T) {
102104
func TestContext_DeleteActive(t *testing.T) {
103105
env := []string{"A7_CONFIG_DIR=" + t.TempDir()}
104106

105-
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443")
107+
_, stderr, err := runA7WithEnv(env, "context", "create", "local", "--server", "https://localhost:7443", "--skip-validation")
106108
require.NoError(t, err, stderr)
107109

108110
_, stderr, err = runA7WithEnv(env, "context", "delete", "local")
@@ -124,6 +126,7 @@ func TestContext_CreateWithCACert(t *testing.T) {
124126
"--token", "test123",
125127
"--gateway-group", "default",
126128
"--ca-cert", fakeCA,
129+
"--skip-validation",
127130
)
128131
require.NoError(t, err, stderr)
129132
}

0 commit comments

Comments
 (0)