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

Microsoft.Network/networkSecurityGroups #60

Closed
slavizh opened this issue May 19, 2020 · 10 comments
Closed

Microsoft.Network/networkSecurityGroups #60

slavizh opened this issue May 19, 2020 · 10 comments
Assignees
Labels
normalization noise related to normalization of properties

Comments

@slavizh
Copy link

slavizh commented May 19, 2020

Describe the noise

Resource type (i.e. Microsoft.Storage/storageAccounts)
Microsoft.Network/networkSecurityGroups

apiVersion (i.e. 2019-04-01)
2019-11-01

Client (PowerShell, Azure CLI, or API)
PowerShell

Relevant ARM Template code (we only need the resource object for the above resourceType and apiVersion, but if it's easier you can include the entire template

Expected response (i.e. "I expected no noise since the template has not been modified since the resources were deployed)
No change in securityRules proeprty

Current (noisy) response (either include a screenshot of the what-if output, or copy/paste the text)

 ~ Microsoft.Network/networkSecurityGroups/test-vnet-2_Subnet1-nsg [2019-11-01]
    ~ properties.securityRules: [
      - 1:

          name:                                "AllowSameSubnetInBound"
          properties.access:                   "Allow"
          properties.description:              "Allow incoming traffic from within the subnet"
          properties.destinationAddressPrefix: "10.0.1.0/24"
          properties.destinationPortRange:     "*"
          properties.direction:                "Inbound"
          properties.priority:                 4093
          properties.protocol:                 "*"
          properties.sourceAddressPrefix:      "10.0.1.0/24"
          properties.sourcePortRange:          "*"

      - 2:

          name:                                "DenyAzureLoadBalancerInBound"
          properties.access:                   "Deny"
          properties.description:              "Deny incoming traffic from Azure load balancers"
          properties.destinationAddressPrefix: "*"
          properties.destinationPortRange:     "*"
          properties.direction:                "Inbound"
          properties.priority:                 4095
          properties.protocol:                 "*"
          properties.sourceAddressPrefix:      "AzureLoadBalancer"
          properties.sourcePortRange:          "*"

    - 3:

          name:                                "AllowAzureLoadBalancerInBound"
          properties.access:                   "Allow"
          properties.description:              "Allow incoming traffic from Azure load balancers"
          properties.destinationAddressPrefix: "VirtualNetwork"
          properties.destinationPortRange:     "*"
          properties.direction:                "Inbound"
          properties.priority:                 4094
          properties.protocol:                 "*"
          properties.sourceAddressPrefix:      "AzureLoadBalancer"
          properties.sourcePortRange:          "*"

      - 4:

          name:                                "DenyVnetInBound"
          properties.access:                   "Deny"
          properties.description:              "Deny all inbound traffic"
          properties.destinationAddressPrefix: "VirtualNetwork"
          properties.destinationPortRange:     "*"
          properties.direction:                "Inbound"
          properties.priority:                 4096
          properties.protocol:                 "*"
          properties.sourceAddressPrefix:      "VirtualNetwork"
          properties.sourcePortRange:          "*"

      ]


Additional context
Add any other context about the problem here.

@shenglol shenglol added the defaults Noise caused by property default values label May 20, 2020
@shenglol shenglol self-assigned this Jun 4, 2020
@shenglol shenglol added fix in progress The fix for the issue is in progress and removed fix in progress The fix for the issue is in progress labels Jun 18, 2020
@shenglol
Copy link
Collaborator

Hi @slavizh , do you mind checking if those security rules were added by Policy effects? We recently updated the What-If back-end to integrate changes made by Policy effects into What-If results, which should fix this sort of noise and will be deployed soon.

@shenglol shenglol added question Further information is requested and removed defaults Noise caused by property default values labels Jun 18, 2020
@slavizh
Copy link
Author

slavizh commented Jun 19, 2020

@shenglol No, they are not added by policies. This deployment is from test environment where we do not have any policies. The rules are coming from configuration in the template.

@shenglol
Copy link
Collaborator

I see. However I could not reproduce the noise. I've also checked with REST API that properties.securityRules has the correct behavior. Can you share the template you are using?

@shenglol shenglol added investigate 🔍 Further investigation needed and removed question Further information is requested labels Jun 19, 2020
@slavizh
Copy link
Author

slavizh commented Jun 22, 2020

Yes. Send it via private e-mail.

@slavizh
Copy link
Author

slavizh commented Jun 22, 2020

Additionally this noisy also appears:

~ properties.privateEndpointNetworkPolicies:    "Enabled" => "enabled"
        ~ properties.privateLinkServiceNetworkPolicies: "Enabled" => "enabled

@shenglol
Copy link
Collaborator

I was able to reproduce the noise with the template. This turned out to be a known limitation for What-If. The issue is that the securityRules property of NSG are also modeled as a child resource (Microsoft.Network/networkSecurityGroups/securityRules), but What-If only understands the resources that are directly declared in the template.

In an ARM template, if you specify some security rules as child resources of a NSG but don't put them in the NSG's securityRules property, they will show up as What-If noise, because they get merged to the NSG's securityRules property in the NSG's GET responses.

We have a general issue #78 to track this, and I've added this one there.

The privateEndpointNetworkPolicies noise is a casing problem and I'll fix that.

@shenglol shenglol added normalization noise related to normalization of properties and removed investigate 🔍 Further investigation needed labels Jun 22, 2020
@slavizh
Copy link
Author

slavizh commented Jun 23, 2020

Thanks. Yes this is a common practice due to sometimes not being able to put the resources inside the main resource. In those case you add them as separate resource. The cases for that approach are often related solving some ARM template language restrictions or easier way to write code.

@shenglol shenglol added partial fix committed The partial fix for the issue has been checked into an internal repo and is awaiting deployment and removed partial fix committed The partial fix for the issue has been checked into an internal repo and is awaiting deployment labels Aug 10, 2020
@alex-frankel
Copy link
Contributor

@shenglol - Am I right that this is part of the same fix that we did for VNETs? So once w51 is rolled out we can close this?

@shenglol
Copy link
Collaborator

shenglol commented Jan 6, 2021

Not yet. The custom metadata need to be updated to set a flag to enable inline nested resource handling for NSG. We currently only turned it on for VNETs because we want to make sure it doesn't break things. Since it's been working without big issues I think we can enable it for more resource types now. This should be fixed with this week's metadata regeneration.

@alex-frankel alex-frankel assigned Xynoclafe and unassigned shenglol Feb 4, 2021
@Xynoclafe
Copy link

The fix for this should be available. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normalization noise related to normalization of properties
Projects
None yet
Development

No branches or pull requests

4 participants