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

Moved /api/cluster/leadership handler under public routes (requires no authentication) #3101

Merged
merged 1 commit into from Apr 3, 2018

Conversation

aantono
Copy link
Contributor

@aantono aantono commented Mar 29, 2018

As it was brought up in Slack, the /cluster/leadership URL is mostly being called from various load balancers and other sources that don't typically do authentication. Having /cluster handler be under API endpoint, which otherwise might need to be guarded with authentication, prevents access to /cluster/leadership URL from load balancers. Moving it under Ping endpoint, will solve this problem, and, as it seems from the intent of /cluster/leadership URL, it actually belongs under Ping to begin with, as it serves similar purpose.

@CumpsD
Copy link

CumpsD commented Mar 29, 2018

Thanks @aantono!

@juliens
Copy link
Member

juliens commented Mar 29, 2018

Thanks for your contribution!

WDYT about keeping /api/cluster/leader, enabling it with api option and just disabling auth on this endpoint ?

@CumpsD
Copy link

CumpsD commented Mar 29, 2018

that would serve the use case too

@aantono
Copy link
Contributor Author

aantono commented Mar 29, 2018

@juliens do you mean to disable auth on just /api/cluster/leader without disabling auth on the rest of the /api/* URLs?

@mmatur mmatur added this to the 1.7 milestone Mar 30, 2018
@mmatur mmatur added kind/bug/fix a bug fix and removed kind/enhancement a new or improved feature. labels Mar 30, 2018
@mmatur mmatur modified the milestones: 1.7, 1.6 Mar 30, 2018
@juliens
Copy link
Member

juliens commented Mar 30, 2018

@aantono yes, we need to keep auth on /api/* but no auth on /api/cluster/leader

As this endpoint was introduce in 1.6.0, can you put it in 1.6.0 to fix the auth bug ?

@aantono
Copy link
Contributor Author

aantono commented Mar 30, 2018

You bet. Can you point me to the right place to tweak that config? Looked for it myself, but can’t find the right nobs to turn ;)

@juliens
Copy link
Member

juliens commented Mar 30, 2018

You need two things:

  • rebase your branch on top of v1.6 git rebase --onto upstream/v1.6 HEAD^
  • Change the base branch ( edit next to the PR Title and then you can change it)

@aantono aantono force-pushed the cluster-api-change branch 2 times, most recently from be01d17 to 8617671 Compare March 30, 2018 15:48
@aantono aantono changed the base branch from master to v1.6 March 30, 2018 15:53
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@aantono aantono changed the title Moved /cluster/leadership handler under Ping endpoint Moved /api/cluster/leadership handler under public routes (requires no authentication) Mar 30, 2018
Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants