gitops-catalog icon indicating copy to clipboard operation
gitops-catalog copied to clipboard

possibly depricate installplan-approver?

Open itewk opened this issue 1 year ago • 6 comments

Over in the helm-charts repo we now have a super charged version of the installplan-approver from this repository that does things even more robustly. makign sure to only approve the correct installplan for instance.

maybe we should depricate the installplan-approver here and/or at least cross link from here to the more advanced helm chart?

https://github.com/redhat-cop/helm-charts/tree/master/charts/operators-installer

thoughts?

itewk avatar Jun 16 '23 15:06 itewk

@pittar @zisisli @carlmes @PendaGTP @strangiato thoughts?

itewk avatar Jun 16 '23 15:06 itewk

Hey @itewk . I really like the new Helm-based approver. However, not everyone will be using Helm and there's a certain simplicity to referencing the existing approver in your Kustomize file.

I agree that we should at a minimum reference the more robust Helm version in the README for the installplan-approver. I'll have to dig into your code a bit to see how much could be back ported into the simple Kustomize version.

pittar avatar Jun 16 '23 15:06 pittar

Agreed with @pittar!

We should align them as best we can and provide both the helm and kustomize options.

Lots of customers are very kustomize driven and avoid helm like it is the plague. Both the helm and kustomize patterns should be maintained as best as possible to support the different options.

strangiato avatar Jun 16 '23 15:06 strangiato

@pittar you could probably backport all of it into customize as well. we are then just maintinaing it in both palces. i went with helm because i wanted all teh approval stuff to be claned up after the fact, and helm hooks (which intigrate with argo hooks) are great for that. additionally, syncing parameters accross resources far easier with helm then kustomize.

minimally you could take the bash in the new Job for the more advanced approval logic if nothing else. maybe leave some comment in both jobs in both repos that they are copies of eachothers logic so if we ever change one someone can go change the other.

itewk avatar Jun 16 '23 15:06 itewk

@pittar , @mertel-rh made a PR that ports the functionality from the helm chart (as best as possible) to this customize.

https://github.com/redhat-cop/gitops-catalog/pull/243/

itewk avatar Sep 08 '23 14:09 itewk

@itewk , yes - looking at it now! Thanks!

pittar avatar Sep 08 '23 14:09 pittar