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

Cloudflare service token rotation #1057

Conversation

BojanZelic
Copy link
Contributor

This PR closes #1041

It adds support for cloudflare_access_service_token renewal if terraform is ran within the timeperiod defined in min_days_for_renewal.

See example usage:

# Generate a service token that will renew if terraform is ran within 30 days of expiration
resource "cloudflare_access_service_token" "my_app" {
  account_id = "d41d8cd98f00b204e9800998ecf8427e"
  name       = "CI/CD app renewed"
  min_days_for_renewal = 30

  lifecycle {
    create_before_destroy = true
  }
}

@jacobbednarz
Copy link
Member

I would also like to explore an integration test for this behaviour however I don't exactly have a good approach planned out yet. Thoughts?

@BojanZelic
Copy link
Contributor Author

I would also like to explore an integration test for this behaviour however I don't exactly have a good approach planned out yet. Thoughts?

For an integration test I see two tests possibly:

  1. set min_days_for_renewal = 365... run the apply 2x and make sure that the 2nd time it regenerates the token
  2. set min_days_for_renewal = 364... run the apply 2x and make sure that it doesn't regenerate the token

@BojanZelic
Copy link
Contributor Author

@jacobbednarz I've added an acceptance test & updated a few things with the PR. Let me know if you have any further suggestions.

@@ -100,6 +137,7 @@ func resourceCloudflareAccessServiceTokenCreate(d *schema.ResourceData, meta int
d.Set("name", serviceToken.Name)
d.Set("client_id", serviceToken.ClientID)
d.Set("client_secret", serviceToken.ClientSecret)
d.Set("expires_at", serviceToken.ExpiresAt.Format(time.RFC3339))
Copy link
Member

Choose a reason for hiding this comment

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

cloudflare.AccessServiceTokenCreateResponse doesn't have an ExpiresAt field - see https://github.com/cloudflare/cloudflare-go/blob/master/access_service_tokens.go#L37-L44. we'll need to get this into cloudflare-go first

@jacobbednarz jacobbednarz added the workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. label May 17, 2021
@jacobbednarz
Copy link
Member

looks like a good approach! other than adding the field into cloudflare-go, this is looking ace. thanks!

@jacobbednarz
Copy link
Member

this is now ready to update from master to get this prepared for merge

@jacobbednarz jacobbednarz added workflow/pending-contributor-response Indicates an issue or PR requires a response from a contributor. and removed workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. labels May 21, 2021
@BojanZelic BojanZelic force-pushed the cloudflare_service_token_rotation branch from 695626d to cdaca76 Compare May 21, 2021 15:22
@BojanZelic
Copy link
Contributor Author

@jacobbednarz I've rebased from master... let me know if you need anything else to get this merged.

@jacobbednarz
Copy link
Member

thanks, looks great and the integration tests are passing locally 🎉

@jacobbednarz jacobbednarz merged commit 5659bb6 into cloudflare:master May 21, 2021
@BojanZelic BojanZelic deleted the cloudflare_service_token_rotation branch May 21, 2021 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow/pending-contributor-response Indicates an issue or PR requires a response from a contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloudflare_access_service_token should allow automatic rotation
2 participants