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

helmCharts valuesFile does not work when file path contains filepath up (cd ..) #5494

Closed
arkmech opened this issue Dec 18, 2023 · 10 comments
Closed
Labels
area/helm kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@arkmech
Copy link

arkmech commented Dec 18, 2023

What happened?

File Structure

├── k8s/
| ├── base/
| | ├── helm/
| | | ├── gitops/
| | | | ├── argo-cd/ (Helm Chart Pulled)
| | | | ├── base-argo-cd-values.yaml
| | | | ├── kustomization.yaml
| ├── dev/
| | ├── helm/
| | | ├── gitops/
| | | | ├── dev-argo-cd-values.yaml
| | | | ├── kustomization.yaml

base-argo-cd-values.yaml
Currently Empty

k8s/base/helm/gitops/kustomization.yaml

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
helmGlobals:
  chartHome: argo-cd
helmCharts:
- namespace: gitops-system
  name: gitops
  releaseName: argo-cd
  valuesFile: base-argo-cd-values.yaml

k8s/dev/helm/gitops/kustomization.yaml

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
helmGlobals:
  chartHome: ../../../base/helm/gitops
helmCharts:
- namespace: gitops-system
  name: gitops
  releaseName: argo-cd
  valuesFile: ../../../base/helm/gitops/base-argo-cd-values.yaml
  additionalValuesFiles:
  - kind-argo-cd-values.yaml
  1. Documentation states valuesFile takes a file path, it works for filePaths going down, but does not work when file path contains .. to go up a directory.

Run:
kustomize build k8s/dev/helm/gitops --enable-helm

Error:
1)

Error: security; file '/Users/me/project/k8s/base/helm/gitops/base-argo-cd-values.yaml' is not in or below '/Users/me/project/k8s/dev/helm/gitops'
Error: no repo specified for pull, no chart found at ''

I looked at this issue and it doesn't seem fixed: #5163

Per docs:
https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_helmchartinflationgenerator_

repo: [Optional] The URL of the repository which contains the chart. If this is provided, the generator will try to fetch remote charts. Otherwise it will try to load local chart in chartHome.

Even though I don't specify the repo key. Its still trying to pull repo instead of looking at chartHome

Having base-argo-cd-values.yaml in a file location without .. works. For example in the same directory or nested directory

What did you expect to happen?

kustomize builds successfully.

How can we reproduce it (as minimally and precisely as possible)?

See file structure and yaml above.

Expected output

Actual output

Kustomize version

v5.3.0

Operating system

MacOS

@arkmech arkmech added the kind/bug Categorizes issue or PR as related to a bug. label Dec 18, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 18, 2023
@koba1t
Copy link
Member

koba1t commented Jan 11, 2024

I think maybe this is a security check that prevents kustomizations from reading files outside their own directory root.
https://kubectl.docs.kubernetes.io/faq/kustomize/#security-file-foo-is-not-in-or-below-bar

Could you try load-restrictor option that resolv your problem?

/area helm
/triage need-information

@k8s-ci-robot
Copy link
Contributor

@koba1t: The label(s) triage/need-information cannot be applied, because the repository doesn't have them.

In response to this:

I think maybe this is a security check that prevents kustomizations from reading files outside their own directory root.
https://kubectl.docs.kubernetes.io/faq/kustomize/#security-file-foo-is-not-in-or-below-bar

Could you try load-restrictor option that resolv your problem?

/area helm
/triage need-information

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@koba1t
Copy link
Member

koba1t commented Jan 11, 2024

/triage needs-information

@k8s-ci-robot k8s-ci-robot added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 11, 2024
@max06
Copy link

max06 commented Feb 1, 2024

Since I just had the same issue: Yep, that helps!

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 1, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 1, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2024
@alexrecuenco
Copy link

alexrecuenco commented Aug 8, 2024

/triage needs-information

Could this be re-opened?

I think this could be reconsidered, if you wouldn't mind. I am struggling to see if this is currently restricted as a recommendation in general, if using --load-restrictor=LoadRestrictionsNone is always recommended, or if there is something else that we are trying to discourage.

Because, to me, chartHome would be handled similarly to resources/components: An external place where we grab resources to inflate/modify. I am not sure I get the restriction of having the LoadRestrictor prevent external access to it.

My example would be the following use. I use a helm chart

  • in overlays/dev the chart is inflated once for a dev app
  • inoverlays/test to inflate the chart a few times for each client (SaSS)
  • in overlays/prod to do the same, but without experimental features.

What I want to do is to have a $ROOT/charts directory with all the charts I've downloaded and kept.

Then I would have some base/kustomization.yaml with other resources

Resource descriptions

In dev you would have one version inflated, with experimental support

# overlays/dev/kustomize.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: dev

resources: 
  - ../../base

patches:
 - ...
 
helmGlobals:
  chartHome: ../../charts/
helmCharts:
  - name: ruby-base
    valuesInline:
      resourceName: dev-client-1
      experimental_assets: true

Multiple versions inflated in test

# overlays/test/kustomize.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: test
resources: 
  - ../../base

patches:
 - ...

helmGlobals:
  chartHome: ../../charts/
helmCharts:
  - name: ruby-base
    valuesInline:
      resourceName: test-client-1
      experimental_assets: true
  - name: ruby-base
    valuesInline:
      resourceName: test-client-1
      experimental_assets: false

And finally, in prod multiple for each client, without any experimental/beta features

# overlays/production/kustomize.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: production
resources: 
  - ../../base

patches:
 - ...

helmGlobals:
  chartHome: ../../charts/
helmCharts:
  - name: ruby-base
    valuesInline:
      resourceName: client-1
      experimental_assets: false
  - name: ruby-base
    valuesInline:
      resourceName: client-2
      experimental_assets: false

Of course, --load-restrictor=LoadRestrictionsNone would do it, but it feels —as a non-expert user— as if I am disabling something that I shouldn't

@koba1t
Copy link
Member

koba1t commented Aug 12, 2024

@alexrecuenco
According to the comments, We prefer to chartHome dir put under the root of the kustomization.yaml file.
Because the helmCharts generator reads values.yaml from charts dir if valueFiles is not defined.
https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_helmchartinflationgenerator_

// ChartHome might be consulted by the plugin (to read
// values files below it), so it must be located under
// the loader root (unless root restrictions are
// disabled, in which case this can be an absolute path).
if p.ChartHome == "" {
p.ChartHome = types.HelmDefaultHome
}

p.ValuesFile = filepath.Join(p.absChartHome(), p.Name, "values.yaml")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

6 participants