VAULT-35631: support RLQ fields "group_by" and "secondary_rate"

[bosouza]

trafficstars

Description

This PR adds supports to the new group_by and secondary_rate fields to the vault_quota_rate_limit resource, added to the API in https://github.com/hashicorp/vault/pull/30447. These new collective and entity-based rate limits are an enterprise-only functionality, but the default option group_by: ip is allowed on community edition and maintains the standard, backwards-compatible behavior of rate limit quotas.

As described in code comments I had a lot of trouble with the default value semantics for the fields, as leaving group_by unset is allowed and defaults to ip, and leaving secondary_rate unset makes it default to 0 on ip and none modes, and rate on the entity-based modes. There are 2 approaches I considered:

1. Optional + set state if in config: this is the approach that's already being used by the inheritable field, it is configured as an Optional field but not Computed, and we only call the Set function to commit its value to the tf state if it was already in the config/state, which means that if the user doesn't specify anything in the TF config then the value won't be written to the state.

Pros:

• when the practitioner updates the inheritable field from some value to unset in the TF config the result if for the TF state to contain a zero value for the setting. Still not perfect, but the SDK v2 can't distinguish between zero value and unset, so that's the best we can hope for

Cons:

• doesn't work with TF import. That's actually a bug we have, even if inheritable is set in the backend the field will be left unset in the resulting imported configuration
• the default value of fields like inheritable can't be referenced by the TF config, it's only visible when set.

2. Optional + computed: mark the field as both optional and computed, so regardless if the group_by and secondary_rate fields are set in the TF config they will show up

Pros:

• TF import works properly
• fields can always be depended upon to be a source of truth, even having the default values assigned by the external API

Cons:

• when the practitioner changes a field from a value to unset the terraform state is maintained. As seen in the tests, that results in a bug where removing secondary_rate doesn't bring it back to the default state expected if the RLQ had been originally created with the field unset.

Overall it seems like the approach 2 is the best. I considered migrating inheritable to this same approach, fixing the bug with terraform import, but that could be considered a breaking change so I'm not sure if it's a viable approach.

Jira: VAULT-35631

Checklist

• [x] Added CHANGELOG entry (only for user-facing changes)
• [x] Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestQuotaRateLimitGroupBy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -run=TestQuotaRateLimitGroupBy -timeout 30m ./...
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/consts   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/framework/base   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/framework/client [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/framework/errutil        [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/framework/model  [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/group   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/mfa     [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/pki      [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/provider/fwprovider      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.014s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/framework/validators     0.016s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  0.034s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider 0.020s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/internal/providertest     [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/rotation [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/sync     [no test files]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/vault/secrets/ephemeral  0.014s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/vault/sys        0.013s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/testutil  0.005s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      0.006s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util/mountutil    0.003s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/vault     0.014s
...

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request