Add support for vault_quota_rate_limit interval and block_interval by mdgreenfield · Pull Request #1084 · hashicorp/terraform-provider-vault

mdgreenfield · 2021-06-28T22:55:01Z

Allows users to specify either in terms of duration or number of
seconds utilizing Vault's parseutil.ParseDurationSecond logic. See
https://github.com/hashicorp/vault/blob/ed33ed1a0a5ecb9d41ab22b4899aa318239ac031/sdk/helper/parseutil/parseutil.go#L96-L147

Closes #1049

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

Release note for CHANGELOG:

Add support for `vault_quota_rate_limit` `interval` and `block_interval`

Output from acceptance testing:

$ ➜  terraform-provider-vault git:(quota-rate-limit-changes) make testacc TESTARGS='-run=TestQuotaRateLimit'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestQuotaRateLimit -timeout 120m
?       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]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/codegen   (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    0.622s [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    0.846s [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    1.799s [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        0.477s [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    1.562s [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      1.101s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/util      (cached) [no tests to run]
WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
        A future release of golang/protobuf will delete this package,
        which has long been excluded from the compatibility promise.

=== RUN   TestQuotaRateLimit
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 0
  interval = 1
  name = tf-test-5534064214137632519
  path =
  rate = 157
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 0
  interval = 1
  name = tf-test-5534064214137632519
  path =
  rate = 157
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 0
  interval = 1
  name = tf-test-5534064214137632519
  path =
  rate = 157
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 0
  interval = 1
  name = tf-test-5534064214137632519
  path =
  rate = 157
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 0
  interval = 1
  name = tf-test-5534064214137632519
  path =
  rate = 157
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 120
  interval = 60
  name = tf-test-5534064214137632519
  path =
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 120
  interval = 60
  name = tf-test-5534064214137632519
  path =
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 120
  interval = 60
  name = tf-test-5534064214137632519
  path =
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 120
  interval = 60
  name = tf-test-5534064214137632519
  path =
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 120
  interval = 60
  name = tf-test-5534064214137632519
  path =
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 60
  interval = 120
  name = tf-test-5534064214137632519
  path = sys/
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 60
  interval = 120
  name = tf-test-5534064214137632519
  path = sys/
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 60
  interval = 120
  name = tf-test-5534064214137632519
  path = sys/
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 60
  interval = 120
  name = tf-test-5534064214137632519
  path = sys/
  rate = 525.3
vault_quota_rate_limit.foobar:
  ID = tf-test-5534064214137632519
  provider = provider.vault
  block_interval = 60
  interval = 120
  name = tf-test-5534064214137632519
  path = sys/
  rate = 525.3
--- PASS: TestQuotaRateLimit (0.60s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     1.018s

mdgreenfield · 2021-06-28T22:57:16Z

+				Type:         schema.TypeString,
+				Optional:     true,
+				Description:  "If set, when a client reaches a rate limit threshold, the client will be prohibited from any further requests until after the 'block_interval' has elapsed.",
+				StateFunc:    durationSecond,


Because read requests return these values in seconds and not a duration the configuration value must be computed in terms of seconds to prevent a perpetual diff.

Since vault converts the duration strings to seconds (int), we prefer to only support schema.TypeInt for these sorts of Vault fields. This should greatly simplify your PR.

hashicorp-cla · 2022-03-12T17:19:50Z

All committers have signed the CLA.

benashz

Thanks for the contribution. We can scale down this PR by making interval and block_interval schema.TypeInt.

benashz · 2022-05-30T19:37:48Z

+				Type:         schema.TypeString,
+				Optional:     true,
+				Description:  "If set, when a client reaches a rate limit threshold, the client will be prohibited from any further requests until after the 'block_interval' has elapsed.",
+				StateFunc:    durationSecond,


Since vault converts the duration strings to seconds (int), we prefer to only support schema.TypeInt for these sorts of Vault fields. This should greatly simplify your PR.

benashz · 2022-05-30T19:41:44Z

@mdgreenfield please let us know if you have spare cycles to work on this PR. If not we would be happy to take it over.

Thanks,

Ben

mdgreenfield · 2022-05-31T19:38:56Z

Hi @benashz I can address the comments in the next day or two.

Allows users to specify either in terms of duration or number of seconds utilizing Vault's `parseutil.ParseDurationSecond` logic. See https://github.com/hashicorp/vault/blob/ed33ed1a0a5ecb9d41ab22b4899aa318239ac031/sdk/helper/parseutil/parseutil.go#L96-L147 Closes hashicorp#1049

mdgreenfield · 2022-06-01T21:38:50Z

@benashz I've updated the PR switching to schema.TypeInt.

I did notice that some fields, e.g. the TTL fields on the auth mount resource, use schema.TypeString and make use of validateDuration in the vault package. That may predate the move towards using schema.TypeInt and I suspect we can't really just go back and update it to schema.TypeInt without being backwards incompatible.

mdgreenfield · 2022-06-01T21:41:56Z

The updated acceptance testing output:

$ make testacc TESTARGS='-run=TestQuotaRateLimit'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -v -run=TestQuotaRateLimit -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]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.415s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    1.823s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    1.803s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    1.801s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        0.941s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    1.073s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      2.053s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  1.038s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
?       github.com/hashicorp/terraform-provider-vault/testutil  [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/util      0.946s [no tests to run]
=== RUN   TestQuotaRateLimit
--- PASS: TestQuotaRateLimit (81.98s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     83.321s

benashz

Looks good! I added a few minor suggestions that should be addressed.

benashz · 2022-06-02T22:08:59Z

-	rateLimt := fmt.Sprintf("%.1f", whole+decimal)
-	// Vault retuns floats with trailing zeros trimmed
-	return strings.TrimRight(strings.TrimRight(rateLimt, "0"), ".")
+	rateLimit := fmt.Sprintf("%.1f", whole+decimal)


Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>

mdgreenfield · 2022-06-04T15:23:22Z

Thanks @benashz for the review. I learned a few things about writing Terraform providers from your comments.

Changes have been added to the PR.

benashz

LGTM!

Thank you for you contribution to HashiCorp!

…ashicorp#1084) Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>

github-actions Bot added documentation size/L labels Jun 28, 2021

mdgreenfield commented Jun 28, 2021

View reviewed changes

Comment thread util/util.go Outdated

mdgreenfield commented Jun 28, 2021

View reviewed changes

benashz reviewed May 30, 2022

View reviewed changes

benashz self-assigned this May 30, 2022

benashz added the feature-parity label May 30, 2022

benashz added this to the 3.7.0 milestone May 30, 2022

mdgreenfield force-pushed the quota-rate-limit-changes branch from 65c4767 to bbb5741 Compare June 1, 2022 21:17

github-actions Bot added size/M and removed size/L labels Jun 1, 2022

mdgreenfield force-pushed the quota-rate-limit-changes branch 4 times, most recently from 3875b7b to d2c5ba3 Compare June 1, 2022 21:27

Update vault_quota_rate_limit interval interval fields to int

2d8f99c

mdgreenfield force-pushed the quota-rate-limit-changes branch from d2c5ba3 to 2d8f99c Compare June 1, 2022 21:29

benashz reviewed Jun 2, 2022

View reviewed changes

Apply suggestions from code review

968be5c

Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>

benashz approved these changes Jun 6, 2022

View reviewed changes

benashz merged commit 19ea794 into hashicorp:main Jun 6, 2022

Uh oh!

Conversation

mdgreenfield commented Jun 28, 2021

Community Note

Uh oh!

Uh oh!

mdgreenfield Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

benashz May 30, 2022

Choose a reason for hiding this comment

Uh oh!

hashicorp-cla commented Mar 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benashz left a comment

Choose a reason for hiding this comment

Uh oh!

benashz May 30, 2022

Choose a reason for hiding this comment

Uh oh!

benashz commented May 30, 2022

Uh oh!

mdgreenfield commented May 31, 2022

Uh oh!

mdgreenfield commented Jun 1, 2022

Uh oh!

mdgreenfield commented Jun 1, 2022

Uh oh!

benashz left a comment

Choose a reason for hiding this comment

Uh oh!

benashz Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdgreenfield commented Jun 4, 2022

Uh oh!

benashz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hashicorp-cla commented Mar 12, 2022 •

edited

Loading