feat: add opt-in toggle for managedFields inclusion in deserialization#4746
feat: add opt-in toggle for managedFields inclusion in deserialization#4746YuqiGuo105 wants to merge 1 commit intokubernetes-client:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: YuqiGuo105 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @YuqiGuo105! |
c939470 to
684d221
Compare
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
|
This file is auto generated from Kubernetes swagger, I think the fact that the field is missing means that the field must be missing from the swagger. We should fix that in the upstream swagger. |
Thanks for reviewing! I want to clarify the root cause — this is not a swagger / code generation issue. The managedFields field is present in both the upstream swagger and the generated V1ObjectMeta.java (line 105). The server also returns the field in the HTTP response body (confirmed by the reporter in the linked issue). The data is being silently dropped during Gson deserialization by V1MetadataExclusionStrategy — a hand-written (non-generated) class that tells Gson to skip the managedFields field on V1ObjectMeta. This strategy is injected into the generated JSON.java via json.diff after each code generation run. This was an intentional change introduced in 22.0.0 with no public opt-out API, which is what broke the users in the linked issue. This PR adds a simple toggle (JSON.setIncludeManagedFields(true)) while keeping the default behavior unchanged. |
| gson = gsonBuilder.create(); | ||
| } | ||
|
|
||
| static { |
There was a problem hiding this comment.
I don't think this part of the change is necessary (the method refactor etc) please remove.
| import io.kubernetes.client.openapi.models.V1ObjectMeta; | ||
|
|
||
| public class V1MetadataExclusionStrategy implements com.google.gson.ExclusionStrategy { | ||
| private final boolean includeManagedFields; |
There was a problem hiding this comment.
Rather than modifying this class, let's just not add the exclusion strategy if includeManagedFields is true in the JSON class below.
Since 22.0.0, V1MetadataExclusionStrategy has excluded managedFields from Gson (de)serialization by default to reduce noise. This broke users who rely on managedFields data. There was no supported way to re-enable them. Before this change, managedFields were silently dropped on every API response even when the server returned them. The only workaround required reflection-based hacking into Gson private internals. After this change, call JSON.setIncludeManagedFields(true) once at startup to opt in. The default remains false so all existing users are unaffected. Changes: - V1MetadataExclusionStrategy: add boolean includeManagedFields constructor param; when true, shouldSkipField() skips the exclusion check entirely - JSON: add static includeManagedFields flag (default false); extract the 700-line static initializer into initGson() helper; add public static setIncludeManagedFields(boolean) that flips the flag and reinitialises the shared Gson instance - JSONTest: three new tests covering default exclusion, opt-in inclusion, and restoration of default after toggling - scripts/patches/json.diff: updated to include the new hunks so the changes survive future OpenAPI code regeneration
684d221 to
642de36
Compare
Summary
Since 22.0.0,
V1MetadataExclusionStrategyhas silently excludedmanagedFieldsfrom Gson (de)serialization by default to reduce noise. This broke users who rely onmanagedFieldsdata and there was no supported way to re-enable them.Closes #3995
Workflow: Before this fix
managedFieldswere silently dropped on every API response with no recourse:The only workaround was reflection-based hacking into Gson's private
factoriesfield.Workflow: After this fix
Call
JSON.setIncludeManagedFields(true)once at startup to opt in. Everything else is unchanged:To restore the default (exclude
managedFields):The default remains
false(managedFields excluded) so all existing users are unaffected.Changes
V1MetadataExclusionStrategy.javaboolean includeManagedFieldsconstructor param; whentrue,shouldSkipField()returnsfalseimmediately (no exclusion)JSON.javaincludeManagedFieldsflag (defaultfalse); extract 700-linestatic {}intoinitGson()helper; addpublic static setIncludeManagedFields(boolean)that flips the flag and reinitialises the shared Gson instanceJSONTest.javascripts/patches/json.diff