Skip to content

Commit

Permalink
Rwork the token clean up mechanism (#719)
Browse files Browse the repository at this point in the history 
Rework the token secrets cleanup mechanism by fetching kubernetes secrets and deleting the redundant secrets directly, instead of fetching personal access tokens and then deleting the secrets according to the PAT objects.
  • Loading branch information
@vinokurig
vinokurig authored Sep 10, 2024
1 parent c21e03c commit 3445db7
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 31 deletions.
70 changes: 40 additions & 30 deletions ...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,19 +188,7 @@ private List<PersonalAccessToken> doGetPersonalAccessTokens(
cheUser.getUserId(),
oAuthProviderName);
for (KubernetesNamespaceMeta namespaceMeta : namespaceFactory.list()) {
List<Secret> secrets =
namespaceFactory
.access(null, namespaceMeta.getName())
.secrets()
.get(KUBERNETES_PERSONAL_ACCESS_TOKEN_LABEL_SELECTOR);

// sort secrets to get the newest one first
// Assign to new list to avoid UnsupportedOperationException (ImmutableList)
secrets =
secrets.stream()
.sorted(Comparator.comparing(secret -> secret.getMetadata().getCreationTimestamp()))
.collect(Collectors.toList());
Collections.reverse(secrets);
List<Secret> secrets = doGetPersonalAccessTokenSecrets(namespaceMeta);

for (Secret secret : secrets) {
LOG.debug("Checking secret {}", secret.getMetadata().getName());
Expand Down Expand Up @@ -257,6 +245,29 @@ private List<PersonalAccessToken> doGetPersonalAccessTokens(
return result;
}

private List<Secret> doGetPersonalAccessTokenSecrets(KubernetesNamespaceMeta namespaceMeta)
throws ScmConfigurationPersistenceException {
try {
List<Secret> secrets =
namespaceFactory
.access(null, namespaceMeta.getName())
.secrets()
.get(KUBERNETES_PERSONAL_ACCESS_TOKEN_LABEL_SELECTOR);

// sort secrets to get the newest one first
// Assign to new list to avoid UnsupportedOperationException (ImmutableList)
secrets =
secrets.stream()
.sorted(Comparator.comparing(secret -> secret.getMetadata().getCreationTimestamp()))
.collect(Collectors.toList());
Collections.reverse(secrets);
return secrets;
} catch (InfrastructureException e) {
LOG.debug("Failed to get personal access token secret", e);
throw new ScmConfigurationPersistenceException(e.getMessage(), e);
}
}

/**
* Returns the name of the SCM provider. If the name is not set, the name of the token is used.
* This is used to support back compatibility with the old token secrets, which do not have the
Expand Down Expand Up @@ -354,28 +365,27 @@ public void forceRefreshPersonalAccessToken(String scmServerUrl)
scmPersonalAccessTokenFetcher.refreshPersonalAccessToken(subject, scmServerUrl);
gitCredentialManager.createOrReplace(personalAccessToken);
store(personalAccessToken);
removePreviousTokensIfPresent(subject, scmServerUrl);
removePreviousTokenSecretsIfPresent(scmServerUrl);
}

private void removePreviousTokensIfPresent(Subject subject, String scmServerUrl)
private void removePreviousTokenSecretsIfPresent(String scmServerUrl)
throws ScmConfigurationPersistenceException, UnsatisfiedScmPreconditionException {
List<PersonalAccessToken> personalAccessTokens =
doGetPersonalAccessTokens(subject, null, scmServerUrl);
for (int i = 1; i < personalAccessTokens.size(); i++) {
PersonalAccessToken token = personalAccessTokens.get(i);
if (token.getScmProviderUrl().equals(scmServerUrl)) {
try {
String namespace = getFirstNamespace();
cheServerKubernetesClientFactory
.create()
.secrets()
.inNamespace(namespace)
.withName(token.getScmTokenId())
.delete();
} catch (InfrastructureException e) {
throw new ScmConfigurationPersistenceException(e.getMessage(), e);
try {
for (KubernetesNamespaceMeta namespaceMeta : namespaceFactory.list()) {
List<Secret> secrets = doGetPersonalAccessTokenSecrets(namespaceMeta);
for (int i = 1; i < secrets.size(); i++) {
Secret secret = secrets.get(i);
if (secret.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL).equals(scmServerUrl)) {
cheServerKubernetesClientFactory
.create()
.secrets()
.inNamespace(getFirstNamespace())
.delete(secret);
}
}
}
} catch (InfrastructureException e) {
throw new ScmConfigurationPersistenceException(e.getMessage(), e);
}
}

Expand Down
66 changes: 65 additions & 1 deletion ...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ public void shouldDeleteInvalidTokensOnGet() throws Exception {
}

@Test(dependsOnMethods = "shouldDeleteInvalidTokensOnGet")
public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
public void shouldReturnFirstValidTokenAndDeleteTheInvalidOne() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(singletonList(meta));
Expand Down Expand Up @@ -529,4 +529,68 @@ public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
assertEquals(token.get().getScmTokenId(), "id2");
verify(nonNamespaceOperation, times(1)).delete(eq(secret1));
}

@Test
public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(singletonList(meta));
KubernetesNamespace kubernetesnamespace = Mockito.mock(KubernetesNamespace.class);
KubernetesSecrets secrets = Mockito.mock(KubernetesSecrets.class);
when(kubernetesnamespace.secrets()).thenReturn(secrets);
when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
Map<String, String> data1 =
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
Map<String, String> data2 =
Map.of("token", Base64.getEncoder().encodeToString("token2".getBytes(UTF_8)));
ObjectMeta meta1 =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-01T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"github",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"id1"))
.build();
ObjectMeta meta2 =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-02T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"github",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"id2"))
.build();
Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build();
Secret secret2 = new SecretBuilder().withMetadata(meta2).withData(data2).build();
when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(secret1, secret2));
when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation);
when(kubernetesnamespace.secrets()).thenReturn(secrets);
PersonalAccessToken token =
new PersonalAccessToken(
"http://host1", "provider", "cheUser", "username", "token-name", "tid-24", "token123");
when(scmPersonalAccessTokenFetcher.refreshPersonalAccessToken(
any(org.eclipse.che.commons.subject.Subject.class), eq("http://host1")))
.thenReturn(token);

// when
personalAccessTokenManager.forceRefreshPersonalAccessToken("http://host1");

// then
verify(nonNamespaceOperation, times(1)).delete(eq(secret1));
}
}

0 comments on commit 3445db7

Please sign in to comment.