From ad8bbf85c2e76250b44efbff369d626f0e9316ec Mon Sep 17 00:00:00 2001 From: Ivan Bodrov Date: Thu, 2 Jan 2025 12:07:02 -0500 Subject: [PATCH 1/2] concord-server: simplify UserInfo, UserInfoProcessor --- .../processors/CurrentUserInfoProcessor.java | 5 +- .../InitiatorUserInfoProcessor.java | 5 +- .../processors/UserInfoProcessor.java | 55 +++++++++---------- .../concord/server/user/UserInfoProvider.java | 25 ++++----- 4 files changed, 43 insertions(+), 47 deletions(-) diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/CurrentUserInfoProcessor.java b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/CurrentUserInfoProcessor.java index eb72538506..d87a7f508e 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/CurrentUserInfoProcessor.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/CurrentUserInfoProcessor.java @@ -20,6 +20,7 @@ * ===== */ +import com.fasterxml.jackson.databind.ObjectMapper; import com.walmartlabs.concord.sdk.Constants; import com.walmartlabs.concord.server.process.pipelines.processors.signing.Signing; import com.walmartlabs.concord.server.user.UserManager; @@ -31,7 +32,7 @@ public class CurrentUserInfoProcessor extends UserInfoProcessor { @Inject - public CurrentUserInfoProcessor(UserManager userManager, Signing signing) { - super(Constants.Request.CURRENT_USER_KEY, userManager, signing); + public CurrentUserInfoProcessor(UserManager userManager, Signing signing, ObjectMapper objectMapper) { + super(Constants.Request.CURRENT_USER_KEY, userManager, signing, objectMapper); } } diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/InitiatorUserInfoProcessor.java b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/InitiatorUserInfoProcessor.java index 43183311f4..9522e546cd 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/InitiatorUserInfoProcessor.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/InitiatorUserInfoProcessor.java @@ -20,6 +20,7 @@ * ===== */ +import com.fasterxml.jackson.databind.ObjectMapper; import com.walmartlabs.concord.sdk.Constants; import com.walmartlabs.concord.server.process.pipelines.processors.signing.Signing; import com.walmartlabs.concord.server.user.UserManager; @@ -31,7 +32,7 @@ public class InitiatorUserInfoProcessor extends UserInfoProcessor { @Inject - public InitiatorUserInfoProcessor(UserManager userManager, Signing signing) { - super(Constants.Request.INITIATOR_KEY, userManager, signing); + public InitiatorUserInfoProcessor(UserManager userManager, Signing signing, ObjectMapper objectMapper) { + super(Constants.Request.INITIATOR_KEY, userManager, signing, objectMapper); } } diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/UserInfoProcessor.java b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/UserInfoProcessor.java index 5169a3487a..28233564ad 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/UserInfoProcessor.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/process/pipelines/processors/UserInfoProcessor.java @@ -20,17 +20,20 @@ * ===== */ -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; import com.walmartlabs.concord.server.process.Payload; import com.walmartlabs.concord.server.process.pipelines.processors.signing.Signing; import com.walmartlabs.concord.server.sdk.ConcordApplicationException; -import com.walmartlabs.concord.server.user.UserInfoProvider.BaseUserInfo; +import com.walmartlabs.concord.server.user.UserInfoProvider; import com.walmartlabs.concord.server.user.UserManager; -import org.immutables.value.Value; import java.util.HashMap; import java.util.Map; +import java.util.Optional; + +import static java.util.Objects.requireNonNull; /** * Collects and stores the current user's data. @@ -40,22 +43,31 @@ public abstract class UserInfoProcessor implements PayloadProcessor { private final String key; private final UserManager userManager; private final Signing signing; + private final ObjectMapper objectMapper; + + public UserInfoProcessor(String key, + UserManager userManager, + Signing signing, + ObjectMapper objectMapper) { - public UserInfoProcessor(String key, UserManager userManager, Signing signing) { - this.key = key; - this.userManager = userManager; - this.signing = signing; + this.key = requireNonNull(key); + this.userManager = requireNonNull(userManager); + this.signing = requireNonNull(signing); + this.objectMapper = requireNonNull(objectMapper); } @Override public Payload process(Chain chain, Payload payload) { - BaseUserInfo info = userManager.getCurrentUserInfo(); + var info = userManager.getCurrentUserInfo(); + var result = objectMapper.convertValue(info, ObjectNode.class); if (signing.isEnabled()) { - info = sign(info); + Optional.ofNullable(info.username()) + .map(this::sign) + .ifPresent(signature -> result.set("usernameSignature", signature)); } - Map m = new HashMap<>(); + Map m = new HashMap<>(); m.put(key, info); payload = payload.mergeValues(Payload.CONFIGURATION, m); @@ -63,28 +75,11 @@ public Payload process(Chain chain, Payload payload) { return chain.process(payload); } - private BaseUserInfo sign(BaseUserInfo i) { - if (i == null || i.username() == null) { - return i; - } - + private TextNode sign(String username) { try { - String s = signing.sign(i.username()); - return SignedUserInfo.from(i).usernameSignature(s).build(); + return TextNode.valueOf(signing.sign(username)); } catch (Exception e) { throw new ConcordApplicationException("Error while singing process data: " + e.getMessage(), e); } } - - @Value.Immutable - @JsonSerialize(as = ImmutableSignedUserInfo.class) - @JsonDeserialize(as = ImmutableSignedUserInfo.class) - public interface SignedUserInfo extends BaseUserInfo { - - String usernameSignature(); - - public static ImmutableSignedUserInfo.Builder from(BaseUserInfo i) { - return ImmutableSignedUserInfo.builder().from(i); - } - } } diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserInfoProvider.java b/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserInfoProvider.java index 2cc5b82921..ff0b279f7d 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserInfoProvider.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/user/UserInfoProvider.java @@ -30,6 +30,8 @@ import java.util.Set; import java.util.UUID; +import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_EMPTY; + public interface UserInfoProvider { UserType getUserType(); @@ -44,9 +46,16 @@ public interface UserInfoProvider { UserInfo getInfo(UUID id, String username, String userDomain); UUID create(String username, String domain, String displayName, String email, Set roles); - - @JsonInclude(JsonInclude.Include.NON_EMPTY) - interface BaseUserInfo { + + @Value.Immutable + @JsonInclude(NON_EMPTY) + @JsonSerialize(as = ImmutableUserInfo.class) + @JsonDeserialize(as = ImmutableUserInfo.class) + interface UserInfo { + + static ImmutableUserInfo.Builder builder() { + return ImmutableUserInfo.builder(); + } @Nullable UUID id(); @@ -69,14 +78,4 @@ interface BaseUserInfo { @Nullable Map attributes(); } - - @Value.Immutable - @JsonSerialize(as = ImmutableUserInfo.class) - @JsonDeserialize(as = ImmutableUserInfo.class) - interface UserInfo extends BaseUserInfo { - - static ImmutableUserInfo.Builder builder() { - return ImmutableUserInfo.builder(); - } - } } From 1337d58b03a54ac7ab84a8b3eeb71722b5e7e685 Mon Sep 17 00:00:00 2001 From: Ivan Bodrov Date: Thu, 16 Jan 2025 23:18:32 -0500 Subject: [PATCH 2/2] concord-server-it: retry attachment download --- .../concord/it/server/AnsibleProjectIT.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/it/server/src/test/java/com/walmartlabs/concord/it/server/AnsibleProjectIT.java b/it/server/src/test/java/com/walmartlabs/concord/it/server/AnsibleProjectIT.java index 555330278e..5f7b96d563 100644 --- a/it/server/src/test/java/com/walmartlabs/concord/it/server/AnsibleProjectIT.java +++ b/it/server/src/test/java/com/walmartlabs/concord/it/server/AnsibleProjectIT.java @@ -31,10 +31,7 @@ import java.io.InputStream; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import static com.walmartlabs.concord.it.common.ServerClient.*; import static java.util.Collections.singletonMap; @@ -124,12 +121,11 @@ public void testFailure() throws Exception { // --- - ProcessApi processApi = new ProcessApi(getApiClient()); waitForStatus(getApiClient(), spr.getInstanceId(), ProcessEntry.StatusEnum.FAILED); // --- - try (InputStream resp = processApi.downloadAttachment(spr.getInstanceId(), "ansible_stats.json")) { + try (InputStream resp = downloadAttachment(spr.getInstanceId(), "ansible_stats.json")) { assertNotNull(resp); Map stats = fromJson(resp); @@ -180,7 +176,6 @@ public void test(Map input) throws Exception { // --- - ProcessApi processApi = new ProcessApi(getApiClient()); ProcessEntry psr = waitForCompletion(getApiClient(), spr.getInstanceId()); // --- @@ -193,7 +188,7 @@ public void test(Map input) throws Exception { // --- - try (InputStream resp = processApi.downloadAttachment(spr.getInstanceId(), "ansible_stats.json")) { + try (InputStream resp = downloadAttachment(spr.getInstanceId(), "ansible_stats.json")) { assertNotNull(resp); Map stats = fromJson(resp); @@ -207,4 +202,19 @@ public void test(Map input) throws Exception { private static InputStream resource(String path) { return AnsibleProjectIT.class.getResourceAsStream(path); } + + private InputStream downloadAttachment(UUID instanceId, String name) throws InterruptedException, ApiException { + int attemptsLeft = 3; + while (true) { + try { + return new ProcessApi(getApiClient()).downloadAttachment(instanceId, name); + } catch (ApiException e) { + if (attemptsLeft-- > 0 && (e.getCode() == 404 || e.getCode() >= 500)) { + Thread.sleep(500); + } else { + throw e; + } + } + } + } }