From 34c279644a27e587139fb58dd40ac158e3ce188a Mon Sep 17 00:00:00 2001 From: LoginovIlia Date: Wed, 15 Apr 2026 14:42:00 -0400 Subject: [PATCH] validations: add exclusiveMaximum property validation --- docs/validations.md | 37 ++++ pkg/runner/registry.go | 1 + pkg/validations/property/exclusive_maximum.go | 162 +++++++++++++++++ .../property/exclusive_maximum_test.go | 167 ++++++++++++++++++ pkg/validations/property/nullable.go | 1 + test/exclusivemaximumactivated/a.yaml | 27 +++ test/exclusivemaximumactivated/b.yaml | 28 +++ test/exclusivemaximumactivated/expected.json | 20 +++ test/exclusivemaximumdeactivated/a.yaml | 28 +++ test/exclusivemaximumdeactivated/b.yaml | 27 +++ .../exclusivemaximumdeactivated/expected.json | 20 +++ 11 files changed, 518 insertions(+) create mode 100644 pkg/validations/property/exclusive_maximum.go create mode 100644 pkg/validations/property/exclusive_maximum_test.go create mode 100644 test/exclusivemaximumactivated/a.yaml create mode 100644 test/exclusivemaximumactivated/b.yaml create mode 100644 test/exclusivemaximumactivated/expected.json create mode 100644 test/exclusivemaximumdeactivated/a.yaml create mode 100644 test/exclusivemaximumdeactivated/b.yaml create mode 100644 test/exclusivemaximumdeactivated/expected.json diff --git a/docs/validations.md b/docs/validations.md index 8f9f731..63ee829 100644 --- a/docs/validations.md +++ b/docs/validations.md @@ -218,3 +218,40 @@ validations: configuration: removalPolicy: Allow ``` + +### exclusiveMaximum + +Validates compatibility of changes to the `exclusiveMaximum` constraint on a property. +When `exclusiveMaximum` is activated, the maximum value itself becomes an exclusive upper bound, which +tightens the allowed range for writers and is considered a breaking change. +When `exclusiveMaximum` is removed, the maximum value becomes an inclusive upper bound, which loosens the +allowed range for writers, but can break readers that were built against the previously stricter constraint. +Because whether either direction is actually breaking depends on whether you care about writer or reader +semantics, both directions are flagged by default and can be independently configured. + +#### Configuration + +The `exclusiveMaximum` validation can be configured to allow adding and/or removing the constraint when you know the change is safe: + +- `additionPolicy` - controls whether adding `exclusiveMaximum` is considered compatible. Allowed values are `Allow` and `Disallow`. When set to `Allow`, adding `exclusiveMaximum` is not flagged. The default is `Disallow` to remain maximally conservative. +- `removalPolicy` - controls whether removing `exclusiveMaximum` is considered compatible. Allowed values are `Allow` and `Disallow`. When set to `Allow`, removing `exclusiveMaximum` is not flagged. The default is `Disallow` to remain maximally conservative. + +Example configuration that allows adding `exclusiveMaximum`: + +```yaml +validations: + - name: exclusiveMaximum + enforcement: Error + configuration: + additionPolicy: Allow +``` + +Example configuration that allows removing `exclusiveMaximum`: + +```yaml +validations: + - name: exclusiveMaximum + enforcement: Error + configuration: + removalPolicy: Allow +``` diff --git a/pkg/runner/registry.go b/pkg/runner/registry.go index 545b9d9..59a7720 100644 --- a/pkg/runner/registry.go +++ b/pkg/runner/registry.go @@ -32,6 +32,7 @@ func init() { property.RegisterDefault(defaultRegistry) property.RegisterEnum(defaultRegistry) property.RegisterMaximum(defaultRegistry) + property.RegisterExclusiveMaximum(defaultRegistry) property.RegisterMaxItems(defaultRegistry) property.RegisterMaxLength(defaultRegistry) property.RegisterMaxProperties(defaultRegistry) diff --git a/pkg/validations/property/exclusive_maximum.go b/pkg/validations/property/exclusive_maximum.go new file mode 100644 index 0000000..9e2a815 --- /dev/null +++ b/pkg/validations/property/exclusive_maximum.go @@ -0,0 +1,162 @@ +// Copyright 2026 The Kubernetes Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//nolint:dupl +package property + +import ( + "errors" + "fmt" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "sigs.k8s.io/crdify/pkg/config" + "sigs.k8s.io/crdify/pkg/validations" +) + +var ( + _ validations.Validation = (*ExclusiveMaximum)(nil) + _ validations.Comparator[apiextensionsv1.JSONSchemaProps] = (*ExclusiveMaximum)(nil) +) + +const exclusiveMaximumValidationName = "exclusiveMaximum" + +// RegisterExclusiveMaximum registers the ExclusiveMaximum validation +// with the provided validation registry. +func RegisterExclusiveMaximum(registry validations.Registry) { + registry.Register(exclusiveMaximumValidationName, exclusiveMaximumFactory) +} + +// exclusiveMaximumFactory is a function used to initialize an ExclusiveMaximum validation +// implementation based on the provided configuration. +func exclusiveMaximumFactory(cfg map[string]interface{}) (validations.Validation, error) { + exclusiveCfg := &ExclusiveMaximumConfig{} + + err := ConfigToType(cfg, exclusiveCfg) + if err != nil { + return nil, fmt.Errorf("parsing config: %w", err) + } + + err = ValidateExclusiveMaximumConfig(exclusiveCfg) + if err != nil { + return nil, fmt.Errorf("validating exclusiveMaximum config: %w", err) + } + + return &ExclusiveMaximum{ExclusiveMaximumConfig: *exclusiveCfg}, nil +} + +// ValidateExclusiveMaximumConfig ensures provided ExclusiveMaximumConfig is valid and defaults missing values. +func ValidateExclusiveMaximumConfig(in *ExclusiveMaximumConfig) error { + if in == nil { + return nil + } + + switch in.AdditionPolicy { + case ExclusiveMaximumAdditionPolicyAllow, ExclusiveMaximumAdditionPolicyDisallow: + // valid entries + case ExclusiveMaximumAdditionPolicy(""): + in.AdditionPolicy = ExclusiveMaximumAdditionPolicyDisallow + default: + return fmt.Errorf("%w : %q (valid values: %q, %q)", errUnknownExclusiveMaximumAdditionPolicy, in.AdditionPolicy, ExclusiveMaximumAdditionPolicyAllow, ExclusiveMaximumAdditionPolicyDisallow) + } + + switch in.RemovalPolicy { + case ExclusiveMaximumRemovalPolicyAllow, ExclusiveMaximumRemovalPolicyDisallow: + // valid entries + case ExclusiveMaximumRemovalPolicy(""): + in.RemovalPolicy = ExclusiveMaximumRemovalPolicyDisallow + default: + return fmt.Errorf("%w : %q (valid values: %q, %q)", errUnknownExclusiveMaximumRemovalPolicy, in.RemovalPolicy, ExclusiveMaximumRemovalPolicyAllow, ExclusiveMaximumRemovalPolicyDisallow) + } + + return nil +} + +var errUnknownExclusiveMaximumAdditionPolicy = errors.New("unknown addition policy") +var errUnknownExclusiveMaximumRemovalPolicy = errors.New("unknown removal policy") + +// ExclusiveMaximumAdditionPolicy represents how adding the exclusiveMaximum constraint should be evaluated. +type ExclusiveMaximumAdditionPolicy string + +const ( + // ExclusiveMaximumAdditionPolicyAllow treats adding exclusiveMaximum when it was previously absent as compatible. + ExclusiveMaximumAdditionPolicyAllow ExclusiveMaximumAdditionPolicy = "Allow" + // ExclusiveMaximumAdditionPolicyDisallow treats adding exclusiveMaximum when it was previously absent as incompatible. + ExclusiveMaximumAdditionPolicyDisallow ExclusiveMaximumAdditionPolicy = "Disallow" +) + +// ExclusiveMaximumRemovalPolicy represents how loosening the exclusiveMaximum constraint should be evaluated. +type ExclusiveMaximumRemovalPolicy string + +const ( + // ExclusiveMaximumRemovalPolicyAllow treats loosening exclusiveMaximum as compatible. + ExclusiveMaximumRemovalPolicyAllow ExclusiveMaximumRemovalPolicy = "Allow" + // ExclusiveMaximumRemovalPolicyDisallow treats loosening exclusiveMaximum as incompatible. + ExclusiveMaximumRemovalPolicyDisallow ExclusiveMaximumRemovalPolicy = "Disallow" +) + +// ExclusiveMaximumConfig contains additional configuration for the ExclusiveMaximum validation. +type ExclusiveMaximumConfig struct { + // AdditionPolicy dictates whether adding exclusiveMaximum when it was previously absent is compatible. + // Allowed values are Allow and Disallow. Defaults to Disallow. + AdditionPolicy ExclusiveMaximumAdditionPolicy `json:"additionPolicy,omitempty"` + // RemovalPolicy dictates whether loosening exclusiveMaximum is compatible. + // Allowed values are Allow and Disallow. Defaults to Disallow. + RemovalPolicy ExclusiveMaximumRemovalPolicy `json:"removalPolicy,omitempty"` +} + +// ExclusiveMaximum is a Validation that can be used to identify +// incompatible changes to the exclusiveMaximum constraint of CRD properties. +type ExclusiveMaximum struct { + ExclusiveMaximumConfig + enforcement config.EnforcementPolicy +} + +// Name returns the name of the ExclusiveMaximum validation. +func (e *ExclusiveMaximum) Name() string { + return exclusiveMaximumValidationName +} + +// SetEnforcement sets the EnforcementPolicy for the ExclusiveMaximum validation. +func (e *ExclusiveMaximum) SetEnforcement(policy config.EnforcementPolicy) { + e.enforcement = policy +} + +// Compare compares an old and a new JSONSchemaProps, checking for incompatible changes to the exclusiveMaximum constraint of a property. +// In order for callers to determine if diffs to a JSONSchemaProps have been handled by this validation +// the JSONSchemaProps.ExclusiveMaximum field will be reset to 'false' as part of this method. +// It is highly recommended that only copies of the JSONSchemaProps to compare are provided to this method +// to prevent unintentional modifications. +func (e *ExclusiveMaximum) Compare(a, b *apiextensionsv1.JSONSchemaProps) validations.ComparisonResult { + var err error + + switch { + case a.ExclusiveMaximum == b.ExclusiveMaximum: + // nothing to do + case !a.ExclusiveMaximum && b.ExclusiveMaximum && e.AdditionPolicy != ExclusiveMaximumAdditionPolicyAllow: + err = fmt.Errorf("%w : %t -> %t", ErrExclusiveMaximumActivated, a.ExclusiveMaximum, b.ExclusiveMaximum) + case a.ExclusiveMaximum && !b.ExclusiveMaximum && e.RemovalPolicy != ExclusiveMaximumRemovalPolicyAllow: + err = fmt.Errorf("%w : %t -> %t", ErrExclusiveMaximumRemoved, a.ExclusiveMaximum, b.ExclusiveMaximum) + } + + a.ExclusiveMaximum = false + b.ExclusiveMaximum = false + + return validations.HandleErrors(e.Name(), e.enforcement, err) +} + +// ErrExclusiveMaximumActivated represents an error state when a property transitions from inclusive to exclusive maximum. +var ErrExclusiveMaximumActivated = errors.New("exclusive maximum activated when it was not previously") + +// ErrExclusiveMaximumRemoved represents an error state when a property transitions from exclusive to inclusive maximum. +var ErrExclusiveMaximumRemoved = errors.New("exclusive maximum removed when it was not previously") diff --git a/pkg/validations/property/exclusive_maximum_test.go b/pkg/validations/property/exclusive_maximum_test.go new file mode 100644 index 0000000..d05b38c --- /dev/null +++ b/pkg/validations/property/exclusive_maximum_test.go @@ -0,0 +1,167 @@ +// Copyright 2026 The Kubernetes Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package property + +import ( + "errors" + "testing" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/ptr" + internaltesting "sigs.k8s.io/crdify/pkg/validations/internal/testing" +) + +func TestExclusiveMaximum(t *testing.T) { + testcases := []internaltesting.Testcase[apiextensionsv1.JSONSchemaProps]{ + { + Name: "no diff, not flagged", + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + Flagged: false, + ComparableValidation: &ExclusiveMaximum{}, + }, + { + Name: "tightened flagged by default", + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + ExclusiveMaximum: true, + }, + Flagged: true, + ComparableValidation: &ExclusiveMaximum{}, + }, + { + Name: "tightened allowed via config", + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + ExclusiveMaximum: true, + }, + Flagged: false, + ComparableValidation: &ExclusiveMaximum{ + ExclusiveMaximumConfig: ExclusiveMaximumConfig{AdditionPolicy: ExclusiveMaximumAdditionPolicyAllow}, + }, + }, + { + Name: "loosened flagged by default", + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + ExclusiveMaximum: true, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + Flagged: true, + ComparableValidation: &ExclusiveMaximum{}, + }, + { + Name: "loosening allowed via config", + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + ExclusiveMaximum: true, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + Flagged: false, + ComparableValidation: &ExclusiveMaximum{ + ExclusiveMaximumConfig: ExclusiveMaximumConfig{RemovalPolicy: ExclusiveMaximumRemovalPolicyAllow}, + }, + }, + } + + internaltesting.RunTestcases(t, testcases...) +} + +func TestValidateExclusiveMaximumConfig(t *testing.T) { + testcases := []struct { + name string + cfg *ExclusiveMaximumConfig + wantErr error + wantAddition ExclusiveMaximumAdditionPolicy + wantRemoval ExclusiveMaximumRemovalPolicy + }{ + { + name: "nil config", + cfg: nil, + }, + { + name: "defaults policies", + cfg: &ExclusiveMaximumConfig{}, + wantAddition: ExclusiveMaximumAdditionPolicyDisallow, + wantRemoval: ExclusiveMaximumRemovalPolicyDisallow, + }, + { + name: "allows valid addition policy", + cfg: &ExclusiveMaximumConfig{AdditionPolicy: ExclusiveMaximumAdditionPolicyAllow}, + wantAddition: ExclusiveMaximumAdditionPolicyAllow, + wantRemoval: ExclusiveMaximumRemovalPolicyDisallow, + }, + { + name: "allows valid removal policy", + cfg: &ExclusiveMaximumConfig{RemovalPolicy: ExclusiveMaximumRemovalPolicyAllow}, + wantAddition: ExclusiveMaximumAdditionPolicyDisallow, + wantRemoval: ExclusiveMaximumRemovalPolicyAllow, + }, + { + name: "invalid addition policy mentions valid values", + cfg: &ExclusiveMaximumConfig{AdditionPolicy: "invalid"}, + wantErr: errUnknownExclusiveMaximumAdditionPolicy, + }, + { + name: "invalid removal policy mentions valid values", + cfg: &ExclusiveMaximumConfig{RemovalPolicy: "invalid"}, + wantErr: errUnknownExclusiveMaximumRemovalPolicy, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + err := ValidateExclusiveMaximumConfig(tc.cfg) + if tc.wantErr != nil { + if !errors.Is(err, tc.wantErr) { + t.Fatalf("expected error %v, got %v", tc.wantErr, err) + } + + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tc.cfg == nil { + return + } + + if tc.cfg.AdditionPolicy != tc.wantAddition { + t.Fatalf("expected addition policy %q, got %q", tc.wantAddition, tc.cfg.AdditionPolicy) + } + + if tc.cfg.RemovalPolicy != tc.wantRemoval { + t.Fatalf("expected removal policy %q, got %q", tc.wantRemoval, tc.cfg.RemovalPolicy) + } + }) + } +} diff --git a/pkg/validations/property/nullable.go b/pkg/validations/property/nullable.go index 4377b94..c0cd5b6 100644 --- a/pkg/validations/property/nullable.go +++ b/pkg/validations/property/nullable.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +//nolint:dupl package property import ( diff --git a/test/exclusivemaximumactivated/a.yaml b/test/exclusivemaximumactivated/a.yaml new file mode 100644 index 0000000..54b296d --- /dev/null +++ b/test/exclusivemaximumactivated/a.yaml @@ -0,0 +1,27 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: exclusivemaximums.example.com +spec: + group: example.com + names: + kind: ExclusiveMaximum + listKind: ExclusiveMaximumList + plural: exclusivemaximums + singular: exclusivemaximum + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + size: + type: integer + maximum: 10 diff --git a/test/exclusivemaximumactivated/b.yaml b/test/exclusivemaximumactivated/b.yaml new file mode 100644 index 0000000..cdf24b2 --- /dev/null +++ b/test/exclusivemaximumactivated/b.yaml @@ -0,0 +1,28 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: exclusivemaximums.example.com +spec: + group: example.com + names: + kind: ExclusiveMaximum + listKind: ExclusiveMaximumList + plural: exclusivemaximums + singular: exclusivemaximum + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + size: + type: integer + maximum: 10 + exclusiveMaximum: true diff --git a/test/exclusivemaximumactivated/expected.json b/test/exclusivemaximumactivated/expected.json new file mode 100644 index 0000000..b63280b --- /dev/null +++ b/test/exclusivemaximumactivated/expected.json @@ -0,0 +1,20 @@ +{ + "sameVersionValidation": [ + { + "version": "v1", + "propertyComparisons": [ + { + "property": "^.spec.size", + "comparisonResults": [ + { + "name": "exclusiveMaximum", + "errors": [ + "exclusive maximum activated when it was not previously : false -\u003e true" + ] + } + ] + } + ] + } + ] +} diff --git a/test/exclusivemaximumdeactivated/a.yaml b/test/exclusivemaximumdeactivated/a.yaml new file mode 100644 index 0000000..cdf24b2 --- /dev/null +++ b/test/exclusivemaximumdeactivated/a.yaml @@ -0,0 +1,28 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: exclusivemaximums.example.com +spec: + group: example.com + names: + kind: ExclusiveMaximum + listKind: ExclusiveMaximumList + plural: exclusivemaximums + singular: exclusivemaximum + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + size: + type: integer + maximum: 10 + exclusiveMaximum: true diff --git a/test/exclusivemaximumdeactivated/b.yaml b/test/exclusivemaximumdeactivated/b.yaml new file mode 100644 index 0000000..54b296d --- /dev/null +++ b/test/exclusivemaximumdeactivated/b.yaml @@ -0,0 +1,27 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: exclusivemaximums.example.com +spec: + group: example.com + names: + kind: ExclusiveMaximum + listKind: ExclusiveMaximumList + plural: exclusivemaximums + singular: exclusivemaximum + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + size: + type: integer + maximum: 10 diff --git a/test/exclusivemaximumdeactivated/expected.json b/test/exclusivemaximumdeactivated/expected.json new file mode 100644 index 0000000..9cd69cf --- /dev/null +++ b/test/exclusivemaximumdeactivated/expected.json @@ -0,0 +1,20 @@ +{ + "sameVersionValidation": [ + { + "version": "v1", + "propertyComparisons": [ + { + "property": "^.spec.size", + "comparisonResults": [ + { + "name": "exclusiveMaximum", + "errors": [ + "exclusive maximum removed when it was not previously : true -\u003e false" + ] + } + ] + } + ] + } + ] +}