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

Mock server doesn't take KubernetesSerialization customisation into account #5293

Closed
metacosm opened this issue Jun 29, 2023 · 7 comments · Fixed by #5310 or #5349
Closed

Mock server doesn't take KubernetesSerialization customisation into account #5293

metacosm opened this issue Jun 29, 2023 · 7 comments · Fixed by #5310 or #5349
Assignees
Milestone

Comments

@metacosm
Copy link
Collaborator

Describe the bug

This is particularly annoying in Quarkus' context where you'd expect the customised bean to be injected in the client returned by the mock server.

Fabric8 Kubernetes Client version

SNAPSHOT

Steps to reproduce

Customise the KubernetesSerialization, use the mock server, realise that the server doesn't use the custom configuration.

Expected behavior

The mock server should use the custom serialisation configuration.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.25.3@latest

Environment

macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

@shawkins
Copy link
Contributor

My initial thought is that the mock server relies on generic parsing / jsonnode manipulation, then what it returns to the client will be parsed again. Is that leaving some unexpected artifacts in the returned json that you expect KuberentesSerialization would have addressed?

@shawkins
Copy link
Contributor

shawkins commented Jul 5, 2023

Double-checked the parsing that is being done, KubernetesAttributesExtractor.toKubernetesResource is problematic wrt Serialization / KubernetesSerialization as it does not specify generic or jsonnode parsing. All of the other locations seem fine. Are you hitting a probem related to KubernetesAttributesExtractor?

shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jul 5, 2023
@shawkins shawkins self-assigned this Jul 5, 2023
@manusa manusa added this to the 6.8.0 milestone Jul 5, 2023
manusa pushed a commit that referenced this issue Jul 5, 2023
@jonsie
Copy link

jonsie commented Jul 20, 2023

If I want to use a KubernetesSerialization that has ObjectMapper configured with a specific module (like KotlinModule), how does this fix address that? While the application itself seems to run fine since we can provide a KubernetesSerialization object to the client at boot, we can't unit test CRDs that use Kotlin data classes. Any thoughts?

@shawkins
Copy link
Contributor

we can't unit test CRDs that use Kotlin data classes. Any thoughts?

Are you saying that your CRDs will fail to parse or that you expect the mock server to do validation?

The mock server does not do any CR validation - it uses only a minimal metadata from CRDs.

@jonsie
Copy link

jonsie commented Jul 20, 2023

Basically, when using the KubernetesMockServerExtension (via @EnableKubernetesMockClient) the KubernetesClient that it bootstraps is non-configurable WRT Jackson. In previous versions we were able to configure the Serialization singleton via #jsonMapper - while this was ugly it did allow us to add custom modules. Now, we are blocked on a few services upgrading because we can not customize the KubernetesClient KubernetesSerialization used in tests. Essentially, we have CRDs that are composed of Kotlin data classes, or we have enabled/disabled certain serialization features, etc. While we can configure the ObjectMapper (via KubernetesSerialization) for the app at run time, we can't configure the test client for unit tests.

Is there something I'm missing or is it just currently not-configurable? I assume this issue raised here is the same issue I'm referring to, but could be mistaken.

@shawkins
Copy link
Contributor

Basically, when using the KubernetesMockServerExtension (via @EnableKubernetesMockClient) the KubernetesClient that it bootstraps is non-configurable WRT Jackson.

Ah, yes when using annotations like KubernetesTest or EnableKubernetesMockClient the client is being injected without a way to configure it.

I assume this issue raised here is the same issue I'm referring to, but could be mistaken.

The change made for this pr was only about the deserialization being performed by the mock server.

This is a general issue with the client building logic - anything that a user may be doing to customize their clients via the Config is also not avaialable.

The quickest solution would be adding more methods like KubernetesMockServer.createClient - one that takes a KubernetesSerialization, or one that takes a KubernetesClientBuilder (but we may have to consider merging the Config). Then instead of relying on the injected client, you'd have to call that method to create your specialized one.

@jonsie
Copy link

jonsie commented Jul 20, 2023

The quickest solution would be adding more methods like KubernetesMockServer.createClient - one that takes a KubernetesSerialization, or one that takes a KubernetesClientBuilder (but we may have to consider merging the Config). Then instead of relying on the injected client, you'd have to call that method to create your specialized one.

KubernetesClientBuiler would work. Honestly even just an option to specify KubernetesSerialization would be fine, and leave the Config alone. For the most part Jackson config is more important since that has overlap in the application domain (which impacts unit testing) whereas Config is more environment specific (though being able to specify all the client fields in the mock client would be useful).

shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jul 20, 2023
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jul 20, 2023
manusa pushed a commit that referenced this issue Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants