-
Notifications
You must be signed in to change notification settings - Fork 38
Various concerns with the new nxrm-aws-resiliency
chart
#49
Comments
I totally agree, this chart need to be more flexible, allow us to customize and resuse on existing envs.
|
I agree with lrstanley, the current state of the chart is not production ready in my opinion. I've noticed some more things to add:
|
Thank you @lrstanley for your feedback on the Helm Charts! We appreciate you taking the time to compose this list. Please see our response below for each item 1.Helm charts should rarely ever be specific to a single cloud provider. The implications of this means Nexus helm-based deployments are not portable across cloud providers, and will require a lot of additional work to port to other cloud providers if needed (we're currently AWS, but this limits our options). The whole point of Kubernetes abstracting everything via plugable resources is to support portability. 2.Usage of AWS secrets-store-csi-driver should be optional, and should honestly not be recommended unless teams are already running the CSI driver. It adds quite a bit of additional complexity, that only makes sense if operators are already running these deployments.
Misc other chart-level issues:
|
Thank you @muckelba for taking the time to provide your feedback on the Helm Charts. Please see our response below against each item: Not every image registry is configurable Not every image registry is configurable There is no way to configure multiple ingresses for docker registries running on different (internal) ports There is no (documented) way to use an existing ingress solution like traefik. It seems like setting new ALBs, etc up is required |
Thank you for the quick response and taking the time to answer.
Sorry, i should have clarified that more precisely. I'm talking about 4 containers (external-dns, fluent-bit, create-nexus-work-dir, directory-creator) using hard coded images (k8s.gcr.io/external-dns/external-dns, amazon/aws-for-fluent-bit, ubuntu, busybox) with no way to configure their registries. This makes pulling impossible in environments without direct access to those registries.
That is fine, but i don't understand why there is a variable for that in values.yaml then. Looks like an oversight to me.
I see, thank you. So i assume that is not something that should be done directly in this chart.
Making the ALB optional would be a good improvement. |
For your information, the nexus helm team released a new chart which addresses almost every concern stated in here: |
Sorry for the delay. Our team should be looking at the new chart soon and will report back on if it resolves most of the issues we've previously raised. |
Hello! We recently started looking at shifting from the existing nexus-repository-manager helm chart, to the new
nxrm-aws-resiliency
chart, and we're noticing a lot of problems that I wanted to make sure to raise, as I feel others will likely raise the same concerns. Some of them have also been reported in other issues as well.secrets-store-csi-driver
should be optional, and should honestly not be recommended unless teams are already running the CSI driver. It adds quite a bit of additional complexity, that only makes sense if operators are already running these deployments.external-dns
controller should be optional, even if using docker functionality. For example, we use wildcard CNAME records through route53, so I can't see a need to add additional complexity in the DNS process just for Nexus.fluentbit
should be optional. I would also say that Prometheus should be supported at some point in the future as another optional extension of the helm chart. We actually started working on a Prometheus exporter that includes various metadata/metrics/etc (written in Go, 50+ different metrics atm, could be a sidecar).load-balancer-controller
should be optional, even if using docker functionality. There are various security risks with dynamic loadbalancer creation, and it also prevents us from using IaC to manage our loadbalancers. We also have a lot of customizations we do to our loadbalancers, and this would prevent us from doing so.values.yaml
looks like it doesn't follow common best practices/standards. The old chart more closely aligned with best practices/standards.pv
andpvc
.nexusNs
,cloudwatchNs
, andexternaldnsNs
.)password
,token
, etc in the name, should be trimmed/masked in the system-information calls, and not returned in the UI.storageclass
resource being created?From just my initial review of the new chart, and after having dealt with 20+ charts from other vendors/platform solutions over the past two years, it seems like the new changes are a step backwards in almost every way and doesn't have the necessary functionality included for a proper enterprise deployment. Looking to see if any of the above improvements can be made.
Thanks!
The text was updated successfully, but these errors were encountered: