feat(helm): helm values can be either a string or a map (#2936) (#11538)

* feat: values can be either a string or a map

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Allow viewing and editing values

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* fix golang lint

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* fix codegen make build green

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Add special cases for handling nil/empty string. strip newline at end of generated yaml. fix unittests

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Add tests. Fix e2e marshalJSON

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* fix codegen. make codegen and make codegen-local give different results to me

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Update helm_test.go

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Fix UI - validate input

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* change implementation; introduce valuesObject instead of values

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* More generated files. Fix tests

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Fix eslint

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* chore: make codegen

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: use sigs.k8s.io/yaml

In the same vein as #13292, since `github.com/ghodss/yaml` is no longer
maintained.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: compact values

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: make manifests

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* docs: add to helm user guide

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* docs: remove any deprecation mentions

Remove any mentions that `Values` is deprecated, for now.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: make codegen

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: add assertion

verify that valuesobject overrides values by checking the number of
replicas.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* test: remove assertion

This wasn't meant to be in there, was a typo.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* chore: minor nit fix

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

---------

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

assets/swagger.json

@@ -4953,6 +4953,17 @@
         }
       }
     },
+    "runtimeRawExtension": {
+      "description": "RawExtension is used to hold extensions in external versions.\n\nTo use this, make a field which has RawExtension as its type in your external, versioned\nstruct, and Object in your internal struct. You also need to register your\nvarious plugin types.\n\n// Internal package:\ntype MyAPIObject struct {\n\truntime.TypeMeta `json:\",inline\"`\n\tMyPlugin runtime.Object `json:\"myPlugin\"`\n}\ntype PluginA struct {\n\tAOption string `json:\"aOption\"`\n}\n\n// External package:\ntype MyAPIObject struct {\n\truntime.TypeMeta `json:\",inline\"`\n\tMyPlugin runtime.RawExtension `json:\"myPlugin\"`\n}\ntype PluginA struct {\n\tAOption string `json:\"aOption\"`\n}\n\n// On the wire, the JSON will look something like this:\n{\n\t\"kind\":\"MyAPIObject\",\n\t\"apiVersion\":\"v1\",\n\t\"myPlugin\": {\n\t\t\"kind\":\"PluginA\",\n\t\t\"aOption\":\"foo\",\n\t},\n}\n\nSo what happens? Decode first uses json or yaml to unmarshal the serialized data into\nyour external MyAPIObject. That causes the raw JSON to be stored, but not unpacked.\nThe next step is to copy (using pkg/conversion) into the internal struct. The runtime\npackage's DefaultScheme has conversion functions installed which will unpack the\nJSON stored in RawExtension, turning it into the correct object type, and storing it\nin the Object. (TODO: In the case where the object is of an unknown type, a\nruntime.Unknown object will be created and stored.)\n\n+k8s:deepcopy-gen=true\n+protobuf=true\n+k8s:openapi-gen=true",
+      "type": "object",
+      "properties": {
+        "raw": {
+          "description": "Raw is the underlying serialization of this object.\n\nTODO: Determine how to detect ContentType and ContentEncoding of 'Raw' data.",
+          "type": "string",
+          "format": "byte"
+        }
+      }
+    },
     "runtimeStreamError": {
       "type": "object",
       "properties": {
@@ -6165,7 +6176,10 @@
         },
         "values": {
           "type": "string",
-          "title": "Values specifies Helm values to be passed to helm template, typically defined as a block"
+          "title": "Values specifies Helm values to be passed to helm template, typically defined as a block. ValuesObject takes precedence over Values, so use one or the other.\n+patchStrategy=replace"
+        },
+        "valuesObject": {
+          "$ref": "#/definitions/runtimeRawExtension"
         },
         "version": {
           "type": "string",

cmd/argocd/commands/app.go

@@ -794,9 +794,11 @@ func unset(source *argoappv1.ApplicationSource, opts unsetOpts) (updated bool, n
 				}
 			}
 		}
