Comprehensive Enhancements to Dependency-Track Helm Chart w/o 3rd party dependencies #20
Conversation
New Bitnami Common v2.19.1 library chart dependency added. This augmentation was introduced to significantly enhance functionality and ensure compatibility with Bitnami Charts, which utilize a widely-recognized API adopted by numerous projects. Signed-off-by: Jiří Altman <[email protected]>
1. **Revise the Ingress Template in Accordance with the Bitnami Common Format:** - Expand the Ingress configuration to incorporate additional parameters such as `selfSigned`, `pathType`, `apiVersion`, `hostname`, `path`, `ingressClassName`, `tls`, `tlsWwwPrefix`, `extraHosts`, `extraPaths`, `extraTls`, `secrets`, and `extraRules`. See [documentation link]. - Integrate these configuration options into `values.yaml`. 2. **Update Service Definitions Across Chart Dependencies:** - Modify `charts/*/service.yaml` to align with the revised Ingress configuration. Specifically, rename the service port from `web` to `http` to enhance consistency across configurations. Signed-off-by: Jiří Altman <[email protected]>
1. **Introduce a New Template for Managing TLS Secrets:** - Implement a template specifically designed to manage TLS secrets, facilitating a more structured approach to security configurations. 2. **Incorporate Conditional Logic for Certificate Management:** - Add conditional logic to distinguish between auto-generated certificates and those provided by the user, ensuring flexible and secure certificate handling. 3. **Enhance `values.yaml` with Additional TLS Configuration Fields:** - Update `values.yaml` to include new fields pertinent to TLS configuration, allowing for expanded customization and control over TLS settings. Signed-off-by: Jiří Altman <[email protected]>
The Ingress section of the values.yaml file is now fully documented, compatible with helm-docs standards. The documentation includes detailed comments to enhance understanding. It provides options for enabling the generation of Ingress records, the use of self-signed certificates, and the specification of the Ingress API version. Signed-off-by: Jiří Altman <[email protected]>
Comments have been added to the TLS section of the `values.yaml` file to improve understanding. These annotations address enabling TLS transport, the process of auto-generating self-signed certificates, the procedure for naming secrets that contain certificates, and the methods for defining paths and content for certificate files when mounted as secrets. Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
A new template for rendering Helm templates in values has been added to the _helpers.tpl file. This function takes a value that may contain a Helm template and evaluate it, which is useful for dynamic configuration values that need to be evaluated during chart rendering. The function also handles different data types and scopes, providing flexibility in its usage. An alias "!TPL" has been created for this function as well. Signed-off-by: Jiří Altman <[email protected]>
…ookup Added two new helper templates to the dependency-track Helm chart. The first function expands the name of the namespace, allowing for more flexible configuration. The second function reuses values from existing secrets or sets them to default values if they don't exist, improving security and configurability. Signed-off-by: Jiří Altman <[email protected]>
Introduces a new helper template that checks if the necessary annotations for cert-manager to manage TLS certificates are present in the Ingress annotations. This enhancement allows Dependency Track to utilize cert-manager for handling its TLS certificates, improving security and certificate management. Signed-off-by: Jiří Altman <[email protected]>
There was a problem hiding this comment.
So I don't see the need for all this complexity. Why should this helm chart be so general?
Personally, I think it should be opinionated to have less complexity, I think the helm chart solve one thing and one thing only. We should make it easy to deploy DT and those options should be extendable.
But at least for me as a maintainer of other open-source projects I think readability is extremely important
I don't see why we would want to align with the Bitnami common, since it brings complexity that we don't need.
For example, the cert-manager stuff.
What is wrong with writing?
ingress:
annotations:
cert-manager.io/cluster-issuer: letsencryptThis can be done without adding lots of .tpl code.
But I do think the addition of extraDeploy is a great one, since it will make it possible for users to use the helm chart in a way they see fit without having to have another helm/kustomize deployment.
So my vote is solve the original issue.
- add ingressClassName
- add TLS support (this is solved with the help of
extraDeploy
| @@ -0,0 +1,114 @@ | |||
| {{- /* | |||
There was a problem hiding this comment.
I'm a big fan of this, something that I use allot in grafana helm charts. But It can be made much easier.
https://github.com/grafana/helm-charts/blob/d2d9da9a4cee6ce5dec9449de8e940a1121136ce/charts/grafana/templates/extra-manifests.yaml#L3
I would also prefer calling it extraObjects since that is what it is, this have nothing to do with a deployment.
There was a problem hiding this comment.
I'm a big fan of this, something that I use allot in grafana helm charts. But It can be made much easier.
https://github.com/grafana/helm-charts/blob/d2d9da9a4cee6ce5dec9449de8e940a1121136ce/charts/grafana/templates/extra-manifests.yaml#L3
Thank you for your feedback on the Helm chart configuration. I understand your concerns about the perceived complexity of the template and appreciate the opportunity to discuss how its design can benefit users across different levels of expertise.
Error Handling and Debugging: My template includes robust error handling and detailed debugging information, which significantly eases the process of identifying and resolving configuration issues. By providing explicit error messages and pinpointing the exact lines where issues occur, I aim to reduce the time users spend troubleshooting. This support is especially valuable for those new to Kubernetes or those dealing with complex deployments.
Flexibility and Safety in Templating: The ability to use both dictionaries and multiline strings in the template offers users the flexibility to choose the method that best suits their needs. This dual approach caters to a variety of use cases, from simple to complex, without compromising the safety and integrity of the deployment scripts. The multiline string option is particularly beneficial for embedding complex logic that might be unwieldy within a dictionary format.
Embedded Documentation and Guidelines: I have included detailed documentation directly within the template to serve as an immediate reference for users. This approach not only aids in understanding the function and structure of the template but also educates users on best practices for YAML configurations. Such embedded guidance is invaluable for ensuring that both new and experienced users can effectively utilize the chart without frequent reference to external documentation.
Transparency and Understandability: The explicit nature of my configurations promotes a clear understanding of how resources are managed, which is crucial in collaborative environments. This transparency helps in maintaining and scaling projects, as it ensures that configurations are understandable and accessible to anyone who might work on or inherit the project in the future.
Advanced Customization Capabilities: Lastly, my template supports advanced customization options without overwhelming those who do not require them. This design strategy allows users who need to deploy customized or non-standard resources to do so within a standardized framework, ensuring that the chart remains useful and adaptable to a broad range of deployment needs.
I believe that these features, while adding a layer of complexity, provide significant value by making the Helm chart more robust, user-friendly, and adaptable. I am open to exploring ways to streamline the experience further and would love to continue this conversation to refine my approach based on user feedback.
Thank you again for your insights, and I look forward to making our Helm chart even better with your continued input.
I would also prefer calling it extraObjects since that is what it is, this have nothing to do with a deployment.
👍 This is fine for me - maybe extraResources sounds better? WDYT? 😉
There was a problem hiding this comment.
extraResources sounds better.
| @@ -0,0 +1,30 @@ | |||
| {{- if and (eq (.Values.tls.enabled | toString) "true") (.Values.tls.existingSecret | empty) }} | |||
There was a problem hiding this comment.
Since you have implemented extraDeploy.yaml is this really needed?
In my experience, the vast majority of Kubernetes users, use cert-manager. Sure, it won't give you encryption all the way to the pod, but we live with it since it's good enough for most companies.
Since you already got extraDeploy, you can easily add a TLS secret through there.
There was a problem hiding this comment.
Thank you for pointing out the use of extraDeploy and the common practice of manual configuration of cert-manager in Kubernetes deployments. I appreciate your perspective and understand that for skilled IT specialists like yourself, the prevalence of manual configuration of cert-manager is quite familiar.
However, our decision to maintain the current configuration separately and explicitly is intentional. For a broader range of users, including those who may not be as familiar with Kubernetes intricacies, the process must be straightforward. Time is very precious, and learning to manually configure settings like these can be quite time-consuming and potentially error-prone for those less experienced with Kubernetes' more advanced configurations. Additionally, extraDeploy is designed for advanced users or specific edge cases, not for everyday use.
We aim to keep our Helm chart as accessible as possible, enabling users to achieve a secure deployment with minimal additional research or modification to the default settings. This approach reduces the initial barrier for new users and speeds up the process of deploying secure, production-ready instances.
I believe that maintaining this feature as part of our default configuration supports our goal of user-friendliness and broad accessibility. However, I'm open to further discussion and exploring ways we could offer more advanced configurations as optional, without compromising the ease of use for new users. Your insights are very valuable, and I look forward to continuing this conversation to find the best path forward.
There was a problem hiding this comment.
I see your point, but I personally don't think it's worth it. Especially since you make it hard to use the helm chart for those of us that actually know Kubernetes which I would assume is the most people that deploy this helm chart.
I would argue that providing a self-signed certificate is an antipattern, and it is as useful as just using http by default. Of course, it won't be as secure, but the usability of forcing the users to import the generated certificate isn't easy, to do a PoC I would just use http.
To help people getting started, I would instead put my time in to writing a few example value files, just like it was done in: https://github.com/DependencyTrack/helm-charts/blob/main/charts/dependency-track/values-minikube.yaml
And spend some time to document best practices on how to setup DT inside Kubernetes.
This is something that you will want to do any way, and pointing the users to stuff like cert-manager is what you want to do anyway.
Sadly I have read lots and lots of helm throughout the years and I can say that some of the template you are doing I would need to google to understand in detail. Which for me makes it too complex to debug, especially if there is someone that don't want to go to beaten path no matter if they are junior or senior.
But this is my point of view, and I understand if you choose not to go with it.
Any way, thanks for improving the helm chart, sadly I haven't been able to use it in production so far, but it's in my todo together with a bunch of other things ;)
There was a problem hiding this comment.
This template extends beyond a basic self-signed certificate, representing a sophisticated mechanism that integrates seamlessly with the Ingress Template. While its primary utility is to facilitate the creation of a self-signed certificate, this is but one aspect of its functionality. For users who rely on their own or an external CA, and do not use Cert Manager, the template provides an easy method to include a certificate without requiring in-depth technical knowledge.
Your current assessment seems to misunderstand the template’s full scope. The functionality of extraResource (formerly known as extraDeploy—please note the name change) is distinct and designed as an advanced tool for specialized scenarios.
Additionally, I would like to clarify that the template is user-friendly. The notion that it is difficult to manage or that a deep understanding of Kubernetes is essential for deploying Helm charts is not accurate. The template also does not restrict advanced users as suggested; it is designed to simplify the process for both novices and experienced professionals alike.
It might be more beneficial to focus on enhancing quality and conducting unit tests rather than engaging in discussions on what might be considered minor points. While I appreciate the importance of thorough documentation, as reflected in the values.yaml and my contributions, it is important to recognize that not all administrators actively seek detailed technical documentation.
Finally, the assertion that adding toggle options complicates the chart, while stating that learning basic configurations is simple, seems contradictory. No offense is intended by these observations.
I hope we can redirect our focus to constructive discussions that enhance the tool’s usability and effectiveness.
There was a problem hiding this comment.
My short answer is, let's agree to disagree. I was asked to give my opinion and I have done that.
I think readability is key to be able to maintain software in the long run, no matter what your needs might be.
I created #23 to showcase how you can solve the same issue, which I personally think is a better option and I know you won't agree on that :).
I will use this helm chart no matter which PR gets merged, as long as the issue gets solved.
FYI, I don't think disagreeing is an issue, we just see things differently and that is fine.
charts/dependency-track/values.yaml
Outdated
|
|
||
| # -- (object) Common annotations for all the resources created by the chart. | ||
| commonAnnotations: | ||
| test: test |
There was a problem hiding this comment.
NIT: test: test should be commented
| pathType: ImplementationSpecific | ||
|
|
||
| # -- Default host for the ingress resource | ||
| hostname: dependency-track.local.gd |
There was a problem hiding this comment.
Maybe something like example.com?
There was a problem hiding this comment.
Thank you for suggesting the use of example.com. I appreciate your attention to using recognizable example domains, and I understand the appeal. We chose dependency-track.local.gd for the Helm Chart to streamline the setup process for users testing the chart in local development environments like Docker Desktop, Rancher Desktop, k3d, or microk8s. Our goal was to ensure that the chart could be deployed with minimal configuration. dependency-track.local.gd resolves directly to localhost, which allows for immediate and hassle-free testing.
While example.com is a standard choice, it typically resolves to NXDOMAIN. This would require users to edit the values.yaml file, potentially complicating the initial setup process.
I value your focus on clarity and adherence to conventions. Could we explore other alternatives that address your concerns while still maintaining ease of setup? Your insights are incredibly valuable for refining our approach and ensuring the chart remains user-friendly. I look forward to your thoughts or any further suggestions you might have!
There was a problem hiding this comment.
We can always provide values-*.yaml files with overrides to work with the various tools. In fact, we already have that for Minikube: https://github.com/DependencyTrack/helm-charts/blob/main/charts/dependency-track/values-minikube.yaml
I agree that the default should be example.com. It's exactly what the domain was created for, and it makes it immediately obvious that overriding is necessary.
| name: http | ||
| {{- end }} | ||
| {{- if .Values.ingress.extraRules | empty | not }} |
There was a problem hiding this comment.
Why do we need to support extrRules?
For me, this helm chart is written to run DT and nothing else.
- Simplified and polished templates to better fit to dependency-track - Updated the condition checks for better readability. - Removed unnecessary 'apiVersion' field in ingress configuration. - Replaced 3rd party common template calls with internal ones for better clarity. - Adjusted TLS secret generation to align with updated conditions. - Removed 'extraHosts' from values.yaml as it's no longer needed. - Added few related options into Values Signed-off-by: Jiří Altman <[email protected]>
Introduced a new feature that allows users to deploy additional Kubernetes resources not included in the main chart package. This extends the capabilities of the chart by enabling custom resource specification. The template iterates over each item in `$.Values.extraDeploy`, passing it through a rendering process, and checks for valid YAML syntax. If all checks pass, the item is rendered as YAML multi-document and printed to output. Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
Signed-off-by: Jiří Altman <[email protected]>
If Longhorn storage was used, and storageClassName was defined, Longhorn didn't work properly. Solution was to remove storageClassName completely but it was not possible (limited by chart design). Signed-off-by: Jiří Altman <[email protected]>
|
I am a big fan of this. |
🛠 Helm Chart Enhancement
dependencytrack.tpl.render,dependencytrack.namespace,dependencytrack.secrets.lookup,dependencytrack.ingress.certManagerRequest) to encapsulate common operations and boost chart maintainability.🔐 TLS Support
values.yamlwith new fields related to TLS configuration, providing clear options for setup.📖 Documentation
values.yamlextensively, adhering to Helm-docs standards to aid user comprehension.📁 Repository and Codebase Maintenance
.gitignorewith new patterns to exclude system generated files and specific IDE/system files..helmignorefile to limit contents packaged into the Helm chart.extraDeploy.yamlto allow specification of extra Kubernetes resources, enhancing the chart's customization and extension capabilities.🧪 How to Test
extraDeploy.yamlto confirm functionality and integration without impacting core components.📅 Impact
As a result of these updates, the Dependency-Track Helm chart is not only more extensible and customizable but also aligns with industry standards and best practices. These improvements significantly enhance usability and potential use cases, ensuring that the chart remains relevant and robust in diverse environments.
Please review the extensive changes and provide feedback. Due to the broad scope of updates, extensive testing is recommended to ensure there are no unforeseen issues.
Closes #17