Handle empty chart path, add Chart type

[jlegrone]

What this PR does / why we need it:

This makes it possible to run ct install --charts ./ from the top level of a chart's directory.

This seems like a pretty straightforward use case for testing changes to a single chart, especially if that chart is in a dedicated source repository.

Which issue this PR fixes

fixes #101

Special notes for your reviewer:

[jlegrone]

/assign @unguiculus

[unguiculus]

Hm, I'm not sure we should do this. The tool is not expected to work like this and, thus, the solution seems a little hacky to me.

[jlegrone]

@unguiculus yes I was a little on the fence about this. My thought was that if --charts takes paths to chart directories, it should gracefully handle pretty much any valid chart path you pass to it, or at least throw a user friendly error if we don't want to support this.

I think by being more explicit about whether functions accept chart paths or chart names, we can avoid the perceived hackiness. It would be a larger change, but would you be open to a new Chart type, with methods for reading chart yaml, getting chart name, etc?

Edit: I had some time over lunch to work on a refactor, let me know what you think!

[jlegrone]

This costs a bit more in terms of memory, but we are now able to catch configurations resulting in bad chart paths up front instead of waiting in sequence for helm install or version checks to fail.

[scottrigby]

@jlegrone there is a lot of nice cleanup in this PR regardless of the initial premise to support adding chart files to the git root. There are some charts that do this (example: https://github.com/goharbor/harbor-helm), but most repos do not seem to do this. I think the main question is whether we want to promote files for a single chart in the git root as a best practice or not 🤔 My initial thoughts on pros & cons? Pro could be "why not?", and a con could be it's not extensible (a charts subdirectory dir allows you to add additional charts as needed, without changing CI etc).

[jlegrone]

My initial thoughts on pros & cons? Pro could be "why not?", and a con could be it's not extensible (a charts subdirectory dir allows you to add additional charts as needed, without changing CI etc).

Why not indeed. 😄I think a lot of the benefits of this library are "day 2" concerns as opposed to things folks will be aware of the very first time they write a helm chart, and as such we should attempt to meet them where they are as long as it's not too much of a burden.

The biggest downside I can think of is that it could end up being a useful convention to put charts in a parent directory that matches the local alias for the registry they get published to, but this has never been required for use so far and seems like something that could cause issues in the future as the "package management" features of helm evolve. Also, there are lots of cases where this convention is not being followed already.

[unguiculus]

I really like the refactoring in this PR. I think we can support a single chart in the root directory but we should try and make it a feature instead of an undocumented hack. The --chart-dirs flag specifies chart parent directories. We should keep it that way and maybe make the flag optional. If the flag is not specified, we should require a single chart in the root. However, I guess this would be a breaking change. We could accept empty value instead (--chart-dirs=, chart-dirs: []) for now and revise that for an upcoming v3.0.0.

[jlegrone]

I think what it may come down to is whether the --charts flag should require all paths specified to be in a --chart-dirs directory. Right now there is nothing preventing this from happening, other than what I consider a bug with the CreateInstallParams function generating invalid release names with some inputs.

I do not think this should be a recommended configuration though since --charts also disables change detection; I think it would be preferable to making a breaking change in the future so that you get the current behavior with --charts ./foo --all and change detection etc with --charts ./foo.

Edit: I'll open a new issue to discuss potential changes to flag behavior as that is a separate topic from this PR.

See #125

[scottrigby]

This looks OK to me, though I'd also like to wait for @unguiculus approval before merging. (PS, #125 is really a separate issue, which I believe shouldn't block this one).