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

Use CMK blocks rather than CMK resources in Terraform #4242

Merged

Conversation

yuvalyaron
Copy link
Collaborator

@yuvalyaron yuvalyaron commented Jan 1, 2025

Resolves #4241

What is being addressed

Currently, CMKs are configured using separate Terraform resources, where the Azure resource is created first, and the CMK is added as a separate resource later.

This approach is problematic for tenants with policies requiring CMK to be enabled, as the resource is initially created without the CMK, resulting in a policy violation even if the CMK is added later.

This PR addresses the issue by using CMK blocks within the resources themselves, instead of defining them as separate resources.

Copy link

github-actions bot commented Jan 1, 2025

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit b887b4a.

♻️ This comment has been updated with latest results.

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

github-actions bot commented Jan 1, 2025

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12572911353 (with refid d812a81a)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

github-actions bot commented Jan 1, 2025

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12573094999 (with refid d812a81a)

(in response to this comment from @yuvalyaron)

@yuvalyaron yuvalyaron enabled auto-merge (squash) January 1, 2025 18:02
@yuvalyaron yuvalyaron disabled auto-merge January 1, 2025 18:24
@yuvalyaron yuvalyaron enabled auto-merge (squash) January 1, 2025 18:24
@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

github-actions bot commented Jan 1, 2025

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12573272383 (with refid d812a81a)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

github-actions bot commented Jan 1, 2025

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12573293655 (with refid d812a81a)

(in response to this comment from @yuvalyaron)

@yuvalyaron yuvalyaron requested a review from tim-p-allen January 1, 2025 21:39
Copy link
Member

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

LGTM as long as its been tested.

@tamirkamara
Copy link
Collaborator

/test-force-approve

Copy link

github-actions bot commented Jan 2, 2025

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit b887b4a)

(in response to this comment from @tamirkamara)

@yuvalyaron yuvalyaron merged commit e74fbe8 into microsoft:main Jan 2, 2025
12 checks passed
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.

Use CMK blocks rather than CMK resources in Terraform
3 participants