refactor(config): OpenIDConnectionUtils.persistOAuthToken should not …

…persist values that no longer exist Signed-off-by: Marc Nuri <marc@marcnuri.com>
fabric8io · Nov 8, 2024 · d569bf2 · d569bf2
1 parent b95bfdf
commit d569bf2
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 20 deletions.
diff --git a/...netes-client-api/src/main/java/io/fabric8/kubernetes/client/internal/KubeConfigUtils.java b/...netes-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/...es-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java b/...es-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/...i/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java b/...i/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))