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

Chart repository index allows to add multiple charts of the same version #3230

Closed
hypnoglow opened this issue Dec 4, 2017 · 8 comments
Closed

Comments

@hypnoglow
Copy link

An oversimplified example:

$ helm create foobar
Creating foobar

$ helm package foobar --version 0.5.2 .
Successfully packaged chart and saved it to: /tmp/foobar-0.5.2.tgz

$ cp foobar-0.5.2.tgz foobar-same.tgz

$ helm repo index .

$ cat index.yaml
apiVersion: v1
entries:
  foobar:
  - apiVersion: v1
    created: 2017-12-04T23:10:01.217542+03:00
    description: A Helm chart for Kubernetes
    digest: 8f02d733d1fc2b9ba65402b87428971346c9235aaa257655c85ffcdd73ce614f
    name: foobar
    urls:
    - foobar-0.5.2.tgz
    version: 0.5.2
  - apiVersion: v1
    created: 2017-12-04T23:10:01.217782+03:00
    description: A Helm chart for Kubernetes
    digest: 8f02d733d1fc2b9ba65402b87428971346c9235aaa257655c85ffcdd73ce614f
    name: foobar
    urls:
    - foobar-same.tgz
    version: 0.5.2
generated: 2017-12-04T23:10:01.216935+03:00

Now index contains multiple records for the same version of the chart. Note that even their digests are same, but I think even in case of different digests index should not contain multiple records of the same version.

This makes all commands involving chart repository ambiguous/incorrect:

$ helm serve --repo-path .
Regenerating index. This may take a moment.
Now serving you on 127.0.0.1:8879

$ helm repo add serve http://127.0.0.1:8879
"serve" has been added to your repositories

$ helm search serve/foobar --versions
NAME        	VERSION	DESCRIPTION
serve/foobar	0.5.2  	A Helm chart for Kubernetes // <----- Only one version

$ helm install serve/foobar --version 0.5.1 // <----- Which one will be installed?

// and so on

This behavior is caused by IndexFile.Add() in pkg/repo/index.go currently just appending a new version to the versions slice without any check. But is there any reason for this? I'm curious because I'm maintaining a helm plugin for chart repositories, and my index code is based on IndexFile implementation in pkg/repo package.

@bacongobbler I think #2606 is kinda related (I think the Issue author meant the same problem for repositories but not for charts), and you said that this behavior is undesirable. Am I correct? Should I get my feet wet with pkg/repo and provide a fix?

@bacongobbler
Copy link
Member

yes, feel free to provide a fix @hypnoglow! Duplicate entries with the same version should not exist. One way to fix this would be to add a prompt to helm repo index when a duplicate entry is found, prompting the user to ask which one they wish to serve. Alternatively, we can error out and inform the user on how to rectify the issue.

@hypnoglow
Copy link
Author

@bacongobbler , I'm working on the index code. I have a question: should we already remove deprecated format support, or that comments are falsy, and we should remove that support only in helm >= 3.0?

@bacongobbler
Copy link
Member

If it’s in there and there’s a code path in Helm 2.0, it shouldn’t be removed. :)

If it’s dead code, go for it! I’d suggest ripping it out in a separate PR though.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@hypnoglow
Copy link
Author

/remove-lifecycle stale

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@bacongobbler
Copy link
Member

duplicate of #2606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants