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

Update buildkite-metrics to use the agent metrics api #40

Merged
merged 4 commits into from
Apr 17, 2018

Conversation

sj26
Copy link
Member

@sj26 sj26 commented Apr 6, 2018

This is super rough, but it works.

@sj26 sj26 added the wip label Apr 6, 2018
@sj26 sj26 self-assigned this Apr 6, 2018
@sj26 sj26 requested a review from lox April 6, 2018 05:24
Copy link
Contributor

@lox lox left a comment

Choose a reason for hiding this comment

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

This is a good reduction in code bulk. Will probably needs some explanation in the README

@lox
Copy link
Contributor

lox commented Apr 9, 2018

Random idea, but what if we didn't pass the agent registration token over the wire and instead used a HMAC'd value with the account_id? Still vulnerable to reply attacks, but at least you can't then use that token to register an agent.

@sj26
Copy link
Member Author

sj26 commented Apr 10, 2018

@lox I'd love to look into a better authentication exchange, especially I'd love to homogenize our tokens so that a cluster token can do some limited agent api stuff, and an agent token can do limited rest api stuff, etc. It'd be a little interesting, trying to create tokens which included enough information to do a mac exchange. But I think we're sufficiently protected against replay by TLS for the moment:

https://www.ssllabs.com/ssltest/analyze.html?d=agent.buildkite.com

@lox
Copy link
Contributor

lox commented Apr 10, 2018

I wasn't proposing any sort of key exchange @sj26, just rather than passing in BUILDKITE_AGENT_TOKEN, we'd pass in hmac($BUILDKITE_AGENT_TOKEN, $ACCOUNT_ID) and use that as the secret.

It means we aren't encouraging people to put the BUILDKITE_AGENT_TOKEN in env anywhere.

@sj26
Copy link
Member Author

sj26 commented Apr 10, 2018

Right, right — for the agent setup and env, not the request channel. Yeah, that also kinda gels with what I mean. I was proposing opaque authorization tokens which actually pack information, like our graphql IDs, e.g. Authorization: Token base64(pack($TOKEN_UUID+$NONCE+hmac($TOKEN_SECRET,$TOKEN_UUID+$NONCE))), so BK can unpack the uuid to lookup the agent registration token and then validate the hmac — and you could generate that on the buildkite.com side to feed in as an env. I was misinterpreting and thinking the metrics agent would also then perform hmac for the request channel, but that's probably superfluous.

But yeah, step 2.

@lox
Copy link
Contributor

lox commented Apr 10, 2018

Gotcha! That makes sense. Reckon getting it working like it is makes sense for now anyway.

@lox
Copy link
Contributor

lox commented Apr 16, 2018

Tests are looking awesome @sj26

@sj26
Copy link
Member Author

sj26 commented Apr 16, 2018

@lox thanks! They're rough, but they work. I think maybe some README updates and this is good for release, with some published caveats that it drops a bunch of metrics from the previous version around builds and historical — this is purely for job/agent workload metrics. Do you know many folks using it beyond the elastic stack?

@lox
Copy link
Contributor

lox commented Apr 16, 2018

Happy to merge with some README changes!

@sj26 sj26 removed the wip label Apr 17, 2018
@lox
Copy link
Contributor

lox commented Apr 17, 2018

🚢

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.

2 participants