feat(gateway): support multiple hosts in ingress#392
Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds support for multiple gateway hosts by introducing a Hosts field on GatewayIngress, utilities to aggregate and deduplicate hosts (including settings-based hosts with {stack} templating), and refactors ingress/TLS generation and tests to use the aggregated host list. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/resources/gateways/ingress.go (1)
88-95:getAllHostsis called twice per reconciliation — consider computing once.Both
withTlsandwithIngressRulesindependently callgetAllHosts, which hits the settings store each time. If the settings value were to change between the two calls, TLS hosts and ingress rule hosts could theoretically diverge. Computing the hosts once increateIngressand closing over the result would be both more efficient and more consistent.♻️ Suggested approach
func createIngress(ctx core.Context, stack *v1beta1.Stack, gateway *v1beta1.Gateway) error { name := types.NamespacedName{ Namespace: stack.Name, Name: "gateway", } if gateway.Spec.Ingress == nil { return core.DeleteIfExists[*v1.Ingress](ctx, name) } + hosts, err := getAllHosts(ctx, gateway) + if err != nil { + return err + } + _, _, err := core.CreateOrUpdate(ctx, name, withAnnotations(ctx, stack, gateway), withLabels(ctx, stack, gateway), withIngressClassName(ctx, stack, gateway), - withIngressRules(ctx, gateway), - withTls(ctx, gateway), + withIngressRules(hosts), + withTls(ctx, gateway, hosts), core.WithController[*v1.Ingress](ctx.GetScheme(), gateway), ) return err }Then update
withIngressRulesandwithTlsto accepthosts []stringdirectly instead of computing them internally.Also applies to: 124-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/resources/gateways/ingress.go` around lines 88 - 95, Compute hosts once in createIngress by calling getAllHosts(ctx, gateway) and store the []string result, then pass that hosts slice into withTls and withIngressRules instead of letting those functions call getAllHosts themselves; update the signatures of withTls and withIngressRules to accept hosts []string (and remove their internal getAllHosts calls), and ensure createIngress uses the single hosts value for both TLS (SecretName/Hosts) and ingress rules so both use the same, cached data.api/formance.com/v1beta1/gateway_types.go (1)
53-68: Deduplication logic is duplicated withgetAllHostsiningress.go.The same seen-map dedup loop appears here and in
internal/resources/gateways/ingress.go:57-68. Consider extracting a smalldedup([]string) []stringhelper to share between the two call sites.This is a minor DRY nit — the current code is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/formance.com/v1beta1/gateway_types.go` around lines 53 - 68, The GetHosts method on GatewayIngress duplicates the deduplication logic used by getAllHosts in internal/resources/gateways/ingress.go; extract a small helper function (e.g. dedupStrings or dedup([]string) []string) and use it from both GatewayIngress.GetHosts and getAllHosts to remove the seen-map loop duplication, preserving current behavior (skip empty strings and preserve first occurrence order).internal/tests/gateway_controller_test.go (1)
167-186: Consider adding a test for host deduplication across spec and settings.The happy path is well covered. A test where a settings host overlaps with the spec host (e.g., settings value includes
"example.net, settings.example.com") would exercise the dedup path ingetAllHostsand guard against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tests/gateway_controller_test.go` around lines 167 - 186, Add a test to cover host deduplication by creating a Settings resource whose value includes the stack/spec host (e.g., "example.net, settings.example.com") and asserting the resulting Ingress produced by the controller contains no duplicates; update the existing Context in gateway_controller_test.go to create hostsSetting via settings.New with a value that includes "example.net" and "settings.example.com", call Create(hostsSetting) in JustBeforeEach and Delete(hostsSetting) in AfterEach, then in the It block load the Ingress and assert ingress.Spec.Rules length and Host entries reflect deduplication (referencing getAllHosts in the controller logic to ensure this path is exercised).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/formance.com/v1beta1/gateway_types.go`:
- Around line 53-68: The GetHosts method on GatewayIngress duplicates the
deduplication logic used by getAllHosts in
internal/resources/gateways/ingress.go; extract a small helper function (e.g.
dedupStrings or dedup([]string) []string) and use it from both
GatewayIngress.GetHosts and getAllHosts to remove the seen-map loop duplication,
preserving current behavior (skip empty strings and preserve first occurrence
order).
In `@internal/resources/gateways/ingress.go`:
- Around line 88-95: Compute hosts once in createIngress by calling
getAllHosts(ctx, gateway) and store the []string result, then pass that hosts
slice into withTls and withIngressRules instead of letting those functions call
getAllHosts themselves; update the signatures of withTls and withIngressRules to
accept hosts []string (and remove their internal getAllHosts calls), and ensure
createIngress uses the single hosts value for both TLS (SecretName/Hosts) and
ingress rules so both use the same, cached data.
In `@internal/tests/gateway_controller_test.go`:
- Around line 167-186: Add a test to cover host deduplication by creating a
Settings resource whose value includes the stack/spec host (e.g., "example.net,
settings.example.com") and asserting the resulting Ingress produced by the
controller contains no duplicates; update the existing Context in
gateway_controller_test.go to create hostsSetting via settings.New with a value
that includes "example.net" and "settings.example.com", call
Create(hostsSetting) in JustBeforeEach and Delete(hostsSetting) in AfterEach,
then in the It block load the Ingress and assert ingress.Spec.Rules length and
Host entries reflect deduplication (referencing getAllHosts in the controller
logic to ensure this path is exercised).
sylr
force-pushed
the
feat/ingress-multiple-hosts
branch
from
bfa5250 to
48d331b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
internal/resources/gateways/ingress.go (1)
82-85:⚠️ Potential issue | 🟠 MajorEmpty
hostsslice produces a TLS block with no host coverage.When
getAllHostsreturns[](triggered by the empty-Hostcatch-all scenario already flagged),withTlsstill creates anIngressTLSentry withHosts: []. Most ingress controllers will silently ignore or mis-handle a TLS stanza with an empty hosts list. This is a downstream effect of the empty-host filtering inDedupHosts, but the impact here is independently observable: TLS is configured but covers nothing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/resources/gateways/ingress.go` around lines 82 - 85, The TLS stanza is being added even when hosts is an empty slice, producing a TLS entry with no host coverage; update the logic in the function that sets t.Spec.TLS (the withTls / TLS assignment block) to only set t.Spec.TLS when the hosts slice returned by getAllHosts/DedupHosts is non-empty—if hosts is empty, leave t.Spec.TLS nil or an empty list so no IngressTLS entry is emitted. Ensure you reference and gate on the same hosts variable used currently for SecretName to avoid creating a TLS block with Hosts: [].api/formance.com/v1beta1/gateway_types.go (1)
54-73:⚠️ Potential issue | 🟠 Major
DedupHostsfiltering empty strings breaks existing catch-all ingresses.When
Hostis set to""(empty) and noHosts/settings are configured,DedupHostsdiscards the empty string,GetHosts()returns[], andwithIngressRulesgenerates zero rules — replacing what was previously a catch-all rule. Existing users relying on an emptyHostfor catch-all behavior will have their ingress rules silently cleared.Consider explicitly handling the empty-host case (e.g., pass it through as-is when it is the only entry) or document the migration path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/formance.com/v1beta1/gateway_types.go` around lines 54 - 73, DedupHosts currently drops empty strings which removes intended catch-all hosts; update DedupHosts (used by GatewayIngress.GetHosts) to preserve a single empty host when that was the only input (or otherwise explicitly present as the intended host) instead of returning an empty slice. Concretely: when deduplication yields no non-empty hosts but the original input contained an empty string as the sole/explicit host, return []string{""}; keep existing dedup behavior for other cases so duplicates and intermediate empty entries are still filtered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/resources/gateways/ingress.go`:
- Around line 53-64: getAllHosts currently dereferences gateway.Spec.Ingress via
gateway.Spec.Ingress.GetHosts() and will panic if Spec.Ingress is nil; add a
defensive nil check inside getAllHosts: if gateway.Spec.Ingress is nil, treat
ingress hosts as an empty slice, otherwise call gateway.Spec.Ingress.GetHosts(),
then proceed to append settingsHosts and return v1beta1.DedupHosts(...) as
before. Update getAllHosts to reference gateway.Spec.Ingress and use a local
variable like ingressHosts to avoid the nil deref and keep behavior consistent
with createIngress.
---
Duplicate comments:
In `@api/formance.com/v1beta1/gateway_types.go`:
- Around line 54-73: DedupHosts currently drops empty strings which removes
intended catch-all hosts; update DedupHosts (used by GatewayIngress.GetHosts) to
preserve a single empty host when that was the only input (or otherwise
explicitly present as the intended host) instead of returning an empty slice.
Concretely: when deduplication yields no non-empty hosts but the original input
contained an empty string as the sole/explicit host, return []string{""}; keep
existing dedup behavior for other cases so duplicates and intermediate empty
entries are still filtered.
In `@internal/resources/gateways/ingress.go`:
- Around line 82-85: The TLS stanza is being added even when hosts is an empty
slice, producing a TLS entry with no host coverage; update the logic in the
function that sets t.Spec.TLS (the withTls / TLS assignment block) to only set
t.Spec.TLS when the hosts slice returned by getAllHosts/DedupHosts is
non-empty—if hosts is empty, leave t.Spec.TLS nil or an empty list so no
IngressTLS entry is emitted. Ensure you reference and gate on the same hosts
variable used currently for SecretName to avoid creating a TLS block with Hosts:
[].
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
config/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
api/formance.com/v1beta1/gateway_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godocs/04-Modules/02-Gateway.mddocs/09-Configuration reference/01-Settings.mdinternal/resources/gateways/ingress.gointernal/tests/gateway_controller_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- api/formance.com/v1beta1/zz_generated.deepcopy.go
- internal/tests/gateway_controller_test.go
- docs/09-Configuration reference/01-Settings.md
| func getAllHosts(ctx core.Context, gateway *v1beta1.Gateway) ([]string, error) { | ||
| settingsHosts, err := settings.GetTrimmedStringSlice(ctx, gateway.Spec.Stack, "gateway", "ingress", "hosts") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| for i, h := range settingsHosts { | ||
| settingsHosts[i] = strings.ReplaceAll(h, "{stack}", gateway.Spec.Stack) | ||
| } | ||
|
|
||
| return v1beta1.DedupHosts(append(gateway.Spec.Ingress.GetHosts(), settingsHosts...)), nil | ||
| } |
There was a problem hiding this comment.
getAllHosts has an implicit nil precondition on gateway.Spec.Ingress.
gateway.Spec.Ingress.GetHosts() on line 63 panics if gateway.Spec.Ingress is nil. The current call-site (createIngress) guards against this, but nothing prevents getAllHosts from being called elsewhere without the guard. A defensive nil check or a documenting comment would make the precondition explicit.
🛡️ Proposed defensive guard
func getAllHosts(ctx core.Context, gateway *v1beta1.Gateway) ([]string, error) {
+ if gateway.Spec.Ingress == nil {
+ return nil, nil
+ }
settingsHosts, err := settings.GetTrimmedStringSlice(ctx, gateway.Spec.Stack, "gateway", "ingress", "hosts")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getAllHosts(ctx core.Context, gateway *v1beta1.Gateway) ([]string, error) { | |
| settingsHosts, err := settings.GetTrimmedStringSlice(ctx, gateway.Spec.Stack, "gateway", "ingress", "hosts") | |
| if err != nil { | |
| return nil, err | |
| } | |
| for i, h := range settingsHosts { | |
| settingsHosts[i] = strings.ReplaceAll(h, "{stack}", gateway.Spec.Stack) | |
| } | |
| return v1beta1.DedupHosts(append(gateway.Spec.Ingress.GetHosts(), settingsHosts...)), nil | |
| } | |
| func getAllHosts(ctx core.Context, gateway *v1beta1.Gateway) ([]string, error) { | |
| if gateway.Spec.Ingress == nil { | |
| return nil, nil | |
| } | |
| settingsHosts, err := settings.GetTrimmedStringSlice(ctx, gateway.Spec.Stack, "gateway", "ingress", "hosts") | |
| if err != nil { | |
| return nil, err | |
| } | |
| for i, h := range settingsHosts { | |
| settingsHosts[i] = strings.ReplaceAll(h, "{stack}", gateway.Spec.Stack) | |
| } | |
| return v1beta1.DedupHosts(append(gateway.Spec.Ingress.GetHosts(), settingsHosts...)), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/resources/gateways/ingress.go` around lines 53 - 64, getAllHosts
currently dereferences gateway.Spec.Ingress via gateway.Spec.Ingress.GetHosts()
and will panic if Spec.Ingress is nil; add a defensive nil check inside
getAllHosts: if gateway.Spec.Ingress is nil, treat ingress hosts as an empty
slice, otherwise call gateway.Spec.Ingress.GetHosts(), then proceed to append
settingsHosts and return v1beta1.DedupHosts(...) as before. Update getAllHosts
to reference gateway.Spec.Ingress and use a local variable like ingressHosts to
avoid the nil deref and keep behavior consistent with createIngress.
48d331b to
a464c64
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/formance.com/v1beta1/gateway_types.go`:
- Around line 28-31: Remove the kubebuilder root/resource markers from the
nested type GatewayIngress: delete the lines with
"+kubebuilder:object:root=true", "+kubebuilder:subresource:status", and
"+kubebuilder:resource:scope=Cluster" that are placed on the GatewayIngress type
definition so it is no longer treated as a top-level API object; this will
prevent generation of DeepCopyObject()/runtime.Object implementations for
GatewayIngress and stop it from being emitted as a standalone CRD—leave
GatewayIngress defined as a plain struct referenced by GatewaySpec (Ingress
*GatewayIngress) and ensure only the real root type (e.g. Gateway) keeps
root/resource kubebuilder markers.
In `@docs/09-Configuration` reference/02-Custom Resource Definitions.md:
- Line 2118: The table entry links to a non-existent fragment
`#gatewayingresstls` for GatewayIngressTLS; locate the GatewayIngressTLS section
header and either add an explicit HTML anchor or update the table link to match
the actual header fragment (exact hyphenation/lowercasing) used by that section
so the link resolves correctly; ensure the table row referencing
`GatewayIngressTLS` uses the same fragment string as the target header.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
api/formance.com/v1beta1/gateway_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godocs/04-Modules/02-Gateway.mddocs/09-Configuration reference/01-Settings.mddocs/09-Configuration reference/02-Custom Resource Definitions.mdinternal/resources/gateways/ingress.gointernal/tests/gateway_controller_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/04-Modules/02-Gateway.md
| // GatewayIngress represents the ingress configuration for the gateway. | ||
| // +kubebuilder:object:root=true | ||
| // +kubebuilder:subresource:status | ||
| // +kubebuilder:resource:scope=Cluster |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Step 1: Check the actual markers on GatewayIngress
echo "=== Checking GatewayIngress markers ==="
rg -n -B5 -A2 'type GatewayIngress struct' api/formance.com/v1beta1/gateway_types.go | head -40
# Step 2: Check the full GatewayIngress definition and context
echo ""
echo "=== Checking full context of GatewayIngress definition ==="
sed -n '20,80p' api/formance.com/v1beta1/gateway_types.go
# Step 3: Check if Gateway type exists and if GatewayIngress is nested
echo ""
echo "=== Checking Gateway type structure ==="
sed -n '1,150p' api/formance.com/v1beta1/gateway_types.go | tail -100Repository: formancehq/operator
Length of output: 4891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for DeepCopyObject generation on GatewayIngress
echo "=== Checking for DeepCopyObject on GatewayIngress ==="
rg -n 'func \(in \*GatewayIngress\) DeepCopyObject' api/formance.com/v1beta1/ || echo "Not found (expected)"
# Check for DeepCopyInto on GatewayIngress (regular nested type)
echo ""
echo "=== Checking for DeepCopyInto on GatewayIngress ==="
rg -n 'func \(in \*GatewayIngress\) DeepCopyInto' api/formance.com/v1beta1/
# Check for any SchemeBuilder registrations of GatewayIngress
echo ""
echo "=== Checking SchemeBuilder registrations ==="
rg -n 'SchemeBuilder\.Register.*GatewayIngress' --type=go api/formance.com/v1beta1/ || echo "Not found (expected)"
# Check if Gateway type is the actual root
echo ""
echo "=== Verifying Gateway is actual root ==="
rg -n -B2 'type Gateway struct' api/formance.com/v1beta1/gateway_types.go | head -20Repository: formancehq/operator
Length of output: 591
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if GatewayIngress is documented as a standalone CRD (unwanted artifact)
echo "=== Checking for GatewayIngress in docs ==="
find . -name "*.md" -type f -exec rg -l 'GatewayIngress' {} \; | head -5
# Verify the exact lines of the markers to confirm what should be removed
echo ""
echo "=== Exact marker lines on GatewayIngress ==="
sed -n '28,33p' api/formance.com/v1beta1/gateway_types.goRepository: formancehq/operator
Length of output: 490
Remove root/resource kubebuilder markers from nested GatewayIngress.
GatewayIngress (lines 29–31) is marked with +kubebuilder:object:root=true, +kubebuilder:subresource:status, and +kubebuilder:resource:scope=Cluster, but this type is nested under GatewaySpec as an optional field (Ingress *GatewayIngress), not an API root resource. These markers cause:
DeepCopyObject()to be generated in the deepcopy file, implementingruntime.ObjectunnecessarilyGatewayIngressto appear as a standalone CRD in documentation- Misleading artifact surface that contradicts its actual nested usage
These markers belong only on root types (like Gateway itself) and should be removed from GatewayIngress.
Proposed fix
// GatewayIngress represents the ingress configuration for the gateway.
-// +kubebuilder:object:root=true
-// +kubebuilder:subresource:status
-// +kubebuilder:resource:scope=Cluster
type GatewayIngress struct {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // GatewayIngress represents the ingress configuration for the gateway. | |
| // +kubebuilder:object:root=true | |
| // +kubebuilder:subresource:status | |
| // +kubebuilder:resource:scope=Cluster | |
| // GatewayIngress represents the ingress configuration for the gateway. | |
| type GatewayIngress struct { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/formance.com/v1beta1/gateway_types.go` around lines 28 - 31, Remove the
kubebuilder root/resource markers from the nested type GatewayIngress: delete
the lines with "+kubebuilder:object:root=true",
"+kubebuilder:subresource:status", and "+kubebuilder:resource:scope=Cluster"
that are placed on the GatewayIngress type definition so it is no longer treated
as a top-level API object; this will prevent generation of
DeepCopyObject()/runtime.Object implementations for GatewayIngress and stop it
from being emitted as a standalone CRD—leave GatewayIngress defined as a plain
struct referenced by GatewaySpec (Ingress *GatewayIngress) and ensure only the
real root type (e.g. Gateway) keeps root/resource kubebuilder markers.
| | `scheme` _string_ | Indicate the scheme.<br />Actually, It should be `https` unless you know what you are doing. | https | | | ||
| | `ingressClassName` _string_ | Ingress class to use | | | | ||
| | `annotations` _object (keys:string, values:string)_ | Custom annotations to add on the ingress | | | | ||
| | `tls` _[GatewayIngressTLS](#gatewayingresstls)_ | Allow to customize the tls part of the ingress | | | |
There was a problem hiding this comment.
Fix broken anchor link for GatewayIngressTLS.
Line 2118 links to #gatewayingresstls, but that fragment is not present, so the reference is broken.
📝 Minimal doc fix
-| `tls` _[GatewayIngressTLS](`#gatewayingresstls`)_ | Allow to customize the tls part of the ingress | | |
+| `tls` _GatewayIngressTLS_ | Allow to customize the tls part of the ingress | | |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `tls` _[GatewayIngressTLS](#gatewayingresstls)_ | Allow to customize the tls part of the ingress | | | | |
| | `tls` _GatewayIngressTLS_ | Allow to customize the tls part of the ingress | | | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 2118-2118: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/09-Configuration` reference/02-Custom Resource Definitions.md at line
2118, The table entry links to a non-existent fragment `#gatewayingresstls` for
GatewayIngressTLS; locate the GatewayIngressTLS section header and either add an
explicit HTML anchor or update the table link to match the actual header
fragment (exact hyphenation/lowercasing) used by that section so the link
resolves correctly; ensure the table row referencing `GatewayIngressTLS` uses
the same fragment string as the target header.
a464c64 to
0a6e2bd
Compare
Add a Hosts field to GatewayIngress CRD and a gateway.ingress.hosts setting to allow serving a stack on multiple hostnames. Hosts from both sources are deduplicated and merged. Each host gets its own ingress rule and all hosts are included in TLS configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… dedup test Address review feedback: - Extract shared DedupHosts function used by both GetHosts and getAllHosts - Compute hosts once in createIngress and pass to withIngressRules/withTls - Add test for host deduplication when spec and settings overlap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sylr
force-pushed
the
feat/ingress-multiple-hosts
branch
from
0a6e2bd to
2c68232
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/formance.com/v1beta1/gateway_types.go (1)
71-74: Consider documenting or handling nil receiver.
GetHosts()will panic if called on a nil*GatewayIngress. While the current call site ingetAllHostsis guarded by a nil check increateIngress, this method could be called from other contexts without protection.Consider either adding a nil check or documenting the precondition:
Option 1: Defensive nil check
// GetHosts returns the deduplicated union of Host and Hosts. func (in *GatewayIngress) GetHosts() []string { + if in == nil { + return nil + } return DedupHosts(append([]string{in.Host}, in.Hosts...)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/formance.com/v1beta1/gateway_types.go` around lines 71 - 74, GetHosts() on *GatewayIngress can panic when receiver is nil; update the GatewayIngress.GetHosts method to defensively handle a nil receiver (e.g., if in == nil return nil or return []string{}) before calling DedupHosts, and ensure any callers such as createIngress/getAllHosts still behave correctly; alternatively, if you choose to require a non-nil receiver, add a clear comment on GatewayIngress.GetHosts documenting that the receiver must be non-nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/formance.com/v1beta1/gateway_types.go`:
- Around line 71-74: GetHosts() on *GatewayIngress can panic when receiver is
nil; update the GatewayIngress.GetHosts method to defensively handle a nil
receiver (e.g., if in == nil return nil or return []string{}) before calling
DedupHosts, and ensure any callers such as createIngress/getAllHosts still
behave correctly; alternatively, if you choose to require a non-nil receiver,
add a clear comment on GatewayIngress.GetHosts documenting that the receiver
must be non-nil.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
config/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
api/formance.com/v1beta1/gateway_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godocs/04-Modules/02-Gateway.mddocs/09-Configuration reference/01-Settings.mdinternal/resources/gateways/ingress.gointernal/tests/gateway_controller_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/04-Modules/02-Gateway.md
- api/formance.com/v1beta1/zz_generated.deepcopy.go
- docs/09-Configuration reference/01-Settings.md
2 participants
Add a Hosts field to GatewayIngress CRD and a gateway.ingress.hosts setting to allow serving a stack on multiple hostnames. Hosts from both sources are deduplicated and merged. Each host gets its own ingress rule and all hosts are included in TLS configuration.