diff --git a/pkg/kapp/crdupgradesafety/change_validator.go b/pkg/kapp/crdupgradesafety/change_validator.go new file mode 100644 index 000000000..4ae3ac936 --- /dev/null +++ b/pkg/kapp/crdupgradesafety/change_validator.go @@ -0,0 +1,216 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package crdupgradesafety + +import ( + "errors" + "fmt" + "reflect" + + "github.com/openshift/crd-schema-checker/pkg/manifestcomparators" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// ChangeValidation is a function that accepts a FieldDiff +// as a parameter and should return: +// - a boolean representation of whether or not the change +// - an error if the change would be unsafe +// has been fully handled (i.e no additional changes exist) +type ChangeValidation func(diff FieldDiff) (bool, error) + +// EnumChangeValidation ensures that: +// - No enums are added to a field that did not previously have +// enum restrictions +// - No enums are removed from a field +// This function returns: +// - A boolean representation of whether or not the change +// has been fully handled (i.e the only change was to enum values) +// - An error if either of the above validations are not satisfied +func EnumChangeValidation(diff FieldDiff) (bool, error) { + // This function resets the enum values for the + // old and new field and compares them to determine + // if there are any additional changes that should be + // handled. Reseting the enum values allows for chained + // evaluations to check if they have handled all the changes + // without having to account for fields other than the ones + // they are designed to handle. This function should only be called when + // returning from this function to prevent unnecessary overwrites of + // these fields. + handled := func() bool { + diff.Old.Enum = []v1.JSON{} + diff.New.Enum = []v1.JSON{} + return reflect.DeepEqual(diff.Old, diff.New) + } + + if len(diff.Old.Enum) == 0 && len(diff.New.Enum) > 0 { + return handled(), fmt.Errorf("enums added when there were no enum restrictions previously") + } + + oldSet := sets.NewString() + for _, enum := range diff.Old.Enum { + if !oldSet.Has(string(enum.Raw)) { + oldSet.Insert(string(enum.Raw)) + } + } + + newSet := sets.NewString() + for _, enum := range diff.New.Enum { + if !newSet.Has(string(enum.Raw)) { + newSet.Insert(string(enum.Raw)) + } + } + + diffSet := oldSet.Difference(newSet) + if diffSet.Len() > 0 { + return handled(), fmt.Errorf("enum values removed: %+v", diffSet.UnsortedList()) + } + + return handled(), nil +} + +// ChangeValidator is a Validation implementation focused on +// handling updates to existing fields in a CRD +type ChangeValidator struct { + // Validations is a slice of ChangeValidations + // to run against each changed field + Validations []ChangeValidation +} + +func (cv *ChangeValidator) Name() string { + return "ChangeValidator" +} + +// Validate will compare each version in the provided existing and new CRDs. +// Since the ChangeValidator is tailored to handling updates to existing fields in +// each version of a CRD. As such the following is assumed: +// - Validating the removal of versions during an update is handled outside of this +// validator. If a version in the existing version of the CRD does not exist in the new +// version that version of the CRD is skipped in this validator. +// - Removal of existing fields is unsafe. Regardless of whether or not this is handled +// by a validator outside this one, if a field is present in a version provided by the existing CRD +// but not present in the same version provided by the new CRD this validation will fail. +// +// Additionally, any changes that are not validated and handled by the known ChangeValidations +// are deemed as unsafe and returns an error. +func (cv *ChangeValidator) Validate(old, new v1.CustomResourceDefinition) error { + errs := []error{} + for _, version := range old.Spec.Versions { + newVersion := manifestcomparators.GetVersionByName(&new, version.Name) + if newVersion == nil { + // if the new version doesn't exist skip this version + continue + } + flatOld := FlattenSchema(version.Schema.OpenAPIV3Schema) + flatNew := FlattenSchema(newVersion.Schema.OpenAPIV3Schema) + + diffs, err := CalculateFlatSchemaDiff(flatOld, flatNew) + if err != nil { + errs = append(errs, fmt.Errorf("calculating schema diff for CRD version %q", version.Name)) + continue + } + + for field, diff := range diffs { + handled := false + for _, validation := range cv.Validations { + ok, err := validation(diff) + if err != nil { + errs = append(errs, fmt.Errorf("version %q, field %q: %w", version.Name, field, err)) + } + if ok { + handled = true + break + } + } + + if !handled { + errs = append(errs, fmt.Errorf("version %q, field %q has unknown change, refusing to determine that change is safe", version.Name, field)) + } + } + } + + if len(errs) > 0 { + return errors.Join(errs...) + } + return nil +} + +type FieldDiff struct { + Old *v1.JSONSchemaProps + New *v1.JSONSchemaProps +} + +// FlatSchema is a flat representation of a CRD schema. +type FlatSchema map[string]*v1.JSONSchemaProps + +// FlattenSchema takes in a CRD version OpenAPIV3Schema and returns +// a flattened representation of it. For example, a CRD with a schema of: +// ```yaml +// +// ... +// spec: +// type: object +// properties: +// foo: +// type: string +// bar: +// type: string +// ... +// +// ``` +// would be represented as: +// +// map[string]*v1.JSONSchemaProps{ +// "^": {}, +// "^.spec": {}, +// "^.spec.foo": {}, +// "^.spec.bar": {}, +// } +// +// where "^" represents the "root" schema +func FlattenSchema(schema *v1.JSONSchemaProps) FlatSchema { + fieldMap := map[string]*v1.JSONSchemaProps{} + + manifestcomparators.SchemaHas(schema, + field.NewPath("^"), + field.NewPath("^"), + nil, + func(s *v1.JSONSchemaProps, _, simpleLocation *field.Path, _ []*v1.JSONSchemaProps) bool { + fieldMap[simpleLocation.String()] = s.DeepCopy() + return false + }) + + return fieldMap +} + +// CalculateFlatSchemaDiff finds fields in a FlatSchema that are different +// and returns a mapping of field --> old and new field schemas. If a field +// exists in the old FlatSchema but not the new an empty diff mapping and an error is returned. +func CalculateFlatSchemaDiff(o, n FlatSchema) (map[string]FieldDiff, error) { + diffMap := map[string]FieldDiff{} + for field, schema := range o { + if _, ok := n[field]; !ok { + return diffMap, fmt.Errorf("field %q in existing not found in new", field) + } + newSchema := n[field] + + // Copy the schemas and remove any child properties for comparison. + // In theory this will focus in on detecting changes for only the + // field we are looking at and ignore changes in the children fields. + // Since we are iterating through the map that should have all fields + // we should still detect changes in the children fields. + oldCopy := schema.DeepCopy() + newCopy := newSchema.DeepCopy() + oldCopy.Properties = nil + newCopy.Properties = nil + if !reflect.DeepEqual(oldCopy, newCopy) { + diffMap[field] = FieldDiff{ + Old: oldCopy, + New: newCopy, + } + } + } + return diffMap, nil +} diff --git a/pkg/kapp/crdupgradesafety/change_validator_test.go b/pkg/kapp/crdupgradesafety/change_validator_test.go new file mode 100644 index 000000000..6c4b943c5 --- /dev/null +++ b/pkg/kapp/crdupgradesafety/change_validator_test.go @@ -0,0 +1,431 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package crdupgradesafety_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/crdupgradesafety" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +func TestEnumChangeValidation(t *testing.T) { + for _, tc := range []struct { + name string + diff crdupgradesafety.FieldDiff + shouldError bool + shouldHandle bool + }{ + { + name: "no change, no error, marked as handled", + diff: crdupgradesafety.FieldDiff{ + Old: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + New: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + }, + shouldHandle: true, + }, + { + name: "enum added, no other changes, no error, marked as handled", + diff: crdupgradesafety.FieldDiff{ + Old: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + New: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + { + Raw: []byte("bar"), + }, + }, + }, + }, + shouldHandle: true, + }, + { + name: "no enums before, enums added, no other changes, error, marked as handled", + diff: crdupgradesafety.FieldDiff{ + Old: &v1.JSONSchemaProps{}, + New: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + }, + shouldHandle: true, + shouldError: true, + }, + { + name: "enum removed, no other changes, error, marked as handled", + diff: crdupgradesafety.FieldDiff{ + Old: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + { + Raw: []byte("bar"), + }, + }, + }, + New: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("bar"), + }, + }, + }, + }, + shouldHandle: true, + shouldError: true, + }, + { + name: "no enum change, other changes, no error, not marked as handled", + diff: crdupgradesafety.FieldDiff{ + Old: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + ID: "bar", + }, + New: &v1.JSONSchemaProps{ + Enum: []v1.JSON{ + { + Raw: []byte("foo"), + }, + }, + ID: "baz", + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := crdupgradesafety.EnumChangeValidation(tc.diff) + assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError) + assert.Equal(t, tc.shouldHandle, handled, "should be handled? - %v", tc.shouldHandle) + assert.Empty(t, tc.diff.Old.Enum) + assert.Empty(t, tc.diff.New.Enum) + }) + } +} + +func TestCalculateFlatSchemaDiff(t *testing.T) { + for _, tc := range []struct { + name string + old crdupgradesafety.FlatSchema + new crdupgradesafety.FlatSchema + expectedDiff map[string]crdupgradesafety.FieldDiff + shouldError bool + }{ + { + name: "no diff in schemas, empty diff, no error", + old: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{}, + }, + new: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{}, + }, + expectedDiff: map[string]crdupgradesafety.FieldDiff{}, + }, + { + name: "diff in schemas, diff returned, no error", + old: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{}, + }, + new: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{ + ID: "bar", + }, + }, + expectedDiff: map[string]crdupgradesafety.FieldDiff{ + "foo": { + Old: &v1.JSONSchemaProps{}, + New: &v1.JSONSchemaProps{ID: "bar"}, + }, + }, + }, + { + name: "diff in child properties only, no diff returned, no error", + old: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "bar": {ID: "bar"}, + }, + }, + }, + new: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "bar": {ID: "baz"}, + }, + }, + }, + expectedDiff: map[string]crdupgradesafety.FieldDiff{}, + }, + { + name: "field exists in old but not new, no diff returned, error", + old: crdupgradesafety.FlatSchema{ + "foo": &v1.JSONSchemaProps{}, + }, + new: crdupgradesafety.FlatSchema{ + "bar": &v1.JSONSchemaProps{}, + }, + expectedDiff: map[string]crdupgradesafety.FieldDiff{}, + shouldError: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + diff, err := crdupgradesafety.CalculateFlatSchemaDiff(tc.old, tc.new) + assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError) + assert.Equal(t, tc.expectedDiff, diff) + }) + } +} + +func TestFlattenSchema(t *testing.T) { + schema := &v1.JSONSchemaProps{ + Properties: map[string]v1.JSONSchemaProps{ + "foo": { + Properties: map[string]v1.JSONSchemaProps{ + "bar": {}, + }, + }, + "baz": {}, + }, + } + + foo := schema.Properties["foo"] + foobar := schema.Properties["foo"].Properties["bar"] + baz := schema.Properties["baz"] + expected := crdupgradesafety.FlatSchema{ + "^": schema, + "^.foo": &foo, + "^.foo.bar": &foobar, + "^.baz": &baz, + } + + actual := crdupgradesafety.FlattenSchema(schema) + + assert.Equal(t, expected, actual) +} + +func TestChangeValidator(t *testing.T) { + for _, tc := range []struct { + name string + changeValidator *crdupgradesafety.ChangeValidator + old v1.CustomResourceDefinition + new v1.CustomResourceDefinition + shouldError bool + }{ + { + name: "no changes, no error", + changeValidator: &crdupgradesafety.ChangeValidator{ + Validations: []crdupgradesafety.ChangeValidation{ + func(_ crdupgradesafety.FieldDiff) (bool, error) { + return false, errors.New("should not run") + }, + }, + }, + old: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + new: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + }, + { + name: "changes, validation successful, change is fully handled, no error", + changeValidator: &crdupgradesafety.ChangeValidator{ + Validations: []crdupgradesafety.ChangeValidation{ + func(_ crdupgradesafety.FieldDiff) (bool, error) { + return true, nil + }, + }, + }, + old: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + new: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + ID: "foo", + }, + }, + }, + }, + }, + }, + }, + { + name: "changes, validation successful, change not fully handled, error", + changeValidator: &crdupgradesafety.ChangeValidator{ + Validations: []crdupgradesafety.ChangeValidation{ + func(_ crdupgradesafety.FieldDiff) (bool, error) { + return false, nil + }, + }, + }, + old: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + new: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + ID: "foo", + }, + }, + }, + }, + }, + }, + shouldError: true, + }, + { + name: "changes, validation failed, change fully handled, error", + changeValidator: &crdupgradesafety.ChangeValidator{ + Validations: []crdupgradesafety.ChangeValidation{ + func(_ crdupgradesafety.FieldDiff) (bool, error) { + return true, errors.New("fail") + }, + }, + }, + old: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + new: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + ID: "foo", + }, + }, + }, + }, + }, + }, + shouldError: true, + }, + { + name: "changes, validation failed, change not fully handled, error", + changeValidator: &crdupgradesafety.ChangeValidator{ + Validations: []crdupgradesafety.ChangeValidation{ + func(_ crdupgradesafety.FieldDiff) (bool, error) { + return false, errors.New("fail") + }, + }, + }, + old: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{}, + }, + }, + }, + }, + }, + new: v1.CustomResourceDefinition{ + Spec: v1.CustomResourceDefinitionSpec{ + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + ID: "foo", + }, + }, + }, + }, + }, + }, + shouldError: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.changeValidator.Validate(tc.old, tc.new) + assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError) + }) + } +} diff --git a/pkg/kapp/crdupgradesafety/preflight.go b/pkg/kapp/crdupgradesafety/preflight.go index 8c35d00b6..687223899 100644 --- a/pkg/kapp/crdupgradesafety/preflight.go +++ b/pkg/kapp/crdupgradesafety/preflight.go @@ -37,6 +37,11 @@ func NewPreflight(df cmdcore.DepsFactory, enabled bool) *Preflight { NewValidationFunc("NoScopeChange", NoScopeChange), NewValidationFunc("NoStoredVersionRemoved", NoStoredVersionRemoved), NewValidationFunc("NoExistingFieldRemoved", NoExistingFieldRemoved), + &ChangeValidator{ + Validations: []ChangeValidation{ + EnumChangeValidation, + }, + }, }, }, } diff --git a/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_added_test.go b/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_added_test.go new file mode 100644 index 000000000..55a82d0c3 --- /dev/null +++ b/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_added_test.go @@ -0,0 +1,122 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Adding enums to an existing field in a CRD is a valid update as long as +// the field was previously restricted to a set of enums. Adding enums when +// the field was not previously restricted is an invalid change and this +// test is testing for that invalid case. +func TestPreflightCRDUpgradeSafetyInvalidFieldChangeEnumsAdded(t *testing.T) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + kubectl := Kubectl{t, env.Namespace, logger} + + testName := "preflightcrdupgradesafetyinvalidfieldchangeenumsadded" + + base := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + base = strings.ReplaceAll(base, "__test-name__", testName) + appName := "preflight-crdupgradesafety-app" + + cleanUp := func() { + kapp.Run([]string{"delete", "-a", appName}) + RemoveClusterResource(t, "ns", testName, "", kubectl) + } + cleanUp() + defer cleanUp() + + update := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + enum: + - foo + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + update = strings.ReplaceAll(update, "__test-name__", testName) + logger.Section("deploy app with CRD update that adds enum to existing field that did not have enums, preflight check enabled, should error", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-a", appName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(base)}) + require.NoError(t, err) + _, err = kapp.RunWithOpts([]string{"deploy", "--preflight=CRDUpgradeSafety", "-a", appName, "-f", "-"}, + RunOpts{StdinReader: strings.NewReader(update), AllowError: true}) + require.Error(t, err) + require.Contains(t, err.Error(), "enums added when there were no enum restrictions previously") + }) +} diff --git a/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_removed_test.go b/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_removed_test.go new file mode 100644 index 000000000..080bb6c36 --- /dev/null +++ b/test/e2e/preflight_crdupgradesafety_invalid_field_change_enums_removed_test.go @@ -0,0 +1,120 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPreflightCRDUpgradeSafetyInvalidFieldChangeEnumsRemoved(t *testing.T) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + kubectl := Kubectl{t, env.Namespace, logger} + + testName := "preflightcrdupgradesafetyinvalidfieldchangeenumsremoved" + + base := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + enum: + - foo + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + base = strings.ReplaceAll(base, "__test-name__", testName) + appName := "preflight-crdupgradesafety-app" + + cleanUp := func() { + kapp.Run([]string{"delete", "-a", appName}) + RemoveClusterResource(t, "ns", testName, "", kubectl) + } + cleanUp() + defer cleanUp() + + update := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + enum: + - bar + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + update = strings.ReplaceAll(update, "__test-name__", testName) + logger.Section("deploy app with CRD update that removes an enum value, preflight check enabled, should error", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-a", appName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(base)}) + require.NoError(t, err) + _, err = kapp.RunWithOpts([]string{"deploy", "--preflight=CRDUpgradeSafety", "-a", appName, "-f", "-"}, + RunOpts{StdinReader: strings.NewReader(update), AllowError: true}) + require.Error(t, err) + require.Contains(t, err.Error(), "enum values removed") + }) +} diff --git a/test/e2e/preflight_crdupgradesafety_valid_field_change_enums_added_test.go b/test/e2e/preflight_crdupgradesafety_valid_field_change_enums_added_test.go new file mode 100644 index 000000000..1e8167076 --- /dev/null +++ b/test/e2e/preflight_crdupgradesafety_valid_field_change_enums_added_test.go @@ -0,0 +1,120 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPreflightCRDUpgradeSafetyValidFieldChangeEnumsAdded(t *testing.T) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + kubectl := Kubectl{t, env.Namespace, logger} + + testName := "preflightcrdupgradesafetyvalidfieldchangeenumsadded" + + base := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + enum: + - foo + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + base = strings.ReplaceAll(base, "__test-name__", testName) + appName := "preflight-crdupgradesafety-app" + + cleanUp := func() { + kapp.Run([]string{"delete", "-a", appName}) + RemoveClusterResource(t, "ns", testName, "", kubectl) + } + cleanUp() + defer cleanUp() + + update := ` +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: memcacheds.__test-name__.example.com +spec: + group: __test-name__.example.com + names: + kind: Memcached + listKind: MemcachedList + plural: memcacheds + singular: memcached + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: string + enum: + - foo + - bar + status: + type: object + type: object + served: true + storage: true + subresources: + status: {} +` + + update = strings.ReplaceAll(update, "__test-name__", testName) + logger.Section("deploy app with CRD update that adds an enum value to an existing field with enum values, preflight check enabled, should not error", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-a", appName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(base)}) + require.NoError(t, err) + _, err = kapp.RunWithOpts([]string{"deploy", "--preflight=CRDUpgradeSafety", "-a", appName, "-f", "-"}, + RunOpts{StdinReader: strings.NewReader(update), AllowError: false}) + require.NoError(t, err) + }) +}