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

Query Frontend: Set CORS header #3414

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

hwoarang
Copy link
Contributor

@hwoarang hwoarang commented Nov 5, 2020

Set CORS to the query frontend component in order to be able
to freely access it from Grafana's living elsewhere.

Signed-off-by: Markos Chandras markos@chandras.me

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Set Access-Control-Allow-Origin Header to Query Frontend

Verification

  • make test-e2e-local
  • Tested also with local grafana connected to a local Thanos frontend using our internal Thanos querier and everything worked fine. No CORS errors anymore.

@GiedriusS
Copy link
Member

Does this also work when you use POST? Because we have:

var corsHeaders = map[string]string{
	"Access-Control-Allow-Headers":  "Accept, Accept-Encoding, Authorization, Content-Type, Origin",
	"Access-Control-Allow-Methods":  "GET, OPTIONS",
	"Access-Control-Allow-Origin":   "*",
	"Access-Control-Expose-Headers": "Date",
}

@hwoarang
Copy link
Contributor Author

hwoarang commented Nov 5, 2020

Does this also work when you use POST? Because we have:

var corsHeaders = map[string]string{
	"Access-Control-Allow-Headers":  "Accept, Accept-Encoding, Authorization, Content-Type, Origin",
	"Access-Control-Allow-Methods":  "GET, OPTIONS",
	"Access-Control-Allow-Origin":   "*",
	"Access-Control-Expose-Headers": "Date",
}

@GiedriusS that's an interesting question. I wonder how it works right now with Querier. I am looking into this.

@hwoarang
Copy link
Contributor Author

hwoarang commented Nov 5, 2020

Even now (thanos 0.16), POST is not allowed from what I see

~$ curl -X POST --head https://thanos.XXXX
HTTP/1.1 405 Method Not Allowed
Allow: GET, OPTIONS
Content-Type: text/plain; charset=utf-8
Date: Thu, 05 Nov 2020 16:43:36 GMT
Vary: Accept-Encoding
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
Content-Length: 19
Connection: keep-alive

🤔

@hwoarang hwoarang force-pushed the set-cors-query-frontend branch 2 times, most recently from 3a4fbba to 3e4ac53 Compare November 5, 2020 17:47
@hwoarang
Copy link
Contributor Author

hwoarang commented Nov 5, 2020

make test-e2e-local fails randomly but I do not thin it's related to this PR. I believe the test-e2e test failure here is similar.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

One suggestion and LGTM (:

cmd/thanos/query_frontend.go Outdated Show resolved Hide resolved
Set CORS to the query frontend component in order to be able
to freely access it from Grafana's living elsewhere. This is similar to
what already happens for the Query component.

Signed-off-by: Markos Chandras <markos@chandras.me>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bwplotka bwplotka merged commit e9ba8ae into thanos-io:master Nov 6, 2020
@hwoarang hwoarang deleted the set-cors-query-frontend branch November 6, 2020 10:26
Oghenebrume50 pushed a commit to Oghenebrume50/thanos that referenced this pull request Dec 7, 2020
Set CORS to the query frontend component in order to be able
to freely access it from Grafana's living elsewhere. This is similar to
what already happens for the Query component.

Signed-off-by: Markos Chandras <markos@chandras.me>
Signed-off-by: Oghenebrume50 <raphlbrume@gmail.com>
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.

3 participants