Skip to content

Add Config Resource filter to lookup introspection endpoint#6928

Closed
a2l007 wants to merge 4 commits intoapache:masterfrom
a2l007:fix_introspect
Closed

Add Config Resource filter to lookup introspection endpoint#6928
a2l007 wants to merge 4 commits intoapache:masterfrom
a2l007:fix_introspect

Conversation

@a2l007
Copy link
Copy Markdown
Contributor

@a2l007 a2l007 commented Jan 28, 2019

The /druid/v1/lookups/introspect/{lookupId} endpoint is presently not secured with any ResourceFilter so every request to this endpoint within a secured cluster would result in the following error:
io.druid.java.util.common.ISE: Request did not have an authorization check performed. This PR fixes the issue.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 30, 2019

Good, the resource filter did its job!

package org.apache.druid.query.lookup;

import com.google.inject.Inject;
import com.sun.jersey.spi.container.ResourceFilters;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This import is unused, and is probably related to the CI failure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is actually being used for the ResourceFilters annotation below.

@a2l007
Copy link
Copy Markdown
Contributor Author

a2l007 commented Feb 4, 2019

It looks like the JerseyTest for LookupIntrospectionResource does not like the ResourceFilter added to this resource. I'll fix the test and update the PR. Sorry for the delay.

@stale
Copy link
Copy Markdown

stale Bot commented Apr 6, 2019

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale Bot added the stale label Apr 6, 2019
@clintropolis
Copy link
Copy Markdown
Member

Oops, I didn't realize this PR existed and also fixed this issue in #7222 which has been merged, so this PR can probably be closed. Sorry about that!

@stale stale Bot removed the stale label Apr 9, 2019
@a2l007
Copy link
Copy Markdown
Contributor Author

a2l007 commented Apr 9, 2019

No worries. I'm happy that the changes are in. Thanks!

@a2l007 a2l007 closed this Apr 9, 2019
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