feat: add ingress chart #1
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new Helm chart for managing application-level ingress, including configuration defaults, rendering templates, and basic documentation.
- Define configurable fields in
values.yamlfor hosts, services, TLS, and rewrite rules. - Implement
templates/ingress.ymlto generate an Ingress resource per service with class-specific annotations. - Provide chart metadata in
Chart.yamland a minimalreadme.md.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| charts/application-ingress/values.yaml | Introduce default values for ingressClass (commented), hosts, services, TLS settings |
| charts/application-ingress/templates/ingress.yml | Render per-service Ingress resources with class-specific defaults and overrides |
| charts/application-ingress/Chart.yaml | Add chart metadata, versioning, and dependency on common chart |
| charts/application-ingress/readme.md | Add basic usage instructions |
Comments suppressed due to low confidence (1)
charts/application-ingress/Chart.yaml:2
- The chart name (
ingress) does not match the directory name (application-ingress), which may confuse tooling and users. Consider renaming the chart toapplication-ingressfor consistency.
name: ingress
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Helm chart for managing application ingress resources, allowing per-service rules, host definitions, and optional TLS settings.
- Add configurable defaults and examples in
values.yamlfor hosts, services, and TLS - Implement an
ingress.ymltemplate with class-specific annotations, path mappings, and TLS support - Provide basic chart metadata (
Chart.yaml) and a minimal README
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| charts/application-ingress/values.yaml | Add default values and comments for ingress chart configuration |
| charts/application-ingress/templates/ingress.yml | Implement Ingress resource templating with class-specific defaults |
| charts/application-ingress/readme.md | Add placeholder documentation linking to values.yaml |
| charts/application-ingress/Chart.yaml | Define chart metadata and dependencies |
Comments suppressed due to low confidence (1)
charts/application-ingress/Chart.yaml:2
- [nitpick] The chart folder is named
application-ingress, butChart.yamlusesname: ingress. Aligning these names improves clarity and consistency for consumers.
name: ingress
| {{- if $.Values.explicitTLS }} | ||
| secretName: {{ $.Values.explicitTLS }} |
There was a problem hiding this comment.
The explicitTLS value is defined as a boolean but used as a secretName. Consider splitting the flag and secret name into separate values (e.g., tls.enabled and tls.secretName) or changing its type to a string.
| {{- if $.Values.explicitTLS }} | |
| secretName: {{ $.Values.explicitTLS }} | |
| {{- if $.Values.tls.enabled }} | |
| secretName: {{ $.Values.tls.secretName }} |
| solution: | ||
| component: ingress | ||
|
|
There was a problem hiding this comment.
[nitpick] The solution.component key is defined in values.yaml but not used in the ingress template. Remove unused values or document its purpose to avoid confusion.
| solution: | |
| component: ingress | |
| # Removed unused solution.component key |
This reverts commit 9952cf2.
No description provided.