Only re-encode subst value if it smells like json and target doc is yaml

[dee0sap]

Description

Currently we always re-encode subst value in an effort to have its encoding ( json or yaml ) match that of the target document. More specifically we are trying to avoid having something that looks like a yaml doc with json mixed into it as this is off putting.

The problems with this are

• More complicated code as we need more error checks in place for the marshall and unmarshall calls that always take place
• At least as implemented, if the value is actually yaml we may lose some formatting and we will lose tags and anchors

With this PR we only perform the re-encoding if the target is not json and if the subst value 'smells' like json.

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🎇 Restructuring
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• [X ] 🔥 Performance Improvements
• [X ] ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Related Tickets & Documents

Please see discussion between myself and @Skarlso

Screenshots

Added tests?

• [x ] 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help
• Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Added to documentation?

• 📜 README.md
• [x ] 🙅 no documentation needed

Checklist:

• [ x] My code follows the style guidelines of this project
• [ x] I have performed a self-review of my code
• [ x] I have commented my code, particularly in hard-to-understand areas
• [ x] I have made corresponding changes to the documentation
• [ x] My changes generate no new warnings
• [ x] I have added tests that prove my fix is effective or that my feature works
• [ x] New and existing unit tests pass locally with my changes
• [ x] Any dependent changes have been merged and published in downstream modules

[Skarlso]

Running the tests for this pr.

[Skarlso]

make test
/Users/skarlso/goprojects/SAP/ocm-controller/bin/controller-gen rbac:roleName=ocm-controller-manager-role crd webhook paths="./api/..." paths="./controllers/..." output:crd:artifacts:config=config/crd/bases
/Users/skarlso/goprojects/SAP/ocm-controller/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./api/..." paths="./controllers/..."
go fmt ./...
go vet ./...
tinygo build -target=wasi -panic=trap -scheduler=none -no-debug -o ./internal/wasm/hostfuncs/resource/testdata/get_resource_bytes.wasm  ./internal/wasm/hostfuncs/resource/testdata/get_resource_bytes.go
tinygo build -target=wasi -panic=trap -scheduler=none -no-debug -o ./internal/wasm/hostfuncs/resource/testdata/get_resource_labels.wasm  ./internal/wasm/hostfuncs/resource/testdata/get_resource_labels.go
tinygo build -target=wasi -panic=trap -scheduler=none -no-debug -o ./internal/wasm/hostfuncs/resource/testdata/get_resource_url.wasm  ./internal/wasm/hostfuncs/resource/testdata/get_resource_url.go
KUBEBUILDER_ASSETS="/Users/skarlso/Library/Application Support/io.kubebuilder.envtest/k8s/1.24.1-darwin-amd64" go test ./... -coverprofile cover.out
        github.com/open-component-model/ocm-controller          coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/api/v1alpha1             coverage: 0.0% of statements
?       github.com/open-component-model/ocm-controller/internal/wasm/hostfuncs/types    [no test files]
?       github.com/open-component-model/ocm-controller/pkg/configdata   [no test files]
?       github.com/open-component-model/ocm-controller/pkg/cache        [no test files]
        github.com/open-component-model/ocm-controller/internal/wasm/hostfuncs          coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/metrics              coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/internal/wasm/hostfuncs/logging          coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/internal/wasm/io         coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/fakes                coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/cache/fakes          coverage: 0.0% of statements
ok      github.com/open-component-model/ocm-controller/controllers      7.269s  coverage: 45.7% of statements
ok      github.com/open-component-model/ocm-controller/internal/wasm/hostfuncs/resource 4.683s  coverage: 45.8% of statements
ok      github.com/open-component-model/ocm-controller/pkg/component    2.663s  coverage: 5.6% of statements
ok      github.com/open-component-model/ocm-controller/pkg/event        1.544s  coverage: 2.7% of statements
?       github.com/open-component-model/ocm-controller/pkg/version      [no test files]
        github.com/open-component-model/ocm-controller/pkg/status               coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/wasm/runtime         coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/wasm/errors          coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/ocm/fakes            coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/snapshot             coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/wasm/config          coverage: 0.0% of statements
        github.com/open-component-model/ocm-controller/pkg/version/generate             coverage: 0.0% of statements
ok      github.com/open-component-model/ocm-controller/pkg/oci  1.650s  coverage: 17.8% of statements
ok      github.com/open-component-model/ocm-controller/pkg/ocm  3.066s  coverage: 28.1% of statements

All tests passed with this pr.

[Skarlso]

@dee0sap Can you please sign your commits? Then we can merge this. :)

[dee0sap]

@dee0sap Can you please sign your commits? Then we can merge this. :)

Done
I had to reset the branch however as I made a mistake when rebasing. Nothing broken but the merges performed by you won't be in the history anymore

[Skarlso]

@dee0sap You will need this pr #791 to fix the lint issue once it's merged and has gone green. 🤞

[Skarlso]

Oh f*ck. The linter update, of course, will kill everything.

[Skarlso]

Sorry, I'm going to bed, we'll have to deal with those tomorrow. Sorry, Dan. :(