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

Clarification request: should RepoFile permit duplicate Entry elements? #2606

Closed
ljnelson opened this issue Jun 21, 2017 · 12 comments
Closed
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. refactor

Comments

@ljnelson
Copy link

(Steven Harris ( @seh ) requested that I create this issue after a discussion we had on Slack.)

While working on microbean-helm, I wanted to understand how the helm command line tool works with chart repositories. That led me into the Go code around processing (e.g.) ~/.helm/repository/repositories.yaml and (e.g.) ~/.helm/repository/cache/stable-index.yaml.

(I am not (yet!) a Go programmer.)

My understanding is that pkg/repo/repo.go contains the RepoFile type for representing the first one, and that pkg/repo/chartrepo.go contains the ChartRepo type for representing the second one. Both of them use a type called Entry, if I'm reading right, to represent what looks like perhaps a union of information used to represent a "thing" in each file. In the case of a RepoFile struct, Entry represents some information you need to talk to a chart repository and there are many of them (as you might expect). In the case of a ChartRepo, Entry seems to represent kind of like its primary key, and there is only one of them. (I don't know why Entry serves this double purpose but I wanted to list what I've learned in case it is helpful here.)

At any rate: it looks to me like RepoFile "has" an Add function that (deliberately?) allows you to add duplicate entries (so I guess this structure allows you to add a representation of a chart repository twice). But then all usage that I can find in the codebase of RepoFile structs seems to key off an entry's name (i.e. it treats this RepoFile structure like a map in terms of its usage.

Is there a reason for permitting duplicates in a RepoFile struct?

@ljnelson
Copy link
Author

This class diagram represents my attempt to make sense of the concepts at work here; I hope it helps.

@thomastaylor312
Copy link
Contributor

@technosophos Do you have an idea on this one? I am not quite sure

@bacongobbler
Copy link
Member

bacongobbler commented Nov 1, 2017

Yes, it should allow duplicate entry elements with the same name. What it should not allow are duplicate entry elements with the same name and the same version. e.g. mychart v0.1.0 and mychart v0.2.0 are OK, but mychart v0.1.0 should not be present in the index.yaml a second time.

I think this question is out of date now since type ChartRepo uses type *Entry whereas type IndexFile uses a type map[string]ChartVersions. Because it uses a map[string]ChartVersions, every name is guaranteed to be unique due to the inherent nature of a map.

Good question, @ljnelson.

@ljnelson
Copy link
Author

ljnelson commented Nov 1, 2017

Thanks for your input. I think, perhaps, you misunderstood my question, or I asked it badly.

So: I understand that an index.yaml may have many entries in it, each of which refers unambiguously to a chart with a particular name and version. For example, the stable one is here: kubernetes-charts.storage.googleapis.com/index.yaml Indeed, there are several entries for wordpress at various versions. I don't, I suppose, have a question about it.

What I am not sure about is: repositories.yaml apparently can have fully duplicate entries in it, since it is merely an array of entries. So I could, for example, have a repositories.yaml like this (note the two duplicate entries):

apiVersion: v1
generated: 2017-05-02T17:14:34.08505128-07:00
repositories:
- caFile: ""
  cache: stable-index.yaml
  certFile: ""
  keyFile: ""
  name: stable
  url: https://kubernetes-charts.storage.googleapis.com/
- caFile: ""
  cache: stable-index.yaml
  certFile: ""
  keyFile: ""
  name: stable
  url: https://kubernetes-charts.storage.googleapis.com/

…at least according to the data structure. My question was: is this deliberate? Should the repositories.yaml above be flagged as invalid?

Thanks again for your time and for responding so courteously to this issue.

@bacongobbler
Copy link
Member

bacongobbler commented Nov 1, 2017

From the pkg side yes, but from the CLI side no. This logic should be rolled into the package, though. :)

I'll re-open this as a good starter refactor if anyone wants to get their feet wet with pkg/repo.

@ljnelson
Copy link
Author

ljnelson commented Nov 1, 2017

Thanks. For posterity, my takeaway is: duplicate entries should not be permitted in a repositories.yaml via any mechanism. I'll double-check that I'm enforcing that in microbean-helm.

@bacongobbler
Copy link
Member

Correct. :)

@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

@ljnelson
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

@thomastaylor312
Copy link
Contributor

/remove-lifecycle stale
/lifecycle frozen

@bacongobbler bacongobbler added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. and removed starter labels Mar 4, 2019
@bacongobbler
Copy link
Member

closing as inactive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. refactor
Projects
None yet
Development

No branches or pull requests

5 participants