Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ checks:
changeEnforcement: Strict
removalEnforcement: Strict
additionEnforcement: Strict
description:
enabled: true
required:
enabled: true
newEnforcement: Strict
Expand Down Expand Up @@ -76,6 +78,8 @@ checks:
changeEnforcement: Strict
removalEnforcement: Strict
additionEnforcement: Strict
description:
enabled: true
required:
enabled: true
newEnforcement: Strict
Expand Down
21 changes: 20 additions & 1 deletion docs/validations.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ checks:
# If set to an unknown value, the "Strict" enforcement strategy
# will be used.
removalEnforcement: { Strict || None }
# AdditionEnforcement is the enforcement strategy that should be used
# additionEnforcement is the enforcement strategy that should be used
# when evaluating if the addition of a default value for a property
# is considered incompatible.
#
Expand All @@ -251,6 +251,25 @@ checks:
additionEnforcement: { Strict || None }
```

### Description

Validates compatibility of changes to a property's description. Changes to descriptions
of properties are generally an acceptable change. If enabled, the validator allows all changes
to the description of a property.

Example configuration for this validation:

```yaml
checks:
version:
# in this example we are configuring the sameVersion validation's enum property validation.
# it is possible to configure this property validation separately in both the sameVersion and
# servedVersion validations.
sameVersion:
description:
enabled: { true || false }
```

### Maximum, MaxLength, MaxItems, and MaxProperties

Validates compatibility of changes to the property constraints related to maximum
Expand Down
42 changes: 28 additions & 14 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ var StrictPropertyCheckConfig = PropertyCheckConfig{
RemovalEnforcement: property.DefaultValidationRemovalEnforcementStrict,
AdditionEnforcement: property.DefaultValidationAdditionEnforcementStrict,
},
Description: DescriptionCheckConfig{
CheckConfig: CheckConfig{
Enabled: true,
},
},
Required: RequiredCheckConfig{
CheckConfig: CheckConfig{
Enabled: true,
Expand Down Expand Up @@ -187,18 +192,19 @@ type VersionCheckConfig struct {
}

type PropertyCheckConfig struct {
Enum EnumCheckConfig `yaml:"enum"`
Default DefaultCheckConfig `yaml:"default"`
Required RequiredCheckConfig `yaml:"required"`
Type TypeCheckConfig `yaml:"type"`
Maximum MaxCheckConfig `yaml:"maximum"`
MaxItems MaxCheckConfig `yaml:"maxItems"`
MaxProperties MaxCheckConfig `yaml:"maxProperties"`
MaxLength MaxCheckConfig `yaml:"maxLength"`
Minimum MinCheckConfig `yaml:"minimum"`
MinItems MinCheckConfig `yaml:"minItems"`
MinProperties MinCheckConfig `yaml:"minProperties"`
MinLength MinCheckConfig `yaml:"minLength"`
Enum EnumCheckConfig `yaml:"enum"`
Default DefaultCheckConfig `yaml:"default"`
Description DescriptionCheckConfig `yaml:"description"`
Required RequiredCheckConfig `yaml:"required"`
Type TypeCheckConfig `yaml:"type"`
Maximum MaxCheckConfig `yaml:"maximum"`
MaxItems MaxCheckConfig `yaml:"maxItems"`
MaxProperties MaxCheckConfig `yaml:"maxProperties"`
MaxLength MaxCheckConfig `yaml:"maxLength"`
Minimum MinCheckConfig `yaml:"minimum"`
MinItems MinCheckConfig `yaml:"minItems"`
MinProperties MinCheckConfig `yaml:"minProperties"`
MinLength MinCheckConfig `yaml:"minLength"`
}

type CheckConfig struct {
Expand All @@ -218,6 +224,10 @@ type DefaultCheckConfig struct {
AdditionEnforcement property.DefaultValidationAdditionEnforcement `json:"additionEnforcement"`
}

type DescriptionCheckConfig struct {
CheckConfig
}

type RequiredCheckConfig struct {
CheckConfig
NewEnforcement property.RequiredValidationNewEnforcement `json:"newEnforcement"`
Expand Down Expand Up @@ -245,7 +255,7 @@ func ValidatorForConfig(cfg Config) *crd.Validator {
}

func ValidationsForCRDChecks(checks CRDChecks) []crd.Validation {
validations := []crd.Validation{}
var validations []crd.Validation
if checks.Scope.Enabled {
validations = append(validations, &crd.Scope{})
}
Expand Down Expand Up @@ -294,7 +304,7 @@ func ServedVersionConfigForServedVersionCheckConfig(cfg ServedVersionCheckConfig
}

func PropertyValidationsForPropertyCheckConfig(cfg PropertyCheckConfig) []property.Validation {
validations := []property.Validation{}
var validations []property.Validation
if cfg.Enum.Enabled {
validations = append(validations, &property.Enum{
RemovalEnforcement: cfg.Enum.RemovalEnforcement,
Expand All @@ -310,6 +320,10 @@ func PropertyValidationsForPropertyCheckConfig(cfg PropertyCheckConfig) []proper
})
}

if cfg.Description.Enabled {
validations = append(validations, &property.Description{})
}

if cfg.Required.Enabled {
validations = append(validations, &property.Required{
NewEnforcement: cfg.Required.NewEnforcement,
Expand Down
21 changes: 21 additions & 0 deletions pkg/validations/property/description.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package property

type Description struct {
}

func (d *Description) Name() string {
return "description"
}

func (d *Description) Validate(diff Diff) (Diff, bool, error) {
reset := func(diff Diff) Diff {
oldProperty := diff.Old()
newProperty := diff.New()
newProperty.Description = ""
oldProperty.Description = ""
return NewDiff(oldProperty, newProperty)
}

resetDiff, handled := IsHandled(diff, reset)
return resetDiff, handled, nil
}
92 changes: 92 additions & 0 deletions pkg/validations/property/description_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package property

import (
"testing"

"github.com/stretchr/testify/require"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)

func TestDescription(t *testing.T) {
type testcase struct {
name string
oldProperty *apiextensionsv1.JSONSchemaProps
newProperty *apiextensionsv1.JSONSchemaProps
err error
handled bool
description *Description
}

for _, tc := range []testcase{
{
name: "no description, no error, handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{},
newProperty: &apiextensionsv1.JSONSchemaProps{},
err: nil,
handled: true,
description: &Description{},
},
{
name: "no diff, no error, handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{
Description: "foo",
},
newProperty: &apiextensionsv1.JSONSchemaProps{
Description: "foo",
},
err: nil,
handled: true,
description: &Description{},
},
{
name: "new description, no error, handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{},
newProperty: &apiextensionsv1.JSONSchemaProps{
Description: "new foo",
},
err: nil,
handled: true,
description: &Description{},
},
{
name: "description removed, no error, handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{
Description: "old foo",
},
newProperty: &apiextensionsv1.JSONSchemaProps{},
err: nil,
handled: true,
description: &Description{},
},
{
name: "description changed, no error, handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{
Description: "old foo",
},
newProperty: &apiextensionsv1.JSONSchemaProps{
Description: "new foo",
},
err: nil,
handled: true,
description: &Description{},
},
{
name: "different field changed, no error, not handled",
oldProperty: &apiextensionsv1.JSONSchemaProps{
ID: "foo",
},
newProperty: &apiextensionsv1.JSONSchemaProps{
ID: "bar",
},
err: nil,
handled: false,
description: &Description{},
},
} {
t.Run(tc.name, func(t *testing.T) {
_, handled, err := tc.description.Validate(NewDiff(tc.oldProperty, tc.newProperty))
require.Equal(t, tc.err, err)
require.Equal(t, tc.handled, handled)
})
}
}