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 Geo-Replicated SQL MI autorotation note #9837

Open
wants to merge 4 commits into
base: live
Choose a base branch
from

Conversation

lukecalderon
Copy link

In sql-docs\azure-sql\database\transparent-data-encryption-byok-key-rotation.md

Added a note to provide clarification when using the same default TDE encryptor across a failover group. This is mentioned in a different page here; however, the article I've updated contradicts the configuration.

The behaviour has been confirmed by the SQL MI Product Group via a support ticket.

Copy link
Contributor

@lukecalderon : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit 4ba5fb3:

✅ Validation status: passed

File Status Preview URL Details
azure-sql/database/transparent-data-encryption-byok-key-rotation.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@Court72
Copy link
Contributor

Court72 commented Jul 3, 2024

@GithubMirek

Can you review the proposed changes?

Important: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged tracking label for the PR review team label Jul 3, 2024
@VanMSFT
Copy link
Member

VanMSFT commented Jul 3, 2024

Thanks, @lukecalderon - Can you help reference the support ticket?

@GithubMirek - Please confirm if these changes are correct.

@lukecalderon
Copy link
Author

lukecalderon commented Jul 4, 2024

Thanks, @lukecalderon - Can you help reference the support ticket?

Sure - MS Support Ref is 2405230030006073

@VanMSFT
Copy link
Member

VanMSFT commented Jul 11, 2024

Following up with Mirek.

Copy link
Contributor

Learn Build status updates of commit 6c8245b:

✅ Validation status: passed

File Status Preview URL Details
azure-sql/database/transparent-data-encryption-byok-key-rotation.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Author

@lukecalderon lukecalderon left a comment

Choose a reason for hiding this comment

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

LGTM

@VanMSFT
Copy link
Member

VanMSFT commented Jul 17, 2024

Hi @lukecalderon - I'm getting contradicting information from the Product Team on this. I'll need to investigate further. Thanks.

@lukecalderon
Copy link
Author

Hi @lukecalderon - I'm getting contradicting information from the Product Team on this. I'll need to investigate further. Thanks.

No problem, sums up my experience too. Happy to provide any further info on it if needed.

@VanMSFT
Copy link
Member

VanMSFT commented Jul 17, 2024

Hi @lukecalderon - I'm getting contradicting information from the Product Team on this. I'll need to investigate further. Thanks.

No problem, sums up my experience too. Happy to provide any further info on it if needed.

Hi @lukecalderon - Looking at the Support case you referenced, it doesn't mention that the issue was due to not having Auto-rotate key set on both servers. That may have been what was mentioned to you, but our Product Team stated that the setting isn't needed on both servers. Auto rotation can ben enabled on either the primary or the secondary server, and should still work. I'll need to fix language on our other doc as well to reflect this.

If you have more to add or know of the PG person that stated this, I can help follow-up. Thanks!

@lukecalderon
Copy link
Author

Hi @lukecalderon - Looking at the Support case you referenced, it doesn't mention that the issue was due to not having Auto-rotate key set on both servers. That may have been what was mentioned to you, but our Product Team stated that the setting isn't needed on both servers. Auto rotation can ben enabled on either the primary or the secondary server, and should still work. I'll need to fix language on our other doc as well to reflect this.

If you have more to add or know of the PG person that stated this, I can help follow-up. Thanks!

Hi @VanMSFT - I was in direct discussion with the engineer (Abdullah Qtaishat) over Teams, who in turn was in discussion with the Product Group, so this may not have made it into the ticketing system.

In our configuration, we had the primary configured with Default TDE/Auto-Rotation:
image

Whilst on the secondary, it was configured without auto-rotation:
image

They both matched, until the primary rotated. The key existed on the secondary server, but a 'background job' got stuck rotating the key onto the secondary. The PG had to manually cancel the job, before I could manually select the new key on the secondary server. Afterwards, the engineer informed me that the PG had stated if the same key is used on the primary and secondary servers, and is the default TDE protector, then auto-rotation must be enabled on both servers.

@VanMSFT
Copy link
Member

VanMSFT commented Jul 17, 2024

Thanks for the additional context, @lukecalderon! I'll check with them and see what they'll say.

@lukecalderon
Copy link
Author

Thanks for the additional context, @lukecalderon! I'll check with them and see what they'll say.

Hi @VanMSFT, how did you get on with the PG?

Copy link
Contributor

Learn Build status updates of commit 8eb748e:

✅ Validation status: passed

File Status Preview URL Details
azure-sql/database/transparent-data-encryption-byok-key-rotation.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@VanMSFT
Copy link
Member

VanMSFT commented Sep 16, 2024

Sorry for the delay @lukecalderon! I'm following up internally.

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

Successfully merging this pull request may close these issues.

5 participants