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

CMK Encryption support for resources in templates #4195

Merged
merged 19 commits into from
Dec 23, 2024

Conversation

yuvalyaron
Copy link
Collaborator

@yuvalyaron yuvalyaron commented Dec 13, 2024

Resolves #4145

What is being addressed

Added support for CMK encryption for resources in templates.

How is this addressed

  • Workspace services use the CMK and encryption identity of the workspace to enable encryption for supporting resources.
  • Shared services use the CMK and encryption identity of the core TRE to enable encryption for supporting resources.

Copy link

github-actions bot commented Dec 13, 2024

Unit Test Results

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

Results for commit 715a331.

♻️ This comment has been updated with latest results.

@marrobi
Copy link
Member

marrobi commented Dec 16, 2024

@yuvalyaron do we know what the upgrade behaviour is here? Might be worth incrementing bundles by a major version to prevent easy upgrade?

Copy link
Collaborator

@LizaShak LizaShak left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@LizaShak LizaShak left a comment

Choose a reason for hiding this comment

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

Let me know when ready :)

@LizaShak LizaShak self-requested a review December 16, 2024 11:53
@yuvalyaron yuvalyaron marked this pull request as ready for review December 18, 2024 20:22
@yuvalyaron
Copy link
Collaborator Author

Let me know when ready :)

ready :)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12400748485 (with refid 64081e76)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

@yuvalyaron do we know what the upgrade behaviour is here? Might be worth incrementing bundles by a major version to prevent easy upgrade?

Upgrading is not an issue as long as the enable_cmk_encryption variable remains unchanged, and we assume it will never change (though the exact approach to how to ensure this is yet to be determined): #4172

Copy link
Collaborator

@LizaShak LizaShak left a comment

Choose a reason for hiding this comment

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

LGTM. Left Just a little comment

@yuvalyaron
Copy link
Collaborator Author

/help

Copy link

🤖 pr-bot 🤖

Hello!

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-destroy-env

Copy link

Destroying PR test environment (RG: rg-tre64081e76)... (run: https://github.com/microsoft/AzureTRE/actions/runs/12452939536)

Copy link

PR test environment destroy complete (RG: rg-tre64081e76)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

🤖 pr-bot 🤖

⚠️ Cannot run tests as PR is not mergeable. Ensure that the PR is open and doesn't have any conflicts.

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/12453158465 (with refid 64081e76)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-force-approve

Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit 715a331)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

The workspace tests fail because the Terraform code from the base workspace in the old release does not include the newly added variables. This issue should be resolved once a new release is created.

@yuvalyaron yuvalyaron merged commit 8baad00 into microsoft:main Dec 23, 2024
12 checks passed
@yuvalyaron yuvalyaron deleted the 4142-cmk-for-templates branch December 23, 2024 09:58
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.

CMK support for resources outside of core
4 participants