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

Ensure that the qr_size can be properly configured. #1750

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

benashz
Copy link
Contributor

@benashz benashz commented Feb 8, 2023

Previously, the identity_mfa_totp resource's schema did not allow the qr_size field to be configured by the user. It was set as Computed, but not Optional. After resolving this issue we discovered that Create/Update functions would always include this param in the vault API request, which negated the benefits of the field being Computed.

In order to resolve the above issue above, and other similar ones, it is now possible to set the VaultAPIGetterFunc by field on the contextFuncConfig. Which means it is possible to use any of the
following schema.ResourceData methods: GetOk(), Get(), GetOkExists()

Relates #1746

Previously, the identity_mfa_totp resource's schema did not allow the
qr_size field to be configured by the user. It was set as Computed,
but not Optional. After resolving this issue we discovered that
Create/Update functions would always include this param in the vault API
request, which negated the benefits of the field being Computed.

In order to resolve the above issue above, and other similar ones, it is
now possible to set the VaultAPIGetterFunc by field on the
contextFuncConfig. Which means it is possible to use any of the
following schema.ResourceData methods: GetOk(), Get(), GetOkExists()
Copy link
Contributor

@vinay-gopalan vinay-gopalan 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! Had 1 question about a potential update for the default getter feature, but LGTM otherwise 😄

internal/identity/mfa/mfa.go Outdated Show resolved Hide resolved
@benashz benashz merged commit f86e54d into main Mar 23, 2023
@benashz benashz deleted the VAULT-13244/fix-identity-mfa-totp-qr_size branch March 23, 2023 18:44
@vinay-gopalan vinay-gopalan modified the milestones: 3.15.0, 3.14.1 Mar 23, 2023
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.

2 participants