Add Secrets Sync support to TFVP #2098
Conversation
There was a problem hiding this comment.
Looks great. Most of my comments are nits, some edge cases we can handle is follow-up PRs or just providing some context along the way.
The only aspect I'd be keen to explore before we merge is to try to make the destination methods generic to remove an important amount of boilerplate code and make new destinations or additional fields easier to fit in in the future. Let me know if that make sense, I'm also happy to pair on this if you want!
| log.Printf("[DEBUG] Deleted AWS sync destination at %q", path) | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Recreate scenarios like modifying the region do not work at the moment with the following error:
│ Namespace: my-ns-1
│ URL: DELETE http://localhost:8200/v1/sys/sync/destinations/aws-sm/my-dest-1
│ Code: 400. Errors:
│
│ * store cannot be deleted because it is still managing secrets
I believe that with @robmonte 's new ?purge parameter available when deleting an application we could gracefully handle the teardown flow by passing this param and polling the destination until the purge operation gets completed by the backend. This can be a separate ticket/PR but food for thought on a possible solution!
| path := secretsSyncDestinationPath(d.Id(), typ) | ||
|
|
||
| log.Printf("[DEBUG] Deleting sync destination at %q", path) | ||
| _, err := client.Logical().DeleteWithContext(ctx, path) |
There was a problem hiding this comment.
Will probably add ?purge parameter to the delete method in a follow-up PR for robustness
|
|
||
| func awsSecretsSyncDestinationDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { | ||
| return syncutil.SyncDestinationDelete(ctx, d, meta, awsSyncType) | ||
| } |
There was a problem hiding this comment.
So clean with the generic utility methods! gg
+ added sync config resource
Description
Adds the following resources:
Checklist