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

Add notifyStash global function #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mscharp
Copy link

@mscharp mscharp commented Apr 15, 2016

No description provided.

@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@ssbarnea
Copy link
Contributor

ssbarnea commented May 6, 2016

@mscharp CREDENTIAL_ID_IN_JENKINS seems like bad naming for a variable. Wouldn't be more appropiate to use something containing SLACK_TOKEN ?

@orrc
Copy link
Member

orrc commented May 6, 2016

As mentioned in the comment above that section, it's not a variable, it's a placeholder.

@ssbarnea
Copy link
Contributor

ssbarnea commented May 6, 2016

@orrc in this case is even worse, putting credentials inside the SCM is a clear no-no. Please try to find a way to keep credentials out of the code that goes into the repository.

Read http://arstechnica.co.uk/security/2016/04/hacking-slack-accounts-as-easy-as-searching-github/ to get an idea about how bad this is.

@ssbarnea
Copy link
Contributor

ssbarnea commented May 6, 2016

Also have a look at my approach at https://github.com/jenkinsci/pipeline-examples/pull/26/files -- clearly not ideal but at least it does not put the token inside the code.

I think that using environment variables or Jenkins API to access these credentials are both "good enough" security approaches. Still, inviting people to put their tokens inside Jenkinsfile, would clearly be not a good idea.

@mscharp
Copy link
Author

mscharp commented May 6, 2016

This code isn't actually putting credentials in the code. There is a "withCredentials" plugin that will use the username/password that has been set up in Jenkins. The CREDENTIAL_ID_IN_JENKINS is just a placeholder for the hash key that you access those credentials by. So, whoever uses this code would replace it with a hash string. I've attached an image of what the credentials look like in Jenkins.

screen shot 2016-05-06 at 9 04 46 am

@mscharp
Copy link
Author

mscharp commented May 6, 2016

And, you are right. There are other ways to access the credentials that you'd need. However, having it hardcoded into this script isn't a big concern for us, as the user associated with those credentials doesn't have rights to do much.

@ssbarnea
Copy link
Contributor

ssbarnea commented May 6, 2016

Ahh, sorry that would have been ok because this ID is useless outside Jenkins. I had the impression that was the slack token.

@nickbroon
Copy link

I'd guess this is a workaround until the following is implemented? jenkinsci/stashnotifier-plugin#92

@tmds
Copy link

tmds commented May 27, 2016

@mscharp how can I use this in a Jenkinsfile?

@tmds
Copy link

tmds commented May 27, 2016

@dcsobral
Copy link

dcsobral commented Aug 5, 2016

Even with the stash notifier plugin working (and supposedly it's working now), I find this is a very interesting example. Maybe make it more generic, like "calling a REST interface with build results"? The actual example can stay pretty much the same.

@kmadel
Copy link
Contributor

kmadel commented Aug 7, 2016

Note, the http-request-plugin has a Pipeline step and allows POST a request body (JSON or whatever). Furthermore, it makes it much easier to handle the response from your REST call.

@kenyee
Copy link

kenyee commented Aug 20, 2016

I was wondering how to do a POST w/ auth info securely as well, so an example of this would be helpful.

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

Successfully merging this pull request may close these issues.

9 participants