Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeepCopy of JSONSchemaProps doesn't deeply copy XValidations. #107954

Closed
benluddy opened this issue Feb 4, 2022 · 8 comments · Fixed by #107956
Closed

DeepCopy of JSONSchemaProps doesn't deeply copy XValidations. #107954

benluddy opened this issue Feb 4, 2022 · 8 comments · Fixed by #107956
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@benluddy
Copy link
Contributor

benluddy commented Feb 4, 2022

Failure cluster 89c7abe10b4e83db5885

Error text:
=== RUN   TestRoundTrip
    roundtrip.go:136: starting group ""
    roundtrip.go:149: finished group ""
    roundtrip.go:136: starting group "apiextensions.k8s.io"
    roundtrip.go:255: 	round tripping to apiextensions.k8s.io/v1, Kind=CustomResourceDefinition v1.CustomResourceDefinition
    roundtrip.go:255: 	round tripping to apiextensions.k8s.io/v1beta1, Kind=CustomResourceDefinition v1beta1.CustomResourceDefinition
    roundtrip.go:255: 	round tripping to apiextensions.k8s.io/v1, Kind=CustomResourceDefinition v1.CustomResourceDefinition
    roundtrip.go:255: 	round tripping to apiextensions.k8s.io/v1beta1, Kind=CustomResourceDefinition v1beta1.CustomResourceDefinition
    roundtrip.go:255: 	round tripping to apiextensions.k8s.io/v1, Kind=CustomResourceDefinition v1.CustomResourceDefinition
    roundtrip.go:409: CustomResourceDefinition: fuzzing a copy altered the original, diff:   &apiextensions.CustomResourceDefinition{

          						XValidations: apiextensions.ValidationRules{
        - 							{Rule: "ɪI=ǯ剢鿠>璼ĨĊ:xx", Message: "腺嬑.ú^x"},
        + 							{Rule: "ɪI=ǯ剢鿠>璼ĨĊ:x", Message: "腺嬑.ú^"},
          						},

Recent failures:

2/4/2022, 5:54:09 AM ci-kubernetes-generate-make-test-count10
2/3/2022, 11:54:15 PM ci-kubernetes-generate-make-test-count10
2/3/2022, 5:54:03 PM ci-kubernetes-generate-make-test-count10
2/3/2022, 5:53:28 AM ci-kubernetes-generate-make-test-count10
2/2/2022, 11:51:28 AM ci-kubernetes-generate-make-test-count10

/kind failing-test
/kind bug
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 4, 2022
@benluddy
Copy link
Contributor Author

benluddy commented Feb 4, 2022

/assign

@liggitt
Copy link
Member

liggitt commented Feb 5, 2022

hmm... latest run of the count10 job with this commit failed:

k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/validation: TestRoundTrip

=== RUN   TestRoundTrip
    validation_test.go:50: seed: 3916589616287113937
    validation_test.go:95: 13: expected
        	&apiextensions.JSONSchemaProps{ID:"AůO画Ĝ}ɽŘ竄", Schema:"Ð", Ref:(*string)(0xc00047e650), Description:"棭榎ȒPɐƓR", Type:"", Nullable:true, Format:"¥Ǣ臄32枍Ê鎍r覝", Title:"æ飷", Default:(*apiextensions.JSON)(0xc00047e630), Maximum:(*float64)(nil), ExclusiveMaximum:false, Minimum:(*float64)(nil), ExclusiveMinimum:true, MaxLength:(*int64)(nil), MinLength:(*int64)(nil), Pattern:"ʤ:|;", MaxItems:(*int64)(nil), MinItems:(*int64)(nil), UniqueItems:false, MultipleOf:(*float64)(nil), Enum:[]apiextensions.JSON(nil), MaxProperties:(*int64)(nil), MinProperties:(*int64)(nil), Required:[]string(nil), Items:(*apiextensions.JSONSchemaPropsOrArray)(nil), AllOf:[]apiextensions.JSONSchemaProps(nil), OneOf:[]apiextensions.JSONSchemaProps(nil), AnyOf:[]apiextensions.JSONSchemaProps(nil), Not:(*apiextensions.JSONSchemaProps)(nil), Properties:map[string]apiextensions.JSONSchemaProps(nil), AdditionalProperties:(*apiextensions.JSONSchemaPropsOrBool)(nil), PatternProperties:map[string]apiextensions.JSONSchemaProps(nil), Dependencies:apiextensions.JSONSchemaDependencies(nil), AdditionalItems:(*apiextensions.JSONSchemaPropsOrBool)(nil), Definitions:apiextensions.JSONSchemaDefinitions(nil), ExternalDocs:(*apiextensions.ExternalDocumentation)(nil), Example:(*apiextensions.JSON)(0xc00047e640), XPreserveUnknownFields:(*bool)(nil), XEmbeddedResource:true, XIntOrString:true, XListMapKeys:[]string(nil), XListType:(*string)(nil), XMapType:(*string)(nil), XValidations:apiextensions.ValidationRules{apiextensions.ValidationRule{Rule:"v7愶u搱k]Ǖ\"ã", Message:"蘷夜 ƪ鱽Ó盶=D]"}}}, got 
        	&apiextensions.JSONSchemaProps{ID:"AůO画Ĝ}ɽŘ竄", Schema:"Ð", Ref:(*string)(0xc00047eaa0), Description:"棭榎ȒPɐƓR", Type:"", Nullable:true, Format:"¥Ǣ臄32枍Ê鎍r覝", Title:"æ飷", Default:(*apiextensions.JSON)(0xc00047ead0), Maximum:(*float64)(nil), ExclusiveMaximum:false, Minimum:(*float64)(nil), ExclusiveMinimum:true, MaxLength:(*int64)(nil), MinLength:(*int64)(nil), Pattern:"ʤ:|;", MaxItems:(*int64)(nil), MinItems:(*int64)(nil), UniqueItems:false, MultipleOf:(*float64)(nil), Enum:[]apiextensions.JSON(nil), MaxProperties:(*int64)(nil), MinProperties:(*int64)(nil), Required:[]string(nil), Items:(*apiextensions.JSONSchemaPropsOrArray)(nil), AllOf:[]apiextensions.JSONSchemaProps(nil), OneOf:[]apiextensions.JSONSchemaProps(nil), AnyOf:[]apiextensions.JSONSchemaProps(nil), Not:(*apiextensions.JSONSchemaProps)(nil), Properties:map[string]apiextensions.JSONSchemaProps(nil), AdditionalProperties:(*apiextensions.JSONSchemaPropsOrBool)(nil), PatternProperties:map[string]apiextensions.JSONSchemaProps(nil), Dependencies:apiextensions.JSONSchemaDependencies(nil), AdditionalItems:(*apiextensions.JSONSchemaPropsOrBool)(nil), Definitions:apiextensions.JSONSchemaDefinitions(nil), ExternalDocs:(*apiextensions.ExternalDocumentation)(nil), Example:(*apiextensions.JSON)(0xc00047eb00), XPreserveUnknownFields:(*bool)(nil), XEmbeddedResource:true, XIntOrString:true, XListMapKeys:[]string(nil), XListType:(*string)(nil), XMapType:(*string)(nil), XValidations:apiextensions.ValidationRules{apiextensions.ValidationRule{Rule:"", Message:""}}}
--- FAIL: TestRoundTrip (0.06s)

@benluddy
Copy link
Contributor Author

benluddy commented Feb 5, 2022

Interesting, that's a different TestRoundTrip? 👀

@benluddy
Copy link
Contributor Author

benluddy commented Feb 5, 2022

It seems to be getting lost here:

if err := json.Unmarshal(openAPIJSON, external); err != nil {

@liggitt
Copy link
Member

liggitt commented Feb 5, 2022

yeah... this is squirrely... that test is doing the following:

  1. fuzzing internal types
  2. converting them to an openapi struct (which populates the extension x-kubernetes-validations pointing to internal ValidationRule structs)
  3. serializes that to openapi json (which serializes Rule and Message in upper-case because there are no json tags on that struct)
  4. deserializes that to v1 JSONSchemaProps, which expects those fields to be rule and message

@liggitt
Copy link
Member

liggitt commented Feb 5, 2022

I'll open another PR strengthening the test, and will tag in @sttts to figure out what the right thing to do here is

this is the first structured extension we're adding that had subfields where capitalization mattered... I'm not 100% sure what we're doing here (serializing the internal types) is correct

@liggitt
Copy link
Member

liggitt commented Feb 5, 2022

PR in progress in #107970

@fedebongio
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants