Skip to content

feat: Ingress controller independent ingress resources, merged hub root and hub sub path ingress#105

Open
maxju wants to merge 4 commits intomasterfrom
103-generic-ingress
Open

feat: Ingress controller independent ingress resources, merged hub root and hub sub path ingress#105
maxju wants to merge 4 commits intomasterfrom
103-generic-ingress

Conversation

@maxju
Copy link
Contributor

@maxju maxju commented Feb 11, 2026

#103

Summary by CodeRabbit

  • New Features

    • Ingress annotations and ingressClassName are now configurable via chart values across all ingress resources; static defaults were replaced with value-driven rendering.
    • Root and sub-path ingress handling for relevant services updated for consistent naming and path behavior.
  • Documentation

    • Added contextual comments guiding ingress controller selection and recommendations for large uploads.

@coderabbitai
Copy link

coderabbitai bot commented Feb 11, 2026

📝 Walkthrough

Walkthrough

Replaces hard-coded Ingress annotations across multiple Helm chart templates with value-driven annotation maps and conditional ingressClassName emission; updates values.yaml with new ingress defaults and comments. Also renames one flame-hub metadata name and adjusts some path/backend formatting.

Changes

Cohort / File(s) Summary
Flame-hub main ingress
charts/flame-hub/templates/ingress.yaml
Replaced static annotations with loops that emit key/value pairs from .Values.global.flameHub.ingress.annotations, conditionally render spec.ingressClassName when .Values.global.flameHub.ingress.className is set, renamed metadata from {{ .Release.Name }}-flame-hub-sub-paths to {{ .Release.Name }}-flame-hub, and adjusted path/backend blocks.
Server ingress templates
charts/flame-hub/templates/server-storage/ingress.yaml, charts/flame-hub/templates/server-telemetry/ingress.yaml, charts/flame-hub/templates/server-core/ingress.yaml, charts/flame-hub/templates/server-messenger/ingress.yaml
Replaced fixed annotation blocks with conditional loops over .Values.*.ingress.annotations (and .Values.serverTelemetry.ingress.subPathAnnotations), quoting values, and added optional spec.ingressClassName when corresponding .Values.*.ingress.className exist. Maintains existing path/rule structure.
Configuration defaults
charts/flame-hub/values.yaml
Added ingress defaults and new keys for className and annotations across global.flameHub, serverCore, serverMessenger, serverStorage, serverTelemetry, and harbor. Introduces example nginx annotation defaults (rewrite-target, proxy-body-size/proxy-buffer-size, timeouts, backend-protocol/upstream-vhost) and explanatory comments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hopped through templates, nibbling hard-coded lines,
Now annotations sprout from values and vines.
With class names conditional and maps in tow,
Flexible ingress gardens happily grow.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: making ingress resources controller-independent and merging hub root/sub-path ingress into a unified resource.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 103-generic-ingress

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
charts/flame-hub/templates/ingress.yaml (1)

6-9: Inconsistent annotation handling pattern.

Unlike the other ingress templates (server-core, server-messenger) which use {{- with .Values...annotations }} to conditionally render the entire annotations: block, this template always renders the annotations: key even when the map is empty. This could result in an empty annotations: block in the generated manifest.

♻️ Suggested fix for consistency
 metadata:
     name: {{ .Release.Name }}-flame-hub
+    {{- with .Values.global.flameHub.ingress.annotations }}
     annotations:
-        {{- range $key, $value := .Values.global.flameHub.ingress.annotations }}
-        {{ $key }}: {{ $value | quote }}
+        {{- range $key, $value := . }}
+        {{ $key }}: {{ $value | quote }}
         {{- end }}
+    {{- end }}
 spec:

Comment @coderabbitai help to get the list of available commands and usage tips.

@tada5hi tada5hi changed the title Feature: Ingress controller independent ingress resources, merged hub root and hub sub path ingress feat: Ingress controller independent ingress resources, merged hub root and hub sub path ingress Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant