Skip to content

PHOENIX-5904 Add log if the configed kerberos principal login failed#34

Closed
infraio wants to merge 2 commits intoapache:masterfrom
infraio:PHOENIX-5904
Closed

PHOENIX-5904 Add log if the configed kerberos principal login failed#34
infraio wants to merge 2 commits intoapache:masterfrom
infraio:PHOENIX-5904

Conversation

@infraio
Copy link
Copy Markdown
Contributor

@infraio infraio commented May 19, 2020

No description provided.

@joshelser
Copy link
Copy Markdown
Member

Reading what you put in the Jira description, are we gaining anything by letting this fall through when hbase security is on but Hadoop security isn't? We can't properly use HBase authentication if Hadoop authentication is off, can we?

Just thinking that we could fail when hadoop.security.authentication=simple and hbase.security.authentication=kerberos.

@infraio
Copy link
Copy Markdown
Contributor Author

infraio commented May 20, 2020

I meet this problem when only configured hbase.security.authentication=kerberos. The log shows "Login successful.", but not. The user is not the configured kerberos user.

@stoty
Copy link
Copy Markdown
Contributor

stoty commented May 25, 2020

I also think that it'd be best to explicitly check both kerberos properties, log an error when they are inconsistent, and only attempt the kerberos login if both are set to kerberos.

I also think that we should mimic whatever HBase does when it encounters these inconsistent settings. If it works in unsecure mode, we should do that as well. If it doesn't work, we should just exit with an error.

If the extra check that you've added still fails, fails then we'd better log an error, and exit early.

@infraio
Copy link
Copy Markdown
Contributor Author

infraio commented May 25, 2020

I also think that we should mimic whatever HBase does when it encounters these inconsistent settings.

If hbase client misconfigured this, it just works without the kerberos. But because the target hbase cluster is in secure mode, so access will fail.

@infraio
Copy link
Copy Markdown
Contributor Author

infraio commented May 26, 2020

Just thinking that we could fail when hadoop.security.authentication=simple and hbase.security.authentication=kerberos.

I thought we don't need to check the inconsistent config. Just check the current user whether has kerberos credential is enough.

@stoty
Copy link
Copy Markdown
Contributor

stoty commented May 27, 2020

I thought we don't need to check the inconsistent config. Just check the current user whether has kerberos credential is enough.

When the HBase/Hadoop setup is correct, PQS already works fine. You have found a case when PQS behaves in a hard to debug way when the settings are inconsistent.
IMO if we want to make it easier for people to spot these kinds of configuration errors, then the most useful thing to do is to check explicitly for this case, and exit early when we know that the setup has no chance of working.

I assume that if we had such a check, it would have saved you a lot of debugging when you encountered the original issue.

@stoty
Copy link
Copy Markdown
Contributor

stoty commented May 27, 2020

Also, please try to add additional commits when changing the PR, instead of force pushing the new one, unless you are rebasing it.
We have no way of seeing what changes you've made to the PR if you always use force push.

@infraio
Copy link
Copy Markdown
Contributor Author

infraio commented May 27, 2020

Also, please try to add additional commits when changing the PR, instead of force pushing the new one, unless you are rebasing it.
We have no way of seeing what changes you've made to the PR if you always use force push.

ok

@infraio
Copy link
Copy Markdown
Contributor Author

infraio commented May 27, 2020

I thought we don't need to check the inconsistent config. Just check the current user whether has kerberos credential is enough.

When the HBase/Hadoop setup is correct, PQS already works fine. You have found a case when PQS behaves in a hard to debug way when the settings are inconsistent.
IMO if we want to make it easier for people to spot these kinds of configuration errors, then the most useful thing to do is to check explicitly for this case, and exit early when we know that the setup has no chance of working.

I assume that if we had such a check, it would have saved you a lot of debugging when you encountered the original issue.

So your advise is? Add a check and exit if inconsistent?

@stoty
Copy link
Copy Markdown
Contributor

stoty commented May 27, 2020

Yes, I think that's the most user-friendly thing to do.

@stoty
Copy link
Copy Markdown
Contributor

stoty commented May 28, 2020

Committed with a minor refactor to improve readability.

@stoty stoty closed this May 28, 2020
@joshelser
Copy link
Copy Markdown
Member

That's great! Thanks Guanghao for your fix! (and Istvan for shepherding in :D)

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