Support complex types as parameter set values

[erikced]

What does this change

This change updates the parameter sets to support number, object, array and boolean values for value sources, e.g.

schemaType: ParameterSet
schemaVersion: 1.1.0
name:  my-params
parameters:
  - name: some-param
    source:
      value: 
        a: [1, 2, 3, 4]
        b: abcd
  - name: some-other-param
      source:
        value: true

so that the parameter set value can be of the same type as the bundle parameter.

Currently this is only supported by manually converting the values to their JSON representation, which is less intuitive

  - name: some-param
    source: 
      value: '{"a": [1, 2, 3, 4], "b": "abcd"}'
  - name: some-other-param
      source:
        value: "true"

It is implemented by updating the unmarsalling code for Source so that when parameter with the value strategy is encountered and the hint is not a string, it is converted to its corresponding JSON string representation.

What issue does it fix

Closes #3331

Notes for the reviewer

Since the unmarshalling - and hence value transformation - is performed before the parameter set is stored in the parameter store the value stored will not necessarily match the input parameter set when running porter parameters apply, which might not be desirable(?). Even though porter will not itself write values that are not strings to the parameter store, it can read such values from the store as the BSON is converted to JSON and then unmarshalled in the same way as parameter sets from file.

Checklist

• Did you write tests?
• Did you write documentation?
• Did you change porter.yaml or a storage document record? Update the corresponding schema file.
• If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

[erikced]

I am a bit confused by this, as the hint should be an empty string at this point? I left it in nonetheless.

[schristoff]

Yeah, you're right, it would be empty here. 🤷🏻‍♀️ Not needed, but not harmful (yet) I assume

[kichristensen]

@erikced Thanks for the PR. This is not the part the I'm most familiar with, maybe @schristoff has some more insights. But I have a few questions.
Will the old complex "stringified" parameters still work, when passing them through the commandline? And what will be the expected way to pass complex types from the command line?
It would be great if you also could update the documentation to describe this functionality?
To make me more certain that this works end-to-end I would really like to see a simple integration test or two for covering this change, would that be possible?

[erikced]

Will the old complex "stringified" parameters still work, when passing them through the commandline? And what will be the expected way to pass complex types from the command line?

This MR should not change anything related to command line parameter usage. --param passes everything along as a string and string values do not even reach unmarshalRawValue so neither behavior nor the expected way to pass complex params on the command line has changed.

It would be great if you also could update the documentation to describe this functionality?

Added missing documentation of the updated format.

To make me more certain that this works end-to-end I would really like to see a simple integration test or two for covering this change, would that be possible?

I have added an integration test validating the handling of parameter set / command line parameter to finalized parameter. It is not a full integration test though, please let me know if you think it is sufficient or if a full e2e test is required.

[kichristensen]

@erikced Looks good to me. Am I correct when I think the new format will technically also work in version 1.0.1 of the parameter file, but it will fail schema validation if it is validated?

[erikced]

@erikced Looks good to me. Am I correct when I think the new format will technically also work in version 1.0.1 of the parameter file, but it will fail schema validation if it is validated?

@kichristensen yes, that should be the case.

As a side note, I am unsure whether the version should be 1.1.0 or 1.0.2, from a semver-like perspective it should probably be 1.1.0 since it is an extension of the functionality and not a patch (whatever that may be for a document) but other document versioning standards would probably treat it as 1.0.2 since all valid 1.0.0/1.0.1 documents are still valid.

[schristoff]

I like 1.1.0 👍