Improve Ingress + TLS configurability

[JuryA]

Enhancements to Ingress Configuration in the Dependency-Track Helm Chart

🛠 Changes

• Ingress Template Improvement: Refined the Ingress resource configuration to better align with modern Kubernetes practices, including support for newer Ingress classes and more efficient handling of TLS configurations.
• New Configurable Values: Introduced new configurable parameters in the values.yaml file, allowing users to dynamically customize Ingress settings such as annotations, TLS secrets, and hostnames.
• Updated Documentation: Added detailed documentation within the values.yaml and README.md files to guide users on configuring the Ingress resource, emphasizing the new parameters and their impacts.

🧪 How to Test

1. Ingress Configuration:
   • Deploy the updated Helm chart into a Kubernetes environment with your specific Ingress settings.
   • Verify that the Ingress behaves as expected, handling traffic according to the new configurations.
   • Confirm that TLS termination is working correctly, if configured.
2. Documentation Review:
   • Review the updated documentation to ensure all new configurations are clearly described and easy to understand.
   • Ensure the examples provided match real-world usage scenarios for better user comprehension.

📝 Notes

• These changes aim to enhance the flexibility and usability of the Dependency-Track chart, particularly for environments requiring advanced Ingress configurations.

Close #17

[nscuro]

Most of this is not relevant to the repository, do we really need this change in .gitignore?

[JuryA]

Well, I can simply revert this commit with the .gitignore changes—I merely adopt the Linux, Windows, VSCode, Idea (JetBrains), and Helm templates as they are. However, I see that, at least for Idea, the configuration is slightly overkill... 🤐

Anyway ... Let me update it slightly...

[nscuro]

Is there a way to achieve the desired outcome without adding 3rd party dependencies?

[JuryA]

Yes, but in my opinion, that's unfortunate because Bitnami is quite stable regarding breaking changes. If it's pegged to major version 2, you can benefit from the fixes and improvements of that library. During the build, the entire library is downloaded, unpacked, and added to the final Helm chart, so it serves merely as a build dependency. The resulting chart is hermetically self-sufficient, eliminating any risk of issues related to unavailability, etc.

However, if you want to maintain control, you can add it as a subchart—the result will be the same, but this approach offers greater control over the origin, and, if necessary, it allows for manual updates.

[nscuro]

I feel like putting to much logic in the expressions makes it a lot harder to read the manifest files. Can we externalize some of it to _helpers.tpl instead?

[JuryA]

Hi! Thank you for initiating the discussion. Let me explain few things:

This refined template, utilized by Bitnami in their charts, constitutes a marked enhancement over the original Ingress, which was candidly inadequate and utterly unsuitable for standard deployments. Initially, configuration was virtually impossible; key elements such as ingressClassName were glaringly absent.

My proposal has transformed this Helm chart into an exceptionally intuitive tool, harmonizing with the familiar structure of Values employed by many DevOps engineers who utilize Bitnami charts or their derivatives. So why attempt to reinvent the wheel?

Moreover, this version supports TLS, cert-manager, and self-signed certificates (including auto-generation) right out of the box—because, as I have stated, I did not create it; I merely adopted it entirely, enhancing it just sufficiently to function as a cohesive unit. Furthermore, this template accommodates numerous modifications and supports various Kubernetes API versions back to version 1.19. It has also proven its efficacy in environments that are pervasive within the corporate sphere.

I recognize the appeal of a streamlined and orderly Helm chart; however, simplicity alone does not equate to deployability. No accolades will be forthcoming for a "simple" Helm chart, particularly given that its deployment demanded a full day and subsequently required adjustments via Kustomize due to the absence of configuration options through Values. Consider the following:

strategicMergePatches:
  - apiVersion: networking.k8s.io/v1
    kind: Ingress
    metadata:
      name: dependency-track-api-server
      namespace: {{ $.Values.namespace | quote }}
    spec:
      ingressClassName: nginx
  - apiVersion: networking.k8s.io/v1
    kind: Ingress
    metadata:
      name: dependency-track-frontend
      namespace: {{ $.Values.namespace | quote }}
    spec:
      ingressClassName: nginx
  - apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: dependency-track-frontend
      namespace: {{ $.Values.namespace | quote }}
    spec:
      template:
        strategy:
          type: Recreate

From my perspective, there is scant latitude for experimentation within this domain; it is prudent to adhere to the tried-and-tested industry standards. This template closely mirrors the Bitnami Nginx chart, available for review at Bitnami Charts. Although some elements might be streamlined, such reductions could prove counterproductive. This template is devoid of extraneous components; indeed, it exemplifies essential functionality. My recommendation is to prioritize product development over the solitary maintenance of a Helm chart, a task that would undoubtedly prove arduous. Moreover, despite the relative stability of the Kubernetes API, occasional shifts occur. Adhering to a tested implementation like this one, actively maintained by Bitnami, guarantees that the Helm chart is cross-platform—effective on OpenShift, AWS, Rancher, and microk8s. Should this not be our paramount concern? Furthermore, all new options in Values have been meticulously documented. Let the code fulfill its purpose when operational; ultimately, users prioritize user experience above all.

---

Certainly, the decision to accept my proposal rests solely with you. I realize my comments might seem a bit forward, but please rest assured that they weren't meant personally. I endeavored to remain as objective as possible. Dependency Track is indeed a remarkable product. 🙂

[nscuro]

[...] constitutes a marked enhancement over the original Ingress, which was candidly inadequate and utterly unsuitable for standard deployments. Initially, configuration was virtually impossible; key elements such as ingressClassName were glaringly absent.

Well, see, this repository was created with the explicit intent to have it built by the community (DependencyTrack/dependency-track#3080 (comment)). It even has a disclaimer in the README about not being production ready:

helm-charts/README.md

Lines 3 to 4 in 008c6a2

	> [!WARNING]
	> The charts in this repository are still being worked on, and are not yet ready for production use.

It's virtually impossible to foresee all the options people might need upfront. So from that perspective, your contribution is very much appreciated.

I recognize the appeal of a streamlined and orderly Helm chart; however, simplicity alone does not equate to deployability. [...] Although some elements might be streamlined, such reductions could prove counterproductive. This template is devoid of extraneous components; indeed, it exemplifies essential functionality. My recommendation is to prioritize product development over the solitary maintenance of a Helm chart, a task that would undoubtedly prove arduous.

I do understand that there is functionality that you (and surely others as well) need, but in the end it will be us maintainers who have to literally maintain this thing going forward.

In asking about simplifying things, and reducing dependencies on 3rd party code, I'm trying to manage risk of us losing grip on how this chart works.

Apologies for the ping from the sideline, but @NissesSenap @heubeck can I get a second opinion on the proposed changes please?

[JuryA]

Well, see, this repository was created with the explicit intent to have it built by the community (DependencyTrack/dependency-track#3080 (comment)). It even has a disclaimer in the README about not being production ready:

Yes, I am aware of that—so I intended to help push it a bit further. That was meant to be constructive criticism, not a complaint. 🙂 Originally, my intention was to keep it as simple as possible, yet at a level that would satisfy a wide range of users with varying demands. Would adding comments help? However, I didn’t want to clutter the charter with unnecessary elements.

[JuryA]

I do understand that there is functionality that you (and surely others as well) need, but in the end it will be us maintainers who have to literally maintain this thing going forward.

In asking about simplifying things, and reducing dependencies on 3rd party code, I'm trying to manage risk of us losing grip on how this chart works.

I fully understand.

[JuryA]

Apologies for the ping from the sideline, but @NissesSenap @heubeck can I get a second opinion on the proposed changes please?

Indeed, should my proposal be adopted, it would undoubtedly be prudent to revise the remainder of the chart accordingly—for consistency’s sake, as well as to ensure the entire chart maintains a uniform standard of excellence. However, I refrained from initiating more extensive modifications, as my primary objective centered around addressing the inadequacies of the poorly configurable Ingress. The amendments I suggested are confined to ingress.yaml, and to a lesser extent, service.yaml—specifically, the naming conventions of the ports. Additionally, I incorporated a dynamically generated TLS secret for self-signed certificates, integral to the Ingress and thereby, a fundamental component of the TLS configuration logic.

Would it be advisable to appropriately adjust the rest of the chart as well, integrating all changes simultaneously? If there’s consensus, I’d be happy to prepare the remainder, this time with enhanced internal documentation. I would certainly complete the documentation for the rest of values.yaml, but only if it makes sense. It’s the most time-consuming part, even though it might not appear so... 😂

[NissesSenap]

Thanks for the ping @nscuro .
I have started to look at the PR, but I didn't look at it in detail due to the dependency of the bitnami helm chart.

I love bitnami for the work that they have done with a number of their helm charts. But they are very complex, and they are supporting many third party resources that they are trying to standardize under with sub charts and that is why they are complex. Which is really useful for them.

But I don't see why you would bring in this complexity to a single helm chart with a single use case. It will make everything hard to read and debug.

I fully understand why you have done what you have done @JuryA , but I personally don't agree and I think it will make it much harder for other people to contribute and use the helm chart due to this.

At the same time, you are trying to solve a real issue as pointed out in the issue, but I would prefer a new PR that don't use the bitnami dependency.

[NissesSenap]

networking.k8s.io/v1 is standard since a very long time. There won't be a networking.k8s.io/v2 ever. Instead, the future is gateway API. So I do not see a need for this feature.

[JuryA]

At the same time, you are trying to solve a real issue as pointed out in the issue, but I would prefer a new PR that don't use the bitnami dependency.

I have created a new PR, #20, and have incorporated all the points we discussed therein.

Important changes:

• Removed the Bitnami Common dependency. (@NissesSenap)
• New helper templates have been added to _helpers.tpl, including documentation where necessary. (@nscuro)
• Refactored ingress.yaml by removing the detection of the K8s API version and other related components. (@NissesSenap)
• Optimized overall to fit the intended purpose and fixed minor bugs that were found.
• Added a new extraDeploy.yaml template as a universal option for adding extra K8s resources for advanced users (which includes documentation for contributors).

May I ask to close this PR as a duplicate?