Skip to content

Commit

Permalink
wip: no new required field with crd-schema-checker
Browse files Browse the repository at this point in the history 
Signed-off-by: everettraven <[email protected]>
  • Loading branch information
@everettraven
everettraven committed Apr 8, 2024
1 parent 2a62912 commit 275f432
Show file tree
Hide file tree
Showing 22 changed files with 1,704 additions and 36 deletions.
1 change: 1 addition & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/k14s/difflib v0.0.0-20240118055029-596a7a5585c3
github.com/k14s/ytt v0.36.0
github.com/mitchellh/go-wordwrap v1.0.1
github.com/openshift/crd-schema-checker v0.0.0-20240404194209-35a9033b1d11
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.29.0 h1:KIA/t2t5UBzoirT4H9tsML45GEbo3ouUnBHsCfD2tVg=
github.com/onsi/gomega v1.29.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
github.com/openshift/crd-schema-checker v0.0.0-20240404194209-35a9033b1d11 h1:eTNDkNRNV5lZvUbVM9Nop0lBcljSnA8rZX6yQPZ0ZnU=
github.com/openshift/crd-schema-checker v0.0.0-20240404194209-35a9033b1d11/go.mod h1:EmVJt97N+pfWFsli/ipXTBZqSG5F5KGQhm3c3IsGq1o=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down
1 change: 1 addition & 0 deletions pkg/kapp/crdupgradesafety/preflight.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func NewPreflight(df cmdcore.DepsFactory, enabled bool) *Preflight {
Validations: []Validation{
NewValidationFunc("NoScopeChange", NoScopeChange),
NewValidationFunc("NoStoredVersionRemoved", NoStoredVersionRemoved),
NewValidationFunc("NoNewRequiredField", NoNewRequiredField),
},
},
}
Expand Down
20 changes: 20 additions & 0 deletions pkg/kapp/crdupgradesafety/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package crdupgradesafety
import (
"errors"
"fmt"
"strings"

"github.com/openshift/crd-schema-checker/pkg/manifestcomparators"
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/util/sets"
)
Expand Down Expand Up @@ -91,3 +93,21 @@ func NoStoredVersionRemoved(old, new v1.CustomResourceDefinition) error {

return nil
}

func NoNewRequiredField(old, new v1.CustomResourceDefinition) error {
reg := manifestcomparators.NewRegistry()
err := reg.AddComparator(manifestcomparators.NoNewRequiredFields())
if err != nil {
return err
}

results, errs := reg.Compare(&old, &new)
if len(errs) > 0 {
return errors.Join(errs...)
}

if len(results[0].Errors) > 0 {
return errors.New(strings.Join(results[0].Errors, "\n"))
}
return nil
}
216 changes: 180 additions & 36 deletions pkg/kapp/crdupgradesafety/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"errors"
"testing"

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

func TestValidator(t *testing.T) {
Expand All @@ -24,15 +26,15 @@ func TestValidator(t *testing.T) {
{
name: "passing validator, no error",
validations: []Validation{
NewValidationFunc("pass", func(_, _ v1.CustomResourceDefinition) error {
NewValidationFunc("pass", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
return nil
}),
},
},
{
name: "failing validator, error",
validations: []Validation{
NewValidationFunc("fail", func(_, _ v1.CustomResourceDefinition) error {
NewValidationFunc("fail", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
return errors.New("boom")
}),
},
Expand All @@ -41,10 +43,10 @@ func TestValidator(t *testing.T) {
{
name: "passing+failing validator, error",
validations: []Validation{
NewValidationFunc("pass", func(_, _ v1.CustomResourceDefinition) error {
NewValidationFunc("pass", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
return nil
}),
NewValidationFunc("fail", func(_, _ v1.CustomResourceDefinition) error {
NewValidationFunc("fail", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
return errors.New("boom")
}),
},
Expand All @@ -55,7 +57,7 @@ func TestValidator(t *testing.T) {
v := Validator{
Validations: tc.validations,
}
var o, n v1.CustomResourceDefinition
var o, n apiextensionsv1.CustomResourceDefinition

err := v.Validate(o, n)
require.Equal(t, tc.shouldErr, err != nil)
Expand All @@ -66,33 +68,33 @@ func TestValidator(t *testing.T) {
func TestNoScopeChange(t *testing.T) {
for _, tc := range []struct {
name string
old v1.CustomResourceDefinition
new v1.CustomResourceDefinition
old apiextensionsv1.CustomResourceDefinition
new apiextensionsv1.CustomResourceDefinition
shouldError bool
}{
{
name: "no scope change, no error",
old: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Scope: v1.ClusterScoped,
old: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Scope: apiextensionsv1.ClusterScoped,
},
},
new: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Scope: v1.ClusterScoped,
new: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Scope: apiextensionsv1.ClusterScoped,
},
},
},
{
name: "scope change, error",
old: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Scope: v1.ClusterScoped,
old: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Scope: apiextensionsv1.ClusterScoped,
},
},
new: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Scope: v1.NamespaceScoped,
new: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Scope: apiextensionsv1.NamespaceScoped,
},
},
shouldError: true,
Expand All @@ -108,28 +110,28 @@ func TestNoScopeChange(t *testing.T) {
func TestNoStoredVersionRemoved(t *testing.T) {
for _, tc := range []struct {
name string
old v1.CustomResourceDefinition
new v1.CustomResourceDefinition
old apiextensionsv1.CustomResourceDefinition
new apiextensionsv1.CustomResourceDefinition
shouldError bool
}{
{
name: "no stored versions, no error",
new: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Versions: []v1.CustomResourceDefinitionVersion{
new: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
},
},
},
},
old: v1.CustomResourceDefinition{},
old: apiextensionsv1.CustomResourceDefinition{},
},
{
name: "stored versions, no stored version removed, no error",
new: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Versions: []v1.CustomResourceDefinitionVersion{
new: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
},
Expand All @@ -139,8 +141,8 @@ func TestNoStoredVersionRemoved(t *testing.T) {
},
},
},
old: v1.CustomResourceDefinition{
Status: v1.CustomResourceDefinitionStatus{
old: apiextensionsv1.CustomResourceDefinition{
Status: apiextensionsv1.CustomResourceDefinitionStatus{
StoredVersions: []string{
"v1alpha1",
},
Expand All @@ -149,17 +151,17 @@ func TestNoStoredVersionRemoved(t *testing.T) {
},
{
name: "stored versions, stored version removed, error",
new: v1.CustomResourceDefinition{
Spec: v1.CustomResourceDefinitionSpec{
Versions: []v1.CustomResourceDefinitionVersion{
new: apiextensionsv1.CustomResourceDefinition{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha2",
},
},
},
},
old: v1.CustomResourceDefinition{
Status: v1.CustomResourceDefinitionStatus{
old: apiextensionsv1.CustomResourceDefinition{
Status: apiextensionsv1.CustomResourceDefinitionStatus{
StoredVersions: []string{
"v1alpha1",
},
Expand All @@ -174,3 +176,145 @@ func TestNoStoredVersionRemoved(t *testing.T) {
})
}
}

func TestNoNewRequiredField(t *testing.T) {
for _, tc := range []struct {
name string
new apiextensionsv1.CustomResourceDefinition
old apiextensionsv1.CustomResourceDefinition
shouldError bool
}{
{
name: "no field added, no error",
old: apiextensionsv1.CustomResourceDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "tests.nofieldremove.com",
},
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "nofieldremove.com",
Names: apiextensionsv1.CustomResourceDefinitionNames{
Kind: "Test",
ListKind: "TestList",
Plural: "tests",
Singular: "test",
},
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
Schema: &apiextensionsv1.CustomResourceValidation{
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"fieldOne": {
Type: "string",
},
},
},
},
},
},
},
},
new: apiextensionsv1.CustomResourceDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "tests.nofieldremove.com",
},
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "nofieldremove.com",
Names: apiextensionsv1.CustomResourceDefinitionNames{
Kind: "Test",
ListKind: "TestList",
Plural: "tests",
Singular: "test",
},
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
Schema: &apiextensionsv1.CustomResourceValidation{
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"fieldOne": {
Type: "string",
},
},
},
},
},
},
},
},
},
{
name: "required field added, error",
old: apiextensionsv1.CustomResourceDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "tests.nofieldremove.com",
},
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "nofieldremove.com",
Names: apiextensionsv1.CustomResourceDefinitionNames{
Kind: "Test",
ListKind: "TestList",
Plural: "tests",
Singular: "test",
},
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
Schema: &apiextensionsv1.CustomResourceValidation{
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"fieldOne": {
Type: "string",
},
},
},
},
},
},
},
},
new: apiextensionsv1.CustomResourceDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "tests.nofieldremove.com",
},
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "nofieldremove.com",
Names: apiextensionsv1.CustomResourceDefinitionNames{
Kind: "Test",
ListKind: "TestList",
Plural: "tests",
Singular: "test",
},
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
Schema: &apiextensionsv1.CustomResourceValidation{
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"fieldOne": {
Type: "string",
},
"fieldTwo": {
Type: "string",
},
},
Required: []string{"fieldTwo"},
},
},
},
},
},
},
shouldError: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
err := NoNewRequiredField(tc.old, tc.new)
assert.Equal(t, tc.shouldError, err != nil)
})
}
}
Loading

0 comments on commit 275f432

Please sign in to comment.