diff --git a/go.mod b/go.mod index 7f385942d..125cb7c9e 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 sigs.k8s.io/controller-runtime v0.21.0 sigs.k8s.io/controller-tools v0.18.0 - sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58 + sigs.k8s.io/crdify v0.4.1-0.20250825182107-69e65223aee0 sigs.k8s.io/yaml v1.6.0 ) diff --git a/go.sum b/go.sum index f66d08458..9b4abf20e 100644 --- a/go.sum +++ b/go.sum @@ -765,8 +765,8 @@ sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytI sigs.k8s.io/controller-runtime v0.21.0/go.mod h1:OSg14+F65eWqIu4DceX7k/+QRAbTTvxeQSNSOQpukWM= sigs.k8s.io/controller-tools v0.18.0 h1:rGxGZCZTV2wJreeRgqVoWab/mfcumTMmSwKzoM9xrsE= sigs.k8s.io/controller-tools v0.18.0/go.mod h1:gLKoiGBriyNh+x1rWtUQnakUYEujErjXs9pf+x/8n1U= -sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58 h1:VTvhbqgZMVoDpHHPuZLaOgzjjsJBhO8+vDKA1COuLCY= -sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= +sigs.k8s.io/crdify v0.4.1-0.20250825182107-69e65223aee0 h1:jfBjW0kwwx2ULnzRrs+Jnepn345JbpAVqzekHBeIGgY= +sigs.k8s.io/crdify v0.4.1-0.20250825182107-69e65223aee0/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= sigs.k8s.io/gateway-api v1.1.0 h1:DsLDXCi6jR+Xz8/xd0Z1PYl2Pn0TyaFMOPPZIj4inDM= sigs.k8s.io/gateway-api v1.1.0/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7npUVDpVXUUOFfW6CgAqE= diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go index 3905535c6..efa86a191 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/crdify/pkg/config" "sigs.k8s.io/crdify/pkg/runner" "sigs.k8s.io/crdify/pkg/validations" + "sigs.k8s.io/crdify/pkg/validations/property" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -131,6 +132,13 @@ func defaultConfig() *config.Config { Name: "description", Enforcement: config.EnforcementPolicyNone, }, + { + Name: "enum", + Enforcement: config.EnforcementPolicyError, + Configuration: map[string]interface{}{ + "additionPolicy": property.AdditionPolicyAllow, + }, + }, }, } } diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go index 71b457de6..2d2a2065f 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go @@ -39,7 +39,7 @@ func newMockPreflight(crd *apiextensionsv1.CustomResourceDefinition, err error) }, preflightOpts...) } -const crdFolder string = "../../../../../testdata/manifests" +const crdFolder string = "testdata/manifests" func getCrdFromManifestFile(t *testing.T, oldCrdFile string) *apiextensionsv1.CustomResourceDefinition { if oldCrdFile == "" { @@ -67,6 +67,14 @@ func getManifestString(t *testing.T, crdFile string) string { return string(buff) } +func wantErrorMsgs(wantMsgs []string) require.ErrorAssertionFunc { + return func(t require.TestingT, haveErr error, _ ...interface{}) { + for _, wantMsg := range wantMsgs { + require.ErrorContains(t, haveErr, wantMsg) + } + } +} + // TestInstall exists only for completeness as Install() is currently a no-op. It can be used as // a template for real tests in the future if the func is implemented. func TestInstall(t *testing.T) { @@ -74,7 +82,7 @@ func TestInstall(t *testing.T) { name string oldCrdPath string release *release.Release - wantErrMsgs []string + requireErr require.ErrorAssertionFunc wantCrdGetErr error }{ { @@ -92,7 +100,7 @@ func TestInstall(t *testing.T) { Name: "test-release", Manifest: "abcd", }, - wantErrMsgs: []string{"json: cannot unmarshal string into Go value of type unstructured.detector"}, + requireErr: wantErrorMsgs([]string{"json: cannot unmarshal string into Go value of type unstructured.detector"}), }, { name: "release with no CRD objects", @@ -108,7 +116,7 @@ func TestInstall(t *testing.T) { Manifest: getManifestString(t, "crd-valid-upgrade.json"), }, wantCrdGetErr: fmt.Errorf("error!"), - wantErrMsgs: []string{"error!"}, + requireErr: wantErrorMsgs([]string{"error!"}), }, { name: "fail to get old crd, not found error", @@ -124,7 +132,7 @@ func TestInstall(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-invalid"), }, - wantErrMsgs: []string{"json: cannot unmarshal"}, + requireErr: wantErrorMsgs([]string{"json: cannot unmarshal"}), }, { name: "valid upgrade", @@ -143,7 +151,7 @@ func TestInstall(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-invalid-upgrade.json"), }, - wantErrMsgs: []string{ + requireErr: wantErrorMsgs([]string{ `scope:`, `storedVersionRemoval:`, `enum:`, @@ -157,7 +165,7 @@ func TestInstall(t *testing.T) { `minLength:`, `minProperties:`, `default:`, - }, + }), }, { name: "new crd validation failure for existing field removal", @@ -168,9 +176,9 @@ func TestInstall(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-field-removed.json"), }, - wantErrMsgs: []string{ + requireErr: wantErrorMsgs([]string{ `existingFieldRemoval:`, - }, + }), }, { name: "new crd validation should not fail on description changes", @@ -188,10 +196,8 @@ func TestInstall(t *testing.T) { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr) err := preflight.Install(context.Background(), tc.release) - if len(tc.wantErrMsgs) != 0 { - for _, expectedErrMsg := range tc.wantErrMsgs { - require.ErrorContains(t, err, expectedErrMsg) - } + if tc.requireErr != nil { + tc.requireErr(t, err) } else { require.NoError(t, err) } @@ -204,7 +210,7 @@ func TestUpgrade(t *testing.T) { name string oldCrdPath string release *release.Release - wantErrMsgs []string + requireErr require.ErrorAssertionFunc wantCrdGetErr error }{ { @@ -222,7 +228,7 @@ func TestUpgrade(t *testing.T) { Name: "test-release", Manifest: "abcd", }, - wantErrMsgs: []string{"json: cannot unmarshal string into Go value of type unstructured.detector"}, + requireErr: wantErrorMsgs([]string{"json: cannot unmarshal string into Go value of type unstructured.detector"}), }, { name: "release with no CRD objects", @@ -238,7 +244,7 @@ func TestUpgrade(t *testing.T) { Manifest: getManifestString(t, "crd-valid-upgrade.json"), }, wantCrdGetErr: fmt.Errorf("error!"), - wantErrMsgs: []string{"error!"}, + requireErr: wantErrorMsgs([]string{"error!"}), }, { name: "fail to get old crd, not found error", @@ -254,7 +260,7 @@ func TestUpgrade(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-invalid"), }, - wantErrMsgs: []string{"json: cannot unmarshal"}, + requireErr: wantErrorMsgs([]string{"json: cannot unmarshal"}), }, { name: "valid upgrade", @@ -273,7 +279,7 @@ func TestUpgrade(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-invalid-upgrade.json"), }, - wantErrMsgs: []string{ + requireErr: wantErrorMsgs([]string{ `scope:`, `storedVersionRemoval:`, `enum:`, @@ -287,7 +293,7 @@ func TestUpgrade(t *testing.T) { `minLength:`, `minProperties:`, `default:`, - }, + }), }, { name: "new crd validation failure for existing field removal", @@ -298,9 +304,9 @@ func TestUpgrade(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-field-removed.json"), }, - wantErrMsgs: []string{ + requireErr: wantErrorMsgs([]string{ `existingFieldRemoval:`, - }, + }), }, { name: "webhook conversion strategy exists", @@ -317,9 +323,9 @@ func TestUpgrade(t *testing.T) { Name: "test-release", Manifest: getManifestString(t, "crd-conversion-no-webhook.json"), }, - wantErrMsgs: []string{ - `validating upgrade for CRD "crontabs.stable.example.com": v1 <-> v2: ^.spec.foobarbaz: enum: allowed enum values removed`, - }, + requireErr: wantErrorMsgs([]string{ + `validating upgrade for CRD "crontabs.stable.example.com": v1 -> v2: ^.spec.foobarbaz: enum: allowed enum values removed`, + }), }, { name: "new crd validation should not fail on description changes", @@ -331,16 +337,43 @@ func TestUpgrade(t *testing.T) { Manifest: getManifestString(t, "crd-description-changed.json"), }, }, + { + name: "success when old crd and new crd contain the exact same validation issues", + oldCrdPath: "crd-conversion-no-webhook.json", + release: &release.Release{ + Name: "test-release", + Manifest: getManifestString(t, "crd-conversion-no-webhook.json"), + }, + }, + { + name: "failure when old crd and new crd contain the exact same validation issues, but new crd introduces another validation issue", + oldCrdPath: "crd-conversion-no-webhook.json", + release: &release.Release{ + Name: "test-release", + Manifest: getManifestString(t, "crd-conversion-no-webhook-extra-issue.json"), + }, + requireErr: func(t require.TestingT, err error, _ ...interface{}) { + require.ErrorContains(t, err, + `validating upgrade for CRD "crontabs.stable.example.com":`, + ) + // The newly introduced issue is reported + require.Contains(t, err.Error(), + `v1 -> v2: ^.spec.extraField: type: type changed : "boolean" -> "string"`, + ) + // The existing issue is not reported + require.NotContains(t, err.Error(), + `v1 -> v2: ^.spec.foobarbaz: enum: allowed enum values removed`, + ) + }, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr) err := preflight.Upgrade(context.Background(), tc.release) - if len(tc.wantErrMsgs) != 0 { - for _, expectedErrMsg := range tc.wantErrMsgs { - require.ErrorContains(t, err, expectedErrMsg) - } + if tc.requireErr != nil { + tc.requireErr(t, err) } else { require.NoError(t, err) } diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook-extra-issue.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook-extra-issue.json new file mode 100644 index 000000000..0bfd13384 --- /dev/null +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook-extra-issue.json @@ -0,0 +1,76 @@ +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "crontabs.stable.example.com" + }, + "spec": { + "group": "stable.example.com", + "versions": [ + { + "name": "v2", + "served": true, + "storage": false, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "foobarbaz": { + "type":"string", + "enum":[ + "bark", + "woof" + ] + }, + "extraField": { + "type":"string" + } + } + } + } + } + } + }, + { + "name": "v1", + "served": true, + "storage": false, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "foobarbaz": { + "type":"string", + "enum":[ + "foo", + "bar", + "baz" + ] + }, + "extraField": { + "type":"boolean" + } + } + } + } + } + } + } + ], + "scope": "Cluster", + "names": { + "plural": "crontabs", + "singular": "crontab", + "kind": "CronTab", + "shortNames": [ + "ct" + ] + } + } +} diff --git a/testdata/manifests/crd-conversion-no-webhook.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook.json similarity index 100% rename from testdata/manifests/crd-conversion-no-webhook.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook.json diff --git a/testdata/manifests/crd-conversion-webhook-old.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-webhook-old.json similarity index 100% rename from testdata/manifests/crd-conversion-webhook-old.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-webhook-old.json diff --git a/testdata/manifests/crd-conversion-webhook.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-webhook.json similarity index 100% rename from testdata/manifests/crd-conversion-webhook.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-webhook.json diff --git a/testdata/manifests/crd-description-changed.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-description-changed.json similarity index 94% rename from testdata/manifests/crd-description-changed.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-description-changed.json index ae30459e3..0e7f9a600 100644 --- a/testdata/manifests/crd-description-changed.json +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-description-changed.json @@ -23,7 +23,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" @@ -70,7 +71,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" diff --git a/testdata/manifests/crd-field-removed.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-field-removed.json similarity index 96% rename from testdata/manifests/crd-field-removed.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-field-removed.json index 86ba06e40..650b13fd4 100644 --- a/testdata/manifests/crd-field-removed.json +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-field-removed.json @@ -22,7 +22,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" @@ -66,7 +67,8 @@ "type": "object", "properties": { "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" diff --git a/testdata/manifests/crd-invalid b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-invalid similarity index 100% rename from testdata/manifests/crd-invalid rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-invalid diff --git a/testdata/manifests/crd-invalid-upgrade.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-invalid-upgrade.json similarity index 92% rename from testdata/manifests/crd-invalid-upgrade.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-invalid-upgrade.json index 4131a68fb..3c95ccb25 100644 --- a/testdata/manifests/crd-invalid-upgrade.json +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-invalid-upgrade.json @@ -24,11 +24,8 @@ "type":"integer" }, "enum": { - "type":"integer", - "enum":[ - 1, - 2 - ] + "type": "string", + "enum": ["a", "b"] }, "minMaxValue": { "type":"integer", diff --git a/testdata/manifests/crd-unhandled-new.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-new.json similarity index 100% rename from testdata/manifests/crd-unhandled-new.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-new.json diff --git a/testdata/manifests/crd-unhandled-old.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-old.json similarity index 100% rename from testdata/manifests/crd-unhandled-old.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-old.json diff --git a/testdata/manifests/crd-valid-upgrade.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-valid-upgrade.json similarity index 93% rename from testdata/manifests/crd-valid-upgrade.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-valid-upgrade.json index 52380dc92..cbc2e3ec1 100644 --- a/testdata/manifests/crd-valid-upgrade.json +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-valid-upgrade.json @@ -22,7 +22,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c", "adding-enum-is-allowed"] }, "minMaxValue": { "type":"integer" @@ -69,7 +70,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c", "adding-enum-is-allowed"] }, "minMaxValue": { "type":"integer" @@ -116,7 +118,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c", "adding-enum-is-allowed"] }, "minMaxValue": { "type":"integer" diff --git a/testdata/manifests/no-crds.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/no-crds.json similarity index 100% rename from testdata/manifests/no-crds.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/no-crds.json diff --git a/testdata/manifests/old-crd.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/old-crd.json similarity index 94% rename from testdata/manifests/old-crd.json rename to internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/old-crd.json index 1f3ff5a4b..5a8c55b32 100644 --- a/testdata/manifests/old-crd.json +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/old-crd.json @@ -23,7 +23,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" @@ -70,7 +71,8 @@ "type":"integer" }, "enum": { - "type":"integer" + "type": "string", + "enum": ["a", "b", "c"] }, "minMaxValue": { "type":"integer" diff --git a/vendor/modules.txt b/vendor/modules.txt index b499ae80b..3d49f8737 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1995,7 +1995,7 @@ sigs.k8s.io/controller-tools/pkg/genall sigs.k8s.io/controller-tools/pkg/loader sigs.k8s.io/controller-tools/pkg/markers sigs.k8s.io/controller-tools/pkg/version -# sigs.k8s.io/crdify v0.4.1-0.20250613143457-398e4483fb58 +# sigs.k8s.io/crdify v0.4.1-0.20250825182107-69e65223aee0 ## explicit; go 1.24.0 sigs.k8s.io/crdify/pkg/config sigs.k8s.io/crdify/pkg/runner diff --git a/vendor/sigs.k8s.io/crdify/pkg/validations/util.go b/vendor/sigs.k8s.io/crdify/pkg/validations/util.go index 4d10bd738..85a5bb699 100644 --- a/vendor/sigs.k8s.io/crdify/pkg/validations/util.go +++ b/vendor/sigs.k8s.io/crdify/pkg/validations/util.go @@ -77,8 +77,7 @@ func FlattenedCRDVersionDiff(a, b map[string]*apiextensionsv1.JSONSchemaProps) m // based on changes to the children properties. The changes to the children // properties should still be evaluated since we are looping through a flattened // map of all the properties for the CRD version - oldSchemaCopy := oldSchema.DeepCopy() - oldSchemaCopy.Properties = nil + oldSchemaCopy := DropChildrenPropertiesFromJSONSchema(oldSchema) newSchema, ok := b[prop] // In the event the property no longer exists on the new version @@ -91,8 +90,7 @@ func FlattenedCRDVersionDiff(a, b map[string]*apiextensionsv1.JSONSchemaProps) m // Do the same copy and unset logic for the new schema properties // before comparison to ensure we are only comparing the individual properties - newSchemaCopy := newSchema.DeepCopy() - newSchemaCopy.Properties = nil + newSchemaCopy := DropChildrenPropertiesFromJSONSchema(newSchema) if !equality.Semantic.DeepEqual(oldSchemaCopy, newSchemaCopy) { diffMap[prop] = Diff{Old: oldSchemaCopy, New: newSchemaCopy} @@ -102,6 +100,20 @@ func FlattenedCRDVersionDiff(a, b map[string]*apiextensionsv1.JSONSchemaProps) m return diffMap } +// DropChildrenPropertiesFromJSONSchema sets properties on a schema +// associated with children schemas to `nil`. Useful when calculating +// differences between a before and after of a given schema +// without the changes to its children schemas influencing the +// diff calculation. +// Returns a copy of the provided apiextensionsv1.JSONSchemaProps with children schemas dropped. +func DropChildrenPropertiesFromJSONSchema(schema *apiextensionsv1.JSONSchemaProps) *apiextensionsv1.JSONSchemaProps { + schemaCopy := schema.DeepCopy() + schemaCopy.Properties = nil + schemaCopy.Items = nil + + return schemaCopy +} + // SchemaWalkerFunc is a function that walks a JSONSchemaProps. // ancestry is an order list of ancestors of s, where index 0 is the root and index len-1 is the direct parent. type SchemaWalkerFunc func(s *apiextensionsv1.JSONSchemaProps, fldPath, simpleLocation *field.Path, ancestry []*apiextensionsv1.JSONSchemaProps) bool diff --git a/vendor/sigs.k8s.io/crdify/pkg/validators/version/served/served.go b/vendor/sigs.k8s.io/crdify/pkg/validators/version/served/served.go index 36c99c43e..bd6b4ff0d 100644 --- a/vendor/sigs.k8s.io/crdify/pkg/validators/version/served/served.go +++ b/vendor/sigs.k8s.io/crdify/pkg/validators/version/served/served.go @@ -81,7 +81,7 @@ func New(opts ...ValidatorOption) *Validator { } // Validate runs the validations configured in the Validator. -func (v *Validator) Validate(_, b *apiextensionsv1.CustomResourceDefinition) map[string]map[string][]validations.ComparisonResult { +func (v *Validator) Validate(a, b *apiextensionsv1.CustomResourceDefinition) map[string]map[string][]validations.ComparisonResult { result := map[string]map[string][]validations.ComparisonResult{} // If conversion webhook is specified and conversion policy is ignore, pass check @@ -89,24 +89,117 @@ func (v *Validator) Validate(_, b *apiextensionsv1.CustomResourceDefinition) map return result } - servedVersions := []apiextensionsv1.CustomResourceDefinitionVersion{} + aResults := v.compareVersionPairs(a) + bResults := v.compareVersionPairs(b) + subtractExistingIssues(bResults, aResults) - for _, version := range b.Spec.Versions { + return bResults +} + +func (v *Validator) compareVersionPairs(crd *apiextensionsv1.CustomResourceDefinition) map[string]map[string][]validations.ComparisonResult { + result := map[string]map[string][]validations.ComparisonResult{} + + for resultVersionPair, versions := range makeVersionPairs(crd) { + result[resultVersionPair] = validations.CompareVersions(versions[0], versions[1], v.unhandledEnforcement, v.comparators...) + } + + return result +} + +func makeVersionPairs(crd *apiextensionsv1.CustomResourceDefinition) map[string][2]apiextensionsv1.CustomResourceDefinitionVersion { + servedVersions := make([]apiextensionsv1.CustomResourceDefinitionVersion, 0, len(crd.Spec.Versions)) + + for _, version := range crd.Spec.Versions { if version.Served { servedVersions = append(servedVersions, version) } } + if len(servedVersions) < 2 { + return nil + } + slices.SortFunc(servedVersions, func(a, b apiextensionsv1.CustomResourceDefinitionVersion) int { return versionhelper.CompareKubeAwareVersionStrings(a.Name, b.Name) }) - for i, oldVersion := range servedVersions[:len(servedVersions)-1] { - for _, newVersion := range servedVersions[i+1:] { - resultVersion := fmt.Sprintf("%s <-> %s", oldVersion.Name, newVersion.Name) - result[resultVersion] = validations.CompareVersions(oldVersion, newVersion, v.unhandledEnforcement, v.comparators...) + pairs := make(map[string][2]apiextensionsv1.CustomResourceDefinitionVersion, numUnidirectionalPermutations(servedVersions)) + + for i, iVersion := range servedVersions[:len(servedVersions)-1] { + for _, jVersion := range servedVersions[i+1:] { + resultVersionPair := fmt.Sprintf("%s -> %s", iVersion.Name, jVersion.Name) + pairs[resultVersionPair] = [2]apiextensionsv1.CustomResourceDefinitionVersion{iVersion, jVersion} } } - return result + return pairs +} + +func numUnidirectionalPermutations[T any](in []T) int { + n := len(in) + + return (n * (n - 1)) / 2 +} + +// subtractExistingIssues removes errors and warnings from b's results that are also found in a's results. +func subtractExistingIssues(b, a map[string]map[string][]validations.ComparisonResult) { + sliceToMapByName := func(in []validations.ComparisonResult) map[string]*validations.ComparisonResult { + out := make(map[string]*validations.ComparisonResult, len(in)) + + for i := range in { + v := &in[i] + out[v.Name] = v + } + + return out + } + + for versionPair, bVersionPairResults := range b { + aVersionPairResults, ok := a[versionPair] + if !ok { + // If the version pair is not found in a, that means + // b introduced a new version, so we'll keep _all_ + // of the comparison results for this pair + continue + } + + for fieldPath, bFieldPathResults := range bVersionPairResults { + aFieldPathResults, ok := aVersionPairResults[fieldPath] + if !ok { + // If this field path is not found in a's results + // for this version pair, that means b introduced a new field + // in an existing schema, so we'll keep _all_ of the comparison + // results for this field path. + continue + } + + aResultMap := sliceToMapByName(aFieldPathResults) + bResultMap := sliceToMapByName(bFieldPathResults) + + for validationName, bValidationResult := range bResultMap { + aValidationResult, ok := aResultMap[validationName] + if !ok { + // If a's results do not include results for this validation, + // that means we ran a new validation for b that we did not + // run for a. We never intend to do that, so if that is somehow + // the case, let's panic and say what our programmer intent was. + panic(fmt.Sprintf("Validation %q not found in a's results for version pair %q. This should never happen because this validator uses the same validation configuration for CRDs a and b.", validationName, versionPair)) + } + + bValidationResult.Errors = slices.DeleteFunc(bValidationResult.Errors, func(bErr string) bool { + return slices.Contains(aValidationResult.Errors, bErr) + }) + if len(bValidationResult.Errors) == 0 { + bValidationResult.Errors = nil + } + + bValidationResult.Warnings = slices.DeleteFunc(bValidationResult.Warnings, func(bWarn string) bool { + return slices.Contains(aValidationResult.Warnings, bWarn) + }) + if len(bValidationResult.Warnings) == 0 { + bValidationResult.Warnings = nil + } + } + } + } }