Skip to content

Commit

Permalink
Slow delete token (line#812)
Browse files Browse the repository at this point in the history 
### motivation:
* Resolve line#797
* It takes a long time to delete a token when central dogma have many project.

### modification:
* Add deletion field to `Token`
* Mark deletion at `appId` Path in `/token.json` when get requested delete token
* Add `purgeToken` feature at `PurgeSchedulingService`
### result:
* When a token deletion request is received, mark deletion and purge it with a background job.
  • Loading branch information
@byungjunn
byungjunn authored Feb 1, 2024
1 parent 717d71b commit 9510c7b
Show file tree
Hide file tree
Showing 9 changed files with 257 additions and 64 deletions.
63 changes: 44 additions & 19 deletions server/src/main/java/com/linecorp/centraldogma/server/internal/api/TokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.linecorp.centraldogma.common.Revision;
import com.linecorp.centraldogma.internal.Jackson;
import com.linecorp.centraldogma.server.command.CommandExecutor;
import com.linecorp.centraldogma.server.internal.api.auth.RequiresAdministrator;
import com.linecorp.centraldogma.server.internal.api.converter.CreateApiResponseConverter;
import com.linecorp.centraldogma.server.metadata.MetadataService;
import com.linecorp.centraldogma.server.metadata.Token;
Expand Down Expand Up @@ -122,13 +123,13 @@ public CompletableFuture<HttpResult<Token>> createToken(@Param String appId,
tokenFuture = mds.createToken(author, appId, isAdmin);
}
return tokenFuture
.thenCompose(unused -> mds.findTokenByAppId(appId))
.thenApply(token -> {
final ResponseHeaders headers = ResponseHeaders.of(HttpStatus.CREATED,
HttpHeaderNames.LOCATION,
"/tokens/" + appId);
return HttpResult.of(headers, token);
});
.thenCompose(unused -> mds.findTokenByAppId(appId))
.thenApply(token -> {
final ResponseHeaders headers = ResponseHeaders.of(HttpStatus.CREATED,
HttpHeaderNames.LOCATION,
"/tokens/" + appId);
return HttpResult.of(headers, token);
});
}

/**
Expand All @@ -145,6 +146,23 @@ public CompletableFuture<Token> deleteToken(ServiceRequestContext ctx,
.thenApply(unused -> token.withoutSecret()));
}

/**
* DELETE /tokens/{appId}/removed
*
* <p>Purges a token of the specified ID that was deleted before.
*/
@Delete("/tokens/{appId}/removed")
@RequiresAdministrator
public CompletableFuture<Token> purgeToken(ServiceRequestContext ctx,
@Param String appId,
Author author, User loginUser) {
return getTokenOrRespondForbidden(ctx, appId, loginUser).thenApplyAsync(
token -> {
mds.purgeToken(author, appId);
return token.withoutSecret();
}, ctx.blockingTaskExecutor());
}

/**
* PATCH /tokens/{appId}
*
Expand All @@ -155,18 +173,25 @@ public CompletableFuture<Token> deleteToken(ServiceRequestContext ctx,
public CompletableFuture<Token> updateToken(ServiceRequestContext ctx,
@Param String appId,
JsonNode node, Author author, User loginUser) {
if (node.equals(activation)) {
return getTokenOrRespondForbidden(ctx, appId, loginUser).thenCompose(
token -> mds.activateToken(author, appId)
.thenApply(unused -> token.withoutSecret()));
}
if (node.equals(deactivation)) {
return getTokenOrRespondForbidden(ctx, appId, loginUser).thenCompose(
token -> mds.deactivateToken(author, appId)
.thenApply(unused -> token.withoutSecret()));
}
throw new IllegalArgumentException("Unsupported JSON patch: " + node +
" (expected: " + activation + " or " + deactivation + ')');
return getTokenOrRespondForbidden(ctx, appId, loginUser).thenCompose(
token -> {
if (token.isDeleted()) {
throw new IllegalArgumentException(
"You can't update the status of the token scheduled for deletion.");
}
if (node.equals(activation)) {
return mds.activateToken(author, appId)
.thenApply(unused -> token.withoutSecret());
}
if (node.equals(deactivation)) {
return mds.deactivateToken(author, appId)
.thenApply(unused -> token.withoutSecret());
}
throw new IllegalArgumentException("Unsupported JSON patch: " + node +
" (expected: " + activation + " or " + deactivation +
')');
}
);
}

private CompletableFuture<Token> getTokenOrRespondForbidden(ServiceRequestContext ctx,
Expand Down
19 changes: 19 additions & 0 deletions ...c/main/java/com/linecorp/centraldogma/server/internal/storage/PurgeSchedulingService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
package com.linecorp.centraldogma.server.internal.storage;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;

import java.time.Duration;
import java.time.Instant;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
Expand All @@ -41,6 +43,8 @@
import com.linecorp.centraldogma.server.command.Command;
import com.linecorp.centraldogma.server.command.CommandExecutor;
import com.linecorp.centraldogma.server.metadata.MetadataService;
import com.linecorp.centraldogma.server.metadata.Token;
import com.linecorp.centraldogma.server.metadata.Tokens;
import com.linecorp.centraldogma.server.storage.project.ProjectManager;

/**
Expand Down Expand Up @@ -80,6 +84,7 @@ public void start(CommandExecutor commandExecutor,
storagePurgingScheduler.start(() -> {
try {
purgeProjectAndRepository(commandExecutor, metadataService);
purgeToken(metadataService);
} catch (Exception e) {
logger.warn("Unexpected purging service failure", e);
}
Expand Down Expand Up @@ -143,6 +148,20 @@ private void purgeRepository(CommandExecutor commandExecutor,
});
}

private static void purgeToken(MetadataService metadataService) {
final Tokens tokens = metadataService.getTokens().join();
final List<String> purging = tokens.appIds().values()
.stream()
.filter(Token::isDeleted)
.map(Token::appId)
.collect(toImmutableList());

if (!purging.isEmpty()) {
logger.info("Purging {} tokens: {}", purging.size(), purging);
purging.forEach(appId -> metadataService.purgeToken(Author.SYSTEM, appId));
}
}

private boolean isDisabled() {
return maxRemovedRepositoryAgeMillis == 0;
}
Expand Down
81 changes: 58 additions & 23 deletions server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,31 +876,66 @@ public CompletableFuture<Revision> destroyToken(Author author, String appId) {
requireNonNull(author, "author");
requireNonNull(appId, "appId");

// Remove the token from every project.
return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
"Delete the token: " + appId,
() -> tokenRepo
.fetch(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, TOKEN_JSON)
.thenApply(tokens -> {
final JsonPointer deletionPath =
JsonPointer.compile("/appIds" + encodeSegment(appId) +
"/deletion");
final Change<?> change = Change.ofJsonPatch(
TOKEN_JSON,
asJsonArray(new TestAbsenceOperation(deletionPath),
new AddOperation(deletionPath,
Jackson.valueToTree(
UserAndTimestamp.of(
author)))));
return HolderWithRevision.of(change, tokens.revision());
}));
}

/**
* Purges the {@link Token} of the specified {@code appId} that was removed before.
*
* <p>Note that this is a blocking method that should not be invoked in an event loop.
*/
public Revision purgeToken(Author author, String appId) {
requireNonNull(author, "author");
requireNonNull(appId, "appId");

final Collection<Project> projects = new SafeProjectManager(projectManager).list().values();
final CompletableFuture<?>[] futures = new CompletableFuture<?>[projects.size()];
int i = 0;
for (final Project p : projects) {
futures[i++] = removeToken(p.name(), author, appId, true).toCompletableFuture();

// Remove the token from projects that only have the token.
for (Project project : projects) {
final ProjectMetadata projectMetadata = fetchMetadata(project.name()).join().object();
final boolean containsTargetTokenInTheProject =
projectMetadata.tokens().values()
.stream()
.anyMatch(token -> token.appId().equals(appId));

if (containsTargetTokenInTheProject) {
removeToken(project.name(), author, appId, true).join();
}
}
return CompletableFuture.allOf(futures).thenCompose(unused -> tokenRepo.push(
INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, "Remove the token: " + appId,
() -> tokenRepo.fetch(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, TOKEN_JSON)
.thenApply(tokens -> {
final Token token = tokens.object().get(appId);
final JsonPointer appIdPath =
JsonPointer.compile("/appIds" + encodeSegment(appId));
final String secret = token.secret();
assert secret != null;
final JsonPointer secretPath =
JsonPointer.compile("/secrets" + encodeSegment(secret));
final Change<?> change = Change.ofJsonPatch(
TOKEN_JSON,
asJsonArray(new RemoveOperation(appIdPath),
new RemoveIfExistsOperation(secretPath)));
return HolderWithRevision.of(change, tokens.revision());
}))
);

return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, "Remove the token: " + appId,
() -> tokenRepo.fetch(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, TOKEN_JSON)
.thenApply(tokens -> {
final Token token = tokens.object().get(appId);
final JsonPointer appIdPath =
JsonPointer.compile("/appIds" + encodeSegment(appId));
final String secret = token.secret();
assert secret != null;
final JsonPointer secretPath =
JsonPointer.compile(
"/secrets" + encodeSegment(secret));
final Change<?> change = Change.ofJsonPatch(
TOKEN_JSON,
asJsonArray(new RemoveOperation(appIdPath),
new RemoveIfExistsOperation(secretPath)));
return HolderWithRevision.of(change, tokens.revision());
})).join();
}

/**
Expand Down
36 changes: 30 additions & 6 deletions server/src/main/java/com/linecorp/centraldogma/server/metadata/Token.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ public final class Token implements Identifiable {
@Nullable
private final UserAndTimestamp deactivation;

@Nullable
private final UserAndTimestamp deletion;

Token(String appId, String secret, boolean isAdmin, UserAndTimestamp creation) {
this(appId, secret, isAdmin, creation, null);
this(appId, secret, isAdmin, creation, null, null);
}

/**
Expand All @@ -76,20 +79,23 @@ public Token(@JsonProperty("appId") String appId,
@JsonProperty("secret") String secret,
@JsonProperty("admin") boolean isAdmin,
@JsonProperty("creation") UserAndTimestamp creation,
@JsonProperty("deactivation") @Nullable UserAndTimestamp deactivation) {
@JsonProperty("deactivation") @Nullable UserAndTimestamp deactivation,
@JsonProperty("deletion") @Nullable UserAndTimestamp deletion) {
this.appId = Util.validateFileName(appId, "appId");
this.secret = Util.validateFileName(secret, "secret");
this.isAdmin = isAdmin;
this.creation = requireNonNull(creation, "creation");
this.deactivation = deactivation;
this.deletion = deletion;
}

private Token(String appId, boolean isAdmin, UserAndTimestamp creation,
@Nullable UserAndTimestamp deactivation) {
@Nullable UserAndTimestamp deactivation, @Nullable UserAndTimestamp deletion) {
this.appId = Util.validateFileName(appId, "appId");
this.isAdmin = isAdmin;
this.creation = requireNonNull(creation, "creation");
this.deactivation = deactivation;
this.deletion = deletion;
secret = null;
}

Expand Down Expand Up @@ -140,29 +146,47 @@ public UserAndTimestamp deactivation() {
return deactivation;
}

/**
* Returns who deleted this token when.
*/
@Nullable
@JsonProperty
public UserAndTimestamp deletion() {
return deletion;
}

/**
* Returns whether this token is activated.
*/
@JsonIgnore
public boolean isActive() {
return deactivation == null;
return deactivation == null && deletion == null;
}

/**
* Returns whether this token is deleted.
*/
@JsonIgnore
public boolean isDeleted() {
return deletion != null;
}

@Override
public String toString() {
// Do not add "secret" to prevent it from logging.
return MoreObjects.toStringHelper(this)
return MoreObjects.toStringHelper(this).omitNullValues()
.add("appId", appId())
.add("isAdmin", isAdmin())
.add("creation", creation())
.add("deactivation", deactivation())
.add("deletion", deletion())
.toString();
}

/**
* Returns a new {@link Token} instance without its secret.
*/
public Token withoutSecret() {
return new Token(appId(), isAdmin(), creation(), deactivation());
return new Token(appId(), isAdmin(), creation(), deactivation(), deletion());
}
}
9 changes: 7 additions & 2 deletions server/src/main/resources/webapp/scripts/app/settings/tokens/tokens.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ <h2>Application Tokens</h2>
<td>{{token.creationTimeStr}}</td>
<td>
<div ng-switch="token.isActive">
<div ng-switch-when="true">Active</div>
<div ng-switch-default>Inactive</div>
<div ng-switch-when="false">
<div ng-switch="token.isDeleted">
<div ng-switch-when="true">Scheduled for deletion</div>
<div ng-switch-default>Inactive</div>
</div>
</div>
<div ng-switch-default>Active</div>
</div>
</td>
</tr>
Expand Down
1 change: 1 addition & 0 deletions server/src/main/resources/webapp/scripts/components/entities/settings.service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ angular.module('CentralDogmaAdmin')
for (i in tokens) {
tokens[i].creationTimeStr = moment(tokens[i].creation.timestamp).fromNow();
tokens[i].isActive = !angular.isDefined(tokens[i].deactivation);
tokens[i].isDeleted = angular.isDefined(tokens[i].deletion);
}
defer.resolve(tokens);
} else {
Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/com/linecorp/centraldogma/server/internal/admin/model/SerializationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void testValidProject() throws IOException {
final Member member = new Member(userLogin, ProjectRole.MEMBER, newCreationTag());
final RepositoryMetadata repositoryMetadata = new RepositoryMetadata("sample", newCreationTag(),
PerRolePermissions.ofDefault());
final Token token = new Token("testApp", "testSecret", false, newCreationTag(), null);
final Token token = new Token("testApp", "testSecret", false, newCreationTag(), null, null);
final ProjectMetadata metadata =
new ProjectMetadata("test",
ImmutableMap.of(repositoryMetadata.name(), repositoryMetadata),
Expand Down Expand Up @@ -141,7 +141,7 @@ void testRemovedProject() throws IOException {
newCreationTag());
final RepositoryMetadata repositoryMetadata = new RepositoryMetadata("sample", newCreationTag(),
PerRolePermissions.ofDefault());
final Token token = new Token("testApp", "testSecret", false, newCreationTag(), null);
final Token token = new Token("testApp", "testSecret", false, newCreationTag(), null, null);
final ProjectMetadata metadata =
new ProjectMetadata("test",
ImmutableMap.of(repositoryMetadata.name(), repositoryMetadata),
Expand Down
Loading

0 comments on commit 9510c7b

Please sign in to comment.