diff --git a/tools/go.mod b/tools/go.mod index 0c330635687..c87a7bdf4da 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -13,7 +13,7 @@ require ( github.com/golangci/golangci-lint/v2 v2.1.6 github.com/google/go-cmp v0.7.0 github.com/mikefarah/yq/v4 v4.44.5 - github.com/openshift/crd-schema-checker v0.0.0-20241113192003-573763d3107a + github.com/openshift/crd-schema-checker v0.0.0-20250905140724-c313b6407231 github.com/russross/blackfriday v2.0.0+incompatible github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 github.com/spf13/cobra v1.9.1 diff --git a/tools/go.sum b/tools/go.sum index d269c6125ce..89a369e4a69 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -515,8 +515,8 @@ github.com/onsi/gomega v1.36.3 h1:hID7cr8t3Wp26+cYnfcjR6HpJ00fdogN6dqZ1t6IylU= github.com/onsi/gomega v1.36.3/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0= github.com/openshift/controller-tools v0.12.1-0.20250402141027-24f590ca0886 h1:Ez6hT01wFM+C1O/tY+n8AugjOTvtqgovqUFOyaF6ObI= github.com/openshift/controller-tools v0.12.1-0.20250402141027-24f590ca0886/go.mod h1:LkvYw1gDIjjhvamu3BGP9z4bgWbtOtvK7KNF3kw1Shg= -github.com/openshift/crd-schema-checker v0.0.0-20241113192003-573763d3107a h1:gBheMF1vVwxfnuGHJ82f/nmUATdtewq60/yhbBqD4+M= -github.com/openshift/crd-schema-checker v0.0.0-20241113192003-573763d3107a/go.mod h1:sTxJ4ZFW9r9fEdbW2v0yMRi6NcyTbx0fII4p83IQ+L8= +github.com/openshift/crd-schema-checker v0.0.0-20250905140724-c313b6407231 h1:8lSGufji9rfiyDxtUl7A4uOyeeP4x0UOOXcsDBFfkGI= +github.com/openshift/crd-schema-checker v0.0.0-20250905140724-c313b6407231/go.mod h1:sTxJ4ZFW9r9fEdbW2v0yMRi6NcyTbx0fII4p83IQ+L8= github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw= github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU= github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w= @@ -986,14 +986,6 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 h1:jpcvIRr3GLoUo sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 h1:/Rv+M11QRah1itp8VhT6HoVx1Ray9eB4DBr+K+/sCJ8= sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3/go.mod h1:18nIHnGi6636UCz6m8i4DhaJ65T6EruyzmoQqI2BVDo= -sigs.k8s.io/kube-api-linter v0.0.0-20250722142827-cf6e50475daa h1:EUPhcYdUXVwMMaTVWG+HCzKlKJRgi7r+gvGvZmzNkTY= -sigs.k8s.io/kube-api-linter v0.0.0-20250722142827-cf6e50475daa/go.mod h1:Jxl3NU9lRf9WJ8dgwgF4U6tLF229jR/KEvtxSwRAKnE= -sigs.k8s.io/kube-api-linter v0.0.0-20250723124227-8eacb1639327 h1:hKuD0+fceBSqCy2Pqe0ifEOTewWIbAOgIVkH9xzbyJw= -sigs.k8s.io/kube-api-linter v0.0.0-20250723124227-8eacb1639327/go.mod h1:Jxl3NU9lRf9WJ8dgwgF4U6tLF229jR/KEvtxSwRAKnE= -sigs.k8s.io/kube-api-linter v0.0.0-20250723124831-1b29e82a0f55 h1:kD9x5uu1/A7wvhwPcuSBk1UiG5wq/nstFxgYALOeZ/Q= -sigs.k8s.io/kube-api-linter v0.0.0-20250723124831-1b29e82a0f55/go.mod h1:Jxl3NU9lRf9WJ8dgwgF4U6tLF229jR/KEvtxSwRAKnE= -sigs.k8s.io/kube-api-linter v0.0.0-20250729132427-47bfeef6cd38 h1:5WuFSvNbquqwM82aBQ36AfsFGsf2Jc0OJM4SCC2rw4w= -sigs.k8s.io/kube-api-linter v0.0.0-20250729132427-47bfeef6cd38/go.mod h1:Jxl3NU9lRf9WJ8dgwgF4U6tLF229jR/KEvtxSwRAKnE= sigs.k8s.io/kube-api-linter v0.0.0-20250808120943-48643eb2563d h1:BcgCRoMLmIxRTLokQ1K1LAle+21fKPqgA6OvzN04xEg= sigs.k8s.io/kube-api-linter v0.0.0-20250808120943-48643eb2563d/go.mod h1:Jxl3NU9lRf9WJ8dgwgF4U6tLF229jR/KEvtxSwRAKnE= sigs.k8s.io/randfill v0.0.0-20250304075658-069ef1bbf016/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY= diff --git a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/defaultcomparators/interfaces.go b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/defaultcomparators/interfaces.go index 5fbd6ae6e09..44904c1b463 100644 --- a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/defaultcomparators/interfaces.go +++ b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/defaultcomparators/interfaces.go @@ -16,6 +16,7 @@ func NewDefaultComparators() manifestcomparators.CRDComparatorRegistry { must(ret.AddComparator(manifestcomparators.NoFieldRemoval())) must(ret.AddComparator(manifestcomparators.NoEnumRemoval())) must(ret.AddComparator(manifestcomparators.NoMaps())) + must(ret.AddComparator(manifestcomparators.NoDataTypeChange())) must(ret.AddComparator(manifestcomparators.MustHaveStatus())) must(ret.AddComparator(manifestcomparators.ListsMustHaveSSATags())) must(ret.AddComparator(manifestcomparators.ConditionsMustHaveProperSSATags())) diff --git a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_must_not_exceed_cost_budget.go b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_must_not_exceed_cost_budget.go index d1db0ff9de2..cdcaf9f6270 100644 --- a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_must_not_exceed_cost_budget.go +++ b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_must_not_exceed_cost_budget.go @@ -54,6 +54,11 @@ func (b mustNotExceedCostBudget) Validate(crd *apiextensionsv1.CustomResourceDef return false } + if schema.XValidations == nil { + // There are no XValidations at this level, we do not need to continue with checks. + return false + } + schemaInfos, schemaWarnings, err := inspectSchema(schema, simpleLocation, len(ancestry) == 0) if err != nil { errsToReport = append(errsToReport, err.Error()) @@ -280,10 +285,6 @@ func isUnboundedCardinality(schema *apiextensions.JSONSchemaProps) bool { } func inspectSchema(schema *apiextensions.JSONSchemaProps, simpleLocation *field.Path, isRoot bool) ([]string, []string, error) { - if schema.XValidations == nil { - return nil, nil, nil - } - typeInfo, err := getDeclType(schema, isRoot) if err != nil { return nil, nil, err diff --git a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_data_type_change.go b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_data_type_change.go new file mode 100644 index 00000000000..a261a8c4071 --- /dev/null +++ b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_data_type_change.go @@ -0,0 +1,90 @@ +package manifestcomparators + +import ( + "fmt" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +type noDataTypeChange struct{} + +func NoDataTypeChange() CRDComparator { + return noDataTypeChange{} +} + +func (noDataTypeChange) Name() string { + return "NoDataTypeChange" +} + +func (noDataTypeChange) WhyItMatters() string { + return "If the data type of fields are changed, then clients that rely on those fields will not be able to read them or write them." +} + +func getFieldsAndTypes(version *apiextensionsv1.CustomResourceDefinitionVersion) map[string]string { + fieldsAndTypes := make(map[string]string) + SchemaHas(version.Schema.OpenAPIV3Schema, field.NewPath("^"), field.NewPath("^"), nil, + func(s *apiextensionsv1.JSONSchemaProps, fldPath, simpleLocation *field.Path, _ []*apiextensionsv1.JSONSchemaProps) bool { + fieldsAndTypes[simpleLocation.String()] = s.Type + return false + }) + + return fieldsAndTypes +} + +type TypeChange struct { + ExistingType string + NewType string +} + +func getChangedTypes(existingFieldsAndTypes map[string]string, newFieldsAndTypes map[string]string) map[string]TypeChange { + changedTypes := make(map[string]TypeChange) + + for existingField, existingType := range existingFieldsAndTypes { + if newType, ok := newFieldsAndTypes[existingField]; ok { + if existingType != newType { + changedTypes[existingField] = TypeChange{ExistingType: existingType, NewType: newType} + } + } + } + + return changedTypes +} + +func (b noDataTypeChange) Compare(existingCRD, newCRD *apiextensionsv1.CustomResourceDefinition) (ComparisonResults, error) { + if existingCRD == nil { + return ComparisonResults{ + Name: b.Name(), + WhyItMatters: b.WhyItMatters(), + + Errors: nil, + Warnings: nil, + Infos: nil, + }, nil + } + errsToReport := []string{} + + for _, newVersion := range newCRD.Spec.Versions { + existingVersion := GetVersionByName(existingCRD, newVersion.Name) + if existingVersion == nil { + continue + } + + existingFieldsAndTypes := getFieldsAndTypes(existingVersion) + newFieldsAndTypes := getFieldsAndTypes(&newVersion) + + changedTypes := getChangedTypes(existingFieldsAndTypes, newFieldsAndTypes) + for changedField, changedType := range changedTypes { + errsToReport = append(errsToReport, fmt.Sprintf("crd/%v version/%v data type of field/%v may not be changed from %v to %v", newCRD.Name, newVersion.Name, changedField, changedType.ExistingType, changedType.NewType)) + } + } + + return ComparisonResults{ + Name: b.Name(), + WhyItMatters: b.WhyItMatters(), + + Errors: errsToReport, + Warnings: nil, + Infos: nil, + }, nil +} diff --git a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_new_required_fields.go b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_new_required_fields.go index f6fad3c315d..8e3825bd81a 100644 --- a/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_new_required_fields.go +++ b/tools/vendor/github.com/openshift/crd-schema-checker/pkg/manifestcomparators/comp_no_new_required_fields.go @@ -24,6 +24,33 @@ func (noNewRequiredFields) WhyItMatters() string { "CRD defaulting requires allowing an object with an empty or missing value to then get defaulted." } +// isFieldOptional checks if a field is optional (ie not required by its parent) +func isFieldOptional( + s *apiextensionsv1.JSONSchemaProps, + ancestors []*apiextensionsv1.JSONSchemaProps, + fldPath *field.Path, + newToFldPath map[*apiextensionsv1.JSONSchemaProps]*field.Path, + newFldPathToRequiredFields map[string]sets.Set[string]) bool { + + // Check if field is an optional array + if s.Type == "array" && (s.MinLength == nil || *s.MinLength == 0) { + return true + } + + // Check if field is not required by its parent + if len(ancestors) > 0 { + parentOfField := ancestors[len(ancestors)-1] + groups := lastIndexOrKeyRegexp.FindStringSubmatch(fldPath.String()) + if len(groups) == 2 { + lastStep := groups[1] + parentRequiredFields := newFldPathToRequiredFields[newToFldPath[parentOfField].String()] + return !parentRequiredFields.Has(lastStep) + } + } + + return false +} + func (b noNewRequiredFields) Compare(existingCRD, newCRD *apiextensionsv1.CustomResourceDefinition) (ComparisonResults, error) { if existingCRD == nil { return ComparisonResults{ @@ -97,10 +124,13 @@ func (b noNewRequiredFields) Compare(existingCRD, newCRD *apiextensionsv1.Custom return false } - existingRequired, existedBefore := existingRequiredFields[fldPath.String()] - if !existedBefore && s.Nullable { - // if the parent of the required field (current element) didn't exist in the schema before AND - // if the parent of the required field is nullable (client doesn't have to set it), + existingRequired, _ := existingRequiredFields[fldPath.String()] + + // Check if this field actually existed in the old schema + _, fieldExistedBefore := existingFldPathToJSONSchemaProps[fldPath.String()] + + if !fieldExistedBefore && isFieldOptional(s, ancestors, fldPath, newToFldPath, newFldPathToRequiredFields) { + // if the parent of the required field didn't exist before AND is optional, // then we can allow a child to be required. return false } @@ -150,10 +180,10 @@ func isAnyAncestorNewAndNullable( for i := len(ancestors) - 1; i >= 0; i-- { ancestor := ancestors[i] ancestorFldPath := newToFldPath[ancestor] - isOptionalArray := ancestor.Type == "array" && (ancestor.MinLength == nil || *ancestor.MinLength == 0) - isAncestoryOptional := ancestor.Nullable || isOptionalArray - if !isAncestoryOptional { - // if this ancestor isn't nullable, then it cannot allow the current element to be required + + // check if the ancestor is optional + if !isFieldOptional(ancestor, ancestors[:i], ancestorFldPath, newToFldPath, newFldPathToRequiredFields) { + // if this ancestor isn't optional, then it cannot allow the current element to be required continue } diff --git a/tools/vendor/modules.txt b/tools/vendor/modules.txt index 6a4d8d9af2e..b7c3f81ec5f 100644 --- a/tools/vendor/modules.txt +++ b/tools/vendor/modules.txt @@ -1086,7 +1086,7 @@ github.com/nunnatsa/ginkgolinter/version # github.com/olekukonko/tablewriter v0.0.5 ## explicit; go 1.12 github.com/olekukonko/tablewriter -# github.com/openshift/crd-schema-checker v0.0.0-20241113192003-573763d3107a +# github.com/openshift/crd-schema-checker v0.0.0-20250905140724-c313b6407231 ## explicit; go 1.22.0 github.com/openshift/crd-schema-checker/pkg/cmd/options github.com/openshift/crd-schema-checker/pkg/defaultcomparators