Merge pull request #304 from pchugh17/dfiu_ignore_pr_creation

Updated to support no-dfiu comment processing to ignore DFIU PRs

README.md

@@ -29,6 +29,7 @@ images they depend on get updated.
   * [Prerequisites](#prerequisites)
   * [Precautions](#precautions)
   * [How To Use It](#how-to-use-it)
+  * [Skip An Image](#additional-notes)
 * [Developer Guide](#developer-guide)
   * [Building](#building)
   * [Running locally](#running-locally)
@@ -130,6 +131,7 @@ optional arguments:
   -f, --auto-merge             NOT IMPLEMENTED / set to automatically merge pull requests if available
   -m PULL_REQ_MESSAGE          message to provide for pull requests
   -c COMMIT_MESSAGE            additional commit message for the commits in pull requests
+  -x IGNORE_IMAGE_STRING       comment snippet after FROM instruction for ignoring a child image. Defaults to 'no-dfiu'
 
 subcommands:
   Specify which feature to perform
@@ -192,6 +194,16 @@ optional arguments:
   -h, --help             show this help message and exit
 ```
 
+### Skip An Image
+
+In case you want the tool to skip updating a particular image tag then add a comment `no-dfiu` after the `FROM` declaration in the Dockerfile. The tool will process the comment following `FROM` declaration and if `no-dfiu` is mentioned, pull request for that image tag will be ignored. You can use an alternate comment string by passing an additional command line parameter `-x IGNORE_IMAGE_STRING`. In that case string mentioned with the parameter, will be used for skipping PR creation.
+
+Example:
+
+```
+FROM imagename:imagetag # no-dfiu
+```
+
 ## Developer Guide
 
 ### Building

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

@@ -82,6 +82,9 @@ static ArgumentParser getArgumentParser() {
                 .type(Boolean.class)
                 .setDefault(false)
                 .help("Only update image tag store. Skip creating PRs");
+        parser.addArgument("-x")
+                .help("comment snippet after FROM instruction for ignoring a child image. " +
+                        "Defaults to 'no-dfiu'");
         return parser;
     }
 

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

@@ -14,6 +14,7 @@ public class FromInstruction {
 
     private static final String NAME = "FROM";
     private static final String INVALID_INSTRUCTION_LINE = "You have not provided a valid FROM instruction line.";
+    private static final String NO_DFIU = "no-dfiu";
     /**
      * The name of the base image
      */
@@ -190,6 +191,24 @@ public boolean hasADifferentTag(String expectedTag) {
         return !tag.trim().equals(expectedTag.trim());
     }
 
+    /**
+     * Determines whether the comment has mentioned {@code ignoreImageString} to ignore creating dfiu PR
+     * If {@code ignoreImageString} present in comment, PR should be ignored
+     * If {@code ignoreImageString} is empty, then by default 'no-dfiu' comment will be searched
+     * @param ignoreImageString comment to search
+     * @return {@code true} if comment is found
+     */
+    public boolean ignorePR(String ignoreImageString) {
+        if (hasComments()) {
+            if (StringUtils.isNotBlank(ignoreImageString)) {
+                return comments.contains(ignoreImageString);
+            } else {
+                return comments.contains(NO_DFIU);
+            }
+        }
+        return false;
+    }
+
     public String getTag() {
         return tag;
     }

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

@@ -55,7 +55,7 @@ public void execute(final Namespace ns, final DockerfileGitHubUtil dockerfileGit
         dockerfileGitHubUtil.createOrUpdateForkBranchToParentDefault(repo, fork, gitForkBranch);
 
         log.info("Modifying on Github...");
-        dockerfileGitHubUtil.modifyAllOnGithub(fork, gitForkBranch.getBranchName(), img, forceTag);
+        dockerfileGitHubUtil.modifyAllOnGithub(fork, gitForkBranch.getBranchName(), img, forceTag, ns.get(Constants.IGNORE_IMAGE_STRING));
         dockerfileGitHubUtil.createPullReq(repo,
                 gitForkBranch.getBranchName(),
                 fork,

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

@@ -36,5 +36,6 @@ private Constants() {
     public static final String GIT_REPO_EXCLUDES = "excludes";
     public static final String GIT_API_SEARCH_LIMIT = "ghapisearchlimit";
     public static final String SKIP_PR_CREATION = "skipprcreation";
+    public static final String IGNORE_IMAGE_STRING = "x";
 
 }

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

@@ -207,7 +207,8 @@ public List<GHRepository> getGHRepositories(Multimap<String, String> pathToDocke
     }
 
     public void modifyAllOnGithub(GHRepository repo, String branch,
-                                  String img, String tag) throws IOException, InterruptedException {
+                                  String img, String tag, String ignoreImageString)
+            throws IOException, InterruptedException {
         List<GHContent> tree = null;
 
         /* There are issues with the GitHub API returning that the GitHub repository exists, but has no content,
@@ -226,21 +227,22 @@ public void modifyAllOnGithub(GHRepository repo, String branch,
         }
         if (tree != null) {
             for (GHContent con : tree) {
-                modifyOnGithubRecursive(repo, con, branch, img, tag);
+                modifyOnGithubRecursive(repo, con, branch, img, tag, ignoreImageString);
             }
         }
     }
 
     protected void modifyOnGithubRecursive(GHRepository repo, GHContent content,
-                                           String branch, String img, String tag) throws IOException {
+                                           String branch, String img, String tag,
+                                           String ignoreImageString) throws IOException {
         /* If we have a submodule; we want to skip.
            Content is submodule when the type is file, but content.getDownloadUrl() is null.
          */
         if (content.isFile() && content.getDownloadUrl() != null) {
-            modifyOnGithub(content, branch, img, tag, "");
+            modifyOnGithub(content, branch, img, tag, "", ignoreImageString);
         } else if(content.isDirectory()) {
             for (GHContent newContent : repo.getDirectoryContent(content.getPath(), branch)) {
-                modifyOnGithubRecursive(repo, newContent, branch, img, tag);
+                modifyOnGithubRecursive(repo, newContent, branch, img, tag, ignoreImageString);
             }
         } else {
             // The only other case is if we have a file, but content.getDownloadUrl() is null
@@ -253,31 +255,34 @@ public GHContent tryRetrievingContent(GHRepository repo, String path, String bra
     }
 
     public void modifyOnGithub(GHContent content,
-                               String branch, String img, String tag, String customMessage) throws IOException {
+                               String branch, String img, String tag,
+                               String customMessage, String ignoreImageString) throws IOException {
         try (InputStream stream = content.read();
              InputStreamReader streamR = new InputStreamReader(stream);
              BufferedReader reader = new BufferedReader(streamR)) {
-            findImagesAndFix(content, branch, img, tag, customMessage, reader);
+            findImagesAndFix(content, branch, img, tag, customMessage, reader, ignoreImageString);
         }
     }
 
-    protected void findImagesAndFix(GHContent content,
-                                    String branch, String img, String tag, String customMessage,
-                                    BufferedReader reader) throws IOException {
+    protected void findImagesAndFix(GHContent content, String branch, String img,
+                                    String tag, String customMessage, BufferedReader reader,
+                                    String ignoreImageString) throws IOException {
         StringBuilder strB = new StringBuilder();
-        boolean modified = rewriteDockerfile(img, tag, reader, strB);
+        boolean modified = rewriteDockerfile(img, tag, reader, strB, ignoreImageString);
         if (modified) {
             content.update(strB.toString(),
                     "Fix Dockerfile base image in /" + content.getPath() + "\n\n" + customMessage, branch);
         }
     }
 
-    protected boolean rewriteDockerfile(String img, String tag, BufferedReader reader, StringBuilder strB) throws IOException {
+    protected boolean rewriteDockerfile(String img, String tag,
+                                        BufferedReader reader, StringBuilder strB,
+                                        String ignoreImageString) throws IOException {
         String line;
         boolean modified = false;
         while ( (line = reader.readLine()) != null ) {
             /* Once true, should stay true. */
-            modified = changeIfDockerfileBaseImageLine(img, tag, strB, line) || modified;
+            modified = changeIfDockerfileBaseImageLine(img, tag, strB, line, ignoreImageString) || modified;
         }
         return modified;
     }
@@ -297,15 +302,18 @@ protected boolean rewriteDockerfile(String img, String tag, BufferedReader reade
      * @param line the inbound line from the Dockerfile
      * @return Whether we've modified the {@code line} that goes into {@code stringBuilder}
      */
-    protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag, StringBuilder stringBuilder, String line) {
+    protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag,
+                                                      StringBuilder stringBuilder, String line,
+                                                      String ignoreImageString) {
         boolean modified = false;
         String outputLine = line;
 
         // Only check/modify lines which contain a FROM instruction
         if (FromInstruction.isFromInstruction(line)) {
             FromInstruction fromInstruction = new FromInstruction(line);
             if (fromInstruction.hasBaseImage(imageToFind) &&
-                    fromInstruction.hasADifferentTag(tag)) {
+                    fromInstruction.hasADifferentTag(tag) &&
+                    !fromInstruction.ignorePR(ignoreImageString)) {
                 fromInstruction = fromInstruction.getFromInstructionWithNewTag(tag);
                 modified = true;
             }
@@ -462,7 +470,7 @@ public void changeDockerfiles(Namespace ns,
                 log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
             } else {
                 modifyOnGithub(content, gitForkBranch.getBranchName(), gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
-                        ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE));
+                        ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE), ns.get(Constants.IGNORE_IMAGE_STRING));
                 isContentModified = true;
                 isRepoSkipped = false;
             }

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

@@ -239,4 +239,26 @@ public Object[][] isFromInstructionWithThisImageAndOlderTagData() {
     public void isFromInstructionWithThisImageAndOlderTag(String line, String imageName, String imageTag, boolean expectedResult) {
         assertEquals(FromInstruction.isFromInstructionWithThisImageAndOlderTag(line, imageName, imageTag), expectedResult);
     }
+
+    @DataProvider
+    public Object[][] commentDataNoDfiu() {
+        return new Object[][] {
+                {"FROM image:tag as builder",       false},
+                {"FROM image:tag#no-dfiu",          true},
+                {"FROM image:tag # no-dfiu",        true},
+                {"FROM image:tag\t# no-dfiu",       true},
+                {"FROM image:\t# no-dfiu # # # ",   true},
+                {"FROM image:",                     false},
+                {"FROM image:test # :no-dfiu",      true},
+                {"FROM image:test # no-dfiu added comments for ignoring dfiu PR",      true},
+                {"FROM no-dfiu:test",               false},
+                {"FROM image:no-dfiu",              false},
+                {"FROM no-dfiu",                    false}
+        };
+    }
+
+    @Test(dataProvider = "commentDataNoDfiu")
+    public void testCommentsWithNoDfiuParsedCorrectly(String input, Boolean expectedResult) {
+        assertEquals(new FromInstruction(input).ignorePR(""), expectedResult);
+    }
 }

dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java

@@ -51,7 +51,7 @@ public void checkPullRequestMade(Map<String, Object> inputMap) throws Exception
         DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
         Mockito.when(dockerfileGitHubUtil.getRepo(Mockito.any())).thenReturn(new GHRepository());
         Mockito.when(dockerfileGitHubUtil.getOrCreateFork(Mockito.any())).thenReturn(new GHRepository());
-        doNothing().when(dockerfileGitHubUtil).modifyAllOnGithub(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+        doNothing().when(dockerfileGitHubUtil).modifyAllOnGithub(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
         GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class);
         when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore);
         doNothing().when(dockerfileGitHubUtil).createPullReq(Mockito.any(), anyString(), Mockito.any(), any());