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

KubernetesMockServer Kubernetes client customization #5349

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

manusa
Copy link
Member

@manusa manusa commented Jul 21, 2023

Description

Supersedes closes #5347
Fixes #5293

Built on top of #5347 to provide a user-friendlier approach to customize the KubernetesClientBuilder instance used by the KubernetesMockServer and the KubernetesMockServerExtension.

The easiest way to leverage the new functionality is by using the enhanced @EnableKubernetesMockClient where you can now pass a reference to a class implementing Consumer<KubernetesClientBuilder> to customize the original KubernetesClientBuilder provided by the KubernetesMockServerExtension.

The following code snippet is an example of such use case:

@EnableKubernetesMockClient(crud = true, kubernetesClientBuilderCustomizer = CustomBuilder.class)
class TheTest {
  // ...
}
class CustomBuilder implements Consumer<KubernetesClientBuilder> {
  @Override
  public void accept(KubernetesClientBuilder builder) {
    /* customize at will */;
  }
}

/cc @jonsie

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

shawkins and others added 3 commits July 20, 2023 14:27
This suite tests exactly the same as `KubernetesMockServerExtensionClientAndServerStaticTests`

Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa added this to the 6.8.0 milestone Jul 21, 2023
@manusa manusa force-pushed the feat/mock-server-client-customizer branch from 80d09be to 396edbc Compare July 21, 2023 10:07
@manusa manusa self-assigned this Jul 21, 2023
@shawkins
Copy link
Contributor

The NamespaceExtension arguably has the same issue.

Also let's just remove the OpenShiftMockServer rather than update it.

@manusa
Copy link
Member Author

manusa commented Jul 21, 2023

The NamespaceExtension arguably has the same issue.

Also let's just remove the OpenShiftMockServer rather than update it.

Agreed to both. However, I'd rather tackle those separately (One new issue for each).
If this PR looks good for the scoped #5293 context I'd rather merge and be able to release along with v6.8.0 on Monday.

@shawkins
Copy link
Contributor

Agreed to both. However, I'd rather tackle those separately (One new issue for each).

Let's give a little thought to the NamespaceExtension before proceeding. Is your plan to similarly change the annotation and add another interface or base class to replace KubernetesMockClientKubernetesClientBuilder? It might be nice to have them be the same to more easily switch from a crud mock to a kubernetestest - maybe provide the whole Config or ConfigBuilder rather than just the url string to the customizer.

Also https://github.com/fabric8io/kubernetes-client/pull/5349/files#diff-9ef31fbfb8fbafe18e5f18f5daa44de9705f3850a2e086aed2d123030a9448deR128 will need some documentation - if someone is using the mockserver manually (not through the annotation), then usage of this method is non-obvious as they likely need to extend KubernetesMockClientKubernetesClientBuilder

@manusa
Copy link
Member Author

manusa commented Jul 21, 2023

I'm not convinced either about the String parameter. Sounds reasonable.
Let me add a further commit with some minor refinements.

@manusa
Copy link
Member Author

manusa commented Jul 21, 2023

3d178e6

should address the expressed concerns

@shawkins
Copy link
Contributor

should address the expressed concerns

It seems like the customizer could just be Consumer - that would work for both the mock server and kubernetestest. Customizing the Config could be done seperately - you could either also provide a plugable Consumer or like other builders the KubernetesClientBuilder could directly provide an editConfig method.

@manusa
Copy link
Member Author

manusa commented Jul 21, 2023

I'm not really following why the current approach won't work for the @KubernetesTest annotation+NamespaceExtension.

In NamespaceExtension we would do something in the lines of kubernetesClientBuilderInstantiator.apply(new Config()). This would even enable users to configure their own cluster instead of relying on autoConfig.

Maybe better to have a quick sync.

@manusa
Copy link
Member Author

manusa commented Jul 21, 2023

Customizing the Config could be done separately

As agreed in internal meeting, let's defer the Config overriding/editing to a follow-up.

Tasks:

@manusa manusa force-pushed the feat/mock-server-client-customizer branch from 3d178e6 to 74dcb05 Compare July 21, 2023 12:19
Copy link
Contributor

@shawkins shawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for pushing to wire this into the annotation.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

82.6% 82.6% Coverage
0.0% 0.0% Duplication

@manusa manusa deleted the feat/mock-server-client-customizer branch July 21, 2023 14:06
@jonsie
Copy link

jonsie commented Jul 21, 2023

Much appreciated, will keep an eye out for the release.

@jonsie
Copy link

jonsie commented Jul 24, 2023

Thanks for the release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mock server doesn't take KubernetesSerialization customisation into account
4 participants