-
Notifications
You must be signed in to change notification settings - Fork 190
[ARO-21136] add network security perimeter to some resources #4396
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
base: master
Are you sure you want to change the base?
Conversation
|
Please rebase pull request. |
|
LGTM |
|
How did you validate these in INT? Do they result in something in the portal we can see? |
I validated it by running the RP deployment in int and then checking the int subscription to see if the NSPs were created and if the resources had been assigned correctly. Which they were. Additionally, the compliance dashboard showed the associated alerts as being resolved. |
|
I think you can just push an empty commit to make this check run. |
| return &arm.Resource{ | ||
| Resource: &armnetwork.NspAssociation{ | ||
| Properties: &armnetwork.NspAssociationProperties{ | ||
| AccessMode: pointerutils.ToPtr(armnetwork.AssociationAccessModeLearning), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: if some day we want to move this to enforce or audit... would it be worth to have it as a func parameter? I like how clean looks right now, so it's not a deal breaker.
| } | ||
| } | ||
|
|
||
| // networkSecurityPerimeterProfile creates a new nsp profile with the hardcoded name `default`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reading the code, this func creates a resource with name nspname/default which is not exactly "hardcoded default".
| cosmosNSPProfile := g.networkSecurityPerimeterProfile("cosmos-nsp") | ||
| cosmosNSPAssociation := g.networkSecurityPerimeterAssociation("cosmos-nsp", "cosmos-nsp-association", "[resourceId('Microsoft.DocumentDB/databaseAccounts', parameters('databaseAccountName'))]") | ||
|
|
||
| rs = append(rs, cosmosNSP, cosmosNSPProfile, cosmosNSPAssociation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to this https://learn.microsoft.com/en-us/azure/private-link/network-security-perimeter-concepts#onboarded-private-link-resources Cosmos DB is not GA, but on "Public Preview"... will it work in all of our regions?
Which issue this PR addresses:
Fixes: https://issues.redhat.com/browse/ARO-21136
What this PR does / why we need it:
Resources affected:
Test plan for issue:
[ ] Deploy and Test in CanaryCanary deployment will be run together with the associated sdp-pipelines and ARO-Pipelines release.