diff --git a/.ainav/config/index.md b/.ainav/config/index.md index 56a01ba5d..9c9a6c1ef 100644 --- a/.ainav/config/index.md +++ b/.ainav/config/index.md @@ -61,7 +61,9 @@ Generated docs: `doc/api_dump/*.md` Admission-webhook validators implement the `Validator` interface (`validator.go`), are listed per-CRD in `registry.go`, and get a default severity in `admission/defaults.go`. -Add a rule = implement + register + add to the defaults table. clusterCapacity validators: +Add a rule = implement + register + add to the defaults table. +Pod-spec syntax validators (`*_podspec_syntax.go` + shared `podspec_syntax.go`): k8s +pod-level syntax rules at WekaCluster/WekaClient admission (OP-361). clusterCapacity validators: `cluster_capacity_chunk_feasibility.go` (greenfield per-FD TLC share ≥ 384 GiB; skipped once the cluster has TLC-bearing drive containers) and `cluster_capacity_protection.go` (min SW≥3, RL≥2, HS≥0 / hotSpare optional — the `3+2+0` floor from `allocator.MinProtectionFloor`). diff --git a/charts/weka-operator/values.yaml b/charts/weka-operator/values.yaml index 3801655c1..d152194e9 100644 --- a/charts/weka-operator/values.yaml +++ b/charts/weka-operator/values.yaml @@ -728,10 +728,23 @@ admissionPolicies: # # violation is reported per unmet dependency. # cluster_skip_default_fs: default # strict: warn | relaxed: warn # + # # Scheduling-related fields (tolerations, nodeSelector, per-role + # # label/annotation maps, failureDomain topology keys/skew, podConfig + # # affinity/topologySpreadConstraints) must have syntax the API server + # # would accept — invalid values are caught at CR apply instead of + # # pod create. + # cluster_podspec_syntax: default # strict: error | relaxed: error + # # # WekaClient.spec.targetCluster must reference an existing # # WekaCluster. # client_target_cluster_exists: default # strict: error | relaxed: warn # + # # Scheduling-related fields (tolerations, nodeSelector, csiConfig + # # advanced labels/tolerations) must have syntax the API server + # # would accept — invalid values are caught at CR apply instead of + # # pod create. + # client_podspec_syntax: default # strict: error | relaxed: error + # # # Decreasing any cores field on a WekaCluster (spec.dynamicTemplate.*Cores) # # is denied — reducing cores on a running cluster can destabilize # # active workloads. Applies on Update only; also blocks unsetting an diff --git a/doc/operator/operations/admission-control.md b/doc/operator/operations/admission-control.md index 8ce80edbd..94bb66840 100644 --- a/doc/operator/operations/admission-control.md +++ b/doc/operator/operations/admission-control.md @@ -2,10 +2,11 @@ ## Overview -The operator runs a validating admission webhook for `WekaCluster` and `WekaClient` -resources. On every `kubectl apply` (or Helm/GitOps equivalent) it runs a battery +The operator runs a validating admission webhook for `WekaCluster`, `WekaClient`, and +`WekaContainer` resources. On every `kubectl apply` (or Helm/GitOps equivalent) it runs a battery of policies and either admits the request, attaches a `kubectl Warning:` line, or -rejects it. The default posture is non-blocking — most policies emit warnings; +rejects it (`WekaContainer` carries update-only policies, e.g. cores decrease). +The default posture is non-blocking — most policies emit warnings; only feasibility-breaking specs are rejected. ## Configuration diff --git a/internal/admission/defaults.go b/internal/admission/defaults.go index 13703df36..defc84f61 100644 --- a/internal/admission/defaults.go +++ b/internal/admission/defaults.go @@ -18,10 +18,12 @@ var ( "cluster_capacity_protection": {Strict: Error, Relaxed: Error}, "cluster_capacity_chunk_feasibility": {Strict: Error, Relaxed: Error}, "cluster_skip_default_fs": {Strict: Warn, Relaxed: Warn}, + "cluster_podspec_syntax": {Strict: Error, Relaxed: Error}, } wekaClientDefaults = map[string]PolicyDefaults{ "client_target_cluster_exists": {Strict: Error, Relaxed: Warn}, + "client_podspec_syntax": {Strict: Error, Relaxed: Error}, } // Update-only defaults: cores-decrease checks are always Error regardless diff --git a/internal/validation/client_podspec_syntax.go b/internal/validation/client_podspec_syntax.go new file mode 100644 index 000000000..28664a0d4 --- /dev/null +++ b/internal/validation/client_podspec_syntax.go @@ -0,0 +1,40 @@ +package validation + +import ( + "context" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// clientPodspecSyntax rejects WekaClients whose scheduling-related fields +// would produce pods the API server rejects at create time (invalid +// toleration keys/enums, label syntax). Pure spec math; see +// podspec_syntax.go for the shared checks. +type clientPodspecSyntax struct{} + +func (clientPodspecSyntax) ID() string { return "client_podspec_syntax" } + +func (clientPodspecSyntax) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + wc, ok := obj.(*wekav1alpha1.WekaClient) + if !ok { + return nil + } + spec := field.NewPath("spec") + var errs field.ErrorList + + errs = append(errs, validateSimpleTolerations(spec.Child("tolerations"), wc.Spec.Tolerations)...) + errs = append(errs, validateRawTolerations(spec.Child("rawTolerations"), wc.Spec.RawTolerations)...) + errs = append(errs, validateLabelMap(spec.Child("nodeSelector"), wc.Spec.NodeSelector)...) + + if csi := wc.Spec.CsiConfig; csi != nil && csi.Advanced != nil { + adv := spec.Child("csiConfig", "advanced") + errs = append(errs, validateLabelMap(adv.Child("nodeLabels"), csi.Advanced.NodeLabels)...) + errs = append(errs, validateLabelMap(adv.Child("controllerLabels"), csi.Advanced.ControllerLabels)...) + errs = append(errs, validateRawTolerations(adv.Child("nodeTolerations"), csi.Advanced.NodeTolerations)...) + errs = append(errs, validateRawTolerations(adv.Child("controllerTolerations"), csi.Advanced.ControllerTolerations)...) + } + return errs +} diff --git a/internal/validation/client_podspec_syntax_test.go b/internal/validation/client_podspec_syntax_test.go new file mode 100644 index 000000000..8d30599c5 --- /dev/null +++ b/internal/validation/client_podspec_syntax_test.go @@ -0,0 +1,88 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestClientPodspecSyntax(t *testing.T) { + v := clientPodspecSyntax{} + base := func() *weka.WekaClient { + return &weka.WekaClient{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} + } + + if errs := v.Validate(context.Background(), nil, base()); len(errs) != 0 { + t.Fatalf("empty spec should pass: %v", errs) + } + + badTol := base() + badTol.Spec.Tolerations = []string{"scitix.ai/nodecheck:NoSchedule"} + if errs := v.Validate(context.Background(), nil, badTol); len(errs) != 1 { + t.Fatalf("bad string toleration: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.tolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } else if !strings.Contains(errs[0].Detail, "rawTolerations") { + t.Fatalf("missing rawTolerations hint: %s", errs[0].Detail) + } + + badRawTol := base() + badRawTol.Spec.RawTolerations = []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Effect: "NoSchedul"}} + if errs := v.Validate(context.Background(), nil, badRawTol); len(errs) != 1 { + t.Fatalf("bad rawToleration effect: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.rawTolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badSel := base() + badSel.Spec.NodeSelector = map[string]string{"bad key!": "x"} + if errs := v.Validate(context.Background(), nil, badSel); len(errs) != 1 { + t.Fatalf("bad nodeSelector: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.nodeSelector") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badNodeLabels := base() + badNodeLabels.Spec.CsiConfig = &weka.ClientCsiConfig{ + Advanced: &weka.AdvancedCsiConfig{NodeLabels: map[string]string{"k": "bad value!"}}, + } + if errs := v.Validate(context.Background(), nil, badNodeLabels); len(errs) != 1 { + t.Fatalf("bad csiConfig.advanced.nodeLabels: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.csiConfig.advanced.nodeLabels") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badControllerTol := base() + badControllerTol.Spec.CsiConfig = &weka.ClientCsiConfig{ + Advanced: &weka.AdvancedCsiConfig{ControllerTolerations: []corev1.Toleration{{Key: "k", Operator: "Bogus"}}}, + } + if errs := v.Validate(context.Background(), nil, badControllerTol); len(errs) != 1 { + t.Fatalf("bad csiConfig.advanced.controllerTolerations: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.csiConfig.advanced.controllerTolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badControllerLabels := base() + badControllerLabels.Spec.CsiConfig = &weka.ClientCsiConfig{ + Advanced: &weka.AdvancedCsiConfig{ControllerLabels: map[string]string{"bad key!": "x"}}, + } + if errs := v.Validate(context.Background(), nil, badControllerLabels); len(errs) != 1 { + t.Fatalf("bad csiConfig.advanced.controllerLabels: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.csiConfig.advanced.controllerLabels") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badNodeTol := base() + badNodeTol.Spec.CsiConfig = &weka.ClientCsiConfig{ + Advanced: &weka.AdvancedCsiConfig{NodeTolerations: []corev1.Toleration{{Key: "k", Operator: "Bogus"}}}, + } + if errs := v.Validate(context.Background(), nil, badNodeTol); len(errs) != 1 { + t.Fatalf("bad csiConfig.advanced.nodeTolerations: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.csiConfig.advanced.nodeTolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } +} diff --git a/internal/validation/cluster_podspec_syntax.go b/internal/validation/cluster_podspec_syntax.go new file mode 100644 index 000000000..7d1a96e06 --- /dev/null +++ b/internal/validation/cluster_podspec_syntax.go @@ -0,0 +1,136 @@ +package validation + +import ( + "context" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// clusterPodspecSyntax rejects WekaClusters whose scheduling-related fields +// would produce pods the API server rejects at create time (invalid +// toleration keys/enums, label syntax, topology keys, affinity). Pure spec +// math; see podspec_syntax.go for the shared checks. +type clusterPodspecSyntax struct{} + +func (clusterPodspecSyntax) ID() string { return "cluster_podspec_syntax" } + +func (clusterPodspecSyntax) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + wc, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + spec := field.NewPath("spec") + var errs field.ErrorList + + errs = append(errs, validateSimpleTolerations(spec.Child("tolerations"), wc.Spec.Tolerations)...) + errs = append(errs, validateRawTolerations(spec.Child("rawTolerations"), wc.Spec.RawTolerations)...) + errs = append(errs, validateLabelMap(spec.Child("nodeSelector"), wc.Spec.NodeSelector)...) + + // RoleNodeSelector and RoleAnnotations have six roles (compute, drive, s3, + // nfs, smbw, dataServices); RoleAffinity below has only five — there is no + // dataServices field on that struct. + roleNodeSelectors := []struct { + role string + sel *map[string]string + }{ + {"compute", wc.Spec.RoleNodeSelector.Compute}, + {"drive", wc.Spec.RoleNodeSelector.Drive}, + {"s3", wc.Spec.RoleNodeSelector.S3}, + {"nfs", wc.Spec.RoleNodeSelector.Nfs}, + {"smbw", wc.Spec.RoleNodeSelector.Smbw}, + {"dataServices", wc.Spec.RoleNodeSelector.DataServices}, + } + for _, r := range roleNodeSelectors { + if r.sel != nil { + errs = append(errs, validateLabelMap(spec.Child("roleNodeSelector", r.role), *r.sel)...) + } + } + + roleAnnotations := []struct { + role string + ann *map[string]string + }{ + {"compute", wc.Spec.RoleAnnotations.Compute}, + {"drive", wc.Spec.RoleAnnotations.Drive}, + {"s3", wc.Spec.RoleAnnotations.S3}, + {"nfs", wc.Spec.RoleAnnotations.Nfs}, + {"smbw", wc.Spec.RoleAnnotations.Smbw}, + {"dataServices", wc.Spec.RoleAnnotations.DataServices}, + } + for _, r := range roleAnnotations { + if r.ann != nil { + errs = append(errs, validateAnnotationMap(spec.Child("roleAnnotations", r.role), *r.ann)...) + } + } + + if fd := wc.Spec.FailureDomain; fd != nil { + // mirror getDefaultRoleTopologySpreadConstraints precedence: label + // wins over compositeLabels; skew is used only with label + if fd.Label != nil { + if *fd.Label == "" { + errs = append(errs, field.Required(spec.Child("failureDomain", "label"), "failureDomain label may not be empty when set")) + } else { + errs = append(errs, validateTopologyKey(spec.Child("failureDomain", "label"), *fd.Label)...) + } + // skew becomes the generated spread constraint's maxSkew (must be > 0) + if fd.Skew != nil && *fd.Skew <= 0 { + errs = append(errs, field.Invalid(spec.Child("failureDomain", "skew"), *fd.Skew, "must be greater than zero")) + } + } else { + for i, l := range fd.CompositeLabels { + p := spec.Child("failureDomain", "compositeLabels").Index(i) + if l == "" { + errs = append(errs, field.Required(p, "failureDomain compositeLabels entries may not be empty")) + } else { + errs = append(errs, validateTopologyKey(p, l)...) + } + } + } + } + + if pc := wc.Spec.PodConfig; pc != nil { + errs = append(errs, validateRawAffinity(spec.Child("podConfig", "affinity"), pc.Affinity)...) + if ra := pc.RoleAffinity; ra != nil { + roleAffinities := []struct { + role string + raw *runtime.RawExtension + }{ + {"compute", ra.Compute}, + {"drive", ra.Drive}, + {"s3", ra.S3}, + {"nfs", ra.Nfs}, + {"smbw", ra.Smbw}, + } + for _, r := range roleAffinities { + if r.raw != nil { + errs = append(errs, validateRawAffinity(spec.Child("podConfig", "roleAffinity", r.role), r.raw)...) + } + } + } + + errs = append(errs, validateRawTopologySpreadConstraints(spec.Child("podConfig", "topologySpreadConstraints"), pc.TopologySpreadConstraints)...) + if rtsc := pc.RoleTopologySpreadConstraints; rtsc != nil { + // RoleTopologySpreadConstraints has five roles (no dataServices field), + // same set as RoleAffinity above. + roleTopologySpreadConstraints := []struct { + role string + raw *runtime.RawExtension + }{ + {"compute", rtsc.Compute}, + {"drive", rtsc.Drive}, + {"s3", rtsc.S3}, + {"nfs", rtsc.Nfs}, + {"smbw", rtsc.Smbw}, + } + for _, r := range roleTopologySpreadConstraints { + if r.raw != nil { + errs = append(errs, validateRawTopologySpreadConstraints(spec.Child("podConfig", "roleTopologySpreadConstraints", r.role), r.raw)...) + } + } + } + } + return errs +} diff --git a/internal/validation/cluster_podspec_syntax_test.go b/internal/validation/cluster_podspec_syntax_test.go new file mode 100644 index 000000000..a123c41c6 --- /dev/null +++ b/internal/validation/cluster_podspec_syntax_test.go @@ -0,0 +1,273 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestClusterPodspecSyntax(t *testing.T) { + v := clusterPodspecSyntax{} + base := func() *weka.WekaCluster { + return &weka.WekaCluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} + } + + if errs := v.Validate(context.Background(), nil, base()); len(errs) != 0 { + t.Fatalf("empty spec should pass: %v", errs) + } + + badTol := base() + badTol.Spec.Tolerations = []string{"scitix.ai/nodecheck:NoSchedule"} + if errs := v.Validate(context.Background(), nil, badTol); len(errs) != 1 { + t.Fatalf("bad string toleration: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.tolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badRawTol := base() + badRawTol.Spec.RawTolerations = []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Effect: "NoSchedul"}} + if errs := v.Validate(context.Background(), nil, badRawTol); len(errs) != 1 { + t.Fatalf("bad rawToleration effect: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.rawTolerations") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badSel := base() + badSel.Spec.NodeSelector = map[string]string{"bad key!": "x"} + if errs := v.Validate(context.Background(), nil, badSel); len(errs) != 1 { + t.Fatalf("bad nodeSelector: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.nodeSelector") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badRole := base() + m := map[string]string{"k": "bad value!"} + badRole.Spec.RoleNodeSelector.Compute = &m + if errs := v.Validate(context.Background(), nil, badRole); len(errs) != 1 { + t.Fatalf("bad roleNodeSelector.compute: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.roleNodeSelector.compute") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badRoleAnn := base() + ann := map[string]string{"bad key!": "x"} + badRoleAnn.Spec.RoleAnnotations.Compute = &ann + if errs := v.Validate(context.Background(), nil, badRoleAnn); len(errs) != 1 { + t.Fatalf("bad roleAnnotations.compute: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.roleAnnotations.compute") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badFD := base() + label := "bad key!" + badFD.Spec.FailureDomain = &weka.FailureDomain{Label: &label} + if errs := v.Validate(context.Background(), nil, badFD); len(errs) != 1 { + t.Fatalf("bad failureDomain.label: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.failureDomain.label") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badComposite := base() + badComposite.Spec.FailureDomain = &weka.FailureDomain{CompositeLabels: []string{"bad key!"}} + if errs := v.Validate(context.Background(), nil, badComposite); len(errs) != 1 { + t.Fatalf("bad failureDomain.compositeLabels: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.failureDomain.compositeLabels") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + emptyLabel := base() + empty := "" + emptyLabel.Spec.FailureDomain = &weka.FailureDomain{Label: &empty} + if errs := v.Validate(context.Background(), nil, emptyLabel); len(errs) != 1 { + t.Fatalf("empty failureDomain.label: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.failureDomain.label") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + emptyComposite := base() + emptyComposite.Spec.FailureDomain = &weka.FailureDomain{CompositeLabels: []string{""}} + if errs := v.Validate(context.Background(), nil, emptyComposite); len(errs) != 1 { + t.Fatalf("empty failureDomain.compositeLabels entry: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.failureDomain.compositeLabels") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badAffinity := base() + badAffinity.Spec.PodConfig = &weka.PodConfiguration{Affinity: &runtime.RawExtension{Raw: []byte("not json")}} + if errs := v.Validate(context.Background(), nil, badAffinity); len(errs) != 1 { + t.Fatalf("bad podConfig.affinity: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.podConfig.affinity") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badRoleAffinity := base() + badRoleAffinity.Spec.PodConfig = &weka.PodConfiguration{ + RoleAffinity: &weka.RoleAffinity{ + Drive: &runtime.RawExtension{Raw: []byte(`{"nodeAffinity": 42}`)}, + }, + } + if errs := v.Validate(context.Background(), nil, badRoleAffinity); len(errs) != 1 { + t.Fatalf("bad podConfig.roleAffinity.drive: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.podConfig.roleAffinity.drive") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badTopo := base() + badTopo.Spec.PodConfig = &weka.PodConfiguration{TopologySpreadConstraints: &runtime.RawExtension{Raw: []byte(`[{"topologyKey":"kubernetes.io/hostname","maxSkew":0,"whenUnsatisfiable":"DoNotSchedule"}]`)}} + if errs := v.Validate(context.Background(), nil, badTopo); len(errs) != 1 { + t.Fatalf("bad podConfig.topologySpreadConstraints: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.podConfig.topologySpreadConstraints[0].maxSkew") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + badRoleTopo := base() + badRoleTopo.Spec.PodConfig = &weka.PodConfiguration{ + RoleTopologySpreadConstraints: &weka.RoleTopologySpreadConstraints{ + Compute: &runtime.RawExtension{Raw: []byte(`[{"topologyKey":"","maxSkew":1,"whenUnsatisfiable":"DoNotSchedule"}]`)}, + }, + } + if errs := v.Validate(context.Background(), nil, badRoleTopo); len(errs) != 1 { + t.Fatalf("bad roleTopologySpreadConstraints.compute: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.podConfig.roleTopologySpreadConstraints.compute[0].topologyKey") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + zone := "topology.kubernetes.io/zone" + zero := 0 + one := 1 + + badSkew := base() + badSkew.Spec.FailureDomain = &weka.FailureDomain{Label: &zone, Skew: &zero} + if errs := v.Validate(context.Background(), nil, badSkew); len(errs) != 1 { + t.Fatalf("failureDomain.skew=0 with label: got %v", errs) + } else if !strings.HasPrefix(errs[0].Field, "spec.failureDomain.skew") { + t.Fatalf("wrong field path: %s", errs[0].Field) + } + + goodSkew := base() + goodSkew.Spec.FailureDomain = &weka.FailureDomain{Label: &zone, Skew: &one} + if errs := v.Validate(context.Background(), nil, goodSkew); len(errs) != 0 { + t.Fatalf("failureDomain.skew=1 with label rejected: %v", errs) + } + + // dead fields are skipped, mirroring factory precedence: + // skew without label is unused; compositeLabels lose to label + deadSkew := base() + deadSkew.Spec.FailureDomain = &weka.FailureDomain{Skew: &zero} + if errs := v.Validate(context.Background(), nil, deadSkew); len(errs) != 0 { + t.Fatalf("unused skew=0 (no label) rejected: %v", errs) + } + deadComposite := base() + deadComposite.Spec.FailureDomain = &weka.FailureDomain{Label: &zone, CompositeLabels: []string{"bad key!"}} + if errs := v.Validate(context.Background(), nil, deadComposite); len(errs) != 0 { + t.Fatalf("unused compositeLabels (label set) rejected: %v", errs) + } +} + +// One case per role field, catching swapped wiring in the role tables. +func TestClusterPodspecSyntaxRoleWiring(t *testing.T) { + v := clusterPodspecSyntax{} + badMap := map[string]string{"bad key!": "x"} + badRaw := &runtime.RawExtension{Raw: []byte("not json")} + + sixRoles := []string{"compute", "drive", "s3", "nfs", "smbw", "dataServices"} + fiveRoles := []string{"compute", "drive", "s3", "nfs", "smbw"} + + set := func(c *weka.WekaCluster, family, role string) { + switch family { + case "roleNodeSelector": + m := badMap + switch role { + case "compute": + c.Spec.RoleNodeSelector.Compute = &m + case "drive": + c.Spec.RoleNodeSelector.Drive = &m + case "s3": + c.Spec.RoleNodeSelector.S3 = &m + case "nfs": + c.Spec.RoleNodeSelector.Nfs = &m + case "smbw": + c.Spec.RoleNodeSelector.Smbw = &m + case "dataServices": + c.Spec.RoleNodeSelector.DataServices = &m + } + case "roleAnnotations": + m := badMap + switch role { + case "compute": + c.Spec.RoleAnnotations.Compute = &m + case "drive": + c.Spec.RoleAnnotations.Drive = &m + case "s3": + c.Spec.RoleAnnotations.S3 = &m + case "nfs": + c.Spec.RoleAnnotations.Nfs = &m + case "smbw": + c.Spec.RoleAnnotations.Smbw = &m + case "dataServices": + c.Spec.RoleAnnotations.DataServices = &m + } + case "podConfig.roleAffinity": + ra := &weka.RoleAffinity{} + switch role { + case "compute": + ra.Compute = badRaw + case "drive": + ra.Drive = badRaw + case "s3": + ra.S3 = badRaw + case "nfs": + ra.Nfs = badRaw + case "smbw": + ra.Smbw = badRaw + } + c.Spec.PodConfig = &weka.PodConfiguration{RoleAffinity: ra} + case "podConfig.roleTopologySpreadConstraints": + rt := &weka.RoleTopologySpreadConstraints{} + switch role { + case "compute": + rt.Compute = badRaw + case "drive": + rt.Drive = badRaw + case "s3": + rt.S3 = badRaw + case "nfs": + rt.Nfs = badRaw + case "smbw": + rt.Smbw = badRaw + } + c.Spec.PodConfig = &weka.PodConfiguration{RoleTopologySpreadConstraints: rt} + } + } + + cases := []struct { + family string + roles []string + }{ + {"roleNodeSelector", sixRoles}, + {"roleAnnotations", sixRoles}, + {"podConfig.roleAffinity", fiveRoles}, + {"podConfig.roleTopologySpreadConstraints", fiveRoles}, + } + for _, tc := range cases { + for _, role := range tc.roles { + t.Run(tc.family+"/"+role, func(t *testing.T) { + c := &weka.WekaCluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} + set(c, tc.family, role) + errs := v.Validate(context.Background(), nil, c) + if len(errs) != 1 { + t.Fatalf("got %d errors (%v), want 1", len(errs), errs) + } + want := "spec." + tc.family + "." + role + if !strings.HasPrefix(errs[0].Field, want) { + t.Fatalf("wrong field path: %s (want prefix %s)", errs[0].Field, want) + } + }) + } + } +} diff --git a/internal/validation/podspec_syntax.go b/internal/validation/podspec_syntax.go new file mode 100644 index 000000000..c10de8931 --- /dev/null +++ b/internal/validation/podspec_syntax.go @@ -0,0 +1,271 @@ +package validation + +// Shared syntax checks for CR fields that are copied verbatim into pod / +// deployment specs. The API server applies these rules only when the Pod is +// created (Gate 2); running them here surfaces the rejection at CR apply. +// Pure spec math — no cluster reads. + +import ( + "encoding/json" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + apivalidation "k8s.io/apimachinery/pkg/api/validation" + metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/runtime" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// validateSimpleTolerations checks the []string convenience form: each entry +// must be a bare taint key. ExpandTolerations puts the whole string into +// Toleration.Key, so anything that is not a qualified name produces a pod the +// API server rejects. +func validateSimpleTolerations(fldPath *field.Path, tolerations []string) field.ErrorList { + var errs field.ErrorList + for i, s := range tolerations { + // "" expands to a valid tolerate-all toleration + if s == "" { + continue + } + if msgs := utilvalidation.IsQualifiedName(s); len(msgs) > 0 { + detail := fmt.Sprintf("must be a bare taint key (e.g. %q): %s", "weka.io/dedicated", strings.Join(msgs, "; ")) + if strings.ContainsAny(s, ":=") { + detail += ". This field takes only the taint key; to specify an effect or value, use rawTolerations instead" + } + errs = append(errs, field.Invalid(fldPath.Index(i), s, detail)) + } + } + return errs +} + +var ( + validTolerationOperators = map[corev1.TolerationOperator]bool{ + "": true, corev1.TolerationOpEqual: true, corev1.TolerationOpExists: true, + } + validTaintEffects = map[corev1.TaintEffect]bool{ + "": true, corev1.TaintEffectNoSchedule: true, corev1.TaintEffectPreferNoSchedule: true, corev1.TaintEffectNoExecute: true, + } +) + +// validateRawTolerations mirrors the syntax rules the API server applies to +// pod .spec.tolerations (upstream pkg/apis/core/validation is not importable). +func validateRawTolerations(fldPath *field.Path, tolerations []corev1.Toleration) field.ErrorList { + var errs field.ErrorList + for i, t := range tolerations { + p := fldPath.Index(i) + if t.Key != "" { + for _, msg := range utilvalidation.IsQualifiedName(t.Key) { + errs = append(errs, field.Invalid(p.Child("key"), t.Key, msg)) + } + } else if t.Operator != corev1.TolerationOpExists { + errs = append(errs, field.Invalid(p.Child("operator"), t.Operator, "operator must be Exists when key is empty")) + } + if !validTolerationOperators[t.Operator] { + errs = append(errs, field.NotSupported(p.Child("operator"), t.Operator, []string{string(corev1.TolerationOpEqual), string(corev1.TolerationOpExists)})) + } + if t.Operator == corev1.TolerationOpExists && t.Value != "" { + errs = append(errs, field.Invalid(p.Child("value"), t.Value, "value must be empty when operator is Exists")) + } + // upstream checks value syntax only under Equal + if t.Operator == "" || t.Operator == corev1.TolerationOpEqual { + for _, msg := range utilvalidation.IsValidLabelValue(t.Value) { + errs = append(errs, field.Invalid(p.Child("value"), t.Value, msg)) + } + } + if !validTaintEffects[t.Effect] { + errs = append(errs, field.NotSupported(p.Child("effect"), t.Effect, []string{string(corev1.TaintEffectNoSchedule), string(corev1.TaintEffectPreferNoSchedule), string(corev1.TaintEffectNoExecute)})) + } + if t.TolerationSeconds != nil && t.Effect != corev1.TaintEffectNoExecute { + errs = append(errs, field.Invalid(p.Child("effect"), t.Effect, "effect must be NoExecute when tolerationSeconds is set")) + } + } + return errs +} + +// validateLabelMap checks label-syntax maps (nodeSelector, csi labels) via +// the same function the API server uses. +func validateLabelMap(fldPath *field.Path, m map[string]string) field.ErrorList { + return metav1validation.ValidateLabels(m, fldPath) +} + +// validateAnnotationMap checks annotation keys + total size (values are free-form). +func validateAnnotationMap(fldPath *field.Path, m map[string]string) field.ErrorList { + return apivalidation.ValidateAnnotations(m, fldPath) +} + +// validateTopologyKey checks a node-label key used as a topology key +// (failureDomain labels, pod-affinity topologyKey). Empty is allowed — +// presence rules stay with the caller. +func validateTopologyKey(fldPath *field.Path, key string) field.ErrorList { + if key == "" { + return nil + } + var errs field.ErrorList + for _, msg := range utilvalidation.IsQualifiedName(key) { + errs = append(errs, field.Invalid(fldPath, key, msg)) + } + return errs +} + +// validateAffinity mirrors the syntax rules the API server applies to pod +// .spec.affinity (upstream pkg/apis/core/validation is not importable). +// Not checked (rare, still caught at pod create): namespaces name syntax, +// matchLabelKeys/mismatchLabelKeys. +func validateAffinity(fldPath *field.Path, aff *corev1.Affinity) field.ErrorList { + if aff == nil { + return nil + } + var errs field.ErrorList + + checkTerm := func(p *field.Path, term corev1.NodeSelectorTerm, required bool) { + for j, expr := range term.MatchExpressions { + ep := p.Child("matchExpressions").Index(j) + for _, msg := range utilvalidation.IsQualifiedName(expr.Key) { + errs = append(errs, field.Invalid(ep.Child("key"), expr.Key, msg)) + } + switch expr.Operator { + case corev1.NodeSelectorOpIn, corev1.NodeSelectorOpNotIn: + if len(expr.Values) == 0 { + errs = append(errs, field.Required(ep.Child("values"), "must be specified when operator is In or NotIn")) + } + case corev1.NodeSelectorOpExists, corev1.NodeSelectorOpDoesNotExist: + if len(expr.Values) > 0 { + errs = append(errs, field.Forbidden(ep.Child("values"), "may not be specified when operator is Exists or DoesNotExist")) + } + case corev1.NodeSelectorOpGt, corev1.NodeSelectorOpLt: + if len(expr.Values) != 1 { + errs = append(errs, field.Required(ep.Child("values"), "must be a single value when operator is Gt or Lt")) + } + default: + errs = append(errs, field.NotSupported(ep.Child("operator"), expr.Operator, []string{"In", "NotIn", "Exists", "DoesNotExist", "Gt", "Lt"})) + } + // upstream checks value syntax in required terms only + if required { + for vi, v := range expr.Values { + for _, msg := range utilvalidation.IsValidLabelValue(v) { + errs = append(errs, field.Invalid(ep.Child("values").Index(vi), v, msg)) + } + } + } + } + for j, f := range term.MatchFields { + fp := p.Child("matchFields").Index(j) + if f.Operator != corev1.NodeSelectorOpIn && f.Operator != corev1.NodeSelectorOpNotIn { + errs = append(errs, field.Invalid(fp.Child("operator"), f.Operator, "not a valid selector operator")) + } else if len(f.Values) != 1 { + errs = append(errs, field.Required(fp.Child("values"), "must be only one value when operator is In or NotIn for node field selector")) + } + if f.Key != "metadata.name" { + errs = append(errs, field.Invalid(fp.Child("key"), f.Key, "not a valid field selector key")) + } else { + for vi, v := range f.Values { + for _, msg := range utilvalidation.IsDNS1123Subdomain(v) { + errs = append(errs, field.Invalid(fp.Child("values").Index(vi), v, msg)) + } + } + } + } + } + if na := aff.NodeAffinity; na != nil { + if req := na.RequiredDuringSchedulingIgnoredDuringExecution; req != nil { + p := fldPath.Child("nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms") + if len(req.NodeSelectorTerms) == 0 { + errs = append(errs, field.Required(p, "must have at least one node selector term")) + } + for i, term := range req.NodeSelectorTerms { + checkTerm(p.Index(i), term, true) + } + } + for i, pref := range na.PreferredDuringSchedulingIgnoredDuringExecution { + pp := fldPath.Child("nodeAffinity", "preferredDuringSchedulingIgnoredDuringExecution").Index(i) + if pref.Weight <= 0 || pref.Weight > 100 { + errs = append(errs, field.Invalid(pp.Child("weight"), pref.Weight, "must be in the range 1-100")) + } + checkTerm(pp.Child("preference"), pref.Preference, false) + } + } + + checkPodTerm := func(tp *field.Path, term corev1.PodAffinityTerm) { + errs = append(errs, metav1validation.ValidateLabelSelector(term.LabelSelector, metav1validation.LabelSelectorValidationOptions{}, tp.Child("labelSelector"))...) + errs = append(errs, metav1validation.ValidateLabelSelector(term.NamespaceSelector, metav1validation.LabelSelectorValidationOptions{}, tp.Child("namespaceSelector"))...) + if term.TopologyKey == "" { + errs = append(errs, field.Required(tp.Child("topologyKey"), "pod affinity terms require a topologyKey")) + } else { + errs = append(errs, validateTopologyKey(tp.Child("topologyKey"), term.TopologyKey)...) + } + } + checkPodAffinityLists := func(base *field.Path, required []corev1.PodAffinityTerm, preferred []corev1.WeightedPodAffinityTerm) { + for i, term := range required { + checkPodTerm(base.Child("requiredDuringSchedulingIgnoredDuringExecution").Index(i), term) + } + for i, w := range preferred { + wp := base.Child("preferredDuringSchedulingIgnoredDuringExecution").Index(i) + if w.Weight <= 0 || w.Weight > 100 { + errs = append(errs, field.Invalid(wp.Child("weight"), w.Weight, "must be in the range 1-100")) + } + checkPodTerm(wp.Child("podAffinityTerm"), w.PodAffinityTerm) + } + } + if pa := aff.PodAffinity; pa != nil { + checkPodAffinityLists(fldPath.Child("podAffinity"), pa.RequiredDuringSchedulingIgnoredDuringExecution, pa.PreferredDuringSchedulingIgnoredDuringExecution) + } + if paa := aff.PodAntiAffinity; paa != nil { + checkPodAffinityLists(fldPath.Child("podAntiAffinity"), paa.RequiredDuringSchedulingIgnoredDuringExecution, paa.PreferredDuringSchedulingIgnoredDuringExecution) + } + return errs +} + +// validateRawAffinity checks a RawExtension that the reconciler will +// unmarshal into v1.Affinity (WekaCluster podConfig.affinity/roleAffinity): +// it must unmarshal cleanly, then passes validateAffinity. +func validateRawAffinity(fldPath *field.Path, raw *runtime.RawExtension) field.ErrorList { + if raw == nil || len(raw.Raw) == 0 { + return nil + } + var aff corev1.Affinity + if err := json.Unmarshal(raw.Raw, &aff); err != nil { + return field.ErrorList{field.Invalid(fldPath, string(raw.Raw), fmt.Sprintf("does not unmarshal into a v1.Affinity: %v", err))} + } + return validateAffinity(fldPath, &aff) +} + +// validateTopologySpreadConstraints mirrors the syntax rules the API server +// applies to pod .spec.topologySpreadConstraints. topologyKey only needs to +// be non-empty (qualified-name would be stricter than upstream). Not +// checked (rare, still caught at pod create): minDomains, duplicate pairs, +// node inclusion policies, matchLabelKeys. +func validateTopologySpreadConstraints(fldPath *field.Path, constraints []corev1.TopologySpreadConstraint) field.ErrorList { + var errs field.ErrorList + for i, c := range constraints { + p := fldPath.Index(i) + if c.MaxSkew <= 0 { + errs = append(errs, field.Invalid(p.Child("maxSkew"), c.MaxSkew, "must be greater than zero")) + } + if c.TopologyKey == "" { + errs = append(errs, field.Required(p.Child("topologyKey"), "topologySpreadConstraints entries require a topologyKey")) + } + if c.WhenUnsatisfiable != corev1.DoNotSchedule && c.WhenUnsatisfiable != corev1.ScheduleAnyway { + errs = append(errs, field.NotSupported(p.Child("whenUnsatisfiable"), c.WhenUnsatisfiable, []string{string(corev1.DoNotSchedule), string(corev1.ScheduleAnyway)})) + } + errs = append(errs, metav1validation.ValidateLabelSelector(c.LabelSelector, metav1validation.LabelSelectorValidationOptions{}, p.Child("labelSelector"))...) + } + return errs +} + +// validateRawTopologySpreadConstraints checks a RawExtension that the +// reconciler will unmarshal into []v1.TopologySpreadConstraint (WekaCluster +// podConfig.topologySpreadConstraints/roleTopologySpreadConstraints): it must +// unmarshal cleanly, mirroring unmarshalTopologySpreadConstraints in +// wekacluster_types.go, then passes validateTopologySpreadConstraints. +func validateRawTopologySpreadConstraints(fldPath *field.Path, raw *runtime.RawExtension) field.ErrorList { + if raw == nil || len(raw.Raw) == 0 { + return nil + } + var constraints []corev1.TopologySpreadConstraint + if err := json.Unmarshal(raw.Raw, &constraints); err != nil { + return field.ErrorList{field.Invalid(fldPath, string(raw.Raw), fmt.Sprintf("does not unmarshal into []v1.TopologySpreadConstraint: %v", err))} + } + return validateTopologySpreadConstraints(fldPath, constraints) +} diff --git a/internal/validation/podspec_syntax_test.go b/internal/validation/podspec_syntax_test.go new file mode 100644 index 000000000..ab7219797 --- /dev/null +++ b/internal/validation/podspec_syntax_test.go @@ -0,0 +1,335 @@ +package validation + +import ( + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateSimpleTolerations(t *testing.T) { + fld := field.NewPath("spec", "tolerations") + cases := []struct { + name string + in []string + wantErrs int + wantHint bool // error detail mentions rawTolerations + }{ + {"empty", nil, 0, false}, + {"empty-string entry expands to valid tolerate-all", []string{""}, 0, false}, + {"bare key", []string{"gpu"}, 0, false}, + {"prefixed key", []string{"scitix.ai/nodecheck"}, 0, false}, + {"key with colon (the OP-361 incident)", []string{"scitix.ai/nodecheck:NoSchedule"}, 1, true}, + {"key with equals", []string{"gpu=true"}, 1, true}, + {"plain invalid chars", []string{"bad key!"}, 1, false}, + {"one good one bad", []string{"gpu", "a:b"}, 1, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + errs := validateSimpleTolerations(fld, tc.in) + if len(errs) != tc.wantErrs { + t.Fatalf("got %d errors (%v), want %d", len(errs), errs, tc.wantErrs) + } + if tc.wantHint && !strings.Contains(errs[0].Detail, "rawTolerations") { + t.Fatalf("expected rawTolerations hint in %q", errs[0].Detail) + } + }) + } +} + +func TestValidateRawTolerations(t *testing.T) { + fld := field.NewPath("spec", "rawTolerations") + sec := int64(30) + cases := []struct { + name string + in []corev1.Toleration + wantErrs int + }{ + {"empty", nil, 0}, + {"full valid", []corev1.Toleration{{Key: "scitix.ai/nodecheck", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule}}, 0}, + {"empty key with Exists (tolerate everything)", []corev1.Toleration{{Operator: corev1.TolerationOpExists}}, 0}, + {"empty operator means Equal", []corev1.Toleration{{Key: "k", Value: "v"}}, 0}, + {"bad key", []corev1.Toleration{{Key: "a:b", Operator: corev1.TolerationOpExists}}, 1}, + {"bad effect enum", []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Effect: "NoSchedul"}}, 1}, + {"bad operator enum", []corev1.Toleration{{Key: "k", Operator: "Sometimes"}}, 1}, + {"Exists with value", []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Value: "v"}}, 1}, + {"Exists with invalid value reports only must-be-empty", []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Value: "bad value!"}}, 1}, + {"bad value syntax", []corev1.Toleration{{Key: "k", Value: "bad value!"}}, 1}, + {"tolerationSeconds without NoExecute", []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule, TolerationSeconds: &sec}}, 1}, + {"tolerationSeconds with NoExecute ok", []corev1.Toleration{{Key: "k", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoExecute, TolerationSeconds: &sec}}, 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if errs := validateRawTolerations(fld, tc.in); len(errs) != tc.wantErrs { + t.Fatalf("got %d errors (%v), want %d", len(errs), errs, tc.wantErrs) + } + }) + } +} + +func TestValidateLabelMap(t *testing.T) { + fld := field.NewPath("spec", "nodeSelector") + cases := []struct { + name string + in map[string]string + wantErrs int + }{ + {"nil", nil, 0}, + {"valid", map[string]string{"weka.io/supports-backends": "true"}, 0}, + {"bad key", map[string]string{"bad key!": "x"}, 1}, + {"bad value", map[string]string{"k": "no spaces allowed"}, 1}, + {"value too long", map[string]string{"k": strings.Repeat("a", 64)}, 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if errs := validateLabelMap(fld, tc.in); len(errs) != tc.wantErrs { + t.Fatalf("got %d errors (%v), want %d", len(errs), errs, tc.wantErrs) + } + }) + } +} + +func TestValidateAnnotationMap(t *testing.T) { + fld := field.NewPath("spec", "roleAnnotations", "compute") + if errs := validateAnnotationMap(fld, map[string]string{"example.com/scrape": "true"}); len(errs) != 0 { + t.Fatalf("valid annotations rejected: %v", errs) + } + if errs := validateAnnotationMap(fld, map[string]string{"bad key!": "x"}); len(errs) != 1 { + t.Fatalf("bad annotation key not rejected") + } +} + +func TestValidateTopologyKey(t *testing.T) { + fld := field.NewPath("spec", "failureDomain", "label") + if errs := validateTopologyKey(fld, "topology.kubernetes.io/zone"); len(errs) != 0 { + t.Fatalf("valid topologyKey rejected: %v", errs) + } + if errs := validateTopologyKey(fld, "bad key!"); len(errs) != 1 { + t.Fatalf("bad topologyKey not rejected") + } + if errs := validateTopologyKey(fld, ""); len(errs) != 0 { + t.Fatalf("empty topologyKey should be skipped (field optional): %v", errs) + } +} + +func TestValidateAffinity(t *testing.T) { + fld := field.NewPath("spec", "affinity") + good := &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{ + MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "weka.io/mode", Operator: corev1.NodeSelectorOpIn, Values: []string{"backend"}}}, + }}, + }, + }} + if errs := validateAffinity(fld, good); len(errs) != 0 { + t.Fatalf("valid affinity rejected: %v", errs) + } + badKey := &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{ + MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "bad key!", Operator: corev1.NodeSelectorOpExists}}, + }}, + }, + }} + if errs := validateAffinity(fld, badKey); len(errs) == 0 { + t.Fatalf("bad match-expression key not rejected") + } + badOp := &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{ + MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "k", Operator: "Maybe"}}, + }}, + }, + }} + if errs := validateAffinity(fld, badOp); len(errs) == 0 { + t.Fatalf("bad node-selector operator not rejected") + } + if errs := validateAffinity(fld, nil); len(errs) != 0 { + t.Fatalf("nil affinity should pass: %v", errs) + } + + podAffinityMissingTopologyKey := &corev1.Affinity{PodAffinity: &corev1.PodAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{ + LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "weka"}}, + }}, + }} + if errs := validateAffinity(fld, podAffinityMissingTopologyKey); len(errs) != 1 { + t.Fatalf("got %d errors (%v), want 1 for missing topologyKey", len(errs), errs) + } + + podAntiAffinityBadSelector := &corev1.Affinity{PodAntiAffinity: &corev1.PodAntiAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{ + LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"bad key!": "x"}}, + TopologyKey: "kubernetes.io/hostname", + }}, + }} + if errs := validateAffinity(fld, podAntiAffinityBadSelector); len(errs) == 0 { + t.Fatalf("bad podAntiAffinity labelSelector not rejected") + } + + podAffinityValid := &corev1.Affinity{PodAffinity: &corev1.PodAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{ + LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "weka"}}, + TopologyKey: "kubernetes.io/hostname", + }}, + }} + if errs := validateAffinity(fld, podAffinityValid); len(errs) != 0 { + t.Fatalf("valid podAffinity term rejected: %v", errs) + } + + podAffinityPreferredMissingTopologyKey := &corev1.Affinity{PodAffinity: &corev1.PodAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{{ + Weight: 50, + PodAffinityTerm: corev1.PodAffinityTerm{ + LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "weka"}}, + }, + }}, + }} + if errs := validateAffinity(fld, podAffinityPreferredMissingTopologyKey); len(errs) != 1 { + t.Fatalf("got %d errors (%v), want 1 for preferred term missing topologyKey", len(errs), errs) + } else if want := "spec.affinity.podAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey"; errs[0].Field != want { + t.Fatalf("wrong field path: %s (want %s)", errs[0].Field, want) + } + + // upstream value-count rules per operator + requiredWithExpr := func(expr corev1.NodeSelectorRequirement) *corev1.Affinity { + return &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{MatchExpressions: []corev1.NodeSelectorRequirement{expr}}}, + }, + }} + } + valueCountCases := []struct { + name string + expr corev1.NodeSelectorRequirement + }{ + {"In without values", corev1.NodeSelectorRequirement{Key: "k", Operator: corev1.NodeSelectorOpIn}}, + {"Exists with values", corev1.NodeSelectorRequirement{Key: "k", Operator: corev1.NodeSelectorOpExists, Values: []string{"v"}}}, + {"Gt with two values", corev1.NodeSelectorRequirement{Key: "k", Operator: corev1.NodeSelectorOpGt, Values: []string{"1", "2"}}}, + {"required In with invalid label value", corev1.NodeSelectorRequirement{Key: "k", Operator: corev1.NodeSelectorOpIn, Values: []string{"bad value!"}}}, + } + for _, tc := range valueCountCases { + if errs := validateAffinity(fld, requiredWithExpr(tc.expr)); len(errs) != 1 { + t.Fatalf("%s: got %d errors (%v), want 1", tc.name, len(errs), errs) + } + } + + emptyRequiredTerms := &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{}, + }} + if errs := validateAffinity(fld, emptyRequiredTerms); len(errs) != 1 { + t.Fatalf("empty required nodeSelectorTerms: got %v, want 1 error", errs) + } + + // preferred terms: weight range enforced, invalid label values allowed + badWeight := &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{{ + Weight: 0, + Preference: corev1.NodeSelectorTerm{ + MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "k", Operator: corev1.NodeSelectorOpIn, Values: []string{"bad value!"}}}, + }, + }}, + }} + if errs := validateAffinity(fld, badWeight); len(errs) != 1 { + t.Fatalf("preferred term weight 0: got %v, want exactly 1 error (invalid values allowed in preferred)", errs) + } else if want := "spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].weight"; errs[0].Field != want { + t.Fatalf("wrong field path: %s (want %s)", errs[0].Field, want) + } + + badMatchFields := requiredWithExpr(corev1.NodeSelectorRequirement{Key: "k", Operator: corev1.NodeSelectorOpExists}) + badMatchFields.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchFields = []corev1.NodeSelectorRequirement{ + {Key: "metadata.namespace", Operator: corev1.NodeSelectorOpIn, Values: []string{"x"}}, + } + if errs := validateAffinity(fld, badMatchFields); len(errs) != 1 { + t.Fatalf("matchFields with bad key: got %v, want 1 error", errs) + } +} + +func TestValidateRawAffinity(t *testing.T) { + fld := field.NewPath("spec", "podConfig", "affinity") + if errs := validateRawAffinity(fld, &runtime.RawExtension{Raw: []byte(`{"nodeAffinity":{}}`)}); len(errs) != 0 { + t.Fatalf("valid raw affinity rejected: %v", errs) + } + if errs := validateRawAffinity(fld, &runtime.RawExtension{Raw: []byte(`{"nodeAffinity": 42}`)}); len(errs) != 1 { + t.Fatalf("non-affinity JSON not rejected") + } + if errs := validateRawAffinity(fld, nil); len(errs) != 0 { + t.Fatalf("nil raw should pass: %v", errs) + } +} + +func TestValidateTopologySpreadConstraints(t *testing.T) { + fld := field.NewPath("spec", "topologySpreadConstraints") + // complete valid constraint; each case below breaks exactly one field + base := func() corev1.TopologySpreadConstraint { + return corev1.TopologySpreadConstraint{ + MaxSkew: 1, + TopologyKey: "kubernetes.io/hostname", + WhenUnsatisfiable: corev1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "weka"}}, + } + } + if errs := validateTopologySpreadConstraints(fld, nil); len(errs) != 0 { + t.Fatalf("nil constraints should pass: %v", errs) + } + if errs := validateTopologySpreadConstraints(fld, []corev1.TopologySpreadConstraint{base()}); len(errs) != 0 { + t.Fatalf("valid constraint rejected: %v", errs) + } + + nonQualifiedKey := base() + nonQualifiedKey.TopologyKey = "MY_ZONE KEY" + if errs := validateTopologySpreadConstraints(fld, []corev1.TopologySpreadConstraint{nonQualifiedKey}); len(errs) != 0 { + t.Fatalf("non-qualified topologyKey should pass (upstream requires only non-empty): %v", errs) + } + + cases := []struct { + name string + mutate func(*corev1.TopologySpreadConstraint) + wantField string + }{ + {"empty topologyKey", func(c *corev1.TopologySpreadConstraint) { c.TopologyKey = "" }, "spec.topologySpreadConstraints[0].topologyKey"}, + {"zero maxSkew", func(c *corev1.TopologySpreadConstraint) { c.MaxSkew = 0 }, "spec.topologySpreadConstraints[0].maxSkew"}, + {"negative maxSkew", func(c *corev1.TopologySpreadConstraint) { c.MaxSkew = -1 }, "spec.topologySpreadConstraints[0].maxSkew"}, + {"empty whenUnsatisfiable", func(c *corev1.TopologySpreadConstraint) { c.WhenUnsatisfiable = "" }, "spec.topologySpreadConstraints[0].whenUnsatisfiable"}, + {"bad whenUnsatisfiable enum", func(c *corev1.TopologySpreadConstraint) { c.WhenUnsatisfiable = "Sometimes" }, "spec.topologySpreadConstraints[0].whenUnsatisfiable"}, + {"bad labelSelector", func(c *corev1.TopologySpreadConstraint) { + c.LabelSelector = &metav1.LabelSelector{MatchLabels: map[string]string{"bad key!": "x"}} + }, "spec.topologySpreadConstraints[0].labelSelector"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := base() + tc.mutate(&c) + errs := validateTopologySpreadConstraints(fld, []corev1.TopologySpreadConstraint{c}) + if len(errs) != 1 { + t.Fatalf("got %d errors (%v), want 1", len(errs), errs) + } + if !strings.HasPrefix(errs[0].Field, tc.wantField) { + t.Fatalf("wrong field path: %s (want prefix %s)", errs[0].Field, tc.wantField) + } + }) + } +} + +func TestValidateRawTopologySpreadConstraints(t *testing.T) { + fld := field.NewPath("spec", "podConfig", "topologySpreadConstraints") + if errs := validateRawTopologySpreadConstraints(fld, nil); len(errs) != 0 { + t.Fatalf("nil raw should pass: %v", errs) + } + if errs := validateRawTopologySpreadConstraints(fld, &runtime.RawExtension{Raw: []byte(`[{"topologyKey":"kubernetes.io/hostname","maxSkew":1,"whenUnsatisfiable":"DoNotSchedule"}]`)}); len(errs) != 0 { + t.Fatalf("valid raw constraints rejected: %v", errs) + } + if errs := validateRawTopologySpreadConstraints(fld, &runtime.RawExtension{Raw: []byte(`not json`)}); len(errs) != 1 { + t.Fatalf("garbage JSON not rejected: %v", errs) + } + if errs := validateRawTopologySpreadConstraints(fld, &runtime.RawExtension{Raw: []byte(`[{"topologyKey":"","maxSkew":1,"whenUnsatisfiable":"DoNotSchedule"}]`)}); len(errs) != 1 { + t.Fatalf("empty topologyKey via raw not rejected: %v", errs) + } + // missing maxSkew/whenUnsatisfiable unmarshal to zero values + if errs := validateRawTopologySpreadConstraints(fld, &runtime.RawExtension{Raw: []byte(`[{"topologyKey":"kubernetes.io/hostname"}]`)}); len(errs) != 2 { + t.Fatalf("missing maxSkew/whenUnsatisfiable via raw not rejected: %v", errs) + } +} diff --git a/internal/validation/registry.go b/internal/validation/registry.go index cad361d9c..6d6326195 100644 --- a/internal/validation/registry.go +++ b/internal/validation/registry.go @@ -19,9 +19,11 @@ var ( &clusterCapacityProtection{}, &clusterCapacityChunkFeasibility{}, &clusterSkipDefaultFs{}, + &clusterPodspecSyntax{}, } WekaClient = []Validator{ &clientTargetClusterExists{}, + &clientPodspecSyntax{}, } // Update-only registries: validators that require both old and new objects. diff --git a/package-lock.json b/package-lock.json index 4bdb985ff..f0f64e237 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56,9 +56,9 @@ } }, "node_modules/@actions/http-client/node_modules/undici": { - "version": "6.23.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-6.23.0.tgz", - "integrity": "sha512-VfQPToRA5FZs/qJxLIinmU59u0r7LXqoJkCzinq3ckNJp3vKEh7jTWN589YQ5+aoAC/TGRLyJLCPKcLQbM8r9g==", + "version": "6.28.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.28.0.tgz", + "integrity": "sha512-LIY910g9TI13YS95lrMFrs8Rm/u/irgHeTWoKCoteeJ04CUJ92eEfj0rVn+7VKMPBpUPiUoBKfhNyLI23EE/KA==", "dev": true, "license": "MIT", "engines": { @@ -6813,9 +6813,9 @@ } }, "node_modules/undici": { - "version": "7.19.2", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.19.2.tgz", - "integrity": "sha512-4VQSpGEGsWzk0VYxyB/wVX/Q7qf9t5znLRgs0dzszr9w9Fej/8RVNQ+S20vdXSAyra/bJ7ZQfGv6ZMj7UEbzSg==", + "version": "7.29.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.29.0.tgz", + "integrity": "sha512-IDxfleLmmbSskfWSUATiN1nfn2rDuvnMOqb5CWR92iIfojA0Ud+ulOAAEQ57LPr9rWmsreUyf5lwyao+7GNNVw==", "dev": true, "license": "MIT", "engines": {