From 50df3651731f17c1ae0f5a763b4e5ab8a12245fd Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Tue, 6 Feb 2024 17:36:35 -0800 Subject: [PATCH 01/11] Skip DFIU PRs for renovate onboarded repos --- .gitignore | 1 + dockerfile-image-update/pom.xml | 5 +++ .../dockerfileimageupdate/CommandLine.java | 4 ++ .../utils/Constants.java | 4 ++ .../utils/PullRequests.java | 37 +++++++++++++++++-- .../utils/PullRequestsTest.java | 6 +-- 6 files changed, 50 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index a91ea149..3007cd07 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.ipr *.iws *.iml +*.DS_Store # Vim files *.swp diff --git a/dockerfile-image-update/pom.xml b/dockerfile-image-update/pom.xml index 80ed25a2..ac6da31d 100644 --- a/dockerfile-image-update/pom.xml +++ b/dockerfile-image-update/pom.xml @@ -69,6 +69,11 @@ + + org.json + json + 20200518 + org.slf4j slf4j-simple diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java index 19d723d6..61ab975a 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java @@ -65,6 +65,10 @@ static ArgumentParser getArgumentParser() { ArgumentParsers.newFor("dockerfile-image-update").addHelp(true).build() .description("Image Updates through Pull Request Automator"); + parser.addArgument("-re", "--" + CHECK_FOR_RENOVATE) + .type(Boolean.class) + .setDefault(false) //To prevent null from being returned by the argument + .help("Check if Renovate app is being used to receive remediation PRs."); parser.addArgument("-l", "--" + GIT_API_SEARCH_LIMIT) .type(Integer.class) .setDefault(1000) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index 8c440d33..70c0f8cb 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -51,4 +51,8 @@ private Constants() { public static final Duration DEFAULT_TOKEN_ADDING_RATE = Duration.ofMinutes(DEFAULT_CONSUMING_TOKEN_RATE); public static final String FILENAME_DOCKERFILE = "dockerfile"; public static final String FILENAME_DOCKER_COMPOSE = "docker-compose"; + + public static final String CHECK_FOR_RENOVATE = "checkforrenovate"; + + public static final String RENOVATE_CONFIG_FILENAME = "renovate.json"; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index bbd059a6..1c17de6d 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -4,6 +4,7 @@ import com.salesforce.dockerfileimageupdate.model.*; import com.salesforce.dockerfileimageupdate.process.*; import net.sourceforge.argparse4j.inf.*; +import org.json.*; import org.kohsuke.github.*; import org.slf4j.*; @@ -27,10 +28,17 @@ public void prepareToCreate(final Namespace ns, pathToDockerfilesInParentRepo.get(currUserRepo).stream().findFirst(); if (forkWithContentPaths.isPresent()) { try { - dockerfileGitHubUtil.changeDockerfiles(ns, - pathToDockerfilesInParentRepo, - forkWithContentPaths.get(), skippedRepos, - gitForkBranch, rateLimiter); + if(ns.getBoolean(Constants.CHECK_FOR_RENOVATE) + && (isRenovateEnabled(Constants.RENOVATE_CONFIG_FILENAME, forkWithContentPaths.get()))) { + log.info("Found file with name %s in the repo %s. Skip sending DFIU PRs.", Constants.RENOVATE_CONFIG_FILENAME, forkWithContentPaths.get().getParent().getFullName()); + } else { + dockerfileGitHubUtil.changeDockerfiles(ns, + pathToDockerfilesInParentRepo, + forkWithContentPaths.get(), skippedRepos, + gitForkBranch, rateLimiter); + } + + } catch (IOException | InterruptedException e) { log.error(String.format("Error changing Dockerfile for %s", forkWithContentPaths.get().getParent().getFullName()), e); exceptions.add((IOException) e); @@ -41,5 +49,26 @@ public void prepareToCreate(final Namespace ns, } ResultsProcessor.processResults(skippedRepos, exceptions, log); } + + private boolean isRenovateEnabled(String fileName, GitHubContentToProcess fork) throws IOException { + try { + GHContent fileContent = fork.getParent().getFileContent(fileName); + InputStream is = fileContent.read(); + BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is)); + StringBuilder sb = new StringBuilder(); + String line; + while ((line = bufferedReader.readLine()) != null) { + sb.append(line); + } + JSONObject json = new JSONObject(sb.toString()); + if (json.has("enabled") && json.get("enabled") == "false") { + return false; + } + } catch (FileNotFoundException e) { + log.debug("The file with name %s not found in the repository."); + return false; + } + return true; + } } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java index 2affc13f..c1c2dc17 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java @@ -21,7 +21,7 @@ public void testPullRequestsPrepareToCreateSuccessful() throws Exception { "image", Constants.TAG, "tag", Constants.STORE, "store", Constants.SKIP_PR_CREATION, - false); + false, Constants.CHECK_FOR_RENOVATE, false); Namespace ns = new Namespace(nsMap); PullRequests pullRequests = new PullRequests(); GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); @@ -51,7 +51,7 @@ public void testPullRequestsPrepareThrowsException() throws Exception { "image", Constants.TAG, "tag", Constants.STORE, "store", Constants.SKIP_PR_CREATION, - false); + false, Constants.CHECK_FOR_RENOVATE, false); Namespace ns = new Namespace(nsMap); PullRequests pullRequests = new PullRequests(); GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); @@ -89,7 +89,7 @@ public void testPullRequestsPrepareToCreateWhenNoDockerfileFound() throws Except "image", Constants.TAG, "tag", Constants.STORE, "store", Constants.SKIP_PR_CREATION, - false); + false, Constants.CHECK_FOR_RENOVATE, false); Namespace ns = new Namespace(nsMap); PullRequests pullRequests = new PullRequests(); GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); From bbb42f839f031059917c6dd4412742a938f49362 Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Wed, 7 Feb 2024 15:01:25 -0800 Subject: [PATCH 02/11] Adding tests --- .../utils/Constants.java | 2 +- .../utils/PullRequests.java | 32 +++++++------ .../utils/PullRequestsTest.java | 48 +++++++++++++++++++ 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index 70c0f8cb..732212ec 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -54,5 +54,5 @@ private Constants() { public static final String CHECK_FOR_RENOVATE = "checkforrenovate"; - public static final String RENOVATE_CONFIG_FILENAME = "renovate.json"; + public static final String RENOVATE_CONFIG_FILEPATH = "renovate.json"; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 1c17de6d..56abd74f 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -28,17 +28,16 @@ public void prepareToCreate(final Namespace ns, pathToDockerfilesInParentRepo.get(currUserRepo).stream().findFirst(); if (forkWithContentPaths.isPresent()) { try { + //If the repository has been onboarded to renovate enterprise, skip sending the DFIU PR if(ns.getBoolean(Constants.CHECK_FOR_RENOVATE) - && (isRenovateEnabled(Constants.RENOVATE_CONFIG_FILENAME, forkWithContentPaths.get()))) { - log.info("Found file with name %s in the repo %s. Skip sending DFIU PRs.", Constants.RENOVATE_CONFIG_FILENAME, forkWithContentPaths.get().getParent().getFullName()); + && (isRenovateEnabled(Constants.RENOVATE_CONFIG_FILEPATH, forkWithContentPaths.get()))) { + log.info("Found file with name %s in the repo %s. Skip sending DFIU PRs to this repository.", Constants.RENOVATE_CONFIG_FILEPATH, forkWithContentPaths.get().getParent().getFullName()); } else { dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo, forkWithContentPaths.get(), skippedRepos, gitForkBranch, rateLimiter); } - - } catch (IOException | InterruptedException e) { log.error(String.format("Error changing Dockerfile for %s", forkWithContentPaths.get().getParent().getFullName()), e); exceptions.add((IOException) e); @@ -50,22 +49,27 @@ public void prepareToCreate(final Namespace ns, ResultsProcessor.processResults(skippedRepos, exceptions, log); } - private boolean isRenovateEnabled(String fileName, GitHubContentToProcess fork) throws IOException { + /** + * Check if the repository is onboarded to Renovate. The signal we are looking for are + * (1) The presence of a file named renovate.json in the root of the repository + * (2) Ensuring that the file does not have the key "enabled" set to "false" + * @param filePath the name of the file that we are searching for in the repo + * @param fork A GitHubContentToProcess object that contains the fork repository that is under process + * @return true if the file is found in the path specified and is not disabled, false otherwise + */ + private boolean isRenovateEnabled(String filePath, GitHubContentToProcess fork) throws IOException { try { - GHContent fileContent = fork.getParent().getFileContent(fileName); + GHContent fileContent = fork.getParent().getFileContent(filePath); InputStream is = fileContent.read(); BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is)); - StringBuilder sb = new StringBuilder(); - String line; - while ((line = bufferedReader.readLine()) != null) { - sb.append(line); - } - JSONObject json = new JSONObject(sb.toString()); - if (json.has("enabled") && json.get("enabled") == "false") { + JSONTokener tokener = new JSONTokener(bufferedReader); + JSONObject json = new JSONObject(tokener); + //If the renovate.json file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason + if (json.has("enabled") && json.get("enabled").toString() == "false") { return false; } } catch (FileNotFoundException e) { - log.debug("The file with name %s not found in the repository."); + log.debug("The file with name %s not found in the repository.", filePath); return false; } return true; diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java index c1c2dc17..ce73f3e1 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java @@ -10,6 +10,7 @@ import java.io.*; import java.util.*; +import java.util.Optional; import static org.mockito.Mockito.*; import static org.testng.Assert.assertThrows; @@ -110,4 +111,51 @@ public void testPullRequestsPrepareToCreateWhenNoDockerfileFound() throws Except eq(pathToDockerfilesInParentRepo), eq(gitHubContentToProcess), anyList(), eq(gitForkBranch),eq(rateLimiter)); } + + @Test + public void testPullRequestsPrepareSkipsSendingPRIfRepoOnboardedToRenovate() throws Exception { + Map nsMap = ImmutableMap.of( + Constants.IMG, "image", + Constants.TAG, "tag", + Constants.STORE,"store", + Constants.SKIP_PR_CREATION,false, + Constants.CHECK_FOR_RENOVATE, true); + + + Namespace ns = new Namespace(nsMap); + PullRequests pullRequests = new PullRequests(); + GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); + PagedSearchIterable contentsFoundWithImage = mock(PagedSearchIterable.class); + GitForkBranch gitForkBranch = mock(GitForkBranch.class); + DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); + RateLimiter rateLimiter = Mockito.spy(new RateLimiter()); + Multimap pathToDockerfilesInParentRepo = ArrayListMultimap.create(); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + pathToDockerfilesInParentRepo.put("repo1", gitHubContentToProcess); + pathToDockerfilesInParentRepo.put("repo2", gitHubContentToProcess); + pathToDockerfilesInParentRepo.put("repo3", gitHubContentToProcess); + GHContent content = mock(GHContent.class); + InputStream inputStream1 = new ByteArrayInputStream("{someKey:someValue}".getBytes()); + InputStream inputStream2 = new ByteArrayInputStream("{enabled:false}".getBytes()); + GHRepository ghRepository = mock(GHRepository.class); + + when(pullRequestSender.forkRepositoriesFoundAndGetPathToDockerfiles(contentsFoundWithImage, gitForkBranch)).thenReturn(pathToDockerfilesInParentRepo); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + //Fetch the content of the renovate.json file for the 3 repos. + // The first one returns a file with regular json content. + // The second one returns a file with the key 'enabled' set to 'false' to replicate a repo that has been onboarded to renovate but has it disabled + // The third repo does not have the renovate.json file + when(ghRepository.getFileContent(anyString())).thenReturn(content).thenReturn(content).thenThrow(new FileNotFoundException()); + when(ghRepository.getFullName()).thenReturn("org/repo"); + when(content.read()).thenReturn(inputStream1).thenReturn(inputStream2); + + pullRequests.prepareToCreate(ns, pullRequestSender, contentsFoundWithImage, + gitForkBranch, dockerfileGitHubUtil, rateLimiter); + + //Verify that the DFIU PR is skipped for the first repo, but is sent to the other two repos + verify(dockerfileGitHubUtil, times(2)).changeDockerfiles(eq(ns), + eq(pathToDockerfilesInParentRepo), + eq(gitHubContentToProcess), anyList(), eq(gitForkBranch), + eq(rateLimiter)); + } } From 62b1d6b294e0176e36b8bc9253fbf482fe77ec66 Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Wed, 7 Feb 2024 15:11:19 -0800 Subject: [PATCH 03/11] Updating version of dependency --- dockerfile-image-update/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockerfile-image-update/pom.xml b/dockerfile-image-update/pom.xml index ac6da31d..931efe9a 100644 --- a/dockerfile-image-update/pom.xml +++ b/dockerfile-image-update/pom.xml @@ -72,7 +72,7 @@ org.json json - 20200518 + 20240205 org.slf4j From 50725f7d7026c6f420d966a3411a829e1e28746e Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Wed, 7 Feb 2024 15:15:13 -0800 Subject: [PATCH 04/11] Fix imports --- .../com/salesforce/dockerfileimageupdate/utils/Constants.java | 2 -- .../salesforce/dockerfileimageupdate/utils/PullRequests.java | 3 ++- .../dockerfileimageupdate/utils/PullRequestsTest.java | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index 732212ec..a1daf355 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -51,8 +51,6 @@ private Constants() { public static final Duration DEFAULT_TOKEN_ADDING_RATE = Duration.ofMinutes(DEFAULT_CONSUMING_TOKEN_RATE); public static final String FILENAME_DOCKERFILE = "dockerfile"; public static final String FILENAME_DOCKER_COMPOSE = "docker-compose"; - public static final String CHECK_FOR_RENOVATE = "checkforrenovate"; - public static final String RENOVATE_CONFIG_FILEPATH = "renovate.json"; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 56abd74f..b4785c89 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -4,7 +4,8 @@ import com.salesforce.dockerfileimageupdate.model.*; import com.salesforce.dockerfileimageupdate.process.*; import net.sourceforge.argparse4j.inf.*; -import org.json.*; +import org.json.JSONObject; +import org.json.JSONTokener; import org.kohsuke.github.*; import org.slf4j.*; diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java index ce73f3e1..99da117f 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java @@ -10,7 +10,6 @@ import java.io.*; import java.util.*; -import java.util.Optional; import static org.mockito.Mockito.*; import static org.testng.Assert.assertThrows; From 5bd598bcc4b24618b40d2557f183a616d4a7081c Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 08:47:35 -0800 Subject: [PATCH 05/11] Checking for boolean value in config file --- .../salesforce/dockerfileimageupdate/utils/PullRequests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index b4785c89..2a30eb72 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -66,7 +66,7 @@ private boolean isRenovateEnabled(String filePath, GitHubContentToProcess fork) JSONTokener tokener = new JSONTokener(bufferedReader); JSONObject json = new JSONObject(tokener); //If the renovate.json file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason - if (json.has("enabled") && json.get("enabled").toString() == "false") { + if (json.has("enabled") && json.getBoolean("enabled") == false) { return false; } } catch (FileNotFoundException e) { From 10a819a81620b287b848dba79d08e6561bc923fd Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 10:58:29 -0800 Subject: [PATCH 06/11] Addressing PR comments --- .../dockerfileimageupdate/CommandLine.java | 2 +- .../utils/Constants.java | 4 +- .../utils/PullRequests.java | 37 ++++++++++--------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java index 61ab975a..745cecf2 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java @@ -65,7 +65,7 @@ static ArgumentParser getArgumentParser() { ArgumentParsers.newFor("dockerfile-image-update").addHelp(true).build() .description("Image Updates through Pull Request Automator"); - parser.addArgument("-re", "--" + CHECK_FOR_RENOVATE) + parser.addArgument("-R", "--" + CHECK_FOR_RENOVATE) .type(Boolean.class) .setDefault(false) //To prevent null from being returned by the argument .help("Check if Renovate app is being used to receive remediation PRs."); diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index a1daf355..194f7112 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -10,6 +10,7 @@ import java.time.Duration; +import java.util.*; /** * @author minho-park @@ -52,5 +53,6 @@ private Constants() { public static final String FILENAME_DOCKERFILE = "dockerfile"; public static final String FILENAME_DOCKER_COMPOSE = "docker-compose"; public static final String CHECK_FOR_RENOVATE = "checkforrenovate"; - public static final String RENOVATE_CONFIG_FILEPATH = "renovate.json"; + //The Renovate configuration file can be in any one of the following locations. Refer to https://docs.renovatebot.com/configuration-options/ + public static final List RENOVATE_CONFIG_FILEPATHS = Arrays.asList("renovate.json", "renovate.json5", ".github/renovate.json", ".github/renovate.json5", ".gitlab/renovate.json", ".gitlab/renovate.json5", ".renovaterc", ".renovaterc.json", ".renovaterc.json5"); } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 2a30eb72..8006993f 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -31,8 +31,8 @@ public void prepareToCreate(final Namespace ns, try { //If the repository has been onboarded to renovate enterprise, skip sending the DFIU PR if(ns.getBoolean(Constants.CHECK_FOR_RENOVATE) - && (isRenovateEnabled(Constants.RENOVATE_CONFIG_FILEPATH, forkWithContentPaths.get()))) { - log.info("Found file with name %s in the repo %s. Skip sending DFIU PRs to this repository.", Constants.RENOVATE_CONFIG_FILEPATH, forkWithContentPaths.get().getParent().getFullName()); + && (isRenovateEnabled(Constants.RENOVATE_CONFIG_FILEPATHS, forkWithContentPaths.get()))) { + log.info("Found a renovate configuration file in the repo %s. Skip sending DFIU PRs to this repository.", forkWithContentPaths.get().getParent().getFullName()); } else { dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo, @@ -52,28 +52,29 @@ public void prepareToCreate(final Namespace ns, /** * Check if the repository is onboarded to Renovate. The signal we are looking for are - * (1) The presence of a file named renovate.json in the root of the repository + * (1) The presence of a file where renovate configurations are stored in the repository * (2) Ensuring that the file does not have the key "enabled" set to "false" - * @param filePath the name of the file that we are searching for in the repo + * @param filePaths the list that contains all the names of the files that we are searching for in the repo * @param fork A GitHubContentToProcess object that contains the fork repository that is under process * @return true if the file is found in the path specified and is not disabled, false otherwise */ - private boolean isRenovateEnabled(String filePath, GitHubContentToProcess fork) throws IOException { - try { - GHContent fileContent = fork.getParent().getFileContent(filePath); - InputStream is = fileContent.read(); - BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is)); - JSONTokener tokener = new JSONTokener(bufferedReader); - JSONObject json = new JSONObject(tokener); - //If the renovate.json file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason - if (json.has("enabled") && json.getBoolean("enabled") == false) { - return false; + private boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { + for (String filePath : filePaths) { + try { + GHContent fileContent = fork.getParent().getFileContent(filePath); + InputStream is = fileContent.read(); + BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is)); + JSONTokener tokener = new JSONTokener(bufferedReader); + JSONObject json = new JSONObject(tokener); + is.close(); + bufferedReader.close(); + //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason + return json.optBoolean("enabled", true); + } catch (FileNotFoundException e) { + log.debug("The file with name %s not found in the repository.", filePath); } - } catch (FileNotFoundException e) { - log.debug("The file with name %s not found in the repository.", filePath); - return false; } - return true; + return false; } } From e1c9a83241aa1f06cb402ef1a774bdbc435577e4 Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 11:20:49 -0800 Subject: [PATCH 07/11] Addressing PR comments --- .../utils/PullRequests.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 8006993f..39ac8a20 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -62,16 +62,18 @@ private boolean isRenovateEnabled(List filePaths, GitHubContentToProcess for (String filePath : filePaths) { try { GHContent fileContent = fork.getParent().getFileContent(filePath); - InputStream is = fileContent.read(); - BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is)); - JSONTokener tokener = new JSONTokener(bufferedReader); - JSONObject json = new JSONObject(tokener); - is.close(); - bufferedReader.close(); - //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason - return json.optBoolean("enabled", true); + JSONObject json; + try (InputStream is = fileContent.read(); + BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is))) { + JSONTokener tokener = new JSONTokener(bufferedReader); + json = new JSONObject(tokener); + //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason + return json.optBoolean("enabled", true); + } catch (IOException e) { + log.debug("Exception while trying to close a resource. Exception: %s", e.getMessage()); + } } catch (FileNotFoundException e) { - log.debug("The file with name %s not found in the repository.", filePath); + log.debug("The file with name %s not found in the repository. Exception: %s", filePath, e.getMessage()); } } return false; From ac16208e11809a506eb8a95dedfe232b92bf1b0c Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 11:58:12 -0800 Subject: [PATCH 08/11] Adding more tests --- .../utils/PullRequests.java | 2 +- .../utils/PullRequestsTest.java | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 39ac8a20..9e957ffe 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -58,7 +58,7 @@ public void prepareToCreate(final Namespace ns, * @param fork A GitHubContentToProcess object that contains the fork repository that is under process * @return true if the file is found in the path specified and is not disabled, false otherwise */ - private boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { + public boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { for (String filePath : filePaths) { try { GHContent fileContent = fork.getParent().getFileContent(filePath); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java index 99da117f..4bc9aead 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/PullRequestsTest.java @@ -6,6 +6,7 @@ import net.sourceforge.argparse4j.inf.*; import org.kohsuke.github.*; import org.mockito.*; +import org.testng.*; import org.testng.annotations.*; import java.io.*; @@ -157,4 +158,70 @@ public void testPullRequestsPrepareSkipsSendingPRIfRepoOnboardedToRenovate() thr eq(gitHubContentToProcess), anyList(), eq(gitForkBranch), eq(rateLimiter)); } + + @Test + public void testisRenovateEnabledReturnsFalseIfRenovateConfigFileNotFound() throws IOException { + PullRequests pullRequests = new PullRequests(); + List filePaths = Collections.singletonList("renovate.json"); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + GHRepository ghRepository = mock(GHRepository.class); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + when(ghRepository.getFileContent(anyString())).thenThrow(new FileNotFoundException()); + Assert.assertFalse(pullRequests.isRenovateEnabled(filePaths, gitHubContentToProcess)); + } + + @Test + public void testisRenovateEnabledReturnsFalseIfRenovateConfigFileFoundButIsDisabled() throws IOException { + PullRequests pullRequests = new PullRequests(); + List filePaths = Collections.singletonList("renovate.json"); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + GHRepository ghRepository = mock(GHRepository.class); + GHContent content = mock(GHContent.class); + InputStream inputStream = new ByteArrayInputStream("{enabled:false}".getBytes()); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + when(ghRepository.getFileContent(anyString())).thenReturn(content); + when(content.read()).thenReturn(inputStream); + Assert.assertFalse(pullRequests.isRenovateEnabled(filePaths, gitHubContentToProcess)); + } + + @Test + public void testisRenovateEnabledReturnsTrueIfRenovateConfigFileFoundButEnabledKeyNotFound() throws IOException { + PullRequests pullRequests = new PullRequests(); + List filePaths = Collections.singletonList("renovate.json"); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + GHRepository ghRepository = mock(GHRepository.class); + GHContent content = mock(GHContent.class); + InputStream inputStream = new ByteArrayInputStream("{someKey:someValue}".getBytes()); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + when(ghRepository.getFileContent(anyString())).thenReturn(content); + when(content.read()).thenReturn(inputStream); + Assert.assertTrue(pullRequests.isRenovateEnabled(filePaths, gitHubContentToProcess)); + } + + @Test + public void testisRenovateEnabledReturnsTrueIfRenovateConfigFileFoundAndResourcesThrowAnException() throws IOException { + PullRequests pullRequests = new PullRequests(); + List filePaths = Collections.singletonList("renovate.json"); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + GHRepository ghRepository = mock(GHRepository.class); + GHContent content = mock(GHContent.class); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + when(ghRepository.getFileContent(anyString())).thenReturn(content); + when(content.read()).thenThrow(new IOException()); + Assert.assertFalse(pullRequests.isRenovateEnabled(filePaths, gitHubContentToProcess)); + } + + @Test + public void testisRenovateEnabledReturnsTrueIfRenovateConfigFileFoundAndEnabledKeySetToTrue() throws IOException { + PullRequests pullRequests = new PullRequests(); + List filePaths = Collections.singletonList("renovate.json"); + GitHubContentToProcess gitHubContentToProcess = mock(GitHubContentToProcess.class); + GHRepository ghRepository = mock(GHRepository.class); + GHContent content = mock(GHContent.class); + InputStream inputStream = new ByteArrayInputStream("{enabled:true}".getBytes()); + when(gitHubContentToProcess.getParent()).thenReturn(ghRepository); + when(ghRepository.getFileContent(anyString())).thenReturn(content); + when(content.read()).thenReturn(inputStream); + Assert.assertTrue(pullRequests.isRenovateEnabled(filePaths, gitHubContentToProcess)); + } } From 77c660fdafaaa892ec41ae2ef5e2a132dcec7e9b Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 12:02:08 -0800 Subject: [PATCH 09/11] Making one resource inline with other --- .../salesforce/dockerfileimageupdate/utils/PullRequests.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 9e957ffe..4f145c0a 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -58,13 +58,12 @@ public void prepareToCreate(final Namespace ns, * @param fork A GitHubContentToProcess object that contains the fork repository that is under process * @return true if the file is found in the path specified and is not disabled, false otherwise */ - public boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { + protected boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { for (String filePath : filePaths) { try { GHContent fileContent = fork.getParent().getFileContent(filePath); JSONObject json; - try (InputStream is = fileContent.read(); - BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is))) { + try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(fileContent.read()))) { JSONTokener tokener = new JSONTokener(bufferedReader); json = new JSONObject(tokener); //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason From 43a3454c26d2f958ee2522772cb18e0b72975b6b Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 13:44:00 -0800 Subject: [PATCH 10/11] Addressing PR comments --- .../utils/PullRequests.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index 4f145c0a..ffa4986c 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -61,21 +61,22 @@ public void prepareToCreate(final Namespace ns, protected boolean isRenovateEnabled(List filePaths, GitHubContentToProcess fork) throws IOException { for (String filePath : filePaths) { try { - GHContent fileContent = fork.getParent().getFileContent(filePath); - JSONObject json; - try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(fileContent.read()))) { - JSONTokener tokener = new JSONTokener(bufferedReader); - json = new JSONObject(tokener); - //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason - return json.optBoolean("enabled", true); - } catch (IOException e) { - log.debug("Exception while trying to close a resource. Exception: %s", e.getMessage()); - } + //If the file has the key 'enabled' set to false, it indicates that while the repo has been onboarded to renovate, it has been disabled for some reason + return readJsonFromContent(fork.getParent().getFileContent(filePath)).optBoolean("enabled", true); } catch (FileNotFoundException e) { log.debug("The file with name %s not found in the repository. Exception: %s", filePath, e.getMessage()); + } catch (IOException e) { + log.debug("Exception while trying to close a resource. Exception: %s", e.getMessage()); } } return false; } + + private JSONObject readJsonFromContent(GHContent content) throws IOException { + try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(content.read()))) { + JSONTokener tokener = new JSONTokener(bufferedReader); + return new JSONObject(tokener); + } + } } From 7fd75213afd634d2eeb6a4e4715e004796967106 Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Thu, 8 Feb 2024 13:46:06 -0800 Subject: [PATCH 11/11] Addressing PR comments --- .../salesforce/dockerfileimageupdate/utils/PullRequests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java index ffa4986c..2f16f3d5 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java @@ -72,6 +72,11 @@ protected boolean isRenovateEnabled(List filePaths, GitHubContentToProce return false; } + /** + * Read the content of a fle from a repository and convert it into a JSON object + * @param content The GHContent object for the content in the repository + * @return json object for the content read from the repository + */ private JSONObject readJsonFromContent(GHContent content) throws IOException { try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(content.read()))) { JSONTokener tokener = new JSONTokener(bufferedReader);