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

Gracefully handle rate limiting error on hcp_vault_secrets_secret resource. #812

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

codergs
Copy link
Contributor

@codergs codergs commented Apr 11, 2024

🛠️ Description

This PR does two things:

  1. Add retries for HTTP status code 429 errors received on the OpenVaultSecretsAppSecret api.
  2. Fixes the already broken TestAccVaultSecretsResourceSecret acceptance test.

🏗️ Acceptance tests

  • Are there any feature flags that are required to use this functionality?
  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

Fri Apr 12 11:23:25 EDT 2024 hashicorp/terraform-provider-hcp > make testacc TESTARGS='-run=TestAccVaultSecrets'  
TF_ACC=1 go test ./internal/... -v -run=TestAccVaultSecrets -timeout 360m -parallel=10 -count 1
?       github.com/hashicorp/terraform-provider-hcp/internal/clients/iampolicy  [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/clients/packerv2   [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider   [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/acctest   [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/customtypes       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/clients    0.375s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/consul     0.638s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/hcpvalidator       0.815s [no tests to run]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/modifiers [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      0.987s [no tests to run]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils  [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/configbuilder    [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/configbuilder/packerconfig       [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/testcheck        [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/testutils/testclient       [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils      [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils/base [no test files]
?       github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/utils/location     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/iam       1.241s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/logstreaming      2.260s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/sources/artifact   2.650s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/packer/sources/version    2.818s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/resourcemanager   2.668s [no tests to run]
=== RUN   TestAccVaultSecretsResourceApp
--- PASS: TestAccVaultSecretsResourceApp (2.74s)
=== RUN   TestAccVaultSecretsResourceSecret
The api rate limit exceeded, attempt: 1 trying in 19 seconds--- PASS: TestAccVaultSecretsResourceSecret (21.65s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/vaultsecrets      27.500s
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/waypoint  3.888s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/webhook   4.513s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider/webhook/validator 3.577s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/providersdkv2      4.874s [no tests to run]
...

Modified rate limit to 1 / minute

HCP provider that is public (current behavior when api rate limit is hit)

terraform {
  required_providers {
    hcp = {
      source  = "hashicorp/hcp"
    }
  }
}

resource "hcp_vault_secrets_app" "example" {
  app_name    = "example-app-name"
  description = "My new app!"
}

resource "hcp_vault_secrets_secret" "example" {
  app_name     = hcp_vault_secrets_app.example.app_name
  secret_name  = "example_secret"
  secret_value = "hashi123"
}
> terraform init

Initializing the backend...

Initializing provider plugins...
- Reusing previous version of localhost/providers/hcp from the dependency lock file
- Finding latest version of hashicorp/hcp...
- Installing hashicorp/hcp v0.86.0...
- Installed hashicorp/hcp v0.86.0 (signed by HashiCorp)
- Using previously-installed localhost/providers/hcp v0.0.1

Terraform has made some changes to the provider dependency selections recorded
in the .terraform.lock.hcl file. Review those changes and commit them to your
version control system if they represent changes you intended to make.

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.
Thu Apr 11 17:50:19 EDT 2024 terraform-provider-hcp/test - (codergs-22398) > terraform paln
Terraform has no command named "paln". Did you mean "plan"?

To see all of Terraform's top-level commands, run:
  terraform -help

> terraform plan
hcp_vault_secrets_app.example: Refreshing state... [id=example-app-name]
hcp_vault_secrets_secret.example: Refreshing state... [id=example-app-name]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

> terraform plan
hcp_vault_secrets_app.example: Refreshing state... [id=example-app-name]
hcp_vault_secrets_secret.example: Refreshing state... [id=example-app-name]

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: [GET /secrets/2023-06-13/organizations/{location.organization_id}/projects/{location.project_id}/apps/{app_name}/open/{secret_name}][429] OpenAppSecret default  &{Code:8 Details:[] Message:this request has exceeded the rate limit of 1 requests per minute for your org, try again in 54 seconds}
│ 
│   with hcp_vault_secrets_secret.example,
│   on main.tf line 15, in resource "hcp_vault_secrets_secret" "example":
│   15: resource "hcp_vault_secrets_secret" "example" {
│ 
│ Error reading secret

HCP provider with this PR fix (latest behavior)

terraform {
  required_providers {
    hcp = {
      source  = "localhost/providers/hcp"
    }
  }
}

resource "hcp_vault_secrets_app" "example" {
  app_name    = "example-app-name"
  description = "My new app!"
}

resource "hcp_vault_secrets_secret" "example" {
  app_name     = hcp_vault_secrets_app.example.app_name
  secret_name  = "example_secret"
  secret_value = "hashi123"
}
> terraform init

Initializing the backend...

Initializing provider plugins...
- Finding latest version of localhost/providers/hcp...
- Installing localhost/providers/hcp v0.0.1...
- Installed localhost/providers/hcp v0.0.1 (unauthenticated)

Terraform has created a lock file .terraform.lock.hcl to record the provider
selections it made above. Include this file in your version control repository
so that Terraform can guarantee to make the same selections by default when
you run "terraform init" in the future.

╷
│ Warning: Incomplete lock file information for providers
│ 
│ Due to your customized provider installation methods, Terraform was forced to calculate lock file checksums locally for the following providers:
│   - localhost/providers/hcp
│ 
│ The current .terraform.lock.hcl file only includes checksums for darwin_amd64, so Terraform running on another platform will fail to install these providers.
│ 
│ To calculate additional checksums for another platform, run:
│   terraform providers lock -platform=linux_amd64
│ (where linux_amd64 is the platform to generate)
╵

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.

Thu Apr 11 17:57:06 EDT 2024 terraform-provider-hcp/test  > terraform plan
hcp_vault_secrets_app.example: Refreshing state... [id=example-app-name]
hcp_vault_secrets_secret.example: Refreshing state... [id=example-app-name]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.
Thu Apr 11 17:57:58 EDT 2024 terraform-provider-hcp/test > terraform plan
hcp_vault_secrets_app.example: Refreshing state... [id=example-app-name]
hcp_vault_secrets_secret.example: Refreshing state... [id=example-app-name]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Newly added DEBUG log line.

# rate limited
Fri Apr 12 11:45:56 EDT 2024 terraform-provider-hcp/test > TF_LOG=DEBUG terraform plan 2>&1 | grep -C 4 "api rate limit"
2024-04-12T11:46:01.620-0400 [DEBUG] provider.terraform-provider-hcp: X-Ratelimit-Remaining: 0
2024-04-12T11:46:01.620-0400 [DEBUG] provider.terraform-provider-hcp: X-Ratelimit-Reset: 23
2024-04-12T11:46:01.621-0400 [DEBUG] provider.terraform-provider-hcp
2024-04-12T11:46:01.621-0400 [DEBUG] provider.terraform-provider-hcp: {"code":8,"message":"this request has exceeded the rate limit of 1 requests per minute for your org, try again in 23 seconds","details":[]}
2024-04-12T11:46:01.621-0400 [DEBUG] provider.terraform-provider-hcp: The api rate limit has been exceeded, retrying in 23 seconds, attempt: 1: @caller=/Users/gsharma/go/src/github.com/hashicorp/terraform-provider-hcp/internal/clients/vault_secrets.go:144 tf_resource_type=hcp_vault_secrets_secret tf_rpc=ReadResource @module=hcp tf_mux_provider="*proto6server.Server" tf_provider_addr=registry.terraform.io/hashicorp/hcp tf_req_id=86b439c7-cd93-cea5-7b0a-6f0b0225bbf8 timestamp=2024-04-12T11:46:01.621-0400

2024-04-12T11:46:24.623-0400 [DEBUG] provider.terraform-provider-hcp: 2024/04/12 11:46:24 [DEBUG] GET /secrets/2023-06-13/organizations/9f3ec4ed-26be-4a6d-a81f-175dbd2ee9aa/projects/1f045511-3018-4b65-beb7-e6c93d79807a/apps/example-app-name/open/example_secret HTTP/1.1
2024-04-12T11:46:24.623-0400 [DEBUG] provider.terraform-provider-hcp: Host: gaurav02-cxkef6v7.dev.pedp-remote.hashicorp.services
2024-04-12T11:46:24.623-0400 [DEBUG] provider.terraform-provider-hcp: User-Agent: Go-http-client/1.1
2024-04-12T11:46:24.623-0400 [DEBUG] provider.terraform-provider-hcp: Accept: application/json

# rate limit reset after 1 min
Fri Apr 12 11:46:24 EDT 2024 terraform-provider-hcp/test  > TF_LOG=DEBUG terraform plan 2>&1 | grep -C 4 "api rate limit"

@codergs codergs requested a review from a team as a code owner April 11, 2024 22:01
@codergs codergs requested review from mkam, aslamovamir, pmmukh and a team and removed request for mkam and aslamovamir April 11, 2024 22:01
@codergs codergs changed the title Add retries to OpenVaultSecretsAppSecret on 429 Add retries to OpenVaultSecretsAppSecret on HTTP Status Code 429 Apr 11, 2024
internal/clients/vault_secrets.go Outdated Show resolved Hide resolved
internal/clients/vault_secrets.go Show resolved Hide resolved
@codergs codergs requested a review from pmmukh April 12, 2024 15:50
@codergs codergs requested a review from a team as a code owner April 12, 2024 15:56
@codergs codergs changed the title Add retries to OpenVaultSecretsAppSecret on HTTP Status Code 429 Gracefully handle rate limiting error on hcp_vault_secrets_secret resource. Apr 12, 2024
Copy link
Contributor

@pmmukh pmmukh 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!

internal/clients/vault_secrets.go Show resolved Hide resolved
internal/clients/vault_secrets.go Show resolved Hide resolved
internal/clients/vault_secrets.go Outdated Show resolved Hide resolved
internal/clients/vault_secrets.go Show resolved Hide resolved
internal/clients/vault_secrets.go Outdated Show resolved Hide resolved
@codergs codergs requested a review from averche April 15, 2024 14:53
var err error
for attempt := 0; attempt < retryCount; attempt++ {
getResp, err = client.VaultSecrets.OpenAppSecret(getParams, nil)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to build a generic module that retries with the right backoff based on status code so that others can also use it?

Copy link
Contributor Author

@codergs codergs Apr 16, 2024

Choose a reason for hiding this comment

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

generic wrapper is doable but is probably a little bigger undertaking at this point. let me revisit it after this so that the issue highlighted is fixed. Given various req/reso possibilities, it will need some thinking.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. This can be implemented later in subsequent PR.

@codergs codergs merged commit 4b46b4d into main Apr 17, 2024
6 checks passed
@codergs codergs deleted the codergs-22398 branch April 17, 2024 13:49
catsby added a commit that referenced this pull request Apr 17, 2024
* main:
  New Resource: Waypoint Add-on (#807)
  Gracefully handle rate limiting error on `hcp_vault_secrets_secret` resource. (#812)
  HCPIE-1244: Improve Identity-Specific Testing (#810)
  Fix code ownership for vault secrets files (#814)
  [COMPLIANCE] Add Copyright and License Headers (#799)
  update tests to use real tf no-code modules (#811)
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.

6 participants