Skip to content

Commit 415e632

Browse files
srenatussrenatus
committed
revision: hash for http datasource needs nesting
We can have multiple http (or s3) data sources per source, so input.sources["source-name"].http.hash doesn't do the trick. It needs to be input.sources["source-name"].http["http-datasource-name"].hash etc. Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
1 parent 94df60a commit 415e632

File tree

6 files changed

+320
-39
lines changed

6 files changed

+320
-39
lines changed

‎e2e/cli/build_revision_error_message.txtar‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ cd ..
1313
! exec $OPACTL build --config config.d/bundle.yml --data-dir tmp
1414
stderr 'BUILD_FAILED'
1515
stderr 'undefined ref: input.sources.policies.s3.bucket'
16-
stderr 'have: "bucket"'
17-
stderr 'want \(one of\): \["hash"\]'
16+
stderr 'have: "s3"'
17+
stderr 'want \(one of\): \["git" "sql"\]'
1818

1919
-- repo/policy.rego --
2020
package rules

‎e2e/cli/build_revision_http_hashsum.txtar‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
# Test revision using HTTP datasource hash
1+
# Test revision using HTTP datasource hash with multiple datasources and sources
22
exec $OPACTL build --config config.d/bundle.yml --data-dir tmp
33
stderr '^1/1 bundles built and pushed successfully$'
44
! stdout .
55

66
exec tar xf bundles/hello-world/bundle.tar.gz /.manifest
77

88
exec cat .manifest
9-
stdout '"revision":"[0-9a-f]{64}"'
10-
stdout '"roots":\["users"\]'
9+
# Revision should be a composite of all three hashes (each 8 chars, separated by dashes)
10+
stdout '"revision":"[0-9a-f]{8}-[0-9a-f]{8}-[0-9a-f]{8}"'
11+
stdout '"roots":\["headers","users","extra"\]'
1112
stdout '"rego_version":0'
1213

1314
-- config.d/bundle.yml --
@@ -17,13 +18,26 @@ bundles:
1718
filesystem:
1819
path: bundles/hello-world/bundle.tar.gz
1920
requirements:
20-
- source: http-data
21-
revision: 'input.sources["http-data"].http.hash'
21+
- source: data-a
22+
- source: data-b
23+
revision: '$"{substring(input.sources["data-a"].http.users.hash, 0, 8)}-{substring(input.sources["data-a"].http.headers.hash, 0, 8)}-{substring(input.sources["data-b"].http.extra.hash, 0, 8)}"'
2224
sources:
23-
http-data:
25+
data-a:
2426
datasources:
2527
- type: http
2628
name: users
2729
path: users
2830
config:
2931
url: http://${HTTP_ENDPOINT}/users
32+
- type: http
33+
name: headers
34+
path: headers
35+
config:
36+
url: http://${HTTP_ENDPOINT}/headers
37+
data-b:
38+
datasources:
39+
- type: http
40+
name: extra
41+
path: extra
42+
config:
43+
url: http://${HTTP_ENDPOINT}/users

‎pkg/service/revision.go‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,16 @@ func ResolveRevision(ctx context.Context, revision string, sourceMetadata map[st
9191
}
9292

9393
func buildInputSchema(sourceMetadata map[string]map[string]any) *ast.SchemaSet {
94-
sourceSchema := map[string]any{
94+
datasourceEntrySchema := map[string]any{
9595
"type": "object",
9696
"properties": map[string]any{
97+
"hash": map[string]any{"type": "string"},
98+
},
99+
}
100+
101+
sourceProps := make(map[string]any, len(sourceMetadata))
102+
for sourceName, types := range sourceMetadata {
103+
props := map[string]any{
97104
"git": map[string]any{
98105
"type": "object",
99106
"properties": map[string]any{
@@ -106,24 +113,26 @@ func buildInputSchema(sourceMetadata map[string]map[string]any) *ast.SchemaSet {
106113
"hash": map[string]any{"type": "string"},
107114
},
108115
},
109-
"http": map[string]any{
110-
"type": "object",
111-
"properties": map[string]any{
112-
"hash": map[string]any{"type": "string"},
113-
},
114-
},
115-
"s3": map[string]any{
116-
"type": "object",
117-
"properties": map[string]any{
118-
"hash": map[string]any{"type": "string"},
119-
},
120-
},
121-
},
122-
}
116+
}
123117

124-
sourceProps := make(map[string]any, len(sourceMetadata))
125-
for sourceName := range sourceMetadata {
126-
sourceProps[sourceName] = sourceSchema
118+
// For http and s3, build schema from actual datasource names in metadata
119+
for _, sourceType := range []string{"http", "s3"} {
120+
if typeData, ok := types[sourceType].(map[string]any); ok {
121+
dsProps := make(map[string]any, len(typeData))
122+
for dsName := range typeData {
123+
dsProps[dsName] = datasourceEntrySchema
124+
}
125+
props[sourceType] = map[string]any{
126+
"type": "object",
127+
"properties": dsProps,
128+
}
129+
}
130+
}
131+
132+
sourceProps[sourceName] = map[string]any{
133+
"type": "object",
134+
"properties": props,
135+
}
127136
}
128137

129138
schemas := ast.NewSchemaSet()

‎pkg/service/revision_test.go‎

Lines changed: 246 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,42 @@ func TestExtractRevisionRefs(t *testing.T) {
8888
{SourceName: "sql-source", Fields: []string{"file", "path"}},
8989
},
9090
},
91+
{
92+
name: "http datasource with name",
93+
revision: `input.sources["my-data"].http.users.hash`,
94+
want: []ReferencedSource{
95+
{SourceName: "my-data", Fields: []string{"http", "users", "hash"}},
96+
},
97+
},
98+
{
99+
name: "http datasource with bracket notation",
100+
revision: `input.sources["my-data"].http["user-list"].hash`,
101+
want: []ReferencedSource{
102+
{SourceName: "my-data", Fields: []string{"http", "user-list", "hash"}},
103+
},
104+
},
105+
{
106+
name: "s3 datasource with name",
107+
revision: `input.sources["my-data"].s3["model-weights"].hash`,
108+
want: []ReferencedSource{
109+
{SourceName: "my-data", Fields: []string{"s3", "model-weights", "hash"}},
110+
},
111+
},
112+
{
113+
name: "multiple datasources from same source",
114+
revision: `$"{input.sources.data.http.users.hash}-{input.sources.data.http.products.hash}"`,
115+
want: []ReferencedSource{
116+
{SourceName: "data", Fields: []string{"http", "users", "hash", "products"}},
117+
},
118+
},
119+
{
120+
name: "mixed git and http datasource",
121+
revision: `$"{input.sources.policies.git.commit}-{input.sources.data.http.users.hash}"`,
122+
want: []ReferencedSource{
123+
{SourceName: "policies", Fields: []string{"git", "commit"}},
124+
{SourceName: "data", Fields: []string{"http", "users", "hash"}},
125+
},
126+
},
91127
}
92128

93129
for _, tt := range tests {
@@ -160,7 +196,7 @@ func TestValidationErrorMessages(t *testing.T) {
160196
wantErrContains: `undefined ref: input.sources.nothere.git.commit`,
161197
},
162198
{
163-
name: "invalid source type - http (schema validation)",
199+
name: "invalid source type - http without datasources (schema validation)",
164200
revision: `input.sources.policies.http.url`,
165201
availableSources: []string{"policies"},
166202
wantErrContains: `undefined ref: input.sources.policies.http.url`,
@@ -172,7 +208,7 @@ func TestValidationErrorMessages(t *testing.T) {
172208
wantErrContains: `undefined ref: input.sources.policies.file.path`,
173209
},
174210
{
175-
name: "invalid source type - s3 (schema validation)",
211+
name: "invalid source type - s3 without datasources (schema validation)",
176212
revision: `input.sources.policies.s3.bucket`,
177213
availableSources: []string{"policies"},
178214
wantErrContains: `undefined ref: input.sources.policies.s3.bucket`,
@@ -200,3 +236,211 @@ func TestValidationErrorMessages(t *testing.T) {
200236
})
201237
}
202238
}
239+
240+
func TestValidationErrorMessagesWithDatasources(t *testing.T) {
241+
tests := []struct {
242+
name string
243+
revision string
244+
sourceMetadata map[string]map[string]any
245+
wantErrContains string
246+
}{
247+
{
248+
name: "unknown datasource name under http",
249+
revision: `input.sources.data.http.unknown.hash`,
250+
sourceMetadata: map[string]map[string]any{
251+
"data": {
252+
"http": map[string]any{
253+
"users": map[string]any{"hash": "abc123"},
254+
"products": map[string]any{"hash": "def456"},
255+
},
256+
},
257+
},
258+
wantErrContains: `undefined ref: input.sources.data.http.unknown.hash`,
259+
},
260+
{
261+
name: "unknown datasource name under s3",
262+
revision: `input.sources.data.s3.unknown.hash`,
263+
sourceMetadata: map[string]map[string]any{
264+
"data": {
265+
"s3": map[string]any{
266+
"model-weights": map[string]any{"hash": "abc123"},
267+
},
268+
},
269+
},
270+
wantErrContains: `undefined ref: input.sources.data.s3.unknown.hash`,
271+
},
272+
{
273+
name: "accessing http.hash directly without datasource name",
274+
revision: `input.sources.data.http.hash`,
275+
sourceMetadata: map[string]map[string]any{
276+
"data": {
277+
"http": map[string]any{
278+
"users": map[string]any{"hash": "abc123"},
279+
},
280+
},
281+
},
282+
wantErrContains: `undefined ref: input.sources.data.http.hash`,
283+
},
284+
{
285+
name: "wrong field name on datasource",
286+
revision: `input.sources.data.http.users.url`,
287+
sourceMetadata: map[string]map[string]any{
288+
"data": {
289+
"http": map[string]any{
290+
"users": map[string]any{"hash": "abc123"},
291+
},
292+
},
293+
},
294+
wantErrContains: `undefined ref: input.sources.data.http.users.url`,
295+
},
296+
{
297+
name: "http not in schema when no http datasources exist",
298+
revision: `input.sources.policies.http.users.hash`,
299+
sourceMetadata: map[string]map[string]any{
300+
"policies": {
301+
"git": map[string]any{"commit": "abc123"},
302+
},
303+
},
304+
wantErrContains: `undefined ref: input.sources.policies.http.users.hash`,
305+
},
306+
}
307+
308+
for _, tt := range tests {
309+
t.Run(tt.name, func(t *testing.T) {
310+
_, err := ResolveRevision(t.Context(), tt.revision, tt.sourceMetadata)
311+
if err == nil {
312+
t.Fatalf("expected error containing %q, got nil", tt.wantErrContains)
313+
}
314+
315+
if !strings.Contains(err.Error(), tt.wantErrContains) {
316+
t.Errorf("error message %q does not contain expected substring %q", err.Error(), tt.wantErrContains)
317+
}
318+
})
319+
}
320+
}
321+
322+
func TestResolveRevision(t *testing.T) {
323+
tests := []struct {
324+
name string
325+
revision string
326+
sourceMetadata map[string]map[string]any
327+
want string
328+
}{
329+
{
330+
name: "empty revision",
331+
revision: "",
332+
want: "",
333+
},
334+
{
335+
name: "static string",
336+
revision: `"v1.0.0"`,
337+
want: "v1.0.0",
338+
},
339+
{
340+
name: "git commit",
341+
revision: `input.sources.policies.git.commit`,
342+
sourceMetadata: map[string]map[string]any{
343+
"policies": {
344+
"git": map[string]any{"commit": "abc123def456"},
345+
},
346+
},
347+
want: "abc123def456",
348+
},
349+
{
350+
name: "sql hash",
351+
revision: `input.sources["sql-source"].sql.hash`,
352+
sourceMetadata: map[string]map[string]any{
353+
"sql-source": {
354+
"sql": map[string]any{"hash": "deadbeef"},
355+
},
356+
},
357+
want: "deadbeef",
358+
},
359+
{
360+
name: "http datasource hash",
361+
revision: `input.sources.data.http.users.hash`,
362+
sourceMetadata: map[string]map[string]any{
363+
"data": {
364+
"http": map[string]any{
365+
"users": map[string]any{"hash": "a1b2c3d4"},
366+
},
367+
},
368+
},
369+
want: "a1b2c3d4",
370+
},
371+
{
372+
name: "s3 datasource hash",
373+
revision: `input.sources.data.s3["model-weights"].hash`,
374+
sourceMetadata: map[string]map[string]any{
375+
"data": {
376+
"s3": map[string]any{
377+
"model-weights": map[string]any{"hash": "s3hash999"},
378+
},
379+
},
380+
},
381+
want: "s3hash999",
382+
},
383+
{
384+
name: "template with git commit substring",
385+
revision: `$"git-{substring(input.sources.policies.git.commit, 0, 7)}"`,
386+
sourceMetadata: map[string]map[string]any{
387+
"policies": {
388+
"git": map[string]any{"commit": "abc123def456"},
389+
},
390+
},
391+
want: "git-abc123d",
392+
},
393+
{
394+
name: "template combining git and http datasource",
395+
revision: `$"{substring(input.sources.policies.git.commit, 0, 7)}-{substring(input.sources.data.http.users.hash, 0, 7)}"`,
396+
sourceMetadata: map[string]map[string]any{
397+
"policies": {
398+
"git": map[string]any{"commit": "abc123def456"},
399+
},
400+
"data": {
401+
"http": map[string]any{
402+
"users": map[string]any{"hash": "fedcba9876543210"},
403+
},
404+
},
405+
},
406+
want: "abc123d-fedcba9",
407+
},
408+
{
409+
name: "template combining two http datasources from same source",
410+
revision: `$"{substring(input.sources.data.http.users.hash, 0, 8)}-{substring(input.sources.data.http.products.hash, 0, 8)}"`,
411+
sourceMetadata: map[string]map[string]any{
412+
"data": {
413+
"http": map[string]any{
414+
"users": map[string]any{"hash": "1111111122222222"},
415+
"products": map[string]any{"hash": "3333333344444444"},
416+
},
417+
},
418+
},
419+
want: "11111111-33333333",
420+
},
421+
{
422+
name: "http datasource with bracket notation",
423+
revision: `input.sources.data.http["user-list"].hash`,
424+
sourceMetadata: map[string]map[string]any{
425+
"data": {
426+
"http": map[string]any{
427+
"user-list": map[string]any{"hash": "bracket-hash"},
428+
},
429+
},
430+
},
431+
want: "bracket-hash",
432+
},
433+
}
434+
435+
for _, tt := range tests {
436+
t.Run(tt.name, func(t *testing.T) {
437+
got, err := ResolveRevision(t.Context(), tt.revision, tt.sourceMetadata)
438+
if err != nil {
439+
t.Fatalf("unexpected error: %v", err)
440+
}
441+
if got != tt.want {
442+
t.Errorf("got %q, want %q", got, tt.want)
443+
}
444+
})
445+
}
446+
}

0 commit comments

Comments
 (0)