Skip to content

Commit

Permalink
fix(common): KubernetesHelper.exportKubernetesClientConfigToFile shou…
Browse files Browse the repository at this point in the history
…ld not set certificate related fields in NamedCluster when `trustCerts=true`

While adapting tests in HelmService for install goal, I'm facing
problems with exported kubeconfig file. It is getting rejected by
Kubernetes as invalid kubeconfig file.

In KubernetesMockServer tests, we enable `trustCerts` option to skip certificate
verification. However, ClusterConfiguration creates a KubernetesClient config object
by merging opinionated default with user provided cluster attributes,
this causes resulting config to contain additional fields from user's
kubernetes environment.

To work around this error, we should not set `certificateAuthority` and
`certificateAuthorityData` fields whenever `trustCerts` is enabled.

Signed-off-by: Rohan Kumar <[email protected]>
  • Loading branch information
rohanKanojia authored Jul 12, 2024
1 parent ee6f352 commit 2b5d271
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -852,10 +852,10 @@ private static NamedCluster createKubeConfigClusterFromClient(io.fabric8.kuberne
if (StringUtils.isNotBlank(kubernetesClientConfig.getMasterUrl())) {
clusterBuilder.withServer(kubernetesClientConfig.getMasterUrl());
}
if (StringUtils.isNotBlank(kubernetesClientConfig.getCaCertFile())) {
if (StringUtils.isNotBlank(kubernetesClientConfig.getCaCertFile()) && !kubernetesClientConfig.isTrustCerts()) {
clusterBuilder.withCertificateAuthority(kubernetesClientConfig.getCaCertFile());
}
if (StringUtils.isNotBlank(kubernetesClientConfig.getCaCertData())) {
if (StringUtils.isNotBlank(kubernetesClientConfig.getCaCertData()) && !kubernetesClientConfig.isTrustCerts()) {
clusterBuilder.withCertificateAuthorityData(kubernetesClientConfig.getCaCertData());
}
if (kubernetesClientConfig.isTrustCerts()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,27 @@ void exportKubernetesClientConfigToFile_whenValidTargetFile_thenWriteKubeConfigT
.hasFieldOrPropertyWithValue("user.authProvider.config.key1", "value1"));
}

@Test
@DisplayName("should work with valid kube config")
void exportKubernetesClientConfigToFile_whenTrustCertsEnabled_thenDoNotAddCertData(@TempDir Path temporaryFolder) throws IOException {
// Given
io.fabric8.kubernetes.client.Config kubernetesClientConfig = createKubernetesClientConfig();
kubernetesClientConfig.setTrustCerts(true);

// When
Path exportedKubeConfig = KubernetesHelper.exportKubernetesClientConfigToFile(kubernetesClientConfig, temporaryFolder.resolve("config"));

// Then
assertThat(exportedKubeConfig).isNotNull();
assertThat(Serialization.unmarshal(exportedKubeConfig.toFile(), io.fabric8.kubernetes.api.model.Config.class))
.satisfies(c -> assertThat(c.getClusters())
.singleElement(InstanceOfAssertFactories.type(NamedCluster.class))
.hasFieldOrPropertyWithValue("name", "example-openshiftapps-com:6443")
.hasFieldOrPropertyWithValue("cluster.server", "https://example-openshiftapps-com:6443/")
.hasFieldOrPropertyWithValue("cluster.certificateAuthority", null)
.hasFieldOrPropertyWithValue("cluster.certificateAuthorityData", null));
}

private Config createKubernetesClientConfig() {
NamedContext currentContext = new NamedContextBuilder().withName("cluster1-context")
.withNewContext()
Expand Down

0 comments on commit 2b5d271

Please sign in to comment.