From e22ec67dc4fc0708000ebb1ed1c5cb8c97c55b6a Mon Sep 17 00:00:00 2001 From: xjusko Date: Thu, 17 Oct 2024 08:54:23 +0200 Subject: [PATCH] add backend checks for BuildInfo --- .../org/jboss/set/BuildTriggerResource.java | 4 +- .../org/jboss/set/GitBuildInfoAssembler.java | 15 +- .../org/jboss/set/model/json/BuildInfo.java | 124 ++++++++++- .../model/json/BuildJMSTriggerPayload.java | 10 +- .../jboss/set/BuildTriggerResourceTest.java | 31 ++- .../jboss/set/model/json/BuildInfoTest.java | 196 ++++++++++++++++++ 6 files changed, 341 insertions(+), 39 deletions(-) create mode 100644 src/test/java/org/jboss/set/model/json/BuildInfoTest.java diff --git a/src/main/java/org/jboss/set/BuildTriggerResource.java b/src/main/java/org/jboss/set/BuildTriggerResource.java index 5d4f3e2..f8168e7 100644 --- a/src/main/java/org/jboss/set/BuildTriggerResource.java +++ b/src/main/java/org/jboss/set/BuildTriggerResource.java @@ -37,14 +37,14 @@ public class BuildTriggerResource { @Path("/trigger") public Response triggerBuild(@NotNull @Valid BuildInfo buildInfo) { String email = idToken.claim(Claims.email).orElse("Email not provided in the token").toString(); - if (buildInfo.streams == null || buildInfo.streams.isEmpty()) { + if (buildInfo.getStreams() == null || buildInfo.getStreams().isEmpty()) { return Response.status(400, "List of streams is empty").build(); } logger.infof("Triggering build for product %s by %s", buildInfo, email); buildTrigger.triggerBuild(BuildJMSTriggerPayload.from(buildInfo, email)); - return Response.ok("Triggered build for " + buildInfo.gitRepo + ":" + buildInfo.projectVersion).build(); + return Response.ok("Triggered build for " + buildInfo.getGitRepo() + ":" + buildInfo.getProjectVersion()).build(); } @GET diff --git a/src/main/java/org/jboss/set/GitBuildInfoAssembler.java b/src/main/java/org/jboss/set/GitBuildInfoAssembler.java index b614f7b..f6953a0 100644 --- a/src/main/java/org/jboss/set/GitBuildInfoAssembler.java +++ b/src/main/java/org/jboss/set/GitBuildInfoAssembler.java @@ -129,13 +129,14 @@ private BuildInfo constructGitlabBuild(String[] splitUrl, URL tagUrl, boolean is } private BuildInfo buildBuildInfo(URL tagUrl, String codespace, String repository, String version, String commitSHA) { - BuildInfo buildInfo = new BuildInfo(); - buildInfo.tag = tagUrl.toString(); - buildInfo.commitSha = commitSHA; - buildInfo.gitRepo = String.format("%s://%s/%s/%s", tagUrl.getProtocol(), tagUrl.getHost(), codespace, repository); - buildInfo.projectVersion = version; - - logger.infof("Build created with following parameters - sha: %s, version: %s, link: %s", buildInfo.commitSha, buildInfo.projectVersion, buildInfo.tag); + BuildInfo buildInfo = new BuildInfo.Builder() + .tag(tagUrl.toString()) + .commitSha(commitSHA) + .gitRepo(String.format("%s://%s/%s/%s", tagUrl.getProtocol(), tagUrl.getHost(), codespace, repository)) + .projectVersion(version) + .build(); + + logger.infof("Build created with following parameters - sha: %s, version: %s, link: %s", buildInfo.getCommitSha(), buildInfo.getProjectVersion(), buildInfo.getTag()); return buildInfo; } } diff --git a/src/main/java/org/jboss/set/model/json/BuildInfo.java b/src/main/java/org/jboss/set/model/json/BuildInfo.java index d9079ea..07175c3 100644 --- a/src/main/java/org/jboss/set/model/json/BuildInfo.java +++ b/src/main/java/org/jboss/set/model/json/BuildInfo.java @@ -1,24 +1,130 @@ package org.jboss.set.model.json; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Pattern; +import jakarta.validation.constraints.Size; import java.util.List; +@JsonDeserialize(builder = BuildInfo.Builder.class) public class BuildInfo { - @NotNull - public String tag; + @NotBlank + @Pattern(regexp = "^https://.+$", message = "Tag must start with https://") + private final String tag; - @NotNull - public String gitRepo; + @NotBlank + @Pattern(regexp = "^https://.+$", message = "Repository must start with https://") + private final String gitRepo; - @NotNull - public String projectVersion; + @NotBlank + private final String projectVersion; - @NotNull - public String commitSha; + @NotBlank + @Pattern(regexp = "^[a-fA-F0-9]{7,40}$", message = "Commit SHA must be a valid hexadecimal string (7 to 40 characters).") + private final String commitSha; - public List streams; + // Maximum of 5 streams allowed, based on Stream.values().length. + @Size(max = 5, message = "The maximum number of streams allowed is 5.") + private final List streams; + + BuildInfo(Builder builder) { + this.tag = builder.tag; + this.gitRepo = builder.gitRepo; + this.projectVersion = builder.projectVersion; + this.commitSha = builder.commitSha; + this.streams = builder.streams; + } + + public Builder toBuilder() { + return new Builder() + .tag(this.tag) + .gitRepo(this.gitRepo) + .projectVersion(this.projectVersion) + .commitSha(this.commitSha) + .streams(this.streams); + } + + public @NotNull String getTag() { + return tag; + } + + public @NotNull String getGitRepo() { + return gitRepo; + } + + public @NotNull String getProjectVersion() { + return projectVersion; + } + + public @NotNull String getCommitSha() { + return commitSha; + } + + public List getStreams() { + return streams; + } + + @JsonPOJOBuilder(withPrefix = "") + public static class Builder { + private String tag; + private String gitRepo; + private String projectVersion; + private String commitSha; + private List streams; + + public Builder tag(String tag) { + this.tag = tag.trim(); + return this; + } + + public Builder gitRepo(String gitRepo) { + this.gitRepo = gitRepo.trim(); + return this; + } + + public Builder projectVersion(String projectVersion) { + this.projectVersion = projectVersion.trim(); + return this; + } + + public Builder commitSha(String commitSha) { + this.commitSha = commitSha.trim(); + return this; + } + + public Builder streams(List streams) { + this.streams = streams; + return this; + } + + // Build method + public BuildInfo build() { + validateFields(); + return new BuildInfo(this); + } + + private void validateFields() { + if (tag == null || !tag.startsWith("https://")) { + throw new IllegalArgumentException("Tag must start with https://"); + } + + if (gitRepo == null || !gitRepo.startsWith("https://")) { + throw new IllegalArgumentException("Repository must start with https://"); + } + + if (commitSha == null || !commitSha.matches("^[a-fA-F0-9]{7,40}$")) { + throw new IllegalArgumentException("Commit SHA must be a valid hexadecimal string (7 to 40 characters)."); + } + + if (streams != null && streams.size() > Stream.values().length) { + throw new IllegalArgumentException(String.format("The maximum number of streams allowed is %d.", Stream.values().length)); + } + } + } @Override public String toString() { diff --git a/src/main/java/org/jboss/set/model/json/BuildJMSTriggerPayload.java b/src/main/java/org/jboss/set/model/json/BuildJMSTriggerPayload.java index ed76f0d..06411c5 100644 --- a/src/main/java/org/jboss/set/model/json/BuildJMSTriggerPayload.java +++ b/src/main/java/org/jboss/set/model/json/BuildJMSTriggerPayload.java @@ -16,11 +16,11 @@ public static BuildJMSTriggerPayload from(BuildInfo buildInfo, String email) { BuildJMSTriggerPayload createBuildPayload = new BuildJMSTriggerPayload(); createBuildPayload.email = email; - createBuildPayload.tag = buildInfo.tag; - createBuildPayload.gitRepo = buildInfo.gitRepo; - createBuildPayload.projectVersion = buildInfo.projectVersion; - createBuildPayload.commitSha = buildInfo.commitSha; - createBuildPayload.streams = getStreams(buildInfo.streams); + createBuildPayload.tag = buildInfo.getTag(); + createBuildPayload.gitRepo = buildInfo.getGitRepo(); + createBuildPayload.projectVersion = buildInfo.getProjectVersion(); + createBuildPayload.commitSha = buildInfo.getCommitSha(); + createBuildPayload.streams = getStreams(buildInfo.getStreams()); return createBuildPayload; } diff --git a/src/test/java/org/jboss/set/BuildTriggerResourceTest.java b/src/test/java/org/jboss/set/BuildTriggerResourceTest.java index b7d636a..4b0c5d0 100644 --- a/src/test/java/org/jboss/set/BuildTriggerResourceTest.java +++ b/src/test/java/org/jboss/set/BuildTriggerResourceTest.java @@ -23,8 +23,8 @@ public class BuildTriggerResourceTest { private static final String USER_EMAIL = "user@email.com"; - private static final String BUILD_INFO_TAG = "git+https://github.com/non-exisitent/repo/tree/1.0.0.Final.git"; - private static final String BUILD_INFO_GIT_REPO = "git+https://github.com/non-exisitent/repo.git"; + private static final String BUILD_INFO_TAG = "https://github.com/non-exisitent/repo/tree/1.0.0.Final.git"; + private static final String BUILD_INFO_GIT_REPO = "https://github.com/non-exisitent/repo.git"; private static final String BUILD_INFO_PROJECT_VERSION = "1.0.0.Final"; private static final String BUILD_INFO_COMMIT_SHA = "61ec86095de795f5fb817a7cc824d8d7cfb9ae51"; private static final List BUILD_INFO_STREAMS = List.of(EAP_7_3_X.frontEnd); @@ -41,8 +41,7 @@ public class BuildTriggerResourceTest { @Claim(key = "email", value = USER_EMAIL) }) public void testTriggerEndpoint() throws Exception { - BuildInfo buildInfo = createTestBuildInfoWithoutStream(); - buildInfo.streams = BUILD_INFO_STREAMS; + BuildInfo buildInfo = createTestBuildInfo(); given() .body(objectMapper.writeValueAsString(buildInfo)) @@ -68,8 +67,7 @@ public void testTriggerEndpoint() throws Exception { @Disabled("Anonymous identity doesn't work in tests. (https://github.com/quarkusio/quarkus/issues/21888)") @TestSecurity public void testTriggerEndpointNoAuth() throws Exception { - BuildInfo buildInfo = createTestBuildInfoWithoutStream(); - buildInfo.streams = BUILD_INFO_STREAMS; + BuildInfo buildInfo = createTestBuildInfo(); given() .header("Authorization", "") @@ -87,8 +85,9 @@ public void testTriggerEndpointNoAuth() throws Exception { @Claim(key = "email", value = USER_EMAIL) }) public void testTriggerEndpointInvalidStream() throws Exception { - BuildInfo buildInfo = createTestBuildInfoWithoutStream(); - buildInfo.streams = List.of("Invalid"); + BuildInfo buildInfo = createTestBuildInfo().toBuilder() + .streams(List.of("Invalid")) + .build(); given() .body(objectMapper.writeValueAsString(buildInfo)) @@ -98,13 +97,13 @@ public void testTriggerEndpointInvalidStream() throws Exception { .statusCode(422); } - private BuildInfo createTestBuildInfoWithoutStream() { - BuildInfo buildInfo = new BuildInfo(); - buildInfo.tag = BUILD_INFO_TAG; - buildInfo.gitRepo = BUILD_INFO_GIT_REPO; - buildInfo.projectVersion = BUILD_INFO_PROJECT_VERSION; - buildInfo.commitSha = BUILD_INFO_COMMIT_SHA; - - return buildInfo; + private BuildInfo createTestBuildInfo() { + return new BuildInfo.Builder() + .tag(BUILD_INFO_TAG) + .gitRepo(BUILD_INFO_GIT_REPO) + .projectVersion(BUILD_INFO_PROJECT_VERSION) + .commitSha(BUILD_INFO_COMMIT_SHA) + .streams(BUILD_INFO_STREAMS) + .build(); } } diff --git a/src/test/java/org/jboss/set/model/json/BuildInfoTest.java b/src/test/java/org/jboss/set/model/json/BuildInfoTest.java new file mode 100644 index 0000000..11bd12f --- /dev/null +++ b/src/test/java/org/jboss/set/model/json/BuildInfoTest.java @@ -0,0 +1,196 @@ +package org.jboss.set.model.json; + +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; + +class BuildInfoTest { + + private static final String VALID_TAG = "https://github.com/test/repo"; + private static final String VALID_REPO = "https://github.com/test/repo.git"; + private static final String VALID_VERSION = "1.0.0"; + private static final String VALID_COMMIT_SHA = "61ec86095de795f5fb817a7cc824d8d7cfb9ae51"; + private static final List VALID_STREAMS = List.of("EAP_7_3_X"); + + private static final String INVALID_TAG = "invalid-tag"; + private static final String INVALID_REPO = "invalid-url"; + private static final String INVALID_COMMIT_SHA = "invalidSha"; + + @Test + void testValidBuildInfo() { + Assertions.assertDoesNotThrow(() -> { + new BuildInfo.Builder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + }, "Expected no exception for valid BuildInfo creation"); + } + + @Test + void testInvalidTag() { + IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> { + new BuildInfo.Builder() + .tag(INVALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + }); + + Assertions.assertEquals("Tag must start with https://", thrown.getMessage()); + } + + @Test + void testInvalidCommitSha() { + IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> { + new BuildInfo.Builder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(INVALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + }); + + Assertions.assertEquals("Commit SHA must be a valid hexadecimal string (7 to 40 characters).", thrown.getMessage()); + } + + @Test + void testModifyValidBuildInfoWithInvalidTag() { + BuildInfo originalBuildInfo = new BuildInfo.Builder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + + IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> { + originalBuildInfo.toBuilder() + .gitRepo(INVALID_REPO) + .build(); + }); + + Assertions.assertEquals("Repository must start with https://", thrown.getMessage()); + } + + @Test + void testExceedingStreamLimit() { + IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> { + new BuildInfo.Builder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(Collections.nCopies(Stream.values().length + 1, Stream.EAP_7_3_X.backEnd)) + .build(); + }); + + Assertions.assertTrue(thrown.getMessage().contains("The maximum number of streams allowed is")); + } + + // Test field annotations validation + + private final Validator validator; + + public BuildInfoTest() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + this.validator = factory.getValidator(); + } + + // Custom builder for testing that bypasses Builder validation + private static class TestBuilder extends BuildInfo.Builder { + @Override + public BuildInfo build() { + // Directly construct and return a BuildInfo object without validation + return new BuildInfo(this); + } + } + + @Test + void testAnnotationValidBuildInfo() { + BuildInfo buildInfo = new TestBuilder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + + var violations = validator.validate(buildInfo); + Assertions.assertTrue(violations.isEmpty(), "Expected no validation violations for valid BuildInfo"); + } + + @Test + void testAnnotationInvalidTag() { + BuildInfo buildInfo = new TestBuilder() + .tag(INVALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + + var violations = validator.validate(buildInfo); + Assertions.assertFalse(violations.isEmpty(), "Expected validation violations for invalid tag"); + Assertions.assertEquals(1, violations.size()); + Assertions.assertEquals("Tag must start with https://", violations.iterator().next().getMessage()); + } + + @Test + void testAnnotationInvalidCommitSha() { + BuildInfo buildInfo = new TestBuilder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(INVALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + + var violations = validator.validate(buildInfo); + Assertions.assertFalse(violations.isEmpty(), "Expected validation violations for invalid commit SHA"); + Assertions.assertEquals(1, violations.size()); + Assertions.assertEquals("Commit SHA must be a valid hexadecimal string (7 to 40 characters).", violations.iterator().next().getMessage()); + } + + @Test + void testAnnotationInvalidStreamsSize() { + BuildInfo buildInfo = new TestBuilder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion(VALID_VERSION) + .commitSha(VALID_COMMIT_SHA) + .streams(Collections.nCopies(Stream.values().length + 1, Stream.EAP_7_3_X.backEnd)) + .build(); + + var violations = validator.validate(buildInfo); + Assertions.assertFalse(violations.isEmpty(), "Expected validation violations for exceeding maximum streams"); + Assertions.assertEquals(1, violations.size()); + Assertions.assertTrue(violations.iterator().next().getMessage().contains("The maximum number of streams allowed is")); + } + + @Test + void testEmptyProjectVersion() { + BuildInfo buildInfo = new TestBuilder() + .tag(VALID_TAG) + .gitRepo(VALID_REPO) + .projectVersion("") + .commitSha(VALID_COMMIT_SHA) + .streams(VALID_STREAMS) + .build(); + + var violations = validator.validate(buildInfo); + Assertions.assertFalse(violations.isEmpty(), "Expected validation violations for blank projectVersion"); + Assertions.assertEquals(1, violations.size()); + Assertions.assertEquals("must not be blank", violations.iterator().next().getMessage()); + } +}