Merge pull request #1125 from salesforce/W-14174147

Add a debug flag that enables wire logging for git http requests

dockerfile-image-update/pom.xml

@@ -144,6 +144,16 @@
             <artifactId>bucket4j-core</artifactId>
             <version>8.1.1</version>
         </dependency>
+        <dependency>
+            <groupId>com.squareup.okhttp3</groupId>
+            <artifactId>okhttp</artifactId>
+            <version>4.9.1</version>
+        </dependency>
+        <dependency>
+            <groupId>com.squareup.okhttp3</groupId>
+            <artifactId>logging-interceptor</artifactId>
+            <version>4.9.1</version>
+        </dependency>
     </dependencies>
 
     <build>

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java

@@ -10,21 +10,29 @@
 
 import com.google.common.reflect.ClassPath;
 import com.salesforce.dockerfileimageupdate.subcommands.ExecutableWithNamespace;
-import com.salesforce.dockerfileimageupdate.utils.Constants;
 import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil;
 import com.salesforce.dockerfileimageupdate.utils.GitHubUtil;
 import net.sourceforge.argparse4j.ArgumentParsers;
 import net.sourceforge.argparse4j.impl.Arguments;
 import net.sourceforge.argparse4j.inf.*;
+import okhttp3.OkHttpClient;
+import okhttp3.logging.HttpLoggingInterceptor;
+
 import org.kohsuke.github.GitHub;
 import org.kohsuke.github.GitHubBuilder;
+import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.Comparator;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import static com.salesforce.dockerfileimageupdate.utils.Constants.*;
 
@@ -45,8 +53,8 @@ public static void main(String[] args)
         Namespace ns = handleArguments(parser, args);
         if (ns == null)
             System.exit(1);
-        Class<?> runClass = loadCommand(allClasses, ns.get(Constants.COMMAND));
-        DockerfileGitHubUtil dockerfileGitHubUtil = initializeDockerfileGithubUtil(ns.get(Constants.GIT_API));
+        Class<?> runClass = loadCommand(allClasses, ns.get(COMMAND));
+        DockerfileGitHubUtil dockerfileGitHubUtil = initializeDockerfileGithubUtil(gitApiUrl(ns), gitApiToken(), ns.getBoolean(DEBUG));
 
         /* Execute given command. */
         ((ExecutableWithNamespace)runClass.getDeclaredConstructor().newInstance()).execute(ns, dockerfileGitHubUtil);
