Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Bumped to latest version of Trino. Adjusted overrides in OpaAuthorizer #32

Closed
wants to merge 1 commit into from

Conversation

parj
Copy link

@parj parj commented Dec 24, 2023

No description provided.

@stackable-bot
Copy link

stackable-bot commented Dec 24, 2023

CLA assistant check
All committers have signed the CLA.

@lfrancke
Copy link
Member

Thank you for the contribution!
Please be aware, that this project is currently in the process of being donated to Trino itself.

I'm only on my phone now and don't have the links at hand. Happy to look at this in detail after the holidays

@parj parj force-pushed the main branch 2 times, most recently from 962e7fa to a5f9176 Compare December 24, 2023 13:41
@maltesander
Copy link
Member

Hi @parj,

this trinodb/trino#19532 is based on this repo but heavily improved. Once this is merged, Trino has native OPA support :-)

Not sure if it makes sense to go forward in this repo?

Im fine with doing a review and merging this, but then we probably should archive this repo anyways @lfrancke?

Our latest Trino (428) image is already using the upstream OPA changes.

@lfrancke
Copy link
Member

Make sense to me. Thanks Malte!

@sbernauer sbernauer self-assigned this Jan 18, 2024
@sbernauer
Copy link
Member

sbernauer commented Jan 22, 2024

Hi @parj,
many thanks for your contribution, we really appreciate you taking the time to make the proposed changes!

However - as @maltesander hinted previously - we should archive this repository.
The reason is that trinodb/trino#19532 is better and is already at the latest Trino version (as it tries to merge into Trinos master branch).

Additionally, the Trino SPI (mainly the SystemAccessControl interface) has many changes since 414: https://github.com/trinodb/trino/commits/74616968c958b1e35f61ad99a82bca281ed85f5b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java?since=2023-04-19&until=2024-01-17 (e.g. the checkCanCreateFunction or the removal of the checkCanGrantExecuteFunctionPrivilege function).
We would need address this changes in this PR, as merging this as-is would probably cause issues.

Would you be willing to give trinodb/trino#19532 a try to see if it works for you? One easy way for you to test it, is to use our Trino 428 image docker.stackable.tech/stackable/trino:428-stackable23.11.0, which already contains the new authorizer from trinodb/trino#19532.

@lfrancke
Copy link
Member

Hi,
just a heads up: if we don't hear back from you by tomorrow we will close this and archive the repository

@lfrancke lfrancke closed this Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants