Add NewPatternFromString and bundle.SetDefault; use it for volumes & dashboards

[denik]

Changes

• New dyn.NewPatternFromString to construct pattern from strings.
• New bundle.SetDefault() for easier setting of defaults.
• Replace configure_dashboards_defaults.go and configure_volume_defaults.go with new this new helper.

There could be slight behaviour change: we no longer ignore IsCannotTraverseNilError error. Per team discussions, null should not be set in yaml or python.

Why

• It's more declarative, so less error-prone.
• The string syntax for patterns is easier to read (and write): "resources.dashboards.*.parent_path" vs dyn.NewPattern(dyn.Key("resources"), dyn.Key("dashboards"), dyn.AnyKey(), dyn.Key("parent_path"))
• We're going to migrate many defaults from terraform, so need a compact representation.

Tests

Unit tests for old mutators were converted to acceptance tests in #2733

[denik]

I did not copy this because we discussed that nils should be hard errors. cc @pietern

[andrewnester]

I suggest to remove Internal error part

[denik]

This is to signal that the error is not with the user input.

[pietern]

For future PRs, would be nice to split the dyn changes to keep it self-contained.

[pietern]

In some cases this can still return a NoSuchKeyError.

If the Get fails with this error on a lower-level key, then setting it can still fail.

Consider foo.*.bar.qux. If foo.*.bar doesn't exist, we get the NoSuchKeyError. But the call to SetByPath also fails because it assumes the path leading up to the final component exists.

This is probably OK with the current patterns. But it's a risk, and we should have a hermetic definition of the behavior to future-proof it.

I now see the path is guaranteed to have a single component. Then it always works.

[pietern]

I like the simplification here, but wonder if we need to keep the defaults inlined or split them out to a separate mutator with default in the name so it is easy to locate.

[denik]

I like the simplification here, but wonder if we need to keep the defaults inlined or split them out to a separate mutator with default in the name so it is easy to locate.

Well what I'm trying to achieve there with this PR and comments PR before it is to being able to read the phase or a big chunk of a phase and understand what's happening. Having extra jump does not help with that.

That said, we can revisit this if it grows.

- // Reads (dynamic): resources.dashboards.* (checks for existing parent_path and embed_credentials)
- // Updates (dynamic): resources.dashboards.*.parent_path (sets to workspace.resource_path if not set)
- // Updates (dynamic): resources.dashboards.*.embed_credentials (sets to false if not set)
- // Reads (dynamic): resources.volumes.* (checks for existing volume_type)
- // Updates (dynamic): resources.volumes.*.volume_type (sets to "MANAGED" if not set)
+ {"resources.dashboards.*.parent_path", b.Config.Workspace.ResourcePath},
+ {"resources.dashboards.*.embed_credentials", false},
+ {"resources.volumes.*.volume_type", "MANAGED"},
+	}