Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jaas): fixed auth.jaas.enabled option parsing #5179

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

alexey-kravtsov
Copy link
Contributor

Previous implementation rendered config value as "Quoted(true)", thus value never could be interpreted as true

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

previous implementation rendered config value as "Quoted(true)",
thus value never could be interpreted as true
@jjoyce0510
Copy link
Collaborator

jjoyce0510 commented Jun 15, 2022

This appears to have been working for others - what behavior were you experiencing?

@alexey-kravtsov
Copy link
Contributor Author

This is about the case when AUTH_JAAS_ENABLED is explicitly set to true.
Actual value returned by configs.getValue(JAAS_ENABLED_CONFIG_PATH).toString() is "Quoted(true)", and Boolean.parseBoolean(...) returns false, since under the hood it just checks for "true".equalsIgnoreCase(s); - in this case explicit true is parsed as false

@jjoyce0510
Copy link
Collaborator

Thanks for the explanation & the fix!

Running CI now. Once checks pass we can merge.

@github-actions
Copy link

github-actions bot commented Jun 16, 2022

Unit Test Results (build & test)

381 tests  ±0   381 ✔️ ±0   3m 23s ⏱️ -20s
  89 suites ±0       0 💤 ±0 
  89 files   ±0       0 ±0 

Results for commit 36b86e1. ± Comparison against base commit b4bf1d4.

♻️ This comment has been updated with latest results.

@jjoyce0510 jjoyce0510 merged commit 8dd7dfc into datahub-project:master Jun 16, 2022
@alexey-kravtsov
Copy link
Contributor Author

Thank you!

@alexey-kravtsov alexey-kravtsov deleted the 0_8_38 branch June 17, 2022 11:30
maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022
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.

2 participants