Always build ci workspace without trailing separator (#8788)

Author: daniel-mohedano

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java

@@ -35,7 +35,6 @@
 import datadog.trace.civisibility.source.index.RepoIndexProvider;
 import datadog.trace.civisibility.source.index.RepoIndexSourcePathResolver;
 import datadog.trace.util.Strings;
-import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
@@ -72,7 +71,7 @@ public class CiVisibilityRepoServices {
       LOGGER.info("PR detected: {}", pullRequestInfo);
     }
 
-    repoRoot = appendSlashIfNeeded(getRepoRoot(ciInfo, services.gitClientFactory));
+    repoRoot = getRepoRoot(ciInfo, services.gitClientFactory);
     moduleName = getModuleName(services.config, repoRoot, path);
     ciTags = new CITagsProvider().getCiTags(ciInfo, pullRequestInfo);
 
@@ -126,7 +125,7 @@ private static PullRequestInfo buildPullRequestInfo(
   }
 
   private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFactory) {
-    String ciWorkspace = ciInfo.getNormalizedCiWorkspace();
+    String ciWorkspace = ciInfo.getCiWorkspace();
     if (Strings.isNotBlank(ciWorkspace)) {
       return ciWorkspace;
 
@@ -146,14 +145,6 @@ private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFact
     }
   }
 
-  private static String appendSlashIfNeeded(String repoRoot) {
-    if (repoRoot != null && !repoRoot.endsWith(File.separator)) {
-      return repoRoot + File.separator;
-    } else {
-      return repoRoot;
-    }
-  }
-
   static String getModuleName(Config config, @Nullable String repoRoot, Path path) {
     // if parent process is instrumented, it will provide build system's module name
     String parentModuleName = config.getCiVisibilityModuleName();

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CIInfo.java

@@ -170,7 +170,7 @@ public CIInfo(
     this.ciPipelineNumber = ciPipelineNumber;
     this.ciPipelineUrl = ciPipelineUrl;
     this.ciJobUrl = ciJobUrl;
-    this.ciWorkspace = ciWorkspace;
+    this.ciWorkspace = sanitizeWorkspace(ciWorkspace);
     this.ciNodeName = ciNodeName;
     this.ciNodeLabels = ciNodeLabels;
     this.ciEnvVars = ciEnvVars;
@@ -209,20 +209,18 @@ public String getCiJobUrl() {
     return ciJobUrl;
   }
 
-  /**
-   * @deprecated This method is here only to satisfy CI spec tests. Use {@link
-   *     #getNormalizedCiWorkspace()}
-   */
-  @Deprecated
-  public String getCiWorkspace() {
-    return ciWorkspace;
+  private String sanitizeWorkspace(String workspace) {
+    String realCiWorkspace = FileUtils.toRealPath(workspace);
+    return (realCiWorkspace == null
+            || !realCiWorkspace.endsWith(File.separator)
+            || realCiWorkspace.length() == 1) // root path "/"
+        ? realCiWorkspace
+        : (realCiWorkspace.substring(0, realCiWorkspace.length() - 1));
   }
 
-  public String getNormalizedCiWorkspace() {
-    String realCiWorkspace = FileUtils.toRealPath(ciWorkspace);
-    return (realCiWorkspace == null || realCiWorkspace.endsWith(File.separator))
-        ? realCiWorkspace
-        : (realCiWorkspace + File.separator);
+  /** @return Workspace path without the trailing separator */
+  public String getCiWorkspace() {
+    return ciWorkspace;
   }
 
   public String getCiNodeName() {

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java

@@ -22,7 +22,7 @@ public CITagsProvider() {
   }
 
   public Map<String, String> getCiTags(CIInfo ciInfo, PullRequestInfo pullRequestInfo) {
-    String repoRoot = ciInfo.getNormalizedCiWorkspace();
+    String repoRoot = ciInfo.getCiWorkspace();
     GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
 
     return new CITagsBuilder()

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/CompilerAidedSourcePathResolver.java

@@ -1,6 +1,7 @@
 package datadog.trace.civisibility.source;
 
 import datadog.compiler.utils.CompilerUtils;
+import java.io.File;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -9,7 +10,7 @@ public class CompilerAidedSourcePathResolver implements SourcePathResolver {
   private final String repoRoot;
 
   public CompilerAidedSourcePathResolver(String repoRoot) {
-    this.repoRoot = repoRoot;
+    this.repoRoot = repoRoot.endsWith(File.separator) ? repoRoot : repoRoot + File.separator;
   }
 
   @Nullable

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIInfoTest.groovy

@@ -0,0 +1,22 @@
+package datadog.trace.civisibility.ci
+
+
+import spock.lang.Specification
+
+class CIInfoTest extends Specification {
+
+  def "test ci workspace is correctly sanitized #iterationIndex"() {
+    def builder = CIInfo.builder(null)
+    builder.ciWorkspace(workspacePath)
+    def info = builder.build()
+
+    info.ciWorkspace == sanitizedPath
+
+    where:
+    workspacePath | sanitizedPath
+    null          | null
+    "/"           | "/"
+    "/repo/path"  | "/repo/path"
+    "/repo/path/" | "/repo/path"
+  }
+}

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/codeowners/CodeownersProviderTest.groovy

@@ -8,7 +8,7 @@ import java.nio.file.Files
 
 class CodeownersProviderTest extends Specification {
 
-  private static final String REPO_ROOT = "/repo/root/"
+  private static final String REPO_ROOT = "/repo/root"
 
   def "test codeowners loading: #path"() {
     setup:
@@ -30,10 +30,10 @@ class CodeownersProviderTest extends Specification {
 
     where:
     path << [
-      REPO_ROOT + "CODEOWNERS",
-      REPO_ROOT + ".github/CODEOWNERS",
-      REPO_ROOT + ".gitlab/CODEOWNERS",
-      REPO_ROOT + "docs/CODEOWNERS"
+      REPO_ROOT + "/CODEOWNERS",
+      REPO_ROOT + "/.github/CODEOWNERS",
+      REPO_ROOT + "/.gitlab/CODEOWNERS",
+      REPO_ROOT + "/docs/CODEOWNERS"
     ]
   }
 }

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/CompilerAidedSourcePathResolverTest.groovy

@@ -5,7 +5,7 @@ import spock.lang.Specification
 
 class CompilerAidedSourcePathResolverTest extends Specification {
 
-  public static final String REPO_ROOT = "/repo/root/"
+  public static final String REPO_ROOT = "/repo/root"
   public static final String SOURCE_PATH_VALUE = "/repo/root/path/to/AClassWithSourceInfoInjected.java"
   public static final String SOURCE_PATH_OUTSIDE_REPO_VALUE = "/outside/path/to/AClassWithSourceInfoInjected.java"
 

internal-api/src/main/java/datadog/trace/api/git/GitInfoProvider.java

@@ -55,6 +55,7 @@ public GitInfo getGitInfo(@Nullable String repositoryPath) {
     if (repositoryPath == null) {
       repositoryPath = NULL_PATH_STRING;
     }
+
     return gitInfoCache.computeIfAbsent(repositoryPath, this::buildGitInfo);
   }