Sitelet https://github.com/kubernetes/kops/pull/18523/files
Skip to content

Automated cherry pick of #18522: azure: Scope nodes-to-API NSG rules to the NAT gateway public IP #18523

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.

Sign up for GitHub

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

Merged
kubernetes-prow merged 1 commit into from
Jul 2, 2026
Merged

Automated cherry pick of #18522: azure: Scope nodes-to-API NSG rules to the NAT gateway public IP #18523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions pkg/model/azuremodel/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
}
c.AddTask(networkTask)

ngwPipTask := &azuretasks.PublicIPAddress{
Name: fi.PtrTo(b.NameForVirtualNetwork()),
Lifecycle: b.Lifecycle,
ResourceGroup: b.LinkToResourceGroup(),
IPVersion: network.IPVersionIPv4,
AllocationMethod: network.IPAllocationMethodStatic,
SKU: network.PublicIPAddressSKUNameStandard,
Tags: map[string]*string{},
}
c.AddTask(ngwPipTask)

nsgTask := &azuretasks.NetworkSecurityGroup{
Name: fi.PtrTo(b.Cluster.AzureNetworkSecurityGroupName()),
Lifecycle: b.Lifecycle,
Expand Down Expand Up @@ -237,14 +248,15 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
DestinationPortRange: fi.PtrTo("*"),
})
if b.Cluster.UsesLoadBalancerForKopsController() && b.Cluster.Spec.API.LoadBalancer != nil && b.Cluster.Spec.API.LoadBalancer.Type == kops.LoadBalancerTypePublic {
// TODO: Limit access to necessary source address prefixes instead of "0.0.0.0/0" and "::/0"
// Node traffic to the public load balancer frontend egresses through the NAT gateway, so it
// arrives with the NAT gateway public IP as its source and cannot be matched by the nodes ASG.
nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{
Name: fi.PtrTo("AllowNodesToKubernetesAPI"),
Priority: fi.PtrTo[int32](2000),
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
Protocol: network.SecurityRuleProtocolTCP,
SourceAddressPrefix: fi.PtrTo("*"),
SourcePublicIPAddress: ngwPipTask,
SourcePortRange: fi.PtrTo("*"),
DestinationApplicationSecurityGroupNames: []*string{fi.PtrTo(b.NameForApplicationSecurityGroupControlPlane())},
DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KubeAPIServer)),
Expand All @@ -255,7 +267,7 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
Protocol: network.SecurityRuleProtocolTCP,
SourceAddressPrefix: fi.PtrTo("*"),
SourcePublicIPAddress: ngwPipTask,
SourcePortRange: fi.PtrTo("*"),
DestinationApplicationSecurityGroupNames: []*string{fi.PtrTo(b.NameForApplicationSecurityGroupControlPlane())},
DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)),
Expand Down Expand Up @@ -296,16 +308,6 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
})
c.AddTask(nsgTask)

ngwPipTask := &azuretasks.PublicIPAddress{
Name: fi.PtrTo(b.NameForVirtualNetwork()),
Lifecycle: b.Lifecycle,
ResourceGroup: b.LinkToResourceGroup(),
IPVersion: network.IPVersionIPv4,
AllocationMethod: network.IPAllocationMethodStatic,
SKU: network.PublicIPAddressSKUNameStandard,
Tags: map[string]*string{},
}
c.AddTask(ngwPipTask)
ngwTask := &azuretasks.NatGateway{
Name: fi.PtrTo(b.NameForVirtualNetwork()),
Lifecycle: b.Lifecycle,
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/update_cluster/gossip-azure/kubernetes.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ resource "azurerm_network_security_group" "gossip-k8s-local" {
name = "AllowNodesToKubernetesAPI"
priority = 2000
protocol = "Tcp"
source_address_prefix = "*"
source_address_prefix = azurerm_public_ip.gossip-k8s-local.ip_address
source_port_range = "*"
}
security_rule {
Expand All @@ -396,7 +396,7 @@ resource "azurerm_network_security_group" "gossip-k8s-local" {
name = "AllowNodesToKopsController"
priority = 2001
protocol = "Tcp"
source_address_prefix = "*"
source_address_prefix = azurerm_public_ip.gossip-k8s-local.ip_address
source_port_range = "*"
}
security_rule {
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/update_cluster/minimal_azure/kubernetes.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ resource "azurerm_network_security_group" "minimal-azure-example-com" {
name = "AllowNodesToKubernetesAPI"
priority = 2000
protocol = "Tcp"
source_address_prefix = "*"
source_address_prefix = azurerm_public_ip.minimal-azure-example-com.ip_address
source_port_range = "*"
}
security_rule {
Expand All @@ -396,7 +396,7 @@ resource "azurerm_network_security_group" "minimal-azure-example-com" {
name = "AllowNodesToKopsController"
priority = 2001
protocol = "Tcp"
source_address_prefix = "*"
source_address_prefix = azurerm_public_ip.minimal-azure-example-com.ip_address
source_port_range = "*"
}
security_rule {
Expand Down
30 changes: 29 additions & 1 deletion upup/pkg/fi/cloudup/azuretasks/networksecuritygroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package azuretasks

import (
"context"
"fmt"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
Expand Down Expand Up @@ -89,6 +90,12 @@ func (nsg *NetworkSecurityGroup) Find(c *fi.CloudupContext) (*NetworkSecurityGro
// ApplicationSecurityGroups is for dependency ordering only and is not rendered to the cloud.
ApplicationSecurityGroups: nsg.ApplicationSecurityGroups,
}
expectedRules := make(map[string]*NetworkSecurityRule)
for _, rule := range nsg.SecurityRules {
if rule.Name != nil {
expectedRules[*rule.Name] = rule
}
}
for _, rule := range found.Properties.SecurityRules {
nsr := &NetworkSecurityRule{
Name: rule.Name,
Expand All @@ -101,6 +108,14 @@ func (nsg *NetworkSecurityGroup) Find(c *fi.CloudupContext) (*NetworkSecurityGro
DestinationAddressPrefix: rule.Properties.DestinationAddressPrefix,
DestinationPortRange: rule.Properties.DestinationPortRange,
}
// Map the source address back to the referenced public IP so unchanged rules compare as equal.
if expected := expectedRules[fi.ValueOf(nsr.Name)]; expected != nil && expected.SourcePublicIPAddress != nil {
pipAddress := expected.SourcePublicIPAddress.IPAddress
if pipAddress != nil && nsr.SourceAddressPrefix != nil && *nsr.SourceAddressPrefix == *pipAddress {
nsr.SourcePublicIPAddress = expected.SourcePublicIPAddress
nsr.SourceAddressPrefix = nil
}
}
if len(rule.Properties.SourceAddressPrefixes) > 0 {
nsr.SourceAddressPrefixes = rule.Properties.SourceAddressPrefixes
}
Expand Down Expand Up @@ -181,14 +196,21 @@ func (*NetworkSecurityGroup) RenderAzure(t *azure.AzureAPITarget, a, e, changes
Tags: e.Tags,
}
for _, nsr := range e.SecurityRules {
sourceAddressPrefix := nsr.SourceAddressPrefix
if nsr.SourcePublicIPAddress != nil {
if nsr.SourcePublicIPAddress.IPAddress == nil {
return fmt.Errorf("public IP address %q referenced by security rule %q does not have an allocated address", fi.ValueOf(nsr.SourcePublicIPAddress.Name), fi.ValueOf(nsr.Name))
}
sourceAddressPrefix = nsr.SourcePublicIPAddress.IPAddress
}
securityRule := network.SecurityRule{
Name: nsr.Name,
Properties: &network.SecurityRulePropertiesFormat{
Priority: nsr.Priority,
Access: &nsr.Access,
Direction: &nsr.Direction,
Protocol: &nsr.Protocol,
SourceAddressPrefix: nsr.SourceAddressPrefix,
SourceAddressPrefix: sourceAddressPrefix,
SourceAddressPrefixes: nsr.SourceAddressPrefixes,
SourcePortRange: nsr.SourcePortRange,
DestinationAddressPrefix: nsr.DestinationAddressPrefix,
Expand Down Expand Up @@ -254,10 +276,16 @@ type NetworkSecurityRule struct {
DestinationAddressPrefix *string
DestinationApplicationSecurityGroupNames []*string
DestinationPortRange *string

// SourcePublicIPAddress restricts the rule source to the referenced public IP's allocated address.
SourcePublicIPAddress *PublicIPAddress
}

var _ fi.CloudupHasDependencies = (*NetworkSecurityRule)(nil)

func (e *NetworkSecurityRule) GetDependencies(tasks map[string]fi.CloudupTask) []fi.CloudupTask {
if e.SourcePublicIPAddress != nil {
return []fi.CloudupTask{e.SourcePublicIPAddress}
}
return nil
}
10 changes: 8 additions & 2 deletions upup/pkg/fi/cloudup/azuretasks/networksecuritygroup_terraform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type terraformAzureNetworkSecurityRule struct {
Access *string `cty:"access"`
Direction *string `cty:"direction"`
Protocol *string `cty:"protocol"`
SourceAddressPrefix *string `cty:"source_address_prefix"`
SourceAddressPrefix *terraformWriter.Literal `cty:"source_address_prefix"`
SourceAddressPrefixes []string `cty:"source_address_prefixes"`
SourceApplicationSecurityGroupIDs []*terraformWriter.Literal `cty:"source_application_security_group_ids"`
SourcePortRange *string `cty:"source_port_range"`
Expand Down Expand Up @@ -69,13 +69,19 @@ func (rule *NetworkSecurityRule) toTerraform() *terraformAzureNetworkSecurityRul
access := string(rule.Access)
direction := string(rule.Direction)
protocol := string(rule.Protocol)
var sourceAddressPrefix *terraformWriter.Literal
if rule.SourcePublicIPAddress != nil {
sourceAddressPrefix = terraformWriter.LiteralProperty("azurerm_public_ip", fi.ValueOf(rule.SourcePublicIPAddress.Name), "ip_address")
} else if rule.SourceAddressPrefix != nil {
sourceAddressPrefix = terraformWriter.LiteralFromStringValue(*rule.SourceAddressPrefix)
}
return &terraformAzureNetworkSecurityRule{
Name: rule.Name,
Priority: rule.Priority,
Access: &access,
Direction: &direction,
Protocol: &protocol,
SourceAddressPrefix: rule.SourceAddressPrefix,
SourceAddressPrefix: sourceAddressPrefix,
SourceAddressPrefixes: stringSlice(rule.SourceAddressPrefixes),
SourceApplicationSecurityGroupIDs: applicationSecurityGroupNameIDs(rule.SourceApplicationSecurityGroupNames),
SourcePortRange: rule.SourcePortRange,
Expand Down
184 changes: 184 additions & 0 deletions upup/pkg/fi/cloudup/azuretasks/networksecuritygroup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
/*
Copyright 2026 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package azuretasks

import (
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
network "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/azure"
)

func newTestNetworkSecurityGroup(natGatewayPip *PublicIPAddress) *NetworkSecurityGroup {
return &NetworkSecurityGroup{
Name: to.Ptr("nsg"),
Lifecycle: fi.LifecycleSync,
ResourceGroup: &ResourceGroup{
Name: to.Ptr("rg"),
},
SecurityRules: []*NetworkSecurityRule{
{
Name: to.Ptr("AllowSSH"),
Priority: to.Ptr[int32](100),
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
Protocol: network.SecurityRuleProtocolTCP,
SourceAddressPrefix: to.Ptr("*"),
SourcePortRange: to.Ptr("*"),
DestinationAddressPrefix: to.Ptr("*"),
DestinationPortRange: to.Ptr("22"),
},
{
Name: to.Ptr("AllowNodesToKubernetesAPI"),
Priority: to.Ptr[int32](2000),
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
Protocol: network.SecurityRuleProtocolTCP,
SourcePublicIPAddress: natGatewayPip,
SourcePortRange: to.Ptr("*"),
DestinationAddressPrefix: to.Ptr("*"),
DestinationPortRange: to.Ptr("443"),
},
},
Tags: map[string]*string{
testTagKey: to.Ptr(testTagValue),
},
}
}

func TestNetworkSecurityGroupRenderAzure(t *testing.T) {
cloud := NewMockAzureCloud("eastus")
apiTarget := azure.NewAzureAPITarget(cloud)
nsg := &NetworkSecurityGroup{}
expected := newTestNetworkSecurityGroup(&PublicIPAddress{
Name: to.Ptr("natgw"),
IPAddress: to.Ptr("192.0.2.1"),
})
if err := nsg.RenderAzure(apiTarget, nil, expected, nil); err != nil {
t.Fatalf("unexpected error: %s", err)
}

actual := cloud.NetworkSecurityGroupsClient.NSGs[*expected.Name]
if a, e := *actual.Name, *expected.Name; a != e {
t.Errorf("unexpected Name: expected %s, but got %s", e, a)
}
if a, e := *actual.Properties.SecurityRules[0].Properties.SourceAddressPrefix, "*"; a != e {
t.Errorf("unexpected SourceAddressPrefix: expected %s, but got %s", e, a)
}
if a, e := *actual.Properties.SecurityRules[1].Properties.SourceAddressPrefix, "192.0.2.1"; a != e {
t.Errorf("unexpected SourceAddressPrefix: expected %s, but got %s", e, a)
}
}

func TestNetworkSecurityGroupRenderAzureUnallocatedPublicIPAddress(t *testing.T) {
cloud := NewMockAzureCloud("eastus")
apiTarget := azure.NewAzureAPITarget(cloud)
nsg := &NetworkSecurityGroup{}
expected := newTestNetworkSecurityGroup(&PublicIPAddress{
Name: to.Ptr("natgw"),
})
if err := nsg.RenderAzure(apiTarget, nil, expected, nil); err == nil {
t.Fatalf("expected error rendering a rule whose public IP has no allocated address")
}
}

func TestNetworkSecurityGroupFind(t *testing.T) {
cloud := NewMockAzureCloud("eastus")
ctx := &fi.CloudupContext{
T: fi.CloudupSubContext{
Cloud: cloud,
},
}

natGatewayPip := &PublicIPAddress{
Name: to.Ptr("natgw"),
IPAddress: to.Ptr("192.0.2.1"),
}
nsg := newTestNetworkSecurityGroup(natGatewayPip)
nsg.SecurityRules = append(nsg.SecurityRules, &NetworkSecurityRule{
Name: to.Ptr("AllowNodesToKopsController"),
Priority: to.Ptr[int32](2001),
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
Protocol: network.SecurityRuleProtocolTCP,
SourcePublicIPAddress: natGatewayPip,
SourcePortRange: to.Ptr("*"),
DestinationAddressPrefix: to.Ptr("*"),
DestinationPortRange: to.Ptr("3988"),
})
// Find will return nothing if there is no network security group created.
actual, err := nsg.Find(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if actual != nil {
t.Errorf("unexpected networkSecurityGroup found: %+v", actual)
}

cloud.NetworkSecurityGroupsClient.NSGs[*nsg.Name] = &network.SecurityGroup{
Name: nsg.Name,
ID: to.Ptr("id"),
Properties: &network.SecurityGroupPropertiesFormat{
SecurityRules: []*network.SecurityRule{
{
Name: to.Ptr("AllowNodesToKubernetesAPI"),
Properties: &network.SecurityRulePropertiesFormat{
Priority: to.Ptr[int32](2000),
Access: to.Ptr(network.SecurityRuleAccessAllow),
Direction: to.Ptr(network.SecurityRuleDirectionInbound),
Protocol: to.Ptr(network.SecurityRuleProtocolTCP),
SourceAddressPrefix: to.Ptr("192.0.2.1"),
SourcePortRange: to.Ptr("*"),
DestinationAddressPrefix: to.Ptr("*"),
DestinationPortRange: to.Ptr("443"),
},
},
{
Name: to.Ptr("AllowNodesToKopsController"),
Properties: &network.SecurityRulePropertiesFormat{
Priority: to.Ptr[int32](2001),
Access: to.Ptr(network.SecurityRuleAccessAllow),
Direction: to.Ptr(network.SecurityRuleDirectionInbound),
Protocol: to.Ptr(network.SecurityRuleProtocolTCP),
SourceAddressPrefix: to.Ptr("*"),
SourcePortRange: to.Ptr("*"),
DestinationAddressPrefix: to.Ptr("*"),
DestinationPortRange: to.Ptr("3988"),
},
},
},
},
}
// Find again.
actual, err = nsg.Find(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if a, e := *actual.Name, *nsg.Name; a != e {
t.Errorf("unexpected networkSecurityGroup name: expected %s, but got %s", e, a)
}
// A source matching the referenced public IP maps back to the task reference.
if a := actual.SecurityRules[0]; a.SourcePublicIPAddress != natGatewayPip || a.SourceAddressPrefix != nil {
t.Errorf("expected rule source mapped to the referenced public IP, but got %+v", a)
}
// A non-matching source stays literal, so pre-existing wildcard rules show up as a change.
if a := actual.SecurityRules[1]; a.SourcePublicIPAddress != nil || fi.ValueOf(a.SourceAddressPrefix) != "*" {
t.Errorf("expected rule source to stay literal, but got %+v", a)
}
}
Loading
Loading