deleting breaking remove of helmx.1.rendered dir

[tomassatka]

Before the change when applying transformers user got following error from kustomize binary that kustomize failed and the reason was that in kustomization.yaml file were referred files inside helmx.1 directory however the directory was not existing at that point of time

tomas@tomas-ThinkPad-W550s:~/Develop/test$ helmfile template 
in ./helmfile.yaml: [exit status 1

COMMAND:
  kustomize build /tmp/chartify043502074/test --output /tmp/chartify043502074/test/all.patched.yaml

OUTPUT:
  Error: accumulating resources: accumulation err='accumulating resources from 'helmx.1.rendered/test/templates/serviceaccount.yaml': evalsymlink failure on '/tmp/chartify043502074/test/helmx.1.rendered/test/templates/serviceaccount.yaml' : lstat /tmp/chartify043502074/test/helmx.1.rendered: no such file or directory': evalsymlink failure on '/tmp/chartify043502074/test/helmx.1.rendered/test/templates/serviceaccount.yaml' : lstat /tmp/chartify043502074/test/helmx.1.rendered: no such file or directory]

solution was to remove the os.RemoveAll(helmOutputDir) poiting to helmx.1 dir

After removing and local testing the transformers works again.

Please consider to apply and include into helmfile as updated version of go dependency

[mumoshu]

@tomassatka Hey! Thanks for your efforts ☺️ Would you mind sharing your helmfile.yaml for reproduction? I thought it worked before so this might be either a regression or some edge-case that we didn't encounter before. A example helmfile.yaml helps isolatating the cause, so that I can better review this. Thanks!

[tomassatka]

hi @mumoshu

helm version : version.BuildInfo{Version:"v3.7.1", GitCommit:"1d11fcb5d3f3bf00dbe6fe31b8412839a96b3dc4", GitTreeState:"clean", GoVersion:"go1.16.9"} helmfile version : helmfile version v0.142.0 kustomize version : {Version:kustomize/v4.4.0 GitCommit:63ec6bdb3d737a7c66901828c5743656c49b60e1 BuildDate:2021-09-27T16:24:12Z GoOs:linux GoArch:amd64}

i took the most simple test case i could think of

created simple helm chart called test by helm create test

then added simple helmfile.yaml

releases:
  - name: test
    chart: ./ 
    version: 0.1.0
    transformers: 
    - ./transformer.yaml 

and simple transformer.yaml

apiVersion: builtin
kind: PatchTransformer
metadata:
  name: test-transformer
patch: |-
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    name: test
    labels:
      test-transformer: did-my-job
target:
  kind: Deployment
  name: test

feel free to try out on your own

i also fixed few parts for tests like chart repo url and minor parts around. I dont know how to fix the tests since i dont know the expected behavior.. but at least tests now run during git event..

[mumoshu]

@tomassatka Thanks. I was able to reproduce this. Let me review this real quick to see what's happening and how your fix works.

[mumoshu]

@tomassatka Hey. It turned out you found an edge-case!

So it seems to break only when you had chart: ./. It just work if you had placed your chart under somewhere else. For example, the below works if you created the chart under test/test and helmfile config is at test/helmfile.yaml

$ tree .
.
├── charts
├── Chart.yaml
├── helmfile.yaml
├── templates
│   ├── deployment.yaml
│   ├── _helpers.tpl
│   ├── hpa.yaml
│   ├── ingress.yaml
│   ├── NOTES.txt
│   ├── serviceaccount.yaml
│   ├── service.yaml
│   └── tests
│       └── test-connection.yaml
├── test
│   ├── charts
│   ├── Chart.yaml
│   ├── templates
│   │   ├── deployment.yaml
│   │   ├── _helpers.tpl
│   │   ├── hpa.yaml
│   │   ├── ingress.yaml
│   │   ├── NOTES.txt
│   │   ├── serviceaccount.yaml
│   │   ├── service.yaml
│   │   └── tests
│   │       └── test-connection.yaml
│   └── values.yaml
├── transformer.yaml
└── values.yaml

releases:
  - name: test
    chart: ./test
    version: 0.1.0
    transformers: 
    - ./transformer.yaml 

[icy]

@mumoshu think I have found another edge case, when the chart directory and chart name don't match. Basically we often have the following structure

./MyChartName/.
  Chart.yaml
  templates/

The Chart.yaml defines its name, e.g. chart: MyChartName. This will work well with chart: ./path/to/MyChartName within helmfile. In this case, helmx.1.render directory will not be printed out in kustomize output:

# .... generated and using kustomization.yaml:
kind: ""
apiversion: ""
resources:
- templates/env-vars.yaml
- templates/secrets.yaml
transformers:
- transformers/transformer.0.yaml

However, when there is a mismatch in chart's name vs chart directory name:

./foobar/.
  Chart.yaml  # chart: MyChartName
  templates/

helmfile will print out helmx.1.rendered in the kustomize file (I don't really get why), for example

# .... generated and using kustomization.yaml:
kind: ""
apiversion: ""
resources:
- helmx.1.rendered/MyChartName/templates/env-vars.yaml          # << Note, MyChartName is there
- helmx.1.rendered/MyChartName/templates/secrets.yaml
transformers:
- transformers/transformer.0.yaml

and the helmfile template will immediately generate an error.

I can confirm this bug on my laptop. I may clean up and send a clean example later.

[mumoshu]

@icy Would you mind sharing the relevant part of your helmfile? This bug affects things when your chart path's basename is .. So it would work with chart: ./foobar but not work for chart: ./foobar/. or chart: ./ or chart:..

[icy]

@icy Would you mind sharing the relevant part of your helmfile? This bug affects things when your chart path's basename is .. So it would work with chart: ./foobar but not work for chart: ./foobar/. or chart: ./ or chart:..

Hi @mumoshu, you can fetch the test code from this repo.

https://github.com/icy/helmfile-bug/tree/main/two

I used helmfile v0.143.0. In the code [1], if you rename the the chartName to test the issue is gone.

Please let me know if you need any further information. Thanks.

[1] https://github.com/icy/helmfile-bug/blob/main/two/charts/test/Chart.yaml#L1

[mumoshu]

@tomassatka @icy I finally got some time to work on what we've discussed in the thread and the result is #27. I was able to reproduce the issue using the new integration tests and confirmed the issue was fixed by the use of filepath.Clean before taking the basename of the directory. I'd appreciate it if you could give it a shot by building helmfile yourself replacing the chartify dependency using replace github.com/variantdev/chartify => ../chartify in helmfile's go.mod, or once I cut a new patch version of Helmfile(Not sure when though!

[icy]

@mumoshu Thanks for your work. I have given a test with my test https://github.com/icy/helmfile-bug/tree/main/two and the problem still remains, maybe I missed sth, or my problem is another one?

$ pwd
/home/gfg/go/src/github.com/roboll/helmfile

$ git clog -1
7423913

$ tail -1 go.mod
replace github.com/variantdev/chartify => ../chartify

$ (cd ../chartify && git clog -1 )
7b5aa80

$ go build

$ cd ~/projects/helmfile-bug/two

$ /home/gfg/go/src/github.com/roboll/helmfile/helmfile template 
in ./helmfile.yaml: [exit status 1

COMMAND:
  kustomize build /tmp/chartify1788651568/test --output /tmp/chartify1788651568/test/all.patched.yaml

OUTPUT:
  :: This is {Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:36:27Z GoOs:linux GoArch:amd64}
  Error: accumulating resources: accumulation err='accumulating resources from 'helmx.1.rendered/chartName/templates/test.yaml': evalsymlink failure on '/tmp/chartify1788651568/test/helmx.1.rendered/chartName/templates/test.yaml' : lstat /tmp/chartify1788651568/test/helmx.1.rendered: no such file or directory': evalsymlink failure on '/tmp/chartify1788651568/test/helmx.1.rendered/chartName/templates/test.yaml' : lstat /tmp/chartify1788651568/test/helmx.1.rendered: no such file or directory]

[mumoshu]

@icy Hey! Thanks for reporting. It turned out to be a different one. I was able to reproduce it via a new integration test case and fixed it as you can see at https://github.com/variantdev/chartify/pull/29

[icy]

@icy Hey! Thanks for reporting. It turned out to be a different one. I was able to reproduce it via a new integration test case and fixed it as you can see at #29

I confirm that works well now. Thanks a lot for your work.

[mumoshu]

@icy Thanks for your confirmation ☺️