-		if opts.valuesLiteral && source.Helm.Values != "" {
-			source.Helm.Values = ""
-			updated = true
+		if opts.valuesLiteral && !source.Helm.ValuesIsEmpty() {
+			err := source.Helm.SetValuesString("")
+			if err == nil {
+				updated = true
+			}
 		}
 		for _, valuesFile := range opts.valuesFiles {
 			specValueFiles := source.Helm.ValueFiles

cmd/argocd/commands/app_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
@@ -778,7 +779,7 @@ func Test_unset(t *testing.T) {
 				},
 			},
 			PassCredentials: true,
-			Values:          "some: yaml",
+			ValuesObject:    &runtime.RawExtension{Raw: []byte("some: yaml")},
 			ValueFiles: []string{
 				"values-1.yaml",
 				"values-2.yaml",
@@ -864,9 +865,9 @@ func Test_unset(t *testing.T) {
 	assert.False(t, updated)
 	assert.False(t, nothingToUnset)
 
-	assert.Equal(t, "some: yaml", helmSource.Helm.Values)
+	assert.Equal(t, "some: yaml", helmSource.Helm.ValuesString())
 	updated, nothingToUnset = unset(helmSource, unsetOpts{valuesLiteral: true})
-	assert.Equal(t, "", helmSource.Helm.Values)
+	assert.Equal(t, "", helmSource.Helm.ValuesString())
 	assert.True(t, updated)
 	assert.False(t, nothingToUnset)
 	updated, nothingToUnset = unset(helmSource, unsetOpts{valuesLiteral: true})

cmd/util/app.go

@@ -427,7 +427,10 @@ func setHelmOpt(src *argoappv1.ApplicationSource, opts helmOpts) {
 		src.Helm.IgnoreMissingValueFiles = opts.ignoreMissingValueFiles
 	}
 	if len(opts.values) > 0 {
-		src.Helm.Values = opts.values
+		err := src.Helm.SetValuesString(opts.values)
+		if err != nil {
+			log.Fatal(err)
+		}
 	}
 	if opts.releaseName != "" {
 		src.Helm.ReleaseName = opts.releaseName

docs/operator-manual/application.yaml

@@ -51,7 +51,7 @@ spec:
       # Ignore locally missing valueFiles when installing Helm chart. Defaults to false
       ignoreMissingValueFiles: false
 
-      # Values file as block file
+      # Values file as block file. Prefer to use valuesObject if possible (see below)
       values: |
         ingress:
           enabled: true
@@ -67,6 +67,22 @@ spec:
               hosts:
                 - mydomain.example.com
 
+      # Values file as block file. This takes precedence over values
+      valuesObject:
+        ingress:
+          enabled: true
+          path: /
+          hosts:
+            - mydomain.example.com
+          annotations:
+            kubernetes.io/ingress.class: nginx
+            kubernetes.io/tls-acme: "true"
+          labels: {}
+          tls:
+            - secretName: mydomain-tls
+              hosts:
+                - mydomain.example.com
+
       # Skip custom resource definition installation if chart contains custom resource definitions. Defaults to false
       skipCrds: false
 

docs/user-guide/helm.md

@@ -52,7 +52,28 @@ source:
 
 ## Values
 
-Argo CD supports the equivalent of a values file directly in the Application manifest using the `source.helm.values` key.
+Argo CD supports the equivalent of a values file directly in the Application manifest using the `source.helm.valuesObject` key.
+
+```
+source:
+  helm:
+    valuesObject:
+      ingress:
+        enabled: true
+        path: /
+        hosts:
+          - mydomain.example.com
+        annotations:
+          kubernetes.io/ingress.class: nginx
+          kubernetes.io/tls-acme: "true"
+        labels: {}
+        tls:
+          - secretName: mydomain-tls
+            hosts:
+              - mydomain.example.com
+```
+
+Alternatively, values can be passed in as a string using the `source.helm.values` key.
 
 ```
 source: