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

Add JWT options for acl_auth_method #448

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Conversation

jorgemarey
Copy link
Contributor

Fixes #422

@lgfa29
Copy link
Contributor

lgfa29 commented Apr 4, 2024

Thanks for the PR @jorgemarey!

Have you tested what happens if type changes? Would we leave an orphaned auth method in Nomad? Maybe we would need to update it to have ForceNew: true?

I was also wondering if we should validate config based on type, but I don't think we can do cross-field validation at plan time, at which point we may as well let Nomad knows if something is wrong on apply.

Would you also be able to add a CHANGELOG entry and update the website docs?

Thanks!

@jorgemarey
Copy link
Contributor Author

Hi @lgfa29

Have you tested what happens if type changes? Would we leave an orphaned auth method in Nomad? Maybe we would need to update it to have ForceNew: true?

I didn't notice this, but I just tested and the method seems to change correctly in nomad. So I guess there's no need to set ForceNew to true then.

I was also wondering if we should validate config based on type, but I don't think we can do cross-field validation at plan time, at which point we may as well let Nomad knows if something is wrong on apply.

I don't know how to do this. I you think we can do something on plan time let me know and I'll make the change.

Would you also be able to add a CHANGELOG entry and update the website docs?

Done!

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

Looks great, thank you @jorgemarey!

@lgfa29 lgfa29 merged commit 16a99fb into hashicorp:main Apr 8, 2024
1 check passed
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.

[Feature Request] Add JWT type to nomad_acl_auth_method
2 participants