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 a receive handler to Telemeter for remote-write #223
Conversation
pkg/receive/handler.go
Outdated
} | ||
|
||
type AuthorizationPayload struct { | ||
Cluster string `json:"cluster"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This'll work for now, but doesn't generally work. We need to rethink this a bit. Not a blocker on this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, this is 100% specific to Tollbooth. We’ll need to figure out a way to generalize this and maintain compat with Tollbooth auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that there is a decent way we can keep this generic and not tightly coupled to tollbooth. tollbooth expects all authorization requests to be POST requests with the following body:
{"cluster_id":"<cluster_id>","authorization_token":"<pull_secret_auth_token>"}
We can ensure that the bearer token that prometheus is configured to send matches that format and then we POST that token to the specified URL. This way we don't have to do any custom marshalling/unmarshalling. The only variables then are the authorization URL and the token contents.
The one thing that is funny is that we are taking the bearer token header and using that as the body to of a POST request, but I still think that's better and more generic than what we have today.
pkg/authorize/handler.go
Outdated
res.Body.Close() | ||
} | ||
}() | ||
body, err := ioutil.ReadAll(io.LimitReader(res.Body, 32*1024)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make 32*1024 a constant. :)
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: metalmatze, squat The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
We want to add a remote-write receive handler to Telemeter.
For this we need to add a new authorizer, as we are unable to use JWTs.
Works with a mock authorization-server.
Here's a Prometheus config to test with:
TODOs:
/cc @squat @brancz @kakkoyun