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

Implement kustomizeOptions #109

Conversation

dhaiducek
Copy link
Member

@dhaiducek dhaiducek commented Mar 10, 2023

Allows passing flags equivalent to --enable-helm via the environment variable POLICY_GEN_ENABLE_HELM

ref: https://issues.redhat.com/browse/ACM-9060

Closes #105

@mprahl
Copy link
Member

mprahl commented Mar 14, 2023

@dhaiducek are there any security repercussions with this approach? I'm thinking in particular that when running in App Sub, the Helm execution would end up using App Sub's service account but don't know for sure.

@dhaiducek
Copy link
Member Author

@dhaiducek are there any security repercussions with this approach? I'm thinking in particular that when running in App Sub, the Helm execution would end up using App Sub's service account but don't know for sure.

I was wondering about input on security implications. The risks feel similar to enabling Kustomize. I'm thinking it's toggled on by the user, and I think would call the internal Helm CLI, but that doesn't exist in App Sub, so it'd just fail in that case. (I just tried executing into the subscription container, and helm came up empty.)

That said, I suppose I'll have to do some more digging around ArgoCD, but the situation there might be similar.

@eformat
Copy link

eformat commented Mar 20, 2023

hi @dhaiducek .. this just "works" in argocd .. as it has helm and kustomize support already i.e. in the argocd-repo-server pod they have helm in /usr/local/bin/helm

@gforster
Copy link

gforster commented Oct 4, 2023

This would be helpful. as stated above, this works in ArgoCD. Looks like a maintainer just needs to add a lgtm label?

@ch-stark
Copy link

ch-stark commented Dec 6, 2023

@dhaiducek I think we should approve it, I've a customer waiting on this and we reviewed it again and we do not see the risk, the customer would use ArgoCD and this would just use ArgoCD's Service Account. We could also doc any security related topics. What do you think?

@mprahl
Copy link
Member

mprahl commented Dec 6, 2023

@ch-stark and @dhaiducek , how about we require the user setting an environment variable to enable Helm in Kustomize? That way they opt-in to this feature and ensure it is secure to do so.

In Application Subscription, we can probably enable this by default and document it for ArgoCD.

@dhaiducek
Copy link
Member Author

@ch-stark and @dhaiducek , how about we require the user setting an environment variable to enable Helm in Kustomize? That way they opt-in to this feature and ensure it is secure to do so.

In Application Subscription, we can probably enable this by default and document it for ArgoCD.

That works for me--I've been thinking we just need to get this feature out there, and having a higher-level opt in sounds like a good way to go.

@dhaiducek
Copy link
Member Author

dhaiducek commented Jan 13, 2024

/hold for reviews

...and tests? Though that may be better handled in an integration test...

mprahl
mprahl previously approved these changes Jan 16, 2024
@mprahl
Copy link
Member

mprahl commented Jan 16, 2024

@dhaiducek could you please add documentation of the environment variable in the read me?

mprahl
mprahl previously approved these changes Jan 16, 2024
@dhaiducek
Copy link
Member Author

Updating the README.md caused a really weird rebase (maybe because this PR was originally in stolostron?), but I think it's sorted now...

Allows passing flags equivalent to `--enable-helm` via the
environment variable `POLICY_GEN_ENABLE_HELM`

ref: https://issues.redhat.com/browse/ACM-9060
Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
@dhaiducek
Copy link
Member Author

/unhold

@openshift-ci openshift-ci bot added the lgtm label Jan 17, 2024
Copy link

openshift-ci bot commented Jan 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, mprahl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 4bf2b47 into open-cluster-management-io:main Jan 17, 2024
4 checks passed
@dhaiducek dhaiducek deleted the kustomize-options branch January 17, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFE] Allow kustomize --enable-helm equivalent functionality
6 participants