fixup! Omit scm-username annotation from the PAT secret

eclipse-che · Jul 25, 2023 · 9c245e1 · 9c245e1
1 parent 1edc87c
commit 9c245e1
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 15 deletions.
diff --git a/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java b/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
@@ -11,8 +11,6 @@
  */
 package org.eclipse.che.api.factory.server.scm.kubernetes;
 
-import static com.google.common.base.Strings.isNullOrEmpty;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
 import io.fabric8.kubernetes.api.model.LabelSelector;
@@ -195,17 +193,17 @@ private Optional<PersonalAccessToken> doGetPersonalAccessToken(
             String providerName = annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME);
             String tokenId = annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID);
             String organization = annotations.get(ANNOTATION_SCM_ORGANIZATION);
-            String scmUsername =
+            Optional<String> scmUsername =
                 scmPersonalAccessTokenFetcher.getScmUsername(
                     new PersonalAccessTokenParams(
                         trimmedUrl, providerName, tokenId, token, organization));
-            if (!isNullOrEmpty(scmUsername)) {
+            if (scmUsername.isPresent()) {
               PersonalAccessToken personalAccessToken =
                   new PersonalAccessToken(
                       trimmedUrl,
                       annotations.get(ANNOTATION_CHE_USERID),
                       organization,
-                      scmUsername,
+                      scmUsername.get(),
                       providerName,
                       tokenId,
                       token);

diff --git a/...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java b/...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
@@ -97,7 +97,7 @@ public void shouldTrimBlankCharsInToken() throws Exception {
     when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
     when(kubernetesnamespace.secrets()).thenReturn(secrets);
     when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
-        .thenReturn("user");
+        .thenReturn(Optional.of("user"));
 
     Map<String, String> data =
         Map.of("token", Base64.getEncoder().encodeToString(" token_value \n".getBytes(UTF_8)));
@@ -164,7 +164,7 @@ public void testGetTokenFromNamespace() throws Exception {
     when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
     when(kubernetesnamespace.secrets()).thenReturn(secrets);
     when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
-        .thenReturn("user");
+        .thenReturn(Optional.of("user"));
 
     Map<String, String> data1 =
         Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
@@ -218,7 +218,7 @@ public void shouldGetTokenFromASecretWithSCMUsername() throws Exception {
     when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
     when(kubernetesnamespace.secrets()).thenReturn(secrets);
     when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
-        .thenReturn("user");
+        .thenReturn(Optional.of("user"));
 
     Map<String, String> data =
         Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
@@ -261,7 +261,7 @@ public void shouldGetTokenFromASecretWithoutSCMUsername() throws Exception {
     when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
     when(kubernetesnamespace.secrets()).thenReturn(secrets);
     when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
-        .thenReturn("user");
+        .thenReturn(Optional.of("user"));
 
     Map<String, String> data =
         Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
@@ -298,7 +298,7 @@ public void testGetTokenFromNamespaceWithTrailingSlashMismatch() throws Exceptio
     when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
     when(kubernetesnamespace.secrets()).thenReturn(secrets);
     when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
-        .thenReturn("user");
+        .thenReturn(Optional.of("user"));
 
     Map<String, String> data1 =
         Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));

diff --git a/...y/src/main/java/org/eclipse/che/api/factory/server/scm/ScmPersonalAccessTokenFetcher.java b/...y/src/main/java/org/eclipse/che/api/factory/server/scm/ScmPersonalAccessTokenFetcher.java
@@ -17,7 +17,6 @@
 import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
 import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
 import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException;
-import org.eclipse.che.commons.annotation.Nullable;
 import org.eclipse.che.commons.lang.Pair;
 import org.eclipse.che.commons.subject.Subject;
 
@@ -81,15 +80,14 @@ public boolean isValid(PersonalAccessToken personalAccessToken)
    * {@link PersonalAccessTokenFetcher#isValid(PersonalAccessTokenParams)} method. If any of the
    * fetchers return an scm username, return it. Otherwise, return null.
    */
-  @Nullable
-  public String getScmUsername(PersonalAccessTokenParams params)
+  public Optional<String> getScmUsername(PersonalAccessTokenParams params)
       throws UnknownScmProviderException, ScmUnauthorizedException, ScmCommunicationException {
     for (PersonalAccessTokenFetcher fetcher : personalAccessTokenFetchers) {
       Optional<Pair<Boolean, String>> isValid = fetcher.isValid(params);
       if (isValid.isPresent() && isValid.get().first) {
-        return isValid.get().second;
+        return Optional.of(isValid.get().second);
       }
     }
-    return null;
+    return Optional.empty();
   }
 }