Repository: kubermatic/machine-controller
Pull request #1413 (2022-08-23 09:41:19)

[release/v1.45] Fix unmarshalling for `configvarstring` in cloudProviderSpec
This is an automated cherry-pick of #1367

/assign ahmedwaleedmalik

```release-note
Fix unmarshalling for `configvarstring` in cloudProviderSpec.
```

kubermatic-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1413#" title="Author self-approved">kubermatic-bot</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **ahmedwaleedmalik** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @ahmedwaleedmalik` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubermatic/machine-controller/blob/release/v1.45/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["ahmedwaleedmalik"]} -->
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 6fd98e0f2b7e831dd268ffb3f0706636ddbb9bff</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1413#pullrequestreview-1081818334" title="Approved">ahmedwaleedmalik</a>*, *<a href="https://github.com/kubermatic/machine-controller/pull/1413#" title="Author self-approved">kubermatic-bot</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/release/v1.45/OWNERS)~~ [ahmedwaleedmalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: @kubermatic-bot: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-machine-controller-e2e-openstack-project-auth | ac434ceb9289da0ee9d662ead670ad2f2d4c1b10 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1413/pull-machine-controller-e2e-openstack-project-auth/1562012111637319680) | true | `/test pull-machine-controller-e2e-openstack-project-auth`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1413)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @kubermatic-bot: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-machine-controller-e2e-openstack-project-auth | ac434ceb9289da0ee9d662ead670ad2f2d4c1b10 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1413/pull-machine-controller-e2e-openstack-project-auth/1562012111637319680) | true | `/test pull-machine-controller-e2e-openstack-project-auth`
pull-machine-controller-e2e-azure | ac434ceb9289da0ee9d662ead670ad2f2d4c1b10 | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1413/pull-machine-controller-e2e-azure/1562012114992762880) | true | `/test pull-machine-controller-e2e-azure`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1413)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/providerconfig/types/types.go b/pkg/providerconfig/types/types.go
index 925d1abd..dfdfd448 100644
--- a/pkg/providerconfig/types/types.go
+++ b/pkg/providerconfig/types/types.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strconv"
 
 	clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
 	"github.com/kubermatic/machine-controller/pkg/jsonutil"
@@ -204,7 +205,16 @@ func (configVarString *ConfigVarString) UnmarshalJSON(b []byte) error {
 	if !bytes.HasPrefix(b, []byte("{")) {
 		b = bytes.TrimPrefix(b, []byte(`"`))
 		b = bytes.TrimSuffix(b, []byte(`"`))
-		configVarString.Value = string(b)
+
+		// `Unquote` expects the input string to be inside quotation marks.
+		//  Since we can have a string without any quotations, in which case `TrimPrefix` and
+		// `TrimSuffix` will be noop. We explicitly add quotation marks to the input string
+		// to make sure that `Unquote` never fails.
+		s, err := strconv.Unquote("\"" + string(b) + "\"")
+		if err != nil {
+			return err
+		}
+		configVarString.Value = s
 		return nil
 	}
 	// This type must have the same fields as ConfigVarString but not


Test patch:
diff --git a/pkg/providerconfig/types/types_test.go b/pkg/providerconfig/types/types_test.go
index 1ee10796..53cf64a8 100644
--- a/pkg/providerconfig/types/types_test.go
+++ b/pkg/providerconfig/types/types_test.go
@@ -21,7 +21,7 @@ import (
 	"reflect"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/utils/pointer"
 )
 
@@ -163,6 +163,7 @@ func TestConfigVarStringMarshallingAndUnmarshalling(t *testing.T) {
 
 	testCases := []ConfigVarString{
 		{Value: "val"},
+		{Value: "spe<ialv&lue"},
 		{SecretKeyRef: GlobalSecretKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"}},
 		{Value: "val", SecretKeyRef: GlobalSecretKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"}},
 		{ConfigMapKeyRef: GlobalConfigMapKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"}},
@@ -175,6 +176,11 @@ func TestConfigVarStringMarshallingAndUnmarshalling(t *testing.T) {
 			ConfigMapKeyRef: GlobalConfigMapKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"},
 			SecretKeyRef:    GlobalSecretKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"},
 		},
+		{
+			Value:           "spe<ialv&lue",
+			ConfigMapKeyRef: GlobalConfigMapKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"},
+			SecretKeyRef:    GlobalSecretKeySelector{ObjectReference: v1.ObjectReference{Namespace: "ns", Name: "name"}, Key: "key"},
+		},
 	}
 
 	for _, cvs := range testCases {