Skip to content

Commit 68a55ef

Browse files
committed
fix(charts,test): nil-safe floatingIP validation; replace Russian comment fragment
Two findings from review: 1. nil .Values.floatingIP (operator unset, Helm-coalesced value-table miss, --set floatingIP=null) tripped the fail-fast block. Sprig's `nil | toString` returns the literal string "<nil>", which is truthy AND not a valid IP — so feeding the toString'd value through ipIsValid below would fail-fast on every controlplane render where the operator legitimately left floatingIP unset (single-node clusters, LB-fronted multi-node, anything Helm strips out of the values table). Gate on the RAW .Values.floatingIP truthiness first; coerce through toString INSIDE the gated body, where the numeric-scalar safety net still fires. Both charts. Pinned by TestContract_NetworkMultidoc_VIPGracefulWhenFloatingIPNil plus its Generic_ mirror: nil floatingIP must render cleanly with no Layer2VIPConfig and no "<nil>" leakage in the rendered YAML. 2. One inline test comment contained the Russian verb "pin'ит" mid-sentence. Replace with the English "pins" — committed-text English-only rule per public-content convention. Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 3940ff4 commit 68a55ef

3 files changed

Lines changed: 76 additions & 17 deletions

File tree

charts/cozystack/templates/_helpers.tpl

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -207,18 +207,29 @@ nameservers:
207207
Render-time `fail` with the bad value is much cheaper to
208208
debug.
209209
210-
Coerce through `toString` BEFORE the predicate. An unquoted
211-
numeric YAML scalar (`floatingIP: 192168`) parses as int,
212-
and ipIsValid is a Go function with a string parameter —
213-
passing an int would raise the Go-template
210+
Gate on the RAW .Values.floatingIP first: nil and missing
211+
fields are falsy on the raw value but Sprig's `nil |
212+
toString` returns the literal string "<nil>" — truthy and
213+
not a valid IP, so feeding it to ipIsValid below would
214+
fail-fast on every controlplane render where the operator
215+
left floatingIP unset (single-node clusters, LB-fronted
216+
multi-node, anything Helm coalesces out of the values
217+
table).
218+
219+
Then coerce through toString INSIDE the gated body. An
220+
unquoted numeric YAML scalar (floatingIP: 192168) parses
221+
as int, and ipIsValid is a Go function with a string
222+
parameter — passing an int would raise the Go-template
214223
"wrong type for value; expected string; got int" panic
215-
instead of the friendly fail message. The toString is also
216-
the safety net for any future operator yaml shape we have
217-
not yet thought of. */}}
224+
instead of the friendly fail message. The toString
225+
coercion is also the safety net for any future operator
226+
yaml shape we have not yet thought of. */}}
227+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
218228
{{- $fipStr := .Values.floatingIP | toString }}
219-
{{- if and $fipStr (not (ipIsValid $fipStr)) (eq .MachineType "controlplane") }}
229+
{{- if not (ipIsValid $fipStr) }}
220230
{{- fail (printf "talm: floatingIP %q is not a valid IPv4 / IPv6 literal. Edit values.yaml and re-run." $fipStr) }}
221231
{{- end }}
232+
{{- end }}
222233
{{- /* Operator-declared vipLink override: emit Layer2VIPConfig
223234
regardless of discovery state. Useful when the target link
224235
does not yet exist on the live system at first apply (typical

charts/generic/templates/_helpers.tpl

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,18 +131,29 @@ nameservers:
131131
Render-time `fail` with the bad value is much cheaper to
132132
debug.
133133
134-
Coerce through `toString` BEFORE the predicate. An unquoted
135-
numeric YAML scalar (`floatingIP: 192168`) parses as int,
136-
and ipIsValid is a Go function with a string parameter —
137-
passing an int would raise the Go-template
134+
Gate on the RAW .Values.floatingIP first: nil and missing
135+
fields are falsy on the raw value but Sprig's `nil |
136+
toString` returns the literal string "<nil>" — truthy and
137+
not a valid IP, so feeding it to ipIsValid below would
138+
fail-fast on every controlplane render where the operator
139+
left floatingIP unset (single-node clusters, LB-fronted
140+
multi-node, anything Helm coalesces out of the values
141+
table).
142+
143+
Then coerce through toString INSIDE the gated body. An
144+
unquoted numeric YAML scalar (floatingIP: 192168) parses
145+
as int, and ipIsValid is a Go function with a string
146+
parameter — passing an int would raise the Go-template
138147
"wrong type for value; expected string; got int" panic
139-
instead of the friendly fail message. The toString is also
140-
the safety net for any future operator yaml shape we have
141-
not yet thought of. */}}
148+
instead of the friendly fail message. The toString
149+
coercion is also the safety net for any future operator
150+
yaml shape we have not yet thought of. */}}
151+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
142152
{{- $fipStr := .Values.floatingIP | toString }}
143-
{{- if and $fipStr (not (ipIsValid $fipStr)) (eq .MachineType "controlplane") }}
153+
{{- if not (ipIsValid $fipStr) }}
144154
{{- fail (printf "talm: floatingIP %q is not a valid IPv4 / IPv6 literal. Edit values.yaml and re-run." $fipStr) }}
145155
{{- end }}
156+
{{- end }}
146157
{{- /* Operator-declared vipLink override: emit Layer2VIPConfig
147158
regardless of discovery state. Useful when the target link
148159
does not yet exist on the live system at first apply (typical

pkg/engine/contract_network_multidoc_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ func TestContract_NetworkMultidoc_BridgeConfigEmitted(t *testing.T) {
266266
// Fixture: bridgeWithClusterSubnetLookup has br0 carrying
267267
// 10.5.0.10/24 (global scope) and the IPv4 default route. floatingIP
268268
// 10.5.0.99 is inside that subnet, so link_name_for_address resolves
269-
// to br0; the discovery-derived Layer2VIPConfig pin'ит link=br0.
269+
// to br0; the discovery-derived Layer2VIPConfig pins link=br0.
270270
// Without BridgeConfig emission (the prior shape), this would have
271271
// been a "VIP on undocumented link" symptom; now BridgeConfig
272272
// documents the link explicitly and the chart also emits STP
@@ -774,6 +774,43 @@ func TestContract_NetworkMultidoc_VIPEmitsWithMatchingSubnetEvenWithoutDefaultRo
774774
}
775775
}
776776

777+
// Contract: a nil / unset floatingIP on a controlplane node
778+
// renders without error and emits no Layer2VIPConfig. The
779+
// validation block must gate on the RAW .Values.floatingIP
780+
// truthiness before any toString coercion — Sprig's
781+
// `nil | toString` returns the literal string "<nil>", which is
782+
// truthy and not a valid IP, so a naive predicate on the
783+
// toString'd value would fail-fast on every controlplane render
784+
// where the operator left floatingIP unset (single-node
785+
// clusters, LB-fronted multi-node, anything Helm coalesces out
786+
// of the values table).
787+
func TestContract_NetworkMultidoc_VIPGracefulWhenFloatingIPNil(t *testing.T) {
788+
out := renderCozystackWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{
789+
"floatingIP": nil,
790+
"advertisedSubnets": []any{testAdvertisedSubnet},
791+
})
792+
if strings.Contains(out, "kind: Layer2VIPConfig") {
793+
t.Errorf("Layer2VIPConfig must not emit when floatingIP is nil; got:\n%s", out)
794+
}
795+
if strings.Contains(out, "<nil>") {
796+
t.Errorf("rendered output leaks the Sprig <nil> literal — fail-fast misfired on nil floatingIP:\n%s", out)
797+
}
798+
}
799+
800+
// Generic-chart mirror of the nil-safe contract above.
801+
func TestContract_NetworkMultidoc_Generic_VIPGracefulWhenFloatingIPNil(t *testing.T) {
802+
out := renderGenericWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{
803+
"floatingIP": nil,
804+
"advertisedSubnets": []any{testAdvertisedSubnet},
805+
})
806+
if strings.Contains(out, "kind: Layer2VIPConfig") {
807+
t.Errorf("generic chart: Layer2VIPConfig must not emit when floatingIP is nil; got:\n%s", out)
808+
}
809+
if strings.Contains(out, "<nil>") {
810+
t.Errorf("generic chart: rendered output leaks the Sprig <nil> literal:\n%s", out)
811+
}
812+
}
813+
777814
// Contract: a numeric (non-string) floatingIP — typed without
778815
// quotes in values.yaml so YAML parses it as int — must produce
779816
// the friendly fail-fast error, NOT a Go-template

0 commit comments

Comments
 (0)