Skip to content

Commit 6af75b8

Browse files
pieternpietern
authored
Replace remaining real-domain test hosts with .test (#5189)
## Summary Follow-up to #5125 (Replace bar.com with bar.test in tests). The same footgun lived in many other test fixtures using real-resolving `.com` domains in `Config.Host`, `databricks.yml`'s `workspace.host`, and `.databrickscfg`: `foo.com`, `test.com`, `test2.com`, `myhost.com`, `myworkspace.com`, `x.com`, `a.com`, `www.host[12].com`, `other.host.com`. ## Why this surfaced now The SDK started calling `resolveHostMetadata` on every `Config` init in [databricks/databricks-sdk-go#1542](databricks/databricks-sdk-go#1542) (released in SDK v0.123.0). The CLI pulled this in via #4799 (bump v0.120.0 → v0.126.0). The resolver performs an HTTP fetch against the host's well-known endpoint with a retry loop. For non-resolving hosts (`.test`, `.invalid`, etc.) it fails fast. For real domains it depends on the network: a quick TCP RST or 404 also returns fast; a slow handshake or silent drop triggers a 5-minute retry window. Timeline of `task test (linux, direct)` job duration: | Date | SHA | Duration | Notable | |--------|-----------|----------|-------------------------------| | Apr 16 | f711183 | 497s | SDK 0.127.0 bump | | Apr 21 | a409f3d | 466s | SDK 0.128.0 bump | | Apr 28 | 82aa1b9 | 494s | last fast run | | Apr 29 | e84ed42 | 1077s | #5125 (bar.com -> bar.test) | | Apr 30 | 2867a0e | 1205s | +10min vs baseline | | May 6 | ec76b2e | 1178s | | The 0.127.0 / 0.128.0 bumps did not move the needle — the resolver was already live since the v0.126.0 bump in #4799. The +10min jump appeared on Apr 29, which lines up with the GitHub Actions runner network changing behavior toward these external domains — most likely a firewall or egress-filtering change that stopped fast-failing the lookups, turning every test that set a real-domain `Config.Host` into a 5-minute stall. #5125 fixed the bar.com family and recovered most of the regression. `foo.com` lived in a separate file with a slightly different naming convention and missed that sweep, leaving `TestFilesToSync_Everything*` bleeding ~10min combined — the residual still visible above. ## What this PR does - Mechanical `.com` → `.test` swap across all remaining test fixtures that set a host. Same low-risk pattern as #5125. - Includes the `cmd/root/bundle_test.go` set (`x.com` / `a.com` — both real domains), the schema testdata pass files (`myworkspace.com`), and the `cmd/auth` testdata fixture plus its consumers. - Adds a rule to `AGENTS.md` / `CLAUDE.md` citing [RFC 2606 §2](https://datatracker.ietf.org/doc/html/rfc2606#section-2) so future test fixtures use a non-resolving TLD by default. ## Test plan - [x] `go build ./...` clean - [x] Targeted packages pass: `bundle/config/validate`, `bundle/run`, `libs/auth`, `libs/cmdctx`, `libs/template`, `cmd/auth`, `cmd/root`, `bundle/deploy/terraform`, `bundle/internal/schema` - [x] Watch `task test (linux, direct)` duration on this PR — expect recovery to the ~470s baseline
1 parent d64d08b commit 6af75b8

19 files changed

Lines changed: 66 additions & 64 deletions

File tree

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ parentPath = "/Workspace" + parentPath
123123

124124
**RULE: Do not add defensive `nil` checks for values the caller or framework is documented to always provide.** If a check exists "just in case", either remove it or attach a comment explaining why the invariant might be violated. Direct engine resource methods (`DoCreate`, `DoUpdate`, `RemapState`, etc.) never receive nil receivers or state from the framework, so extra nil-guards there are dead code.
125125

126+
**RULE: Use a non-resolving TLD reserved by [RFC 2606 §2](https://datatracker.ietf.org/doc/html/rfc2606#section-2) (`.test`, `.example`, `.invalid`, `.localhost`) for any test fixture host — `Config.Host`, `databricks.yml`'s `workspace.host`, `.databrickscfg`.** Real domains hit the SDK well-known endpoint resolver and can stall tests for ~5 minutes per call when the runner network can't fast-fail the lookup. The repo convention is `.test` (the TLD RFC 2606 specifically reserves "for use in testing"). See PR #5125 for prior history.
127+
126128
Where a panic is genuinely possible (e.g. `reflect.Type.Elem()` on a non-pointer, division by an empty slice's length), validate at the entry point and return an error.
127129

128130
# Error Handling

‎bundle/config/validate/files_to_sync_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func setupBundleForFilesToSyncTest(t *testing.T) *bundle.Bundle {
6363

6464
m := mocks.NewMockWorkspaceClient(t)
6565
m.WorkspaceClient.Config = &sdkconfig.Config{
66-
Host: "https://foo.com",
66+
Host: "https://foo.test",
6767
}
6868

6969
// The initialization logic in [sync.New] performs a check on the destination path.

‎bundle/deploy/terraform/util_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestParseResourcesStateWithExistingStateFile(t *testing.T) {
7575
"storage": "dbfs:/123456",
7676
"target": "test_dev",
7777
"timeouts": null,
78-
"url": "https://test.com"
78+
"url": "https://test.test"
7979
},
8080
"sensitive_attributes": []
8181
}

‎bundle/internal/schema/testdata/pass/job.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ bundle:
22
name: a job
33

44
workspace:
5-
host: "https://myworkspace.com"
5+
host: "https://myworkspace.test"
66
root_path: /abc
77

88
presets:

‎bundle/internal/schema/testdata/pass/ml.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ bundle:
22
name: ML
33

44
workspace:
5-
host: "https://myworkspace.com"
5+
host: "https://myworkspace.test"
66
root_path: /abc
77

88
presets:

‎bundle/internal/schema/testdata/pass/pipeline.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ bundle:
22
name: a pipeline
33

44
workspace:
5-
host: "https://myworkspace.com"
5+
host: "https://myworkspace.test"
66
root_path: /abc
77

88
presets:

‎bundle/run/pipeline_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func TestPipelineRunnerRestart(t *testing.T) {
6969

7070
m := mocks.NewMockWorkspaceClient(t)
7171
m.WorkspaceClient.Config = &sdk_config.Config{
72-
Host: "https://test.com",
72+
Host: "https://test.test",
7373
}
7474
b.SetWorkpaceClient(m.WorkspaceClient)
7575

‎cmd/auth/auth_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,23 +70,23 @@ func TestValidateProfileHostConflict(t *testing.T) {
7070
// through Cobra's lifecycle (PreRunE on login) and that the root command's
7171
// PersistentPreRunE is NOT shadowed (it initializes logging, IO, user agent).
7272
func TestProfileHostConflictViaCobra(t *testing.T) {
73-
// Point at a config file that has "profile-1" with host https://www.host1.com.
73+
// Point at a config file that has "profile-1" with host https://www.host1.test.
7474
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")
7575

7676
ctx := cmdctx.GenerateExecId(t.Context())
7777
cli := root.New(ctx)
7878
cli.AddCommand(New())
7979

80-
// Set args: auth login --profile profile-1 --host https://other.host.com
80+
// Set args: auth login --profile profile-1 --host https://other.host.test
8181
cli.SetArgs([]string{
8282
"auth", "login",
8383
"--profile", "profile-1",
84-
"--host", "https://other.host.com",
84+
"--host", "https://other.host.test",
8585
})
8686

8787
_, err := cli.ExecuteContextC(ctx)
8888
require.Error(t, err)
89-
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.com", which conflicts with --host "https://other.host.com"`)
89+
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.test", which conflicts with --host "https://other.host.test"`)
9090
assert.Contains(t, err.Error(), "Use --profile only to select a profile")
9191
}
9292

@@ -101,12 +101,12 @@ func TestProfileHostConflictTokenViaCobra(t *testing.T) {
101101
cli.SetArgs([]string{
102102
"auth", "token",
103103
"--profile", "profile-1",
104-
"--host", "https://other.host.com",
104+
"--host", "https://other.host.test",
105105
})
106106

107107
_, err := cli.ExecuteContextC(ctx)
108108
require.Error(t, err)
109-
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.com", which conflicts with --host "https://other.host.com"`)
109+
assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.test", which conflicts with --host "https://other.host.test"`)
110110
}
111111

112112
// TestProfileHostCompatibleViaCobra verifies that matching --profile and --host
@@ -122,7 +122,7 @@ func TestProfileHostCompatibleViaCobra(t *testing.T) {
122122
cli.SetArgs([]string{
123123
"auth", "login",
124124
"--profile", "profile-1",
125-
"--host", "https://www.host1.com",
125+
"--host", "https://www.host1.test",
126126
})
127127

128128
_, err := cli.ExecuteContextC(ctx)

‎cmd/auth/describe_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestGetWorkspaceAuthStatus(t *testing.T) {
4444

4545
status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
4646
err := config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
47-
"host": "https://test.com",
47+
"host": "https://test.test",
4848
"token": "test-token",
4949
"auth_type": "azure-cli",
5050
})
@@ -55,7 +55,7 @@ func TestGetWorkspaceAuthStatus(t *testing.T) {
5555
require.NotNil(t, status)
5656
require.Equal(t, "success", status.Status)
5757
require.Equal(t, "test-user", status.Username)
58-
require.Equal(t, "https://test.com", status.Details.Host)
58+
require.Equal(t, "https://test.test", status.Details.Host)
5959
require.Equal(t, "azure-cli", status.Details.AuthType)
6060

6161
require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value)
@@ -97,7 +97,7 @@ func TestGetWorkspaceAuthStatusError(t *testing.T) {
9797

9898
status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
9999
err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
100-
"host": "https://test.com",
100+
"host": "https://test.test",
101101
"token": "test-token",
102102
"auth_type": "azure-cli",
103103
})
@@ -146,7 +146,7 @@ func TestGetWorkspaceAuthStatusSensitive(t *testing.T) {
146146

147147
status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
148148
err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
149-
"host": "https://test.com",
149+
"host": "https://test.test",
150150
"token": "test-token",
151151
"auth_type": "azure-cli",
152152
})
@@ -196,7 +196,7 @@ func TestGetAccountAuthStatus(t *testing.T) {
196196
err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
197197
"account_id": "test-account-id",
198198
"username": "test-user",
199-
"host": "https://test.com",
199+
"host": "https://test.test",
200200
"token": "test-token",
201201
"auth_type": "azure-cli",
202202
})
@@ -207,7 +207,7 @@ func TestGetAccountAuthStatus(t *testing.T) {
207207
require.Equal(t, "success", status.Status)
208208

209209
require.Equal(t, "test-user", status.Username)
210-
require.Equal(t, "https://test.com", status.Details.Host)
210+
require.Equal(t, "https://test.test", status.Details.Host)
211211
require.Equal(t, "azure-cli", status.Details.AuthType)
212212
require.Equal(t, "test-account-id", status.AccountID)
213213

‎cmd/auth/login_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,10 @@ func TestSetHost(t *testing.T) {
145145
assert.Equal(t, "val from --host", authArguments.Host)
146146

147147
// Test setting host from flag with trailing slash is stripped
148-
authArguments.Host = "https://www.host1.com/"
148+
authArguments.Host = "https://www.host1.test/"
149149
err = setHostAndAccountId(ctx, profile1, &authArguments, []string{})
150150
assert.NoError(t, err)
151-
assert.Equal(t, "https://www.host1.com", authArguments.Host)
151+
assert.Equal(t, "https://www.host1.test", authArguments.Host)
152152

153153
// Test setting host from argument
154154
authArguments.Host = ""
@@ -158,21 +158,21 @@ func TestSetHost(t *testing.T) {
158158

159159
// Test setting host from argument with trailing slash is stripped
160160
authArguments.Host = ""
161-
err = setHostAndAccountId(ctx, profile1, &authArguments, []string{"https://www.host1.com/"})
161+
err = setHostAndAccountId(ctx, profile1, &authArguments, []string{"https://www.host1.test/"})
162162
assert.NoError(t, err)
163-
assert.Equal(t, "https://www.host1.com", authArguments.Host)
163+
assert.Equal(t, "https://www.host1.test", authArguments.Host)
164164

165165
// Test setting host from profile
166166
authArguments.Host = ""
167167
err = setHostAndAccountId(ctx, profile1, &authArguments, []string{})
168168
assert.NoError(t, err)
169-
assert.Equal(t, "https://www.host1.com", authArguments.Host)
169+
assert.Equal(t, "https://www.host1.test", authArguments.Host)
170170

171171
// Test setting host from profile
172172
authArguments.Host = ""
173173
err = setHostAndAccountId(ctx, profile2, &authArguments, []string{})
174174
assert.NoError(t, err)
175-
assert.Equal(t, "https://www.host2.com", authArguments.Host)
175+
assert.Equal(t, "https://www.host2.test", authArguments.Host)
176176

177177
// Test host is not set. Should prompt.
178178
authArguments.Host = ""
@@ -275,14 +275,14 @@ func TestLoadProfileByNameAndClusterID(t *testing.T) {
275275
name: "cluster profile",
276276
profile: "cluster-profile",
277277
configFileEnv: "./testdata/.databrickscfg",
278-
expectedHost: "https://www.host2.com",
278+
expectedHost: "https://www.host2.test",
279279
expectedClusterID: "cluster-from-config",
280280
},
281281
{
282282
name: "profile from home directory (existing)",
283283
profile: "cluster-profile",
284284
homeDirOverride: "testdata",
285-
expectedHost: "https://www.host2.com",
285+
expectedHost: "https://www.host2.test",
286286
expectedClusterID: "cluster-from-config",
287287
},
288288
{

0 commit comments

Comments
 (0)