Skip to content

HIVE-28840: HMS Iceberg Catalog doesn't respect ICEBERG_CATALOG_SERVLET_AUTH - #5710

Merged
okumin merged 3 commits into
apache:masterfrom
okumin:HIVE-28840-catalog-auth
Apr 5, 2025
Merged

okumin merged 3 commits into
apache:masterfrom
okumin:HIVE-28840-catalog-auth

Conversation

@okumin

@okumin okumin commented Mar 23, 2025

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Let HMSCatalog, the Iceberg REST catalog on HMS, respect the parameter.

Why are the changes needed?

We can't configure the auth mode of the catalog API

Does this PR introduce any user-facing change?

No. This feature has not been released yet

Is the change a dependency upgrade?

No

How was this patch tested?

I ran the server on my local machine

@okumin okumin changed the title HIVE-28840: HMH Iceberg Catalog doesn't respect ICEBERG_CATALOG_SERVLET_AUTH HIVE-28840: HMS Iceberg Catalog doesn't respect ICEBERG_CATALOG_SERVLET_AUTH Mar 23, 2025
@deniskuzZ

Copy link
Copy Markdown
Member

hi @okumin, ping in case you missed the comments update

@okumin

okumin commented Apr 3, 2025

Copy link
Copy Markdown
Contributor Author

@deniskuzZ Sorry, I could not respond promptly. I updated the branch

public ServletSecurity(Configuration conf) {
this(conf, isAuthJwt(conf));
public ServletSecurity(String authType, Configuration conf) {
this.conf = conf;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@okumin, why not call the existing constructor?

this(conf, isAuthJwt(authTypeStr));

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.

Just my bad. Updated!

@deniskuzZ deniskuzZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, pending tests

@sonarqubecloud

sonarqubecloud Bot commented Apr 4, 2025

Copy link
Copy Markdown

@okumin
okumin merged commit aa91064 into apache:master Apr 5, 2025
@okumin
okumin deleted the HIVE-28840-catalog-auth branch April 5, 2025 16:24
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.

3 participants