Skip to content

Commit ee62276

Browse files
denikclaudepietern
authored
direct: validate that InputType < StateType < RemoteType (#4442)
## Changes * Validate that StateType is superset of InputType while RemoteType is superset of StateType. * Extend structaccess.Validate to support wildcard paths. ## Why See comments on the tests why it is important. --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
1 parent 70c0de7 commit ee62276

File tree

3 files changed

+303
-12
lines changed

3 files changed

+303
-12
lines changed

‎bundle/direct/dresources/type_test.go‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
package dresources
2+
3+
import (
4+
"reflect"
5+
"slices"
6+
"testing"
7+
8+
"github.com/databricks/cli/libs/structs/structaccess"
9+
"github.com/databricks/cli/libs/structs/structpath"
10+
"github.com/databricks/cli/libs/structs/structtag"
11+
"github.com/databricks/cli/libs/structs/structwalk"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// knownMissingInRemoteType lists fields that exist in StateType but not in RemoteType.
16+
// These are known issues that should be fixed. If a field listed here is found in RemoteType,
17+
// the test fails to ensure the entry is removed from this map.
18+
var knownMissingInRemoteType = map[string][]string{
19+
"clusters": {
20+
"apply_policy_default_values",
21+
},
22+
"jobs": {
23+
"budget_policy_id",
24+
"continuous",
25+
"deployment",
26+
"description",
27+
"edit_mode",
28+
"email_notifications",
29+
"environments",
30+
"format",
31+
"git_source",
32+
"health",
33+
"job_clusters",
34+
"max_concurrent_runs",
35+
"name",
36+
"notification_settings",
37+
"parameters",
38+
"performance_target",
39+
"queue",
40+
"run_as",
41+
"schedule",
42+
"tags",
43+
"tasks",
44+
"timeout_seconds",
45+
"trigger",
46+
"usage_policy_id",
47+
"webhook_notifications",
48+
},
49+
"model_serving_endpoints": {
50+
"ai_gateway",
51+
"budget_policy_id",
52+
"config",
53+
"description",
54+
"email_notifications",
55+
"name",
56+
"rate_limits",
57+
"route_optimized",
58+
"tags",
59+
},
60+
"pipelines": {
61+
"allow_duplicate_names",
62+
"budget_policy_id",
63+
"catalog",
64+
"channel",
65+
"clusters",
66+
"configuration",
67+
"continuous",
68+
"deployment",
69+
"development",
70+
"dry_run",
71+
"edition",
72+
"environment",
73+
"event_log",
74+
"filters",
75+
"gateway_definition",
76+
"id",
77+
"ingestion_definition",
78+
"libraries",
79+
"notifications",
80+
"photon",
81+
"restart_window",
82+
"root_path",
83+
"schema",
84+
"serverless",
85+
"storage",
86+
"tags",
87+
"target",
88+
"trigger",
89+
"usage_policy_id",
90+
},
91+
"quality_monitors": {
92+
"skip_builtin_dashboard",
93+
"warehouse_id",
94+
},
95+
"secret_scopes": {
96+
"backend_azure_keyvault",
97+
"initial_manage_principal",
98+
"scope",
99+
"scope_backend_type",
100+
},
101+
"postgres_branches": {
102+
"branch_id",
103+
"expire_time",
104+
"is_protected",
105+
"no_expiry",
106+
"source_branch",
107+
"source_branch_lsn",
108+
"source_branch_time",
109+
"ttl",
110+
},
111+
"postgres_endpoints": {
112+
"autoscaling_limit_max_cu",
113+
"autoscaling_limit_min_cu",
114+
"disabled",
115+
"endpoint_id",
116+
"endpoint_type",
117+
"no_suspension",
118+
"settings",
119+
"suspend_timeout_duration",
120+
},
121+
"postgres_projects": {
122+
"default_endpoint_settings",
123+
"display_name",
124+
"history_retention_duration",
125+
"pg_version",
126+
"project_id",
127+
},
128+
}
129+
130+
// commonMissingInStateType lists fields that are commonly missing across all resource types.
131+
// These are bundle-specific fields that exist in InputType but not in StateType.
132+
var commonMissingInStateType = []string{
133+
"grants",
134+
"lifecycle",
135+
"permissions",
136+
}
137+
138+
// knownMissingInStateType lists resource-specific fields that exist in InputType but not in StateType.
139+
// Fields in commonMissingInStateType are automatically included for all types.
140+
// Note: Fields with bundle:"internal" or bundle:"readonly" tags are automatically skipped.
141+
var knownMissingInStateType = map[string][]string{
142+
"alerts": {
143+
"file_path",
144+
},
145+
"apps": {
146+
"config",
147+
"source_code_path",
148+
},
149+
"dashboards": {
150+
"file_path",
151+
},
152+
"secret_scopes": {
153+
"backend_type",
154+
"keyvault_metadata",
155+
"name",
156+
},
157+
}
158+
159+
// TestInputSubset validates that all fields in InputType exist in StateType.
160+
//
161+
// This is important because the "changes" block in JSON plan contains paths that work on StateType.
162+
// If that does not align with config, we cannot use the path to relate changes back to input config.
163+
//
164+
// We also resolve pre-deploy references against state, not input.
165+
func TestInputSubset(t *testing.T) {
166+
for resourceType, resource := range SupportedResources {
167+
adapter, err := NewAdapter(resource, resourceType, nil)
168+
require.NoError(t, err)
169+
170+
t.Run(resourceType, func(t *testing.T) {
171+
inputType := adapter.InputConfigType()
172+
stateType := adapter.StateType()
173+
174+
// Validate that all fields in InputType exist in StateType
175+
var missingFields []string
176+
err := structwalk.WalkType(inputType, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) bool {
177+
if path.IsRoot() {
178+
return true
179+
}
180+
// Skip fields marked as internal or readonly in bundle tags
181+
if field != nil {
182+
btag := structtag.BundleTag(field.Tag.Get("bundle"))
183+
if btag.Internal() || btag.ReadOnly() {
184+
return false // don't recurse into internal/readonly fields
185+
}
186+
}
187+
if structaccess.Validate(stateType, path) != nil {
188+
missingFields = append(missingFields, path.String())
189+
return false // don't recurse into missing field
190+
}
191+
return true
192+
})
193+
require.NoError(t, err)
194+
195+
known := append(commonMissingInStateType, knownMissingInStateType[resourceType]...)
196+
197+
// Check that resource-specific known missing fields are actually missing
198+
for _, f := range knownMissingInStateType[resourceType] {
199+
if !slices.Contains(missingFields, f) {
200+
t.Errorf("field %q is listed in knownMissingInStateType but exists in StateType; remove it from the list", f)
201+
}
202+
}
203+
204+
// Filter out known missing fields
205+
var unexpectedMissing []string
206+
for _, f := range missingFields {
207+
if !slices.Contains(known, f) {
208+
unexpectedMissing = append(unexpectedMissing, f)
209+
}
210+
}
211+
212+
if len(unexpectedMissing) > 0 {
213+
t.Errorf("fields in InputType not found in StateType: %v", unexpectedMissing)
214+
}
215+
})
216+
}
217+
}
218+
219+
// TestRemoteSuperset validates that all fields in StateType exist in RemoteType.
220+
//
221+
// This is important because:
222+
// 1. The JSON plan contains paths that work on StateType.
223+
// 2. We include the raw (not remapped) remote value in the "remote_state" field.
224+
// 3. To find a remote value corresponding to a given change, paths must work on RemoteType as well.
225+
//
226+
// Why not include result of RemapState(RemoteType) as remote_state? Because remote_state is also used
227+
// to resolve remote references, which may have more fields than state by design. It's useful
228+
// for debugging to see those values in the detailed plan.
229+
func TestRemoteSuperset(t *testing.T) {
230+
for resourceType, resource := range SupportedResources {
231+
adapter, err := NewAdapter(resource, resourceType, nil)
232+
require.NoError(t, err)
233+
234+
t.Run(resourceType, func(t *testing.T) {
235+
stateType := adapter.StateType()
236+
remoteType := adapter.RemoteType()
237+
238+
// Validate that all fields in StateType exist in RemoteType
239+
var missingFields []string
240+
err := structwalk.WalkType(stateType, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) bool {
241+
if path.IsRoot() {
242+
return true
243+
}
244+
if structaccess.Validate(remoteType, path) != nil {
245+
missingFields = append(missingFields, path.String())
246+
return false // don't recurse into missing field
247+
}
248+
return true
249+
})
250+
require.NoError(t, err)
251+
252+
known := knownMissingInRemoteType[resourceType]
253+
254+
// Check that known missing fields are actually missing
255+
for _, f := range known {
256+
if !slices.Contains(missingFields, f) {
257+
t.Errorf("field %q is listed in knownMissingInRemoteType but exists in RemoteType; remove it from the list", f)
258+
}
259+
}
260+
261+
// Filter out known missing fields
262+
var unexpectedMissing []string
263+
for _, f := range missingFields {
264+
if !slices.Contains(known, f) {
265+
unexpectedMissing = append(unexpectedMissing, f)
266+
}
267+
}
268+
269+
if len(unexpectedMissing) > 0 {
270+
t.Errorf("fields in StateType not found in RemoteType: %v", unexpectedMissing)
271+
}
272+
})
273+
}
274+
}

