Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

[WIP] drivers jenkins -> change trigger to github webhook#444

Open
lwsanty wants to merge 1 commit into
bblfsh:masterfrom
lwsanty:jenkins-webhook4
Open

[WIP] drivers jenkins -> change trigger to github webhook#444
lwsanty wants to merge 1 commit into
bblfsh:masterfrom
lwsanty:jenkins-webhook4

Conversation

@lwsanty

@lwsanty lwsanty commented Oct 30, 2019

Copy link
Copy Markdown
Member

Note this issue must be closed first https://github.com/src-d/infrastructure/issues/1249

Signed-off-by: lwsanty lwsanty@gmail.com


This change is Reviewable

@lwsanty lwsanty requested a review from a team as a code owner October 30, 2019 13:16
@lwsanty lwsanty self-assigned this Oct 30, 2019

@dennwc dennwc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lwsanty)


README.md, line 183 at r1 (raw file):

## License

GPLv3, see [LICENSE](LICENSE)

It's unrelated, so please revert.


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

        [key: 'ref', value: '$.ref']
      ],
      token: '{{.Manifest.Language}}-driver',

Wondering, what this "token" does?


etc/skeleton/Jenkinsfile.tpl, line 53 at r1 (raw file):

      regexpFilterText: '$ref',
      regexpFilterExpression: 'refs/heads/' + BRANCH_NAME

Will it trigger on tags?

Signed-off-by: lwsanty <lwsanty@gmail.com>

@lwsanty lwsanty left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dennwc)


README.md, line 183 at r1 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

It's unrelated, so please revert.

Done.


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

There is a special token parameter. When supplied, the invocation will only trigger jobs with that exact token. The token also allows invocations without any other authentication credentials.

https://wiki.jenkins.io/display/JENKINS/Generic+Webhook+Trigger+Plugin


etc/skeleton/Jenkinsfile.tpl, line 53 at r1 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

Will it trigger on tags?

according to https://github.com/src-d/infrastructure/issues/1249#issuecomment-547509076
our middle-ware will work only with push events, so the answer is no

@lwsanty lwsanty requested a review from dennwc October 30, 2019 13:55

@dennwc dennwc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lwsanty)


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

Previously, lwsanty wrote…

There is a special token parameter. When supplied, the invocation will only trigger jobs with that exact token. The token also allows invocations without any other authentication credentials.

https://wiki.jenkins.io/display/JENKINS/Generic+Webhook+Trigger+Plugin

I see. So now I'm also concerned with https://github.com/src-d/infrastructure/issues/1249#issuecomment-547819998.


etc/skeleton/Jenkinsfile.tpl, line 53 at r1 (raw file):

Previously, lwsanty wrote…

according to https://github.com/src-d/infrastructure/issues/1249#issuecomment-547509076
our middle-ware will work only with push events, so the answer is no

But we may need triggers on tags as well. It should be possible to pass all events from the Github.

@lwsanty lwsanty left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dennwc)


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

I see. So now I'm also concerned with https://github.com/src-d/infrastructure/issues/1249#issuecomment-547819998.

You should not, middleware will be protected by the secret


etc/skeleton/Jenkinsfile.tpl, line 53 at r1 (raw file):
hm, I double checked

Branch pushes and repository tag pushes also trigger webhook push events
https://developer.github.com/webhooks/

So tags will be included.

@lwsanty lwsanty requested a review from dennwc October 30, 2019 15:06

@dennwc dennwc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lwsanty)


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

Previously, lwsanty wrote…

You should not, middleware will be protected by the secret

You mean that the token will be send inside the cluster, while a real secret will be used to expose that middleware?


etc/skeleton/Jenkinsfile.tpl, line 53 at r1 (raw file):

Previously, lwsanty wrote…

hm, I double checked

Branch pushes and repository tag pushes also trigger webhook push events
https://developer.github.com/webhooks/

So tags will be included.

Awesome, thanks!

@lwsanty lwsanty left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dennwc)


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

You mean that the token will be send inside the cluster, while a real secret will be used to expose that middleware?

Generally yes, I just don't know what means real :)

@lwsanty lwsanty requested a review from dennwc October 31, 2019 08:51

@dennwc dennwc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


etc/skeleton/Jenkinsfile.tpl, line 46 at r1 (raw file):

Previously, lwsanty wrote…

Generally yes, I just don't know what means real :)

"Real" means the one that provides security ("go-driver" as a token doesn't).

So OK, assuming we only send insecure tokens inside our own cluster, LGTM.

@lwsanty lwsanty left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion

a discussion (no related file):
blocked until https://github.com/src-d/infrastructure/issues/1249


Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants