Skip to content

Conversation

@jojochuang
Copy link
Contributor

@jojochuang jojochuang commented Feb 7, 2024

What changes were proposed in this pull request?

Add a default value "*" for the configuration property ozone.om.kerberos.principal.pattern to support accessing Ozone cluster in other Kerberos realms.

Please describe your PR in detail:

  • Follow what was done in HDFS-7546.
  • Currently we simply workaround the issue by adding the property to the command line. Adding it to the default makes Ozone more user friendly. E.g.
    hdfs dfs -Dozone.om.kerberos.principal.pattern=* -ls ofs://ozone1707264383/vol1/bucket1/

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10328

How was this patch tested?

@smengcl smengcl requested a review from szetszwo February 7, 2024 23:23
Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @jojochuang !

@Galsza
Copy link
Contributor

Galsza commented Feb 8, 2024

Thank for the patch @jojochuang .

I have two minor questions:
Did you test this change if it really solves the issue?

The default value * this is a looser security option than making the user manually change the config so that they allow cross-realm for ozone. Is it better to enable all realms by default and make the user config it if they want something stricter,
or is it better to disable all realms by default but letting the user know they can config it? I'm fine with either way but the second one seems more secure. (I know it defaults to enabled in HDFS, I just wanted to ask if we want to go the same way with ozone)

@jojochuang
Copy link
Contributor Author

@Galsza yes this configuration value addresses the issue.

IMO
the Kerberos realms already provide the authentication required between the group of hosts.
It is already difficult to try and to set up cross realm communication and this is an unnecessary gotcha.

Or put this way: if a user can easily work around this level of protection with a command line property, then it's not really giving any more safety.

@Galsza
Copy link
Contributor

Galsza commented Feb 8, 2024

@jojochuang Thanks for both of the detailed answers it's clear to me now. LGTM +1

@jojochuang jojochuang merged commit 5715aee into apache:master Feb 8, 2024
@jojochuang
Copy link
Contributor Author

Thanks @smengcl @Galsza this PR is merged.

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants