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

System path added to requiredEnv if env starts with slash #481

Closed
strainovic opened this issue Oct 28, 2022 · 15 comments · Fixed by #487
Closed

System path added to requiredEnv if env starts with slash #481

strainovic opened this issue Oct 28, 2022 · 15 comments · Fixed by #487
Labels
question Further information is requested

Comments

@strainovic
Copy link
Contributor

Operating system

Windows 10

Helmfile Version

0.146.0

Helm Version

3.10.1

Bug description

When we set env variable TEST='/path' and refer to it with "{{ requiredEnv TEST }}", within git bash, output in the rendered manifest will get C:/Program Files/Git/path. If we remove the slash, the manifest will get path.

Any explanation for this?

Thanks

Example helmfile.yaml

not relevant

Error message you've seen (if any)

none

Steps to reproduce

Working Helmfile Version

unknown

Relevant discussion

No response

@yxxhero
Copy link
Member

yxxhero commented Oct 28, 2022

@strainovic please show your helmfile. So I will check what happened.

@yxxhero yxxhero added the question Further information is requested label Oct 28, 2022
@strainovic
Copy link
Contributor Author

helm create test

Content of helmfile.yaml

releases:
- name: test
  chart: ./test
  values:
  - ingress:
      enabled: true
      hosts:
        - host: chart-example.local
          paths:
            - path: "{{ requiredEnv `TEST_PATH` }}"
              pathType: ImplementationSpecific
export TEST_PATH='/aaa'
helmfile template
---
# Source: test/templates/ingress.yaml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test
  labels:
    helm.sh/chart: test-0.1.0
    app.kubernetes.io/name: test
    app.kubernetes.io/instance: test
    app.kubernetes.io/version: "1.16.0"
    app.kubernetes.io/managed-by: Helm
spec:
  rules:
    - host: "chart-example.local"
      http:
        paths:
          - path: C:/Program Files/Git/aaa
            pathType: ImplementationSpecific
            backend:
              service:
                name: test
                port:
                  number: 80

As you can see, path has value of C:/Program Files/Git/aaa

I ran this from the GitBash shell.

@yxxhero
Copy link
Member

yxxhero commented Oct 28, 2022

try:

{{ requiredEnv "TEST_PATH"  | quote }}

@strainovic
Copy link
Contributor Author

strainovic commented Oct 31, 2022

It still does not work :(

releases:
- name: test
  chart: ./test
  values:
  - ingress:
      enabled: true
      hosts:
        - host: chart-example.local
          paths:
            - path: {{ requiredEnv "TEST_PATH" | quote }}
              pathType: ImplementationSpecific
# Source: test/templates/ingress.yaml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test
  labels:
    helm.sh/chart: test-0.1.0
    app.kubernetes.io/name: test
    app.kubernetes.io/instance: test
    app.kubernetes.io/version: "1.16.0"
    app.kubernetes.io/managed-by: Helm
spec:
  rules:
    - host: "chart-example.local"
      http:
        paths:
          - path: C:/Program Files/Git/aaa
            pathType: ImplementationSpecific
            backend:
              service:
                name: test
                port:
                  number: 80

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic I will try to see this. Thanks for your feedback.

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic if use env, what is it will return?

@strainovic
Copy link
Contributor Author

- path: {{ env "TEST_PATH" | quote }}
export TEST_PATH='/aaa'

echo ${TEST_PATH}
/aaa

helmfile template | grep '\- path: '

Building dependency release=test, chart=test
Templating release=test, chart=test
          - path: C:/Program Files/Git/aaa

@strainovic
Copy link
Contributor Author

I tested it on Ubuntu on Windows WSL and it works fine with the same helm/helmfile versions.

helm version
version.BuildInfo{Version:"v3.10.1", GitCommit:"9f88ccb6aee40b9a0535fcc7efea6055e1ef72c9", GitTreeState:"clean", GoVersion:"go1.18.7"}
helmfile version
Helmfile version 0.146.0
helmfile template | grep '\- path: '
Building dependency release=test, chart=test
Templating release=test, chart=test
          - path: /aaa

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic https://github.com/git-for-windows/build-extra/blob/main/ReleaseNotes.md#known-issues

If you specify command-line options starting with a slash, POSIX-to-Windows path conversion will kick in converting e.g. "/usr/bin/bash.exe" to "C:\Program Files\Git\usr\bin\bash.exe". When that is not desired -- e.g. "--upload-pack=/opt/git/bin/git-upload-pack" or "-L/regex/" -- you need to set the environment variable MSYS_NO_PATHCONV temporarily, like so:

MSYS_NO_PATHCONV=1 git blame -L/pathconv/ msys2_path_conv.cc

Alternatively, you can double the first slash to avoid POSIX-to-Windows path conversion, e.g. "//usr/bin/bash.exe".

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic I don't have git bash. Please try to test this. And give feedback. Thanks very much.

@strainovic
Copy link
Contributor Author

Yes, it works! :)

export MSYS_NO_PATHCONV=1
helmfile template | grep '\- path: '

Building dependency release=test, chart=test
Templating release=test, chart=test
          - path: /aaa

Maybe this one should be documented somewhere?

Thanks!

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic could you post a PR? Thanks very much.

@strainovic
Copy link
Contributor Author

Yes, no problem. Can you please tell me where it would be the most appropriate to document?

Thanks :)

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@yxxhero
Copy link
Member

yxxhero commented Oct 31, 2022

@strainovic Thanks very much.

strainovic added a commit to strainovic/helmfile-1 that referenced this issue Oct 31, 2022
@yxxhero yxxhero linked a pull request Oct 31, 2022 that will close this issue
yxxhero pushed a commit that referenced this issue Oct 31, 2022
* Document git for windows env auto-prefix #481

Signed-off-by: Nenad Strainovic <nenad.strainovic@asseco-see.rs>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants