community icon indicating copy to clipboard operation
community copied to clipboard

HIP for git based dependencies

Open dominykas opened this issue 2 years ago • 1 comments

This is an attempt to revive #214, hopefully leading to a revival of https://github.com/helm/helm/pull/11258 down the road.

  • I took the original draft and squashed it into a single commit, while also renaming the file to avoid conflicts
    • I left the attribution to the original authors (incl. @rally25rs, who authored a PR which contained most of the information here), even though after my editing there's little of that original content left
  • I updated the structure to follow the templates
  • I rewrote most parts to clarify language and hopefully add more context
  • I updated the specification for the git based chart URLs to follow the same convention (VCS+protocol) as npm and Python (this was requested in earlier reviews)
  • ~~I updated the dependency repository URL to allow sub-folders (this was also requested in earlier reviews)~~ (still to be done)

There are some things I don't know yet, but I hope these can be resolved during the review process here:

  • whether the implementation recommendation makes sense (I haven't looked at the original code in detail just yet)
  • are there any implications due to version not being semver
  • are there any implications due to version not necessarily matching the version in the Chart.yaml of the dependency

cc @no9 - you asked for it ;) thanks for your help!

dominykas avatar Nov 22 '23 13:11 dominykas

I'm also now thinking about sub-dependencies... Should there be any behaviors prescribed if a dependency is installed from a registry, but it contains a sub-dependency which was originally installed from git?

On npm git dependencies are a major security risk, but dependencies are vendored in Helm, so it's maybe not a concern?

There's also the case of git-based dependencies having sub-dependencies (some of which may also be from git?) - meaning that we probably need to do a recursive helm dep up inside the cloned folders? Which could also result in some infinite loops, if someone puts in a circular dependency that way? Does this need to be solved in the HIP? I'm starting to get worried about the scope a little bit - I wonder if it's possible to move forward with an MVP implementation behind a flag and get some broader feedback?

dominykas avatar Dec 15 '23 11:12 dominykas