@@ -57,30 +65,30 @@ static ArgumentParser getArgumentParser() {
                 ArgumentParsers.newFor("dockerfile-image-update").addHelp(true).build()
                 .description("Image Updates through Pull Request Automator");
 
-        parser.addArgument("-l", "--" + Constants.GIT_API_SEARCH_LIMIT)
+        parser.addArgument("-l", "--" + GIT_API_SEARCH_LIMIT)
                 .type(Integer.class)
                 .setDefault(1000)
                 .help("limit the search results for github api (default: 1000)");
-        parser.addArgument("-o", "--" + Constants.GIT_ORG)
+        parser.addArgument("-o", "--" + GIT_ORG)
                 .help("search within specific organization (default: all of github)");
         /* Currently, because of argument passing reasons, you can only specify one branch. */
-        parser.addArgument("-b", "--" + Constants.GIT_BRANCH)
+        parser.addArgument("-b", "--" + GIT_BRANCH)
                 .help("make pull requests for given branch name (default: main)");
-        parser.addArgument("-g", "--" + Constants.GIT_API)
+        parser.addArgument("-g", "--" + GIT_API)
                 .help("link to github api; overrides environment variable");
         parser.addArgument("-f", "--auto-merge").action(Arguments.storeTrue())
                 .help("NOT IMPLEMENTED / set to automatically merge pull requests if available");
         parser.addArgument("-m")
                 .help("message to provide for pull requests");
         parser.addArgument("-c")
                 .help("additional commit message for the commits in pull requests");
-        parser.addArgument("-e", "--" + Constants.GIT_REPO_EXCLUDES)
+        parser.addArgument("-e", "--" + GIT_REPO_EXCLUDES)
                 .help("regex of repository names to exclude from pull request generation");
         parser.addArgument("-B")
                 .help("additional body text to include in pull requests");
-        parser.addArgument("-s", "--" + Constants.SKIP_PR_CREATION)
+        parser.addArgument("-s", "--" + SKIP_PR_CREATION)
                 .type(Boolean.class)
-                .setDefault(false)
+                .setDefault(false) //To prevent null from being returned by the argument
                 .help("Only update image tag store. Skip creating PRs");
         parser.addArgument("-x")
                 .help("comment snippet mentioned in line just before 'FROM' instruction(Dockerfile)" +
@@ -95,14 +103,19 @@ static ArgumentParser getArgumentParser() {
                 .type(String.class)
                 .required(false)
                 .help("Use RateLimiting when sending PRs. RateLimiting is enabled only if this value is set it's disabled by default.");
+        parser.addArgument("-d", "--" + DEBUG)
+                .type(Boolean.class)
+                .setDefault(false) //To prevent null from being returned by the argument
+                .required(false)
+                .help("Enable debug logging, including git wire logs.");
         return parser;
     }
 
     /*  Adding subcommands to the subcommands list.
             argparse4j allows commands to be truncated, so users can type the first letter (a,c,p) for commands */
     public static Set<ClassPath.ClassInfo> findSubcommands(ArgumentParser parser) throws IOException {
         Subparsers subparsers = parser.addSubparsers()
-                .dest(Constants.COMMAND)
+                .dest(COMMAND)
                 .help("FEATURE")
                 .title("subcommands")
                 .description("Specify which feature to perform")
@@ -181,27 +194,65 @@ public static Class<?> loadCommand(Set<ClassPath.ClassInfo> allClasses, String c
         return runClass;
     }
 
+    public static String gitApiToken() {
+      return gitApiToken(System::getenv, System::exit);
+    }
+
+    public static String gitApiToken(final Function<String, String> envFunc, final Consumer<Integer> exitFunc) {
+      final String token = envFunc.apply("git_api_token");
+      if (Objects.isNull(token)) {
+          log.error("Please provide GitHub token in environment variables.");
+          exitFunc.accept(3);
+      }
+      return token;
+    }
+
+    public static String gitApiUrl(final Namespace ns) throws IOException {
+      return gitApiUrl(ns, System::getenv);
+    }
+
+    public static String gitApiUrl(final Namespace ns, final Function<String, String> envFunc) throws IOException {
+      return Optional.ofNullable(
+                 Optional.ofNullable(ns.getString(GIT_API))
+                   .orElse(envFunc.apply("git_api_url"))
+             )
+             .orElseThrow(() -> new IOException("No Git API URL in environment variables nor on the commmand line."));
+    }
+
+    public static DockerfileGitHubUtil initializeDockerfileGithubUtil(
+        final String gitApiUrl,
+        final String token,
+        final boolean debug) throws IOException
+    {
+      return initializeDockerfileGithubUtil(gitApiUrl, token, () -> new GitHubBuilder(), debug);
+    }
+
     /* Validate API URL and connect to the API using credentials. */
-    public static DockerfileGitHubUtil initializeDockerfileGithubUtil(String gitApiUrl) throws IOException {
-        if (gitApiUrl == null) {
-            gitApiUrl = System.getenv("git_api_url");
-            if (gitApiUrl == null) {
-                throw new IOException("No Git API URL in environment variables.");
-            }
-        }
-        String token = System.getenv("git_api_token");
-        if (token == null) {
-            log.error("Please provide GitHub token in environment variables.");
-            System.exit(3);
-        }
+    public static DockerfileGitHubUtil initializeDockerfileGithubUtil(
+        final String gitApiUrl,
+        final String token,
+        final Supplier<GitHubBuilder> builderFunc,
+        final boolean debug) throws IOException {
 
-        GitHub github = new GitHubBuilder().withEndpoint(gitApiUrl)
+        GitHub github = shouldAddWireLogger(builderFunc.get(), debug)
+                .withEndpoint(gitApiUrl)
                 .withOAuthToken(token)
                 .build();
         github.checkApiUrlValidity();
 
-        GitHubUtil gitHubUtil = new GitHubUtil(github);
+        return new DockerfileGitHubUtil(new GitHubUtil(github));
+    }
+
+    public static GitHubBuilder shouldAddWireLogger(final GitHubBuilder builder, final boolean debug) {
+      if (debug) {
+        HttpLoggingInterceptor logger = new HttpLoggingInterceptor();
+        logger.setLevel(HttpLoggingInterceptor.Level.HEADERS);
+        logger.redactHeader("Authorization");
 
-        return new DockerfileGitHubUtil(gitHubUtil);
+        builder.withConnector(new OkHttpGitHubConnector(new OkHttpClient.Builder() 
+            .addInterceptor(logger)
+            .build()));
+      }
+      return builder;
     }
 }

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java

@@ -54,7 +54,7 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti
             log.error("Could not initialize the Image tage store. Exception: ", e);
         }
 
-        if (ns.get(Constants.SKIP_PR_CREATION)) {
+        if (ns.getBoolean(Constants.SKIP_PR_CREATION)) {
             log.info("Since the flag {} is set to True, the PR creation steps will "
                     + "be skipped.", Constants.SKIP_PR_CREATION);
             return;

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java

@@ -41,6 +41,7 @@ private Constants() {
     public static final String IGNORE_IMAGE_STRING = "x";
     public static final String FILE_NAMES_TO_SEARCH = "filenamestosearch";
     public static final String RATE_LIMIT_PR_CREATION = "rate_limit_pr_creations";
+    public static final String DEBUG = "debug";
     //max number of PRs to be sent (or tokens to be added)  per DEFAULT_RATE_LIMIT_DURATION(per hour in this case)
     public static final long DEFAULT_RATE_LIMIT = 60;
 

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java

@@ -39,7 +39,7 @@ public DockerfileGitHubUtil(GitHubUtil gitHubUtil) {
         this.gitHubUtil = gitHubUtil;
     }
 
-    protected GitHubUtil getGitHubUtil() { return gitHubUtil; }
+    public GitHubUtil getGitHubUtil() { return gitHubUtil; }
 
     /**
      * Return an existing fork in the current user's org or create one if it does not exist

dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/CommandLineTest.java

@@ -9,14 +9,28 @@
 package com.salesforce.dockerfileimageupdate;
 
 import com.google.common.reflect.ClassPath;
+import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil;
+
+import org.kohsuke.github.GitHub;
+import org.kohsuke.github.GitHubBuilder;
+import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
+import org.mockito.Mockito;
 import org.testng.annotations.Test;
 
+import net.sourceforge.argparse4j.inf.Namespace;
+
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Consumer;
+import java.util.function.Function;
 
+import static com.salesforce.dockerfileimageupdate.utils.Constants.GIT_API;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.never;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertSame;
 
 /**
  * Created by afalko on 10/25/17.
@@ -31,4 +45,91 @@ public void testLoadSubcommands() throws Exception {
                 classInfo -> assertThat(expectedSubCommands).contains(classInfo.getSimpleName())
         );
     }
+
+    @Test
+    public void testShouldAddWireLoggerDisabled() {
+      GitHubBuilder builder = Mockito.spy(new GitHubBuilder());
+      CommandLine.shouldAddWireLogger(builder, false);
+
+      Mockito.verify(builder, never()).withConnector(Mockito.any(OkHttpGitHubConnector.class));
+    }
+
+    @Test
+    public void testShouldAddWireLoggerEnabled() {
+      GitHubBuilder builder = Mockito.spy(new GitHubBuilder());
+      CommandLine.shouldAddWireLogger(builder, true);
+
+      Mockito.verify(builder).withConnector(Mockito.any(OkHttpGitHubConnector.class));
+    }
+
+    @Test
+    public void testInitializeDockerfileGithubUtil() throws IOException {
+      GitHubBuilder builder = Mockito.spy(new GitHubBuilder());
+      GitHub github = Mockito.mock(GitHub.class);
+
+      Mockito.doReturn(github).when(builder).build();
+
+      DockerfileGitHubUtil retval = CommandLine.initializeDockerfileGithubUtil("someUrl", "someToken", () -> builder, true);
+
+      Mockito.verify(builder).withEndpoint(Mockito.eq("someUrl"));
+      Mockito.verify(builder).withOAuthToken(Mockito.eq("someToken"));
+      Mockito.verify(builder).withConnector(Mockito.any(OkHttpGitHubConnector.class));
+      Mockito.verify(github).checkApiUrlValidity();
+      assertSame(retval.getGitHubUtil().getGithub(), github);
+    }
+
+    @Test
+    public void testGitApiUrlNonNullNamespace() throws IOException {
+      Namespace ns = Mockito.mock(Namespace.class);
+      Mockito.doReturn("someUrl").when(ns).getString(GIT_API);
+
+      String retval = CommandLine.gitApiUrl(ns, x -> null);
+      assertEquals(retval, "someUrl");
+    }
+
+    @Test
+    public void testGitApiUrlNullNamespace() throws IOException {
+      Namespace ns = Mockito.mock(Namespace.class);
+      Mockito.doReturn(null).when(ns).getString(GIT_API);
+
+      String retval = CommandLine.gitApiUrl(ns, x -> "anotherUrl");
+      assertEquals(retval, "anotherUrl");
+    }
+
+    @Test(expectedExceptions = { IOException.class } )
+    public void testGitApiUrlNamespaceAndEnvNull() throws IOException {
+      Namespace ns = Mockito.mock(Namespace.class);
+      Mockito.doReturn(null).when(ns).getString(GIT_API);
+
+      String retval = CommandLine.gitApiUrl(ns, x -> null);
+      assertEquals(retval, "anotherUrl");
+    }
+
+    @Test
+    public void gitApiTokenAvailable() {
+      Function<String, String> envFunc = x -> {
+        assertEquals(x, "git_api_token");
+        return "token";
+      };
+
+      Consumer<Integer> exitFunc = new Consumer<Integer>() {
+        @Override
+        public void accept(Integer t) {}
+      };
+
+      final String retval = CommandLine.gitApiToken(envFunc, exitFunc);
+      assertEquals(retval, "token");
+    }
+
+    @Test
+    public void gitApiTokenNull() {
+      Function<String, String> envFunc = x -> {
+        assertEquals(x, "git_api_token");
+        return null;
+      };
+
+      Consumer<Integer> exitFunc = x -> assertEquals(x, 3);
+
+      CommandLine.gitApiToken(envFunc, exitFunc);
+    }
 }