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

feat: Add CMEK support for cross-region read replicas #251

Merged
merged 3 commits into from
Oct 13, 2021
Merged

feat: Add CMEK support for cross-region read replicas #251

merged 3 commits into from
Oct 13, 2021

Conversation

imrannayer
Copy link
Collaborator

Fix #249
It is a breaking change. variable read_replicas has an extra key encryption_key_name

@google-cla google-cla bot added the cla: yes label Oct 7, 2021
@comment-bot-dev
Copy link

comment-bot-dev commented Oct 7, 2021

Thanks for the PR! 🚀
✅ Lint checks have passed.

@bharathkkb
Copy link
Member

/gcbrun

@imrannayer imrannayer changed the title Feat/cmek cross region 249 Feat/cmek support for mysql postgresql cross region read replica 249 Oct 8, 2021
@imrannayer
Copy link
Collaborator Author

@bharathkkb can you plz rerun pipeline? It failed due to some API error.

@bharathkkb
Copy link
Member

/gcbrun

@morgante morgante changed the title Feat/cmek support for mysql postgresql cross region read replica 249 feat: Add CMEK support for cross-region read replicas Oct 13, 2021
@morgante morgante merged commit 426724a into terraform-google-modules:master Oct 13, 2021
@jmymy
Copy link
Contributor

jmymy commented Mar 9, 2022

Anyone experiencing weird issues with this?

I have been passing in the var, and now its saying the read replica KMS key will be set to null and requires replica replacement. I let it destroy, and give it another name or wait 7 days for the name to come available, and It creates the new replica instance just fine. However, on the next run, it goes to destroy it again.

@jmymy
Copy link
Contributor

jmymy commented Mar 9, 2022

I had replicas created before this version bump, they were already getting the encryption_key_name passed in as a value, so the state file has the encryption_key_name set. With this bump, its now trying to set that value to null, resulting in a replica recreation.

@jmymy
Copy link
Contributor

jmymy commented Mar 9, 2022

encryption_key_name = (join("-", slice(split("-", lookup(each.value, "zone", var.zone)), 0, 2))) == var.region ? null : each.value.encryption_key_name
https://github.com/terraform-google-modules/terraform-google-sql-db/pull/251/files#diff-2961a2500544e56735fcf3af9d494ba4c33d54dba1211b88782b0c1c4805d81cR32

appears to be the culprit. There is no way i can continue without re-creating the replicas and ensuring that I pass in null the next creation. If i dont pass in null for replicas, the instance still gets created as the provider/api still accept it and the following TF runs will recreate it and be in a loop. Or it does create with null, and the api adds the KMS key to the metadata and thus state, forcing a recreation anyway

@morgante
Copy link
Contributor

morgante commented Mar 9, 2022

@jmymy Can you open an issue w/ the details? Would also be happy to review a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgresSQL CMEK issue with cross region read replica
5 participants