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

GitHub app blog #2988

Merged
merged 17 commits into from
Apr 16, 2020
Merged

GitHub app blog #2988

merged 17 commits into from
Apr 16, 2020

Conversation

timja
Copy link
Member

@timja timja commented Mar 20, 2020

Draft until the plugin is actually released,

PR was: jenkinsci/github-branch-source-plugin#269

cc @bitwiseman @oleg-nenashev @ojacques

@timja
Copy link
Member Author

timja commented Mar 20, 2020

build failed for:

/home/jenkins/workspace/Infra: No space left on device

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks.

I've added some optional comments for you to consider.

If you decide to accept my proposed changes, it will look like this:

screencapture-localhost-4242-blog-2020-03-19-github-app-authentication-2020-03-20-10_44_44

content/_data/authors/timja.adoc Outdated Show resolved Hide resolved
@jglick

This comment has been minimized.

@timja
Copy link
Member Author

timja commented Mar 20, 2020

I’ll check off master later on, We setup about 5 today off the incremental version from the PR

@jglick

This comment has been minimized.

@jglick

This comment has been minimized.

@timja
Copy link
Member Author

timja commented Mar 20, 2020

Never mind, figured it out—I was using the app name where I was supposed to be using its ID (a number).

Hah, I just tested it end 2 end off your PR and it worked fine for me.

Copy link

@ojacques ojacques left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I really like picture in blog posts: not sure what the best visual is, but let me suggest this screenshot which leverages the checks API from Jenkins job - which corresponds to the example in the blog post.

ghapp-checkapi

@oleg-nenashev oleg-nenashev self-requested a review March 25, 2020 10:02
@bitwiseman bitwiseman self-requested a review April 8, 2020 23:08
@bitwiseman
Copy link
Contributor

@timja I've released 2.7.0-beta1 with this feature. I'd like to wait to release this blog post until the GA release (which I plan to do within the week), but I'm open to posting it for the beta.

@timja
Copy link
Member Author

timja commented Apr 14, 2020

@timja I've released 2.7.0-beta1 with this feature. I'd like to wait to release this blog post until the GA release (which I plan to do within the week), but I'm open to posting it for the beta.

i'm open to either, Oleg just posted in advocacy and outreach gitter about possibly publishing it tomorrow, we could discuss there?

https://gitter.im/jenkinsci/advocacy-and-outreach-sig

@bitwiseman
Copy link
Contributor

@timja
Yeah, I should have been clear that I don't consider myself the final word on this. :) I'll pop over there.

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Mostly formatting and wording.

One section that could use a bit more information.

author: timja
---

I'm excited to announce support for authenticating as a GitHub app in Jenkins.
Copy link
Contributor

Choose a reason for hiding this comment

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

"GitHup Apps" is the name of the product. A "GitHub app" is an individual app. Okay.
https://developer.github.com/apps/

Comment on lines 39 to 76
== How do I get an API token in my pipeline?

You can access the Bearer token for the GitHub API by just loading a 'Username/Password' credential as usual,
the plugin will handle authenticating with GitHub in the background:

[source, groovy]
----

pipeline {
agent any

stages{
stage('Check run') {
steps {
withCredentials([usernamePassword(credentialsId: 'githubapp-jenkins',
usernameVariable: 'GITHUB_APP',
passwordVariable: 'GITHUB_JWT_TOKEN')]) {
sh '''
curl -H "Content-Type: application/json" \
-H "Accept: application/vnd.github.antiope-preview+json" \
-H "authorization: Bearer ${GITHUB_JWT_TOKEN}" \
-d '{ "name": "check_run", \
"head_sha": "'${GIT_COMMIT}'", \
"status": "in_progress", \
"external_id": "42", \
"started_at": "2020-03-05T11:14:52Z", \
"output": { "title": "Check run from Jenkins!", \
"summary": "This is a check run which has been generated from Jenkins as GitHub App", \
"text": "...and that is awesome"}}' https://api.github.com/repos/<org>/<repo>/check-runs
'''
}
}
}
}
}


----
Copy link
Contributor

Choose a reason for hiding this comment

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

@timja
It would be helpful if we talked a bit about why users would want to do this and some of the security considerations in this are. Also, we should definitely mention that this token is only valid for one hour. For long running jobs if the user gets the password this way, they should use it right away. This is also a security improvement - tokens visible in the pipeline are not long lived.

@timja timja requested a review from bitwiseman April 15, 2020 19:30
@timja
Copy link
Member Author

timja commented Apr 15, 2020

I've added more info, thanks for the review @bitwiseman,

It's updated for publishing tomorrow, feel free to push any commits you feel improve it as well

@timja timja marked this pull request as ready for review April 15, 2020 19:31
@timja timja requested a review from a team as a code owner April 15, 2020 19:31
@timja
Copy link
Member Author

timja commented Apr 15, 2020

(this is no longer on hold if anyone wants to remove the label)

Copy link
Contributor

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Better late than never, a bunch of minor comments and improvement suggestions

@timja
Copy link
Member Author

timja commented Apr 16, 2020

shall we wait till next week now? I think we missed the best time, could just ship with GA at this point?

@oleg-nenashev
Copy link
Contributor

I am ready to ship in 10 minutes or so if you prefer to do it rather early than later

@oleg-nenashev
Copy link
Contributor

The time is good for US, and we can do reposts tomorrow for APAC/Europe

@timja
Copy link
Member Author

timja commented Apr 16, 2020

sure let's go for it

@oleg-nenashev
Copy link
Contributor

We will need to move images out of the root later. I believe we should move them anyway to the documentation sections later, so I do not mind the image in /content/images for now (this directory is full misplaced images anyway)

@oleg-nenashev oleg-nenashev merged commit df3178c into jenkins-infra:master Apr 16, 2020
@timja
Copy link
Member Author

timja commented Apr 16, 2020

Oops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants