From d569bf22ae069b6bf9f2b31c932ef81053dad266 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Fri, 8 Nov 2024 09:56:45 +0100 Subject: [PATCH] refactor(config): OpenIDConnectionUtils.persistOAuthToken should not persist values that no longer exist Signed-off-by: Marc Nuri --- .../client/internal/KubeConfigUtils.java | 2 +- .../client/utils/OpenIDConnectionUtils.java | 31 ++++++++---------- .../OpenIDConnectionUtilsBehaviorTest.java | 32 ++++++++++++++++++- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/KubeConfigUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/KubeConfigUtils.java index 50810e6c2f7..ce0a7e6df82 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/KubeConfigUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/KubeConfigUtils.java @@ -54,7 +54,7 @@ */ public class KubeConfigUtils { - private static final Logger logger = LoggerFactory.getLogger(io.fabric8.kubernetes.client.Config.class); + private static final Logger logger = LoggerFactory.getLogger(KubeConfigUtils.class); private static final String KUBERNETES_CONFIG_CONTEXT_FILE_KEY = "KUBERNETES_CONFIG_CONTEXT_FILE_KEY"; private static final String KUBERNETES_CONFIG_CLUSTER_FILE_KEY = "KUBERNETES_CONFIG_CLUSTER_FILE_KEY"; diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java index 10b326fb5fc..5675f280681 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java @@ -17,9 +17,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; -import io.fabric8.kubernetes.api.model.AuthInfo; import io.fabric8.kubernetes.api.model.AuthProviderConfig; -import io.fabric8.kubernetes.api.model.NamedAuthInfo; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.http.HttpClient; @@ -197,23 +195,20 @@ public static OAuthToken persistOAuthToken(Config currentConfig, OAuthToken oAut // Persist in file if (currentConfig.getFileWithAuthInfo() != null && currentConfig.getCurrentContext() != null) { try { - final io.fabric8.kubernetes.api.model.Config kubeConfig = KubeConfigUtils - .parseConfig(currentConfig.getFileWithAuthInfo()); - final String userName = currentConfig.getCurrentContext().getContext().getUser(); - NamedAuthInfo namedAuthInfo = kubeConfig.getUsers().stream().filter(n -> n.getName().equals(userName)).findFirst() - .orElseGet(() -> { - NamedAuthInfo result = new NamedAuthInfo(userName, new AuthInfo()); - kubeConfig.getUsers().add(result); - return result; - }); - if (namedAuthInfo.getUser() == null) { - namedAuthInfo.setUser(new AuthInfo()); + final var kubeConfig = KubeConfigUtils.parseConfig(currentConfig.getFileWithAuthInfo()); + final var userName = currentConfig.getCurrentContext().getContext().getUser(); + final var namedAuthInfo = kubeConfig.getUsers().stream() + .filter(n -> n.getName().equals(userName)) + .findFirst() + .orElse(null); + // Update-persist only fields that are still present in the kubeconfig file + if (namedAuthInfo != null + && namedAuthInfo.getUser() != null + && namedAuthInfo.getUser().getAuthProvider() != null + && namedAuthInfo.getUser().getAuthProvider().getConfig() != null) { + namedAuthInfo.getUser().getAuthProvider().getConfig().putAll(authProviderConfig); } - if (namedAuthInfo.getUser().getAuthProvider() == null) { - namedAuthInfo.getUser().setAuthProvider(new AuthProviderConfig()); - } - namedAuthInfo.getUser().getAuthProvider().getConfig().putAll(authProviderConfig); - if (Utils.isNotNullOrEmpty(token)) { + if (Utils.isNotNullOrEmpty(token) && namedAuthInfo != null && namedAuthInfo.getUser() != null) { namedAuthInfo.getUser().setToken(token); } KubeConfigUtils.persistKubeConfigIntoFile(kubeConfig, currentConfig.getFileWithAuthInfo()); diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java index e6630dde04b..7e2a2787faf 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java @@ -501,6 +501,10 @@ void setUp() throws IOException { "- name: user\n" + " user:\n" + " token: original-token\n" + + " auth-provider:\n" + + " config:\n" + + " id-token: original-token\n" + + " refresh-token: original-refresh-token\n" + "contexts:\n" + "- name: context\n" + " context:\n" + @@ -546,6 +550,18 @@ void persistsOAuthTokenInFile() { entry("id-token", "new-token"), entry("refresh-token", "new-refresh-token")); } + + @Test + void skipsOAuthTokenInFileIfNull() { + originalConfig = Config.fromKubeconfig(kubeConfig); + persistOAuthToken(originalConfig, null, "fake.token"); + assertThat(KubeConfigUtils.parseConfig(kubeConfig)) + .extracting(c -> c.getUsers().iterator().next().getUser().getAuthProvider().getConfig()) + .asInstanceOf(InstanceOfAssertFactories.map(String.class, String.class)) + .containsOnly( + entry("id-token", "original-token"), + entry("refresh-token", "original-refresh-token")); + } } @Nested @@ -573,7 +589,11 @@ void setUp() throws IOException { "users:\n" + "- name: user\n" + " user:\n" + - " token: original-token\n").getBytes(StandardCharsets.UTF_8)); + " token: original-token\n" + + " auth-provider:\n" + + " config:\n" + + " id-token: original-token\n" + + " refresh-token: original-refresh-token\n").getBytes(StandardCharsets.UTF_8)); System.setProperty("kubeconfig", kubeConfig.getAbsolutePath() + File.pathSeparator + userConfig.getAbsolutePath()); originalConfig = new ConfigBuilder().withAutoConfigure().build(); persistOAuthToken(originalConfig, oAuthTokenResponse, "updated-token"); @@ -590,6 +610,16 @@ void persistsTokenInUserFile() { .returns("updated-token", c -> c.getUsers().iterator().next().getUser().getToken()); } + @Test + void persistsOAuthTokenInUserFile() { + assertThat(KubeConfigUtils.parseConfig(userConfig)) + .extracting(c -> c.getUsers().iterator().next().getUser().getAuthProvider().getConfig()) + .asInstanceOf(InstanceOfAssertFactories.map(String.class, String.class)) + .containsOnly( + entry("id-token", "new-token"), + entry("refresh-token", "new-refresh-token")); + } + @Test void leavesOtherConfigUntouched() { assertThat(KubeConfigUtils.parseConfig(kubeConfig))