fix #3859: refinements to how a deserialization class is chosen #3899
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shawkins
force-pushed
the
3859
branch
4 times, most recently
from
e3d3cc7
to
d1540fa
Compare
|
With the latest commit there's still a failure - we're trying to parse a Template, but it's class does not have the expected Group template.openshift.io annotation. I can either further relax the parsing, but it seems like that and any other annotation issue should get fixed. |
11 tasks
|
Just so this pr could be standalone, I updated the Template class group. Also updated the apiVersion for the pod test that was failing - and added to the migration guide that the api group name is now strict. |
|
Kudos, SonarCloud Quality Gate passed! |
manusa
reviewed
Mar 2, 2022
Comment on lines
+34
to
+36
| ## Deserialization Resolution | ||
|
|
||
| The group on the object being deserialized is not required to match the prospective class - even for built-in types. This prevents the unintentional parsing of custom types without a registered class as a built-in type of the same name. This also means that you must ensure the apiVersion values are correct on the objects you are deserializing as they will no longer resolve to built-in type of the same name when there is a mistake. |
There was a problem hiding this comment.
Note to self, add this to 5.12.2 changelog if backproted
manusa
approved these changes
Mar 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This updates the search strategy used by the KubernetesDeserializer - to not use classes with conflicting apiGroups.
It goes further to not use classes with conflicting versions - but that leads to several test failures. That was a concern of @andreaTP when dealing with multiple versions - if they are custom resources which do not implement additionalProperties, then it could be a lossful conversion (if ignoring unmatched fields) or an exception when fields don't match.Sorry I wasn't thinking straight, the internal types are only going to be from what's in the PACKAGES, so any laxity made there on version matching won't affect custom types.
Type of change
test, version modification, documentation, etc.)
Checklist