Use dynamic walk for validating unique resource identifiers #1414
Conversation
|
Note: #1425 will add a test to assert resources are maps of string -> pointers to a struct. |
bundle/config/root.go
Outdated
|
|
||
| // Error, encountered a duplicate resource identifier. | ||
| return v, fmt.Errorf("multiple resources named %s (%s at %v, %s at %v)", k, ot, ol, nt, nl) | ||
| } |
There was a problem hiding this comment.
This is the same as above. Any chance we can perform this check after loading everything? Then we could surface every conflict in the same diag. It can also be a read-only mutator at that point instead of functions on the root struct that are invoked every time we merge.
bundle/config/root_test.go
Outdated
| assert.ErrorContains(t, diags.Error(), "multiple resources named foo (job at ./testdata/duplicate_resource_names_in_root/databricks.yml, pipeline at ./testdata/duplicate_resource_names_in_root/databricks.yml)") | ||
| func TestDuplicateIdOnLoadReturnsErrorForJobAndPipeline(t *testing.T) { | ||
| _, diags := Load("./testdata/duplicate_resource_names_in_root_job_and_pipeline/databricks.yml") | ||
| assert.ErrorContains(t, diags.Error(), "multiple resources named foo (job at ./testdata/duplicate_resource_names_in_root_job_and_pipeline/databricks.yml:10:7, pipeline at ./testdata/duplicate_resource_names_in_root_job_and_pipeline/databricks.yml:13:7)") |
There was a problem hiding this comment.
Why is this error OK with slashes on Windows and below you need to run filepath.FromSlash?
| resources: | ||
| jobs: | ||
| foo: | ||
| name: job foo |
There was a problem hiding this comment.
If the test focuses on just the duplicate jobs and hardcodes the loads, these can be resources1 and resources2, for example. I was looking at this and thinking this won't work because there is no include section in this file, while it does because the loads are explicit.
bundle/config/root.go
Outdated
| k := p[1].Key() | ||
| if _, ok := paths[k]; ok { | ||
| // Type and location of the existing resource in the map. | ||
| ot := strings.TrimSuffix(paths[k][0].Key(), "s") |
There was a problem hiding this comment.
we do this suffix trim in many places but shall we just get rid of it and show the message like?
multiple resources named <name> (jobs.<name> at ... and pipelines.<name> at ...)
If not, then better move it to a separate function and reuse
Changes
Using dynamic walk here removes the need to add validation when a new resource type is added. All resources will be automatically validated to have unique keys.
Tests
Modified existing unit tests and added new ones.