Upgrade to yaml.v3

[nick-stroud]

The UnmarshalStrict api is no longer available in v3 but the same functionality is supported using a Decoder with KnownFields(true) set.

Changes from map[interface{}]interface{} to map[string]interface{} were required to avoid panic. It seems that the decoder is providing more explicit type information for deeply nested values.

New behavior on marshaling introduces additional white space in listed items (starting with -). This means that expanded.yaml files generated using expand have different formatting after this change. There is a good summary here.

Tested:

• Ran expand before and after on community/examples/spack-gromacs.yaml and observed diff (whitespace changes only)
• Tested unmarshal with unknown fields (strict behavior). Fails as expected.
• Tested unmarshal with missing fields. Fails as expected.

Submission Checklist

• Have you installed and run this change against pre-commit? (pre-commit install)
• Are all tests passing? (make tests)
• Have you written unit tests to cover this change?
• Is unit test coverage still above 80%?
• Have you updated all applicable documentation?
• Have you followed the guidelines in our Contributing document?

[tpdownes]

When I run ghcp expand on the PR branch and use the spack-gromacs example, I get output that looks like:

blueprint_name: spack-gromacs
validators:
    - validator: test_project_exists
      inputs:
        project_id: ((var.project_id))
...
vars:
    deployment_name: spack-gromacs
    labels:
        ghpc_blueprint: spack-gromacs
        ghpc_deployment: spack-gromacs
    project_id: toolkit-demo-zero-e913
    region: us-central1
    zone: us-central1-c

Lists are indented by 4 characters and, for some reason, the labels map is also. Other maps are indented by 2 characters.

The status quo looks like this

blueprint_name: spack-gromacs
validators:
- validator: test_project_exists
  inputs:
    project_id: ((var.project_id))
- validator: test_region_exists
  inputs:
    project_id: ((var.project_id))
    region: ((var.region))
- validator: test_zone_exists
  inputs:
    project_id: ((var.project_id))
    zone: ((var.zone))
- validator: test_zone_in_region
  inputs:
    project_id: ((var.project_id))
    region: ((var.region))
    zone: ((var.zone))
vars:
  deployment_name: spack-gromacs
  labels:
    ghpc_blueprint: spack-gromacs
    ghpc_deployment: spack-gromacs
  project_id: toolkit-demo-zero-e913
  region: us-central1
  zone: us-central1-c

I believe a few paths are reasonable from here:

1. ghpc expand should produce a "diff-less" YAML file compared to status quo
2. ghpc expand should produce something that passes our linting tests
3. we should alter the behavior of ghpc expand to something we can defend (and linting tests should be modified to match)

[tpdownes]

Since the full checks passed, I don't think they need to be re-run if you address the "superficial" concerns in the review.

[nick-stroud]

I believe a few paths are reasonable from here

First note, that the diff is less noticeable now that indent has been set to 2. Also description has been updated.

From offline discussion:

• Diff-less (option 1) is not achievable with the official yaml package. kubernetes-sigs plans to create a permanent fork to that will re-enable the old white space. This fork does not yet exist and may introduce extra complication to use the fork.
• Decision: We will live with the extra white space in expand.yaml provided by yaml.v3.
• Since this only affects the marshal behavior, we do not need to change the format of our input blueprint. It will mean that expanded blueprints have slightly different whitespace compared to non-expanded blueprints. By changing this line to indent-sequences: consistent (ref) the linter would pass on both blueprints and expanded.yaml. This would also mean that we could introduce blueprints that different formatting. Since we do not submit expanded blueprints into the repo, my suggestion would be to leave the linter config as is.

FYI @tpdownes

[tpdownes]

The final result passes yamllint in its default configuration, rather than ours which mimics common patterns seen in the wild. I am happy enough with that outcome subject to all the upstream library problems you note.