Skip to content
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

[SECURITY] Use HTTPS to resolve dependencies in Maven Build #768

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
name: Don't create issues here, go to https://github.com/jenkinsci/ghprb-plugin/issues
about: No issues here, go to https://github.com/jenkinsci/ghprb-plugin/issues
title: ''
labels: ''
assignees: ''

---

### To report issues, go to the project location at https://github.com/jenkinsci/ghprb-plugin/issues
Copy link
Member

@samrocketman samrocketman Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this file? It doesn't look like it provides anything beneficial to the project because

  1. We're on GitHub.
  2. GitHub issues are pretty standard across all GitHub projects unless it is disabled.
  3. For projects that have GitHub issues disabled, they usually reference where issues are tracked off-site in the README.

As mentioned earlier, there's also GHPRB Jira tracker https://issues.jenkins-ci.org/browse/JENKINS-55793?jql=project%20%3D%20JENKINS%20AND%20status%20in%20(Open%2C%20%22In%20Progress%22%2C%20Reopened)%20AND%20component%20%3D%20ghprb-plugin%20ORDER%20BY%20created%20DESC

Recommendation

Delete this file.

6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
**NOTE**: this plugin is currently maintained at https://github.com/jenkinsci/ghprb-plugin
Copy link
Member

@samrocketman samrocketman Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this note? GHPRB has a Jira project as well.

https://issues.jenkins-ci.org/browse/JENKINS-55793?jql=project%20%3D%20JENKINS%20AND%20status%20in%20(Open%2C%20%22In%20Progress%22%2C%20Reopened)%20AND%20component%20%3D%20ghprb-plugin%20ORDER%20BY%20created%20DESC

At this point it's hard to stop using either without a migration because there's many good reports in both locations.

Recommendation

Delete this line. Alternate recommendation, create a "Filing support issues" section and mention both GitHub issues and JIRA.


# GitHub Pull Request Builder Plugin

This Jenkins plugin builds pull requests from GitHub and will report the results directly to the pull request via
Expand All @@ -15,7 +17,7 @@ build.

A new build can also be started with a comment: ``retest this please``.

You can extend the standard build comment message on github
You can extend the standard build comment message on GitHub
creating a comment file from shell console or any other
jenkins plugin. Contents of that file will be added to the comment on GitHub.
This is useful for posting some build dependent urls for users without
Expand Down Expand Up @@ -93,7 +95,7 @@ For more details, see https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+re
* If you want to use the actual commit in the pull request, use ``${ghprbActualCommit}`` instead of ``${sha1}``
* Under ``Build Triggers``, check ``GitHub pull requests builder``.
* Add admins for this specific job.
* If you want to use GitHub hooks for automatic testing, read the help for ``Use github hooks for build triggering`` in job configuration. Then you can check the checkbox.
* If you want to use GitHub hooks for automatic testing, read the help for ``Use GitHub hooks for build triggering`` in job configuration. Then you can check the checkbox.
* In Advanced, you can modify:
* The crontab line for this specific job. This schedules polling to GitHub for new changes in Pull Requests.
* The whitelisted users for this specific job.
Expand Down
8 changes: 4 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -177,26 +177,26 @@
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</repository>
<repository>
<id>jgit-repository</id>
<name>Eclipse JGit Repository</name>
<url>http://download.eclipse.org/jgit/maven</url>
<url>https://download.eclipse.org/jgit/maven</url>
</repository>
</repositories>

<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>

<distributionManagement>
<repository>
<id>maven.jenkins-ci.org</id>
<url>http://maven.jenkins-ci.org:8081/content/repositories/releases/</url>
<url>https://maven.jenkins-ci.org:8081/content/repositories/releases/</url>
Copy link
Member

@samrocketman samrocketman Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not work. You can't switch to HTTPS and keep the port 8081.

Recommendation

Update the parent pom for jenkinsci and remove repositories from this pom entirely. Plugin poms should not need to be referencing repositories.

</repository>
</distributionManagement>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ f.entry(field: "gitHubAuthId", title:_("GitHub API credentials")) {
f.entry(field: "adminlist", title: _("Admin list")) {
f.textarea(default: descriptor.adminlist)
}
f.entry(field: "useGitHubHooks", title: "Use github hooks for build triggering") {
f.entry(field: "useGitHubHooks", title: "Use GitHub hooks for build triggering") {
Copy link
Member

@samrocketman samrocketman Apr 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes like this could be broken out into a separate pull request. Don't mix changes if you're proposing a large change. This change isn't large but just general advice.

Recommendation

You can leave this. However, if you're interested in getting changes like this out faster than what is blocked by this pull request I recommend opening a separate pull request.

f.checkbox()
}
f.advanced() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<f:entry title="${%Admin list}" field="adminlist">
<f:textarea default="${descriptor.adminlist}"/>
</f:entry>
<f:entry title="Use github hooks for build triggering" field="useGitHubHooks">
<f:entry title="Use GitHub hooks for build triggering" field="useGitHubHooks">
<f:checkbox />
</f:entry>
<f:advanced>
Expand Down