-
Notifications
You must be signed in to change notification settings - Fork 49
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
Skip DFIU PRs for repos onboarded to Renovate Enterprise #1128
Merged
Merged
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
50df365
Skip DFIU PRs for renovate onboarded repos
avimanyum bbb42f8
Adding tests
avimanyum 62b1d6b
Updating version of dependency
avimanyum 50725f7
Fix imports
avimanyum 5bd598b
Checking for boolean value in config file
avimanyum 10a819a
Addressing PR comments
avimanyum e1c9a83
Addressing PR comments
avimanyum ac16208
Adding more tests
avimanyum 77c660f
Making one resource inline with other
avimanyum 43a3454
Addressing PR comments
avimanyum 7fd7521
Addressing PR comments
avimanyum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
*.ipr | ||
*.iws | ||
*.iml | ||
*.DS_Store | ||
|
||
# Vim files | ||
*.swp | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
import com.salesforce.dockerfileimageupdate.model.*; | ||
import com.salesforce.dockerfileimageupdate.process.*; | ||
import net.sourceforge.argparse4j.inf.*; | ||
import org.json.JSONObject; | ||
import org.json.JSONTokener; | ||
import org.kohsuke.github.*; | ||
import org.slf4j.*; | ||
|
||
|
@@ -27,10 +29,16 @@ 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 the repository has been onboarded to renovate enterprise, skip sending the DFIU PR | ||
if(ns.getBoolean(Constants.CHECK_FOR_RENOVATE) | ||
&& (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, | ||
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,32 @@ public void prepareToCreate(final Namespace ns, | |
} | ||
ResultsProcessor.processResults(skippedRepos, exceptions, log); | ||
} | ||
|
||
/** | ||
* Check if the repository is onboarded to Renovate. The signal we are looking for are | ||
* (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 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(List<String> 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to do this in a try-with-resources statement, see https://stackoverflow.com/a/56151320 . If |
||
//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); | ||
} | ||
} | ||
return false; | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there something we can't go with gson dependency that we already have https://github.com/avimanyum/dockerfile-image-update/blob/master/dockerfile-image-update/pom.xml#L116. or you are brining it in for easy of use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah its just a cleaner and easier way to convert to Json object