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

Defender: Azure Cosmos DB should disable public network access #4324

Merged

Conversation

jonnyry
Copy link
Collaborator

@jonnyry jonnyry commented Feb 5, 2025

Resolves #4322

  • Changes Public network access on the two cosmos accounts from Selected networks to Disabled, unless local debugging is enabled
  • Also updates the Azure Portal middleware IPs that get added to the Cosmos firewall - they've changed since they were orignally added (see this article)
  • Middleware IPs are only added added when debugging is enabled, previously they were added all the time

Copy link

github-actions bot commented Feb 5, 2025

Unit Test Results

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

Results for commit a08e374.

♻️ This comment has been updated with latest results.

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

@jonnyry jonnyry requested a review from tamirkamara February 6, 2025 10:09
@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 9, 2025

/test

Copy link

github-actions bot commented Feb 9, 2025

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13227249471 (with refid d5333ffb)

(in response to this comment from @jonnyry)

@jonnyry jonnyry removed the request for review from tamirkamara February 11, 2025 08:37
Copy link
Collaborator

@ShakutaiGit ShakutaiGit left a comment

Choose a reason for hiding this comment

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

This PR conflicts with another PR that was merged yesterday:
https://github.com/microsoft/AzureTRE/pull/4255/files#diff-6d888c37158c152c3302343686aa77e915107e9de3bf18ee4a219f4bc92af597R17
i suggest aligning with main, and I’ll review again afterward.

Copy link
Collaborator

@yuvalyaron yuvalyaron left a comment

Choose a reason for hiding this comment

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

LGTM other than the comments

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

This PR conflicts with another PR that was merged yesterday: https://github.com/microsoft/AzureTRE/pull/4255/files#diff-6d888c37158c152c3302343686aa77e915107e9de3bf18ee4a219f4bc92af597R17 i suggest aligning with main, and I’ll review again afterward.

Hi @ShakutaiGit I've merged with main now if you don't mind taking another look. Thanks.

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/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 @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test-destroy-env

Copy link

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

Copy link

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

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test

Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13291859958 (with refid d5333ffb)

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test-destroy-env

Copy link

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

Copy link

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

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test

Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13294544963 (with refid d5333ffb)

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test-destroy-env

Copy link

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

Copy link

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

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 12, 2025

/test cecd4d6

Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13295640940 (with refid d5333ffb)

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 13, 2025

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13295640940 (with refid d5333ffb)

(in response to this comment from @jonnyry)

@marrobi had several runs like this, despite running a /test-destroy-env prior. Is there something in the sub that needs cleaning manually?

╷
│ Error: A resource with the ID "/subscriptions/c3b89fac-8c45-46c4-8b8c-67f835c0c4c1/resourceGroups/rg-***/providers/Microsoft.Storage/storageAccounts/stalimrej***/providers/Microsoft.EventGrid/eventSubscriptions/evgs-airlock-import-rejected-blob-created" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_eventgrid_event_subscription" for more information.
│ 
│   with module.airlock_resources.azurerm_eventgrid_event_subscription.import_rejected_blob_created,
│   on airlock/eventgrid_topics.tf line 461, in resource "azurerm_eventgrid_event_subscription" "import_rejected_blob_created":
│  461: resource "azurerm_eventgrid_event_subscription" "import_rejected_blob_created" ***
│ 
╵
╷
│ Error: A resource with the ID "/subscriptions/c3b89fac-8c45-46c4-8b8c-67f835c0c4c1/resourceGroups/rg-***/providers/Microsoft.Storage/storageAccounts/stalexapp***/providers/Microsoft.EventGrid/eventSubscriptions/evgs-airlock-export-approved-blob-created" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_eventgrid_event_subscription" for more information.
│ 
│   with module.airlock_resources.azurerm_eventgrid_event_subscription.export_approved_blob_created,
│   on airlock/eventgrid_topics.tf line 498, in resource "azurerm_eventgrid_event_subscription" "export_approved_blob_created":
│  498: resource "azurerm_eventgrid_event_subscription" "export_approved_blob_created" ***
│ 
╵

@marrobi
Copy link
Member

marrobi commented Feb 13, 2025

@jonnyry I've tested this, so happy to force it through. In future after a destroy, if it fails, run a destroy, let us know and we can clean the CI subscription up manually for that TRE ID.

@marrobi
Copy link
Member

marrobi commented Feb 13, 2025

/test-force-approve

Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit cecd4d6)

(in response to this comment from @marrobi)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Feb 13, 2025

/test-force-approve

Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit a08e374)

(in response to this comment from @jonnyry)

@jonnyry jonnyry merged commit 0592bcd into microsoft:main Feb 13, 2025
12 checks passed
@jonnyry jonnyry deleted the jr/upstream-main/115-cosmos-public-network branch February 13, 2025 10:38
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.

Defender: Azure Cosmos DB should disable public network access
4 participants