Skip to content

Commit 8542a59

Browse files
authored
Merge pull request #158 from cozystack/feat/dns1123-validation
feat(charts): centralise DNS-1123 subdomain validation in talm helper
2 parents a3edda7 + f36dd9b commit 8542a59

7 files changed

Lines changed: 443 additions & 4 deletions

File tree

charts/cozystack/templates/_helpers.tpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,12 @@ cluster:
9898
network:
9999
cni:
100100
name: none
101-
dnsDomain: {{ .Values.clusterDomain }}
101+
dnsDomain: {{ include "talm.validate.dns1123subdomain" (dict "value" .Values.clusterDomain "field" "clusterDomain") | quote }}
102102
podSubnets:
103103
{{- toYaml .Values.podSubnets | nindent 6 }}
104104
serviceSubnets:
105105
{{- toYaml .Values.serviceSubnets | nindent 6 }}
106-
clusterName: {{ .Values.clusterName | default .Chart.Name | regexFind "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$" | required "clusterName must be a valid DNS-1123 label" | quote }}
106+
clusterName: {{ include "talm.validate.dns1123subdomain" (dict "value" (.Values.clusterName | default .Chart.Name) "field" "clusterName") | quote }}
107107
controlPlane:
108108
endpoint: {{ required "values.yaml: `endpoint` must be set to the cluster control-plane URL (e.g. https://<vip>:6443). This field is cluster-wide: every node's kubelet and kube-proxy dials it, so it cannot be auto-derived from the current node's IP -- `talm template` runs once per node and has no way to reconcile per-node IPs into a single shared endpoint. For multi-node setups use a VIP (cozystack floatingIP) or an external load balancer; for single-node clusters the node's routable IP works." .Values.endpoint | quote }}
109109
{{- if eq .MachineType "controlplane" }}

charts/generic/templates/_helpers.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ cluster:
5252
{{- toYaml .Values.podSubnets | nindent 6 }}
5353
serviceSubnets:
5454
{{- toYaml .Values.serviceSubnets | nindent 6 }}
55-
clusterName: {{ .Values.clusterName | default .Chart.Name | regexFind "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$" | required "clusterName must be a valid DNS-1123 label" | quote }}
55+
clusterName: {{ include "talm.validate.dns1123subdomain" (dict "value" (.Values.clusterName | default .Chart.Name) "field" "clusterName") | quote }}
5656
controlPlane:
5757
endpoint: {{ required "values.yaml: `endpoint` must be set to the cluster control-plane URL (e.g. https://<vip>:6443). This field is cluster-wide: every node's kubelet and kube-proxy dials it, so it cannot be auto-derived from the current node's IP -- `talm template` runs once per node and has no way to reconcile per-node IPs into a single shared endpoint. For multi-node setups use a VIP or an external load balancer; for single-node clusters the node's routable IP works." .Values.endpoint | quote }}
5858
{{- if eq .MachineType "controlplane" }}

charts/talm/templates/_helpers.tpl

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,3 +475,51 @@ vlans:
475475
busPath: {{ $link.spec.busPath }}
476476
{{- end -}}
477477
{{- end -}}
478+
479+
{{- /* Validate that a value is a well-formed DNS-1123 subdomain
480+
(RFC 1035 syntax + RFC 1123 leading-digit relaxation). On
481+
success returns the value verbatim so callers can pipe it
482+
into `quote` or use it inline. On any violation fails the
483+
render with a precise message naming the field and the
484+
offending value.
485+
486+
Mirrors what k8s.io/apimachinery/pkg/util/validation
487+
.IsDNS1123Subdomain enforces in Go-side flag validation
488+
(talm init --name) so chart-rendered values agree with
489+
values an operator passes through the CLI:
490+
491+
1. non-empty
492+
2. total length <= 253 chars
493+
3. matches RFC 1123 subdomain regex (lowercase, only
494+
[a-z0-9-.], each label starts/ends with [a-z0-9],
495+
no double dots)
496+
497+
Per-label length (63 chars) is NOT enforced — upstream
498+
IsDNS1123Subdomain does not enforce it either, and there
499+
is no Talos-side cluster-name length cap that aligns with
500+
a 63-char floor. Stay symmetric with Go-side validation.
501+
502+
Coercion: .value is rendered through `printf "%v"` before
503+
length / regex checks so an unquoted numeric YAML scalar
504+
(e.g. `clusterName: 123`) becomes the string "123" instead
505+
of crashing the template at `len of type int`. The eq-""
506+
emptiness check also avoids treating numeric 0 as falsy.
507+
508+
Usage:
509+
{{ include "talm.validate.dns1123subdomain"
510+
(dict "value" .Values.clusterName "field" "clusterName") }}
511+
*/ -}}
512+
{{- define "talm.validate.dns1123subdomain" -}}
513+
{{- $field := .field -}}
514+
{{- $value := printf "%v" .value -}}
515+
{{- if eq $value "" -}}
516+
{{- fail (printf "values.yaml: %s must be a non-empty DNS-1123 subdomain (must be lowercase, only [a-z0-9-.], start and end with [a-z0-9], max 253 chars)" $field) -}}
517+
{{- end -}}
518+
{{- if gt (len $value) 253 -}}
519+
{{- fail (printf "values.yaml: %s=%q exceeds 253 characters (DNS-1123 subdomain length limit)" $field $value) -}}
520+
{{- end -}}
521+
{{- if not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$" $value) -}}
522+
{{- fail (printf "values.yaml: %s=%q is not a valid DNS-1123 subdomain (must be lowercase, only [a-z0-9-.], start and end with [a-z0-9])" $field $value) -}}
523+
{{- end -}}
524+
{{- $value -}}
525+
{{- end -}}
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright Cozystack Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Contract: `talm init --name <X>` validates X as a DNS-1123
16+
// subdomain in PreRunE before any file is written. The check uses
17+
// k8s.io/apimachinery/pkg/util/validation.IsDNS1123Subdomain so the
18+
// error wording, character class, and length limits match the
19+
// upstream Kubernetes contract that the rendered Talos config
20+
// downstream relies on.
21+
22+
package commands
23+
24+
import (
25+
"strings"
26+
"testing"
27+
)
28+
29+
// withInitFlagsSnapshot captures the package-level initCmdFlags so a
30+
// test can mutate them without leaking into subsequent tests.
31+
func withInitFlagsSnapshot(t *testing.T) {
32+
t.Helper()
33+
saved := initCmdFlags
34+
t.Cleanup(func() { initCmdFlags = saved })
35+
}
36+
37+
// Contract: a valid DNS-1123 subdomain passes PreRunE without
38+
// touching the validation guard. Includes single-label, multi-label
39+
// (`my.cluster.example`), leading-digit, dashes-in-the-middle. The
40+
// shipped chart names (`cozystack`, `generic`, `talm`) ALL must
41+
// pass — they are valid by construction, so a regression in the
42+
// validator that rejected them would brick every default install.
43+
func TestContract_InitPreRun_AcceptsValidDNS1123Subdomain(t *testing.T) {
44+
withInitFlagsSnapshot(t)
45+
46+
cases := []string{
47+
"cozystack",
48+
"generic",
49+
"talm",
50+
"my-cluster",
51+
"my.cluster.example",
52+
"1leading-digit",
53+
"a", // single character
54+
"prod-2", // trailing digit
55+
}
56+
for _, name := range cases {
57+
t.Run(name, func(t *testing.T) {
58+
initCmdFlags.preset = "cozystack"
59+
initCmdFlags.name = name
60+
initCmdFlags.encrypt = false
61+
initCmdFlags.decrypt = false
62+
initCmdFlags.update = false
63+
initCmdFlags.image = ""
64+
if err := initCmd.PreRunE(initCmd, nil); err != nil {
65+
t.Errorf("expected %q to pass PreRunE, got: %v", name, err)
66+
}
67+
})
68+
}
69+
}
70+
71+
// Contract: an invalid DNS-1123 subdomain is rejected in PreRunE
72+
// with an error that names the offending value AND includes the
73+
// upstream k8s validator message (so the operator sees the precise
74+
// constraint that was violated). Each row covers a distinct
75+
// failure class so a regression that loosens the validator only on
76+
// some axis surfaces here.
77+
func TestContract_InitPreRun_RejectsInvalidDNS1123Subdomain(t *testing.T) {
78+
withInitFlagsSnapshot(t)
79+
80+
cases := []struct {
81+
name string
82+
clusterName string
83+
expectInMsg string // substring of the k8s validator message
84+
}{
85+
{"uppercase", "MyCluster", "lower case"},
86+
{"underscore", "my_cluster", "alphanumeric"},
87+
{"leading dash", "-bad", "alphanumeric"},
88+
{"trailing dash", "bad-", "alphanumeric"},
89+
{"space", "my cluster", "alphanumeric"},
90+
{"empty label between dots", "foo..bar", "alphanumeric"},
91+
{"subdomain too long", strings.Repeat("a", 254), "253"},
92+
}
93+
for _, tc := range cases {
94+
t.Run(tc.name, func(t *testing.T) {
95+
initCmdFlags.preset = "cozystack"
96+
initCmdFlags.name = tc.clusterName
97+
initCmdFlags.encrypt = false
98+
initCmdFlags.decrypt = false
99+
initCmdFlags.update = false
100+
initCmdFlags.image = ""
101+
102+
err := initCmd.PreRunE(initCmd, nil)
103+
if err == nil {
104+
t.Fatalf("expected %q to fail PreRunE", tc.clusterName)
105+
}
106+
if !strings.Contains(err.Error(), `"`+tc.clusterName+`"`) {
107+
t.Errorf("error must quote the offending value, got: %v", err)
108+
}
109+
if !strings.Contains(err.Error(), "DNS-1123 subdomain") {
110+
t.Errorf("error must mention 'DNS-1123 subdomain' for grep-ability, got: %v", err)
111+
}
112+
if !strings.Contains(err.Error(), tc.expectInMsg) {
113+
t.Errorf("error must include upstream substring %q, got: %v", tc.expectInMsg, err)
114+
}
115+
})
116+
}
117+
}
118+
119+
// Contract: validation runs ONLY when --name applies — under
120+
// --encrypt / --decrypt / --update the name flag is not required, so
121+
// the validator must not fire on an empty initCmdFlags.name. Pin
122+
// so a regression that always validates would break the
123+
// regenerate-talosconfig flow operators rely on (they pass --decrypt
124+
// without --name).
125+
//
126+
// These modes also require an existing project root, so the test
127+
// stages a fixture with Chart.yaml + secrets.yaml inside a tempdir
128+
// and points Config.RootDir at it. Without the project-root setup
129+
// PreRunE fails earlier than the validator can be reached.
130+
func TestContract_InitPreRun_SkipsValidationOnExclusiveModes(t *testing.T) {
131+
withInitFlagsSnapshot(t)
132+
133+
cases := []struct {
134+
name string
135+
set func()
136+
}{
137+
{"encrypt", func() { initCmdFlags.encrypt = true }},
138+
{"decrypt", func() { initCmdFlags.decrypt = true }},
139+
{"update", func() { initCmdFlags.update = true }},
140+
}
141+
for _, tc := range cases {
142+
t.Run(tc.name, func(t *testing.T) {
143+
dir := t.TempDir()
144+
makeProjectRoot(t, dir)
145+
setRoot(t, dir)
146+
147+
initCmdFlags.preset = ""
148+
initCmdFlags.name = ""
149+
initCmdFlags.encrypt = false
150+
initCmdFlags.decrypt = false
151+
initCmdFlags.update = false
152+
initCmdFlags.image = ""
153+
tc.set()
154+
155+
if err := initCmd.PreRunE(initCmd, nil); err != nil {
156+
t.Errorf("expected --%s with empty name to pass PreRunE, got: %v", tc.name, err)
157+
}
158+
})
159+
}
160+
}

pkg/commands/init.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/siderolabs/talos/pkg/machinery/config"
3636
"github.com/siderolabs/talos/pkg/machinery/config/generate"
3737
"github.com/siderolabs/talos/pkg/machinery/config/generate/secrets"
38+
"k8s.io/apimachinery/pkg/util/validation"
3839
)
3940

4041
var initCmdFlags struct {
@@ -101,6 +102,15 @@ var initCmd = &cobra.Command{
101102
if initCmdFlags.name == "" {
102103
return fmt.Errorf("cluster name is required (use --name or -N flag)")
103104
}
105+
// Validate the operator-supplied cluster name against the same
106+
// DNS-1123 subdomain rule the chart helpers enforce at render
107+
// time. Without this check an invalid name reaches the bundle
108+
// generator and surfaces as an opaque downstream error; pinning
109+
// it here means the operator sees the precise upstream message
110+
// (length, character class, etc.) before any file is written.
111+
if errs := validation.IsDNS1123Subdomain(initCmdFlags.name); len(errs) > 0 {
112+
return fmt.Errorf("--name %q is not a valid DNS-1123 subdomain: %s", initCmdFlags.name, strings.Join(errs, "; "))
113+
}
104114
return nil
105115
},
106116
RunE: func(cmd *cobra.Command, args []string) error {

pkg/engine/contract_cluster_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func TestContract_Cluster_ClusterDomain_Cozystack(t *testing.T) {
237237
for _, cell := range cozystackCells() {
238238
t.Run(cell.name, func(t *testing.T) {
239239
out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion)
240-
assertContains(t, out, "dnsDomain: cozy.local")
240+
assertContains(t, out, `dnsDomain: "cozy.local"`)
241241
})
242242
}
243243
}

0 commit comments

Comments
 (0)