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

Add BYOK resource #330

Merged
merged 7 commits into from
May 23, 2024
Merged

Add BYOK resource #330

merged 7 commits into from
May 23, 2024

Conversation

emerkle826
Copy link
Contributor

Fixes #320

Copy link

@CalebAlbers CalebAlbers left a comment

Choose a reason for hiding this comment

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

Thank you so much for doing this @emerkle826! Once these changes get merged and released, I'll get our code updated to use it. If you're open to it, I'd be happy to create a PR to add some example terraform that creates an AWS KMS key and configures Astra BYOK with it.

I left a few comments around clarifying expectations in documentation (specifically around not being able to delete keys via DevOps API, which has bitten us in the past).

Looking forward to using these! 🙂

Comment on lines 5 to 7
description: |-
astra_customer_key provides a Customer Key resource for Astra's Bring Your Own Key (BYOK).
---

Choose a reason for hiding this comment

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

I think a support ticket is required to delete a customer key, which wouldn't be idiomatic for terraform providers normally - I think it is worth calling that out in the docs for the resource so people know that manual clean-up is required.

}

func resourceCustomerKeyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
// Delete not yet supported via DevOps API

Choose a reason for hiding this comment

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

Would it be worth calling one of the diagnostic functions to return a warning to the user during a destroy run so they are aware manual cleanup is needed?


# astra_cloud_accounts (Data Source)

Retrieve a list of Cloud Accounts within an Organization

Choose a reason for hiding this comment

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

This is probably more a of a question for the DevOps API folks, but will this only ever return an array of length 1, or are there situations where Astra organizations will have more than one cloud accounts for a given cloud provider and region?

I ask since the primary use case for this data source is to be able to feed it into an AWS KMS access policy, which would require creating an ARN based on the provider_id returned in this data source's response. I'm curious if it is safe to just refer to it with data.astra_cloud_accounts.this[0].provider_id, or if some HCL manipulation needs to be done to turn it into an array of provider IDs/ARNs for the KMS key policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For GCP, I happen to have 2 accounts, so it can be more than 1. That may only be the case for GCP, but the code handles both AWS and GCP.

Choose a reason for hiding this comment

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

This can happen to aws too.

@emerkle826 emerkle826 merged commit 858a644 into main May 23, 2024
4 checks passed
@emerkle826 emerkle826 deleted the issue-320 branch May 23, 2024 22:30
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.

BYOK Configuration with Terraform
4 participants