‎libs/structs/structaccess/get_test.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ import (
1111

1212
// unexported global test case type
1313
type testCase struct {
14-
name string
15-
path string
16-
want any
17-
wantSelf bool
18-
errFmt string
19-
notFound string // if set, expect NotFoundError with this message
14+
name string
15+
path string
16+
want any
17+
wantSelf bool
18+
errFmt string
19+
notFound string // if set, expect NotFoundError with this message
20+
getOnlyErr string // if set, Validate passes but Get returns this error
2021
}
2122

2223
func testGet(t *testing.T, obj any, path string, want any) {
@@ -181,9 +182,9 @@ func runCommonTests(t *testing.T, obj any) {
181182

182183
// Errors common to both
183184
{
184-
name: "wildcard not supported",
185-
path: "items[*].id",
186-
errFmt: "wildcards not supported: items[*].id",
185+
name: "wildcard not supported for Get",
186+
path: "items[*].id",
187+
getOnlyErr: "wildcards not supported: items[*].id",
187188
},
188189
{
189190
name: "missing field",
@@ -275,6 +276,10 @@ func runCommonTests(t *testing.T, obj any) {
275276
require.ErrorAs(t, err, &notFound)
276277
return
277278
}
279+
if tt.getOnlyErr != "" {
280+
require.EqualError(t, err, tt.getOnlyErr)
281+
return
282+
}
278283
if tt.errFmt != "" {
279284
require.EqualError(t, err, tt.errFmt)
280285
var notFound *NotFoundError

‎libs/structs/structaccess/typecheck.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,21 @@ func Validate(t reflect.Type, path *structpath.PathNode) error {
5252
continue
5353
}
5454

55-
// Handle wildcards
56-
if node.DotStar() || node.BracketStar() {
57-
return fmt.Errorf("wildcards not supported: %s", path.String())
55+
// Handle wildcards - treat like index/key access
56+
if node.BracketStar() {
57+
kind := cur.Kind()
58+
if kind != reflect.Slice && kind != reflect.Array {
59+
return fmt.Errorf("%s: cannot use [*] on %s", node.String(), kind)
60+
}
61+
cur = cur.Elem()
62+
continue
63+
}
64+
if node.DotStar() {
65+
if cur.Kind() != reflect.Map {
66+
return fmt.Errorf("%s: cannot use .* on %s", node.String(), cur.Kind())
67+
}
68+
cur = cur.Elem()
69+
continue
5870
}
5971

6072
// Handle key-value selector: validates that we can index the slice/array

0 commit comments

Comments
 (0)