Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect merge of ValuesYamlFile for helmv3.NewRelease #2958

Closed
KevinFairise2 opened this issue Apr 17, 2024 · 1 comment · Fixed by #2963
Closed

Incorrect merge of ValuesYamlFile for helmv3.NewRelease #2958

KevinFairise2 opened this issue Apr 17, 2024 · 1 comment · Fixed by #2963
Assignees
Labels
area/helm kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@KevinFairise2
Copy link
Contributor

KevinFairise2 commented Apr 17, 2024

What happened?

Helm values are not merged as expected when providing an array with several yaml files to ValueYamlFiles.
For instance with the following values:
(1)

image:
  repository: bitnami/nginx

and
(2)

image:
  tag: 1.25.0

We'd expect the following values to be applied

image:
  tag: 1.25.0
  repository: bitnami/nginx

However only (1) or (2) is applied depending on the order you put them in the list.

Note: The behavior described above is the current Helm behavior, when specifying several values file, Helm will deep merge them together. helm/helm#1620

The following lines seem to be responsible for the unexpected behavior:

func decodeRelease(pm resource.PropertyMap, label string) (*Release, error) {
var release Release
values := map[string]any{}
stripped := pm.MapRepl(nil, mapReplExtractValues)
logger.V(9).Infof("[%s] Decoding release: %#v", label, stripped)
if v, ok := stripped["valueYamlFiles"]; ok {
switch reflect.TypeOf(v).Kind() {
case reflect.Slice, reflect.Array:
s := reflect.ValueOf(v)
for i := 0; i < s.Len(); i++ {
val := s.Index(i).Interface()
switch t := val.(type) {
case *resource.Asset:
b, err := t.Bytes()
if err != nil {
return nil, err
}
if err = yaml.Unmarshal(b, &values); err != nil {
return nil, err
}
default:
return nil, fmt.Errorf("unsupported type for 'valueYamlFiles' arg: %T", v)
}
}
}
}
var err error
if err = mapstructure.Decode(stripped, &release); err != nil {
return nil, fmt.Errorf("decoding failure: %w", err)
}
release.Values, err = mergeMaps(values, release.Values, release.AllowNullValues)
if err != nil {
return nil, err
}
return &release, nil
}

The merge of the values rely on yaml.Unmarshall which does not handle correctly the nested fields.

Related PR: #2963

Let me know if I should add anything to the issue

Example

package main

import (
	corev1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/core/v1"
	helmv3 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/helm/v3"
	metav1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/meta/v1"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi/config"
)

func main() {
	pulumi.Run(func(ctx *pulumi.Context) error {
		cfg := config.New(ctx, "")
		k8sNamespace, err := cfg.Try("k8sNamespace")
		if err != nil {
			k8sNamespace = "default"
		}
		appLabels := pulumi.StringMap{
			"app": pulumi.String("nginx-ingress"),
		}

		// Create a new namespace (user supplies the name of the namespace)
		ingressNs, err := corev1.NewNamespace(ctx, "ingressns", &corev1.NamespaceArgs{
			Metadata: &metav1.ObjectMetaArgs{
				Labels: pulumi.StringMap(appLabels),
				Name:   pulumi.String(k8sNamespace),
			},
		})
		if err != nil {
			return err
		}
		helmValues1 := `
image:
  repository: bitnami/nginx
  `
		helmValues2 := `
image:
  tag: 1.25.0  
`
		var helmValues pulumi.AssetOrArchiveArray
		helmValues = append(helmValues, pulumi.NewStringAsset(helmValues2), pulumi.NewStringAsset(helmValues1))
		// Use Helm to install the Nginx ingress controller
		ingresscontroller, err := helmv3.NewRelease(ctx, "ingresscontroller", &helmv3.ReleaseArgs{
			Chart:     pulumi.String("nginx"),
			Namespace: ingressNs.Metadata.Name(),
			RepositoryOpts: &helmv3.RepositoryOptsArgs{
				Repo: pulumi.String("https://charts.bitnami.com/bitnami"),
			},
			SkipCrds:       pulumi.Bool(true),
			ValueYamlFiles: helmValues,
			Version:        pulumi.String("16.0.3"),
		})
		if err != nil {
			return err
		}

		// Export some values for use elsewhere
		ctx.Export("name", ingresscontroller.Name)
		return nil
	})
}

Running the following code with kubectl properly configured to a Kubernetes cluster can show the issue.

You will find the deployed resource with different tag on the image depending on the order of the helmValues:
append(helmValues, pulumi.NewStringAsset(helmValues2), pulumi.NewStringAsset(helmValues1))
or
append(helmValues, pulumi.NewStringAsset(helmValues1), pulumi.NewStringAsset(helmValues2))

Output of pulumi about

CLI          
Version      3.103.1
Go Version   go1.21.5
Go Compiler  gc

Plugins
NAME        VERSION
go          unknown
kubernetes  4.0.3

Host     
OS       darwin
Version  13.5
Arch     arm64

This project is written in go: executable='/usr/local/go/bin/go' version='go version go1.21.6 darwin/arm64'

Current Stack: organization/pulumi-helm-debug/dev

TYPE                           URN
pulumi:pulumi:Stack            urn:pulumi:dev::pulumi-helm-debug::pulumi:pulumi:Stack::pulumi-helm-debug-dev
pulumi:providers:kubernetes    urn:pulumi:dev::pulumi-helm-debug::pulumi:providers:kubernetes::default
kubernetes:core/v1:Namespace   urn:pulumi:dev::pulumi-helm-debug::kubernetes:core/v1:Namespace::ingressns
kubernetes:helm.sh/v3:Release  urn:pulumi:dev::pulumi-helm-debug::kubernetes:helm.sh/v3:Release::ingresscontroller


OPP TYPE  URN
creating  urn:pulumi:dev::pulumi-helm-debug::kubernetes:helm.sh/v3:Release::ingresscontroller
creating  urn:pulumi:dev::pulumi-helm-debug::kubernetes:helm.sh/v3:Release::ingresscontroller


Backend        
Name           
URL            file://~
User          
Organizations  
Token type     personal

Dependencies:
NAME                                        VERSION
github.com/pulumi/pulumi-kubernetes/sdk/v4  4.0.3
github.com/pulumi/pulumi/sdk/v3             3.96.1

Pulumi locates its logs in /var/folders/_b/sjpwr13931b95__1m3thg78h0000gp/T/ by default

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@KevinFairise2 KevinFairise2 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Apr 17, 2024
@blampe
Copy link
Contributor

blampe commented Apr 18, 2024

Note: The behavior described above is the current Helm behavior, when specifying several values file, Helm will deep merge them together. helm/helm#1620

Thank you for this context. If our behavior differs from the helm CLI that is indeed a bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants