Peng-7929:TerraformSDK_apikey_expiry#374
Conversation
hhellyer
left a comment
hhellyer
left a comment
There was a problem hiding this comment.
I think the fixed expiry durations probably need to be removed but otherwise this looks good.
I think this makes sense provided the api key is in use elsewhere in terraform config but not if it's used by scripts running outside terraform. (Which is not a problem with this PR.)
This doesn't expose the /renew functionality of the api endpoints which means the operations probably need to be carried out with a key with mange_api_keys permission. It might be worth us checking if there's way to expose that function that is meaningful inside terraform.
| ForceNew: true, | ||
| ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { | ||
| v := val.(string) | ||
| if v != "" && v != "10d" && v != "30d" && v != "90d" { |
There was a problem hiding this comment.
I don't think we are enforcing the limited set of durations via the REST api just the portal so we should probably not enforce it here. The durations will need to fix the patten of "<number>d" though.
There was a problem hiding this comment.
Updated the validation to use a regex pattern ^\d+d$ instead of hardcoded 10d/30d/90d. Now accepts any duration format while still enforcing the <number>d pattern. Tests added for custom durations
| ), | ||
| }, | ||
| { | ||
| Config: testAccAPIKeyWithExpiryDuration(name, "30d"), |
There was a problem hiding this comment.
What is this testing? I don't think we can change the expiry duration on an api key once it's created.
(Edit - I realised that since terraform will recreate the object this probably does work, I'm not sure it's much use because of the key value changing. I think this is probably fine as it's just the way terraform works, customers should probably avoid doing it but we already have this for normal keys above.)
There was a problem hiding this comment.
Since Terraform recreates the resource when expiry_duration changes, technically the update works, but the API key/secrets will change as part of the recreation. I added a note in the docs to make that behaviour clearer
| resource "ns1_apikey" "rotating_key" { | ||
| name = "rotating-api-key" | ||
|
|
||
| # Enable automatic secret rotation every 30 days |
There was a problem hiding this comment.
Is this comment right? I think we are enforcing secret expiry every 30 days not enabling it to happen automatically. (And I wonder if we should use "renew" or "rotate" but I think it's used interchangeably everywhere else.)
There was a problem hiding this comment.
updated the wording in the examples/docs to “secret expiration” instead of “automatic rotation” since terraform itself isn’t performing the renew/rotate operation.
hhellyer
left a comment
hhellyer
left a comment
There was a problem hiding this comment.
LGTM - This should be able to be merged and published now.
2 participants
Add support for API key expiry and automatic secret rotation in Terraform provider.
Changes
expiry_durationfield (10d/30d/90d)secretsfield to view secret metadataDependencies