Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STONEBLD-1514 CLI and contaminated test jars #872

Merged
merged 2 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions java-components/build-request-processor/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@
<artifactId>quarkus-junit5</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5-internal</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class DeployCommand implements Runnable {
private static final String DOT_JAR = ".jar";
private static final String DOT_POM = ".pom";
private static final String DOT = ".";
private static final Set<String> ALLOWED_CONTAMINANTS = Set.of("-tests.jar");
final BeanManager beanManager;
final ResultsUpdater resultsUpdater;

Expand All @@ -55,7 +56,7 @@ public class DeployCommand implements Runnable {
@CommandLine.Option(required = false, names = "--task-run-name")
String taskRun;

@CommandLine.Option(required = false, names = "--source-path")
@CommandLine.Option(required = true, names = "--source-path")
Path sourcePath;

@CommandLine.Option(required = false, names = "--logs-path")
Expand All @@ -64,10 +65,10 @@ public class DeployCommand implements Runnable {
@CommandLine.Option(required = false, names = "--build-info-path")
Path buildInfoPath;

@CommandLine.Option(required = false, names = "--scm-uri")
@CommandLine.Option(required = true, names = "--scm-uri")
String scmUri;

@CommandLine.Option(required = false, names = "--scm-commit")
@CommandLine.Option(required = true, names = "--scm-commit")
rnc marked this conversation as resolved.
Show resolved Hide resolved
String commit;

@CommandLine.Option(names = "--registry-host", defaultValue = "quay.io")
Expand Down Expand Up @@ -109,6 +110,8 @@ public void run() {

Map<String, Set<String>> contaminatedPaths = new HashMap<>();
Map<String, Set<String>> contaminatedGavs = new HashMap<>();
// Represents directories that should not be deployed i.e. if a single artifact (barring test jars) is
// contaminated then none of the artifacts will be deployed.
Set<Path> toRemove = new HashSet<>();
Map<Path, Gav> jarFiles = new HashMap<>();
Files.walkFileTree(deploymentPath, new SimpleFileVisitor<>() {
Expand All @@ -118,26 +121,27 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
Optional<Gav> gav = getGav(name);
gav.ifPresent(
gav1 -> gavs.add(gav1.getGroupId() + ":" + gav1.getArtifactId() + ":" + gav1.getVersion()));
Log.debugf("Checking %s for contaminants", name);
Log.debugf("Checking %s for contaminants with GAV %s", name, gav);
//we check every file as we also want to catch .tar.gz etc
var info = ClassFileTracker.readTrackingDataFromFile(Files.newInputStream(file), name);
for (var i : info) {
if (!allowedSources.contains(i.source)) {
Log.errorf("%s was contaminated by %s from %s", name, i.gav, i.source);
gav.ifPresent(g -> contaminatedGavs.computeIfAbsent(i.gav,
s -> new HashSet<>())
.add(g.getGroupId() + ":" + g.getArtifactId() + ":" + g.getVersion()));
int index = name.lastIndexOf("/");
if (index != -1) {
contaminatedPaths
.computeIfAbsent(name.substring(0, index), s -> new HashSet<>())
.add(i.gav);
if (ALLOWED_CONTAMINANTS.stream().noneMatch(a -> file.getFileName().toString().endsWith(a))) {
gav.ifPresent(g -> contaminatedGavs.computeIfAbsent(i.gav, s -> new HashSet<>())
.add(g.getGroupId() + ":" + g.getArtifactId() + ":" + g.getVersion()));
int index = name.lastIndexOf("/");
if (index != -1) {
contaminatedPaths.computeIfAbsent(name.substring(0, index),
s -> new HashSet<>()).add(i.gav);
} else {
contaminatedPaths.computeIfAbsent("", s -> new HashSet<>()).add(i.gav);
}
toRemove.add(file.getParent());
} else {
contaminatedPaths.computeIfAbsent("", s -> new HashSet<>()).add(i.gav);
Log.debugf("Ignoring contaminant for %s", file.getFileName());
}
toRemove.add(file.getParent());
}

}
if (gav.isPresent()) {
//now add our own tracking data
Expand Down Expand Up @@ -188,6 +192,9 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
FileUtil.deleteRecursive(i);
}

//update the DB with contaminant information
Log.infof("Contaminants: %s", contaminatedPaths);
Log.infof("Contaminated GAVS: %s", contaminatedGavs);
Log.infof("GAVs to deploy: %s", gavs);
if (gavs.isEmpty()) {
Log.errorf("No content to deploy found in deploy directory");
Expand All @@ -203,10 +210,8 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
}
//we still deploy, but without the contaminates
java.nio.file.Path deployFile = deploymentPath;
Log.infof("Contaminants: %s", contaminatedPaths);
Log.infof("Contaminated GAVS: %s", contaminatedGavs);
//update the DB with contaminant information

// This means the build failed to produce any deployable output.
// If everything is contaminated we still need the task to succeed so we can resolve the contamination.
for (var i : contaminatedGavs.entrySet()) {
gavs.removeAll(i.getValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.List;
import java.util.Map;

Expand All @@ -21,6 +18,8 @@
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.GenericContainer;

import com.redhat.hacbs.resources.util.HashUtil;

import io.quarkus.test.junit.main.QuarkusMainLauncher;
import io.quarkus.test.junit.main.QuarkusMainTest;
import io.vertx.core.json.JsonArray;
Expand Down Expand Up @@ -136,7 +135,7 @@ private Path createDeploymentRepo() throws IOException {
Files.createFile(shaFile);
}

String sha1 = sha1(testContent);
String sha1 = HashUtil.sha1(testContent);
Files.writeString(shaFile, sha1);
}

Expand Down Expand Up @@ -184,24 +183,6 @@ private URL getRegistryURL(String path) throws IOException {
return new URL("http://" + this.container.getHost() + ":" + port + "/v2/" + path);
}

private String sha1(String value) {
return sha1(value.getBytes(StandardCharsets.UTF_8));
}

private String sha1(byte[] value) {
try {
MessageDigest md = MessageDigest.getInstance("SHA-1");
byte[] digest = md.digest(value);
StringBuilder sb = new StringBuilder(40);
for (int i = 0; i < digest.length; ++i) {
sb.append(Integer.toHexString((digest[i] & 0xFF) | 0x100).substring(1, 3));
}
return sb.toString();
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException(e);
}
}

class ContainerRegistryDetails {
String repoName;
List<String> tags;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package com.redhat.hacbs.container.analyser.deploy;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
import java.util.logging.LogRecord;

import jakarta.enterprise.inject.spi.BeanManager;
import jakarta.inject.Inject;

import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import com.redhat.hacbs.container.results.ResultsUpdater;
import com.redhat.hacbs.resources.util.HashUtil;

import io.quarkus.test.LogCollectingTestResource;
import io.quarkus.test.common.QuarkusTestResource;
import io.quarkus.test.common.ResourceArg;
import io.quarkus.test.junit.QuarkusTest;

@QuarkusTest
@QuarkusTestResource(value = LogCollectingTestResource.class, restrictToAnnotatedClass = true, initArgs = @ResourceArg(name = LogCollectingTestResource.LEVEL, value = "FINE"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I don't really like tests like this, they are really fragile and tied to the implementation.

I would create two repos at https://github.com/orgs/jvm-build-service-test-data/repositories , one called 'unbuildable' or something like that, and one where the test jar is contaminated by unbuildable. I think you should have access, both to be able to create the repos and release to central.

Then add the gav of the contaiminated one to the minikube test matrix.

Over time I want to build up https://github.com/orgs/jvm-build-service-test-data to contain all sorts of different problems we have solved, but that only have one or two classes so they build super quick.

public class DeployContaminateTest {
private static final String GROUP = "com.company.foo";
private static final String VERSION = "3.25.8";
public static final String FOO_BAR = "foo-bar";
public static final String FOO_BAZ = "foo-baz";
private static final String DOT = ".";
private static final String SHA_1 = "sha1";
public static final String COMMIT = "3cf2d99b47f0a05466d1d0a2e09d8740faeda149";
public static final String REPO = "https://github.com/foo/bar";
private Map<String, String> ARTIFACT_FILE_MAP = Map.of(
FOO_BAR, "foobar-" + VERSION + "-tests.jar",
FOO_BAZ, "foobaz-" + VERSION + ".jar");

@Inject
ResultsUpdater resultsUpdater;

@BeforeEach
public void clearLogs() {
LogCollectingTestResource.current().clear();
}

@Test
public void testDeployOnlyContaminated() throws IOException, URISyntaxException {
Path onDiskRepo = Paths.get("target/test-data/artifacts").toAbsolutePath();
onDiskRepo.toFile().mkdirs();
Path source = Files.createTempDirectory("hacbs");
Files.writeString(source.resolve("pom.xml"), "");

TestDeployment testDeployment = new TestDeployment(null, resultsUpdater);
testDeployment.deploymentPath = onDiskRepo.toAbsolutePath();
testDeployment.imageId = "test-image";
testDeployment.scmUri = REPO;
testDeployment.commit = COMMIT;
testDeployment.sourcePath = source.toAbsolutePath();
testDeployment.allowedSources = Set.of("redhat", "rebuilt"); // Default value

try {
testDeployment.run();
fail("No exception thrown");
} catch (Exception e) {
List<LogRecord> logRecords = LogCollectingTestResource.current().getRecords();
assertTrue(e.getMessage().contains("deploy failed"));
assertTrue(logRecords.stream()
.anyMatch(r -> LogCollectingTestResource.format(r)
.contains("No content to deploy found in deploy directory")));
}
}

@Test
public void testDeployWithContaminated()
throws IOException, URISyntaxException {
Path onDiskRepo = createDeploymentRepo();
Path source = Files.createTempDirectory("hacbs");
Files.writeString(source.resolve("pom.xml"), "");

TestDeployment testDeployment = new TestDeployment(null, resultsUpdater);
testDeployment.deploymentPath = onDiskRepo.toAbsolutePath();
testDeployment.imageId = "test-image";
testDeployment.scmUri = REPO;
testDeployment.commit = COMMIT;
testDeployment.sourcePath = source.toAbsolutePath();
testDeployment.allowedSources = Set.of("redhat", "rebuilt"); // Default value

testDeployment.run();
List<LogRecord> logRecords = LogCollectingTestResource.current().getRecords();
// logRecords.forEach(r -> System.out.println("*** " + LogCollectingTestResource.format(r)));
assertTrue(logRecords.stream()
.anyMatch(r -> LogCollectingTestResource.format(r).contains(
"com/company/foo/foo-bar/3.25.8/foobar-3.25.8-tests.jar was contaminated by org.jboss.metadata:jboss-metadata-common:9.0.0.Final from central")));
assertTrue(logRecords.stream().noneMatch(r -> r.getMessage().contains("Removing")));
assertTrue(logRecords.stream().anyMatch(r -> LogCollectingTestResource.format(r)
.contains("GAVs to deploy: [com.company.foo:foo-bar:3.25.8, com.company.foo:foo-baz:3.25.8")));
}

private Path createDeploymentRepo()
throws IOException, URISyntaxException {
Path artifacts = Paths.get("target/test-data/artifacts").toAbsolutePath();
FileUtils.deleteDirectory(artifacts.toFile());
Files.createDirectories(artifacts);

// Add data to artifacts folder
for (Map.Entry<String, String> artifactFile : ARTIFACT_FILE_MAP.entrySet()) {
String groupPath = GROUP.replace(DOT, File.separator);
Path testDir = Paths.get(artifacts.toString(), groupPath, artifactFile.getKey(),
VERSION);
Files.createDirectories(testDir);
Path testFile = Paths.get(testDir.toString(), artifactFile.getValue());

if (artifactFile.getKey().contains(FOO_BAR)) {
JarOutputStream jar = new JarOutputStream(new FileOutputStream(testFile.toString()));
Path iconClass = Paths.get(getClass().getResource("/").toURI())
.resolve("../../../cli/src/test/resources/Icon.class").normalize();
JarEntry entry = new JarEntry(iconClass.getFileName().toString());
jar.putNextEntry(entry);
jar.write(Files.readAllBytes(iconClass));
jar.closeEntry();
jar.close();
} else {
Files.createFile(testFile);
String testContent = "Just some data for " + artifactFile.getKey();
Files.writeString(testFile, testContent);
}

Path shaFile = Paths.get(testFile + DOT + SHA_1);
Files.createFile(shaFile);
String sha1 = HashUtil.sha1(Files.readAllBytes(testFile));
Files.writeString(shaFile, sha1);
}

return artifacts;
}

public static class TestDeployment extends DeployCommand {
public TestDeployment(BeanManager beanManager, ResultsUpdater resultsUpdater) {
super(beanManager, resultsUpdater);
}

@Override
protected void doDeployment(Path deployDir, Path sourcePath, Path logsPath, Set<String> gavs)
throws Exception {
System.out.println("Skipping doDeployment for " + deployDir + " from " + sourcePath);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import picocli.CommandLine;

//@QuarkusTest
public class VerifyBuiltArtifactsCommandTest {
private Properties properties;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
%test.quarkus.log.category."com.redhat.hacbs".level=DEBUG
4 changes: 4 additions & 0 deletions java-components/cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
<groupId>io.github.redhat-appstudio.jvmbuild</groupId>
<artifactId>hacbs-resource-model</artifactId>
</dependency>
<dependency>
<groupId>io.github.redhat-appstudio.jvmbuild</groupId>
<artifactId>hacbs-classfile-tracker</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-picocli</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.github.redhatappstudio.jvmbuild.cli;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Set;

import com.redhat.hacbs.classfile.tracker.ClassFileTracker;
import com.redhat.hacbs.classfile.tracker.TrackingData;

import picocli.CommandLine;

@CommandLine.Command(name = "diagnostic", mixinStandardHelpOptions = true, description = "Print diagnostic information")
public class DiagnosticCommand {
@CommandLine.Command(name = "print-class-tracking-data", mixinStandardHelpOptions = true, description = "Print class tracking information")
public void classdump(@CommandLine.Parameters() Path fileName) {
rnc marked this conversation as resolved.
Show resolved Hide resolved
String name = fileName.toString();
try {
System.out.println("Looking for " + fileName);
Set<TrackingData> result = ClassFileTracker.readTrackingDataFromFile(Files.newInputStream(fileName), name);

if (result.isEmpty()) {
System.out.println("No tracking data found");
} else {
System.out.println("Found " + result.size() + " tracking data:");
for (TrackingData data : result) {
System.out.println(data.toString());
}
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
BuildCommand.class,
ArtifactCommand.class,
RebuiltCommand.class,
SetupCommand.class
SetupCommand.class,
DiagnosticCommand.class
})
@Vetoed
public class MainCommand {
Expand Down
Loading