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

Properly handle multiple webhook triggers for a BuildConfig #8551

Closed
jhadvig opened this issue Apr 18, 2016 · 5 comments · Fixed by #8580
Closed

Properly handle multiple webhook triggers for a BuildConfig #8551

jhadvig opened this issue Apr 18, 2016 · 5 comments · Fixed by #8580
Assignees
Labels
component/build kind/bug Categorizes issue or PR as related to a bug. priority/P2

Comments

@jhadvig
Copy link
Member

jhadvig commented Apr 18, 2016

Currently the buildConfig trigger array can hold multiple triggers per one type - eg. multiple github/generic triggers. This could lead to a confusion since there is no point to have multiple triggers of one type and so it should be limited one per type.
Based on conversation with @bparees

@liggitt
Copy link
Contributor

liggitt commented Apr 18, 2016

there is no point to have multiple triggers of one type

not true... I like being able to trigger builds from different repos with different secrets so I can revoke just one

@bparees
Copy link
Contributor

bparees commented Apr 18, 2016

@liggitt well you can't today since the existing implementation is broken :)

@bparees bparees self-assigned this Apr 18, 2016
@liggitt
Copy link
Contributor

liggitt commented Apr 18, 2016

then we should fix that

@bparees bparees added kind/bug Categorizes issue or PR as related to a bug. priority/P2 component/build labels Apr 18, 2016
@bparees
Copy link
Contributor

bparees commented Apr 18, 2016

the issue here is this code:
https://github.com/openshift/origin/blob/master/pkg/build/webhook/generic/generic.go#L27
(and the equivalent in the github webhook) which just finds the first trigger of type Generic and tries to match the secret against it.

I guess we should be iterating all the triggers of type Generic and checking if the supplied secret matches any of them, though something about that bothers me since it makes it easier to brute force guess a secret (each guess you make is automatically tried against all possible matches).

@bparees bparees changed the title Limit number of build trigger type to a single trigger per type Properly handle multiple webhook triggers for a BuildConfig Apr 18, 2016
@bparees bparees assigned PI-Victor and unassigned bparees Apr 18, 2016
@PI-Victor
Copy link
Contributor

i'm on it, will submit a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/build kind/bug Categorizes issue or PR as related to a bug. priority/P2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants