Skip to content

Refactor cmd/helm to allow library usage#12725

Closed
eddiezane wants to merge 1 commit into
helm:mainfrom
defenseunicorns:ez/refactor-cmd
Closed

Refactor cmd/helm to allow library usage#12725
eddiezane wants to merge 1 commit into
helm:mainfrom
defenseunicorns:ez/refactor-cmd

Conversation

@eddiezane

Copy link
Copy Markdown

DO NOT SUBMIT - need to fix a few tests still

What this PR does / why we need it:

The Zarf project currently ships additional vendored tools that may be needed in an airgapped environment. Helm doesn't expose the root command and makes use of internal APIs (version for example) so we are a bit stuck at the moment with attempting to vendor a quasi-fork in tree.

It looks like someone else was attempting to do something similar #12122.


This PR refactors the code base to allow for importing the root Helm command as library code. I modeled it after how we structured kubectl.

https://github.com/kubernetes/kubernetes/blob/f40ff3d9ffb2a06ebb256b6e4c5d1b62079bb3b9/cmd/kubectl/kubectl.go#L29

https://github.com/kubernetes/kubernetes/blob/f40ff3d9ffb2a06ebb256b6e4c5d1b62079bb3b9/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go#L100

I do not believe this to be a breaking change as anything that was in cmd/helm was part of a main package and not exported. I think the bigger question is what surface (if any) do you want to expose.

Here's an example of what this looks like to be used.

Special notes for your reviewer:

My initial goal here is to find out if y'all are open to a change like this - doesn't have to be in this shape. My approach was to publicly expose the bare minimum to make it possible to embed the root Helm command. Aside from the NewRootCmd this included the debug/warning log functions and the PluginError. Would love a discussion and I'm happy to modify/resubmit as needed.

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

cc @marckhouzam @arikmaor

DO NOT SUBMIT

Signed-off-by: Eddie Zaneski <eddiezane@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants