Suppress access denied exception when listing all tables/views in a Glue database #14998
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
findinpath
commented
Nov 11, 2022
plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java
Outdated
Show resolved
Hide resolved
findinpath
force-pushed
the
glue-gettables-on-schema-access-denied
branch
from
dc02c86
to
20b01f1
Compare
findinpath
commented
Nov 18, 2022
| * </pre> | ||
| */ | ||
| @Test | ||
| public void testGetAllEntitiesOnAccessDeniedDatabase() |
There was a problem hiding this comment.
@pettyjamesm do you have any insights about how this fix can be cleanly tested without using a manually defined Glue permission policy?
While trying to programmatically change the AWS Glue permission policies we've (see #14849 ) bumped into the constraint that the changes perform on the Glue Data Catalog programmatically may overwrite either configuration AWS Glue configuration affecting builds which run on the same time or remove existing AWS Glue permission policies which were previously manually configured.
There was a problem hiding this comment.
I think your options are basically either:
- mock the AWS client to throw access denied in the specific test
- manually define a separate Glue permission policy / IAM role for this purpose in test
- use a "create if not exists" pattern to programatically define an IAM role / Glue table with the specific required permissions such that repeated / concurrent runs will only create it once and therefore shouldn't generally interfere with each other while also allowing new environments to create the required objects on-demand during the first run
…ue database Co-authored-by: albericgenius <cnuliuweiren@gmail.com>
findinpath
force-pushed
the
glue-gettables-on-schema-access-denied
branch
from
20b01f1
to
ed4080c
Compare
findinpath
changed the title
ebyhr
approved these changes
Nov 20, 2022
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fix #14746
As per offline discussion due to difficulties which may be caused by the fact that the permission policy for the Glue database which has
glue:GetTablespermission denied we've decided to put up this PR without corresponding tests. Do note however that the PR changes have been locally tested against AWS Glue.Non-technical explanation
Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text: