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.

Sign up for GitHub

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

Jump to bottom

Replace sub-command execution with proper code for Helm #33

Open
NotTheEvilOne opened this issue Nov 28, 2023 · 1 comment · May be fixed by #34
Open

Replace sub-command execution with proper code for Helm #33

NotTheEvilOne opened this issue Nov 28, 2023 · 1 comment · May be fixed by #34
Labels
Container Issues or pull requests relevant for Team 2: Container Infra and Tooling

Comments

@NotTheEvilOne
Copy link

Reviewing the current operator I found helmTemplatewhich calls the helm executable to render the chart template.

cluster-stack-operator/internal/controller/clusterstackrelease_controller.go

Line 312 in ad039ca

func helmTemplate(chartPath, releaseName, namespace string) ([]byte, error) {

I would consider this function bad practice that needs replacement as soon as possible because of at least the following reasons:

  • Running an executable without any additional checks of unknown source even thought you think it's provided by the Dockerfile during the build process is an security issue
  • Downloading a binary for a single task seems bloated additionally
  • Each executable in a file system might be an unnecessary attack vector on containers.
  • KISS principle

There are at least two libraries (Helm [1] and go-helm-client [2]) that would just add the dependencies required to get the job done.

[1] https://github.com/helm/helm
[2] https://github.com/mittwald/go-helm-client
@janiskemper
Copy link
Member

We followed the approach of ArgoCD (see e.g. https://github.com/argoproj/argo-cd/blob/820f4d861a7789f299143ed89816001091abf923/util/helm/cmd.go#L54).

If there is an easier way, I'm open to other approaches! If you want to contribute, I can also support you on this.

NotTheEvilOne added a commit that referenced this issue Dec 1, 2023
@NotTheEvilOne
Replace Helm binary with pure go packages based solution.
19cbca6 
Closes: #33
Signed-off-by: Tobias Wolf <wolf@b1-systems.de>
NotTheEvilOne added a commit that referenced this issue Dec 1, 2023
@NotTheEvilOne
Replace Helm binary with pure go packages based solution.
ff77803 
Closes: #33
Signed-off-by: Tobias Wolf <wolf@b1-systems.de>
@NotTheEvilOne NotTheEvilOne linked a pull request Dec 1, 2023 that will close this issue
Open
aniruddha2000 added a commit that referenced this issue Feb 22, 2024
@aniruddha2000
✨ Add manual upgrade if no k8s version is changed (#33)
4814dcd 
Add manual upgrade if no k8s version is changed

Signed-off-by: Aniruddha Basak <aniruddha.basak@syself.com>
@jschoone jschoone added the Container Issues or pull requests relevant for Team 2: Container Infra and Tooling label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Container Issues or pull requests relevant for Team 2: Container Infra and Tooling
Projects
Sovereign Cloud Stack
Status: Backlog
Milestone
No milestone
3 participants
@jschoone @NotTheEvilOne @janiskemper