Skip to content

Add validation for exclusiveMinimum #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
veepeeaee wants to merge 1 commit into
base: main
Choose a base branch
from
+65 −2
Open

Add validation for exclusiveMinimum #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions pkg/validations/property/min.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ var (
ErrNetNewMinimumConstraint = errors.New("minimum constraint added when there was none previously")
// ErrMinimumIncreased represents an error state where a minimum constaint on a property was increased.
ErrMinimumIncreased = errors.New("minimum increased")
// ErrExclusiveMinimumAdded represents an error state where an exclusiveMininum is added to a property.
ErrExclusiveMinimumAdded = errors.New("exclusiveMinimum added")
)

var (
Expand Down Expand Up @@ -97,12 +99,22 @@ func (m *Minimum) SetEnforcement(policy config.EnforcementPolicy) {
// It is highly recommended that only copies of the JSONSchemaProps to compare are provided to this method
// to prevent unintentional modifications.
func (m *Minimum) Compare(a, b *apiextensionsv1.JSONSchemaProps) validations.ComparisonResult {
err := MinVerification(a.Minimum, b.Minimum)
var errs []error

if err := MinVerification(a.Minimum, b.Minimum); err != nil {
errs = append(errs, err)
}

if !a.ExclusiveMinimum && b.ExclusiveMinimum {
errs = append(errs, ErrExclusiveMinimumAdded)
}
Comment on lines +108 to +110
Copy link
Contributor

@everettraven everettraven Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While setting exclusiveMinimum and minimum are pretty tightly coupled, I think this would make more sense to be an entirely separate check than integrated as part of the minimum check.

I've historically tried to separate each of these validations to check a singular schema property to keep scope as small as possible until we explicitly need to start coupling checks together to unlock more advanced use cases. Keeping to that pattern for now would be my preference.


a.Minimum = nil
b.Minimum = nil
a.ExclusiveMinimum = false
b.ExclusiveMinimum = false

return validations.HandleErrors(m.Name(), m.enforcement, err)
return validations.HandleErrors(m.Name(), m.enforcement, errs...)
}

var (
Expand Down
51 changes: 51 additions & 0 deletions pkg/validations/property/min_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,57 @@ func TestMinimum(t *testing.T) {
Flagged: false,
ComparableValidation: &Minimum{},
},
{
Name: "exclusiveMinimum changed from false to true, flagged",
Old: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: false,
},
New: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: true,
},
Flagged: true,
ComparableValidation: &Minimum{},
},
{
Name: "exclusiveMinimum changed from true to false, not flagged",
Old: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: true,
},
New: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: false,
},
Flagged: false,
ComparableValidation: &Minimum{},
},
{
Name: "net new exclusiveMinimum, flagged",
Old: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
},
New: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: true,
},
Flagged: true,
ComparableValidation: &Minimum{},
},
{
Name: "no diff exclusiveMinimum, not flagged",
Old: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: true,
},
New: &apiextensionsv1.JSONSchemaProps{
Minimum: ptr.To(10.0),
ExclusiveMinimum: true,
},
Flagged: false,
ComparableValidation: &Minimum{},
},
}

internaltesting.RunTestcases(t, testcases...)
Expand Down