-
Notifications
You must be signed in to change notification settings - Fork 159
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
add policy-generator component #251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@adetalhouet @gnunn1 this is awaiting your reviews :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Actually, thinking more about this; the kustomize plugins are versioned with ACM release. Shouldn't we have an overlay structure to account for the different ACM releases. Also, shouldn't this patch be part of the ACM sub-folder. |
I don't know what the official ACM policy is on this, technically there is no issue using the latest and greatest since it is completely decoupled from ACM and brings the benefit of having the latest version available with the newest features. |
Otherwise LGTM |
I'm not as sure as you are on this. Given the plugin generates CR out of CRDs, and the CRDs are coupled to ACM release cycle, I do foresee potential disconnect if using the latest plugin with say ACM 2.3 release. (but that concern is more theoretical than something I actually faced). |
That's a fair point, thanks for pointing that out. |
README.md should be corrected as in sections "Purpose" and "Known Incompatibilities" it looks like copied from kustomize-build-enable-helm component README.md file. Probably the latter was used as a template. |
This adds a new component for patching the argocd instance to enable the ACM policy generator plugin.
Since this component has a conflict with the kustomize-build-enable-helm component I have added some notes on both to indicate that they are not compatible with each other.