Skip to content

Commit 4766ce5

Browse files
Merge pull request #18522 from hakman/azure-scope-api-access-v3
azure: Scope nodes-to-API NSG rules to the NAT gateway public IP
2 parents 568d0e0 + 9a1948d commit 4766ce5

8 files changed

Lines changed: 251 additions & 20 deletions

File tree

pkg/model/azuremodel/network.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
4747
}
4848
c.AddTask(networkTask)
4949

50+
ngwPipTask := &azuretasks.PublicIPAddress{
51+
Name: fi.PtrTo(b.NameForVirtualNetwork()),
52+
Lifecycle: b.Lifecycle,
53+
ResourceGroup: b.LinkToResourceGroup(),
54+
IPVersion: network.IPVersionIPv4,
55+
AllocationMethod: network.IPAllocationMethodStatic,
56+
SKU: network.PublicIPAddressSKUNameStandard,
57+
Tags: map[string]*string{},
58+
}
59+
c.AddTask(ngwPipTask)
60+
5061
nsgTask := &azuretasks.NetworkSecurityGroup{
5162
Name: fi.PtrTo(b.Cluster.AzureNetworkSecurityGroupName()),
5263
Lifecycle: b.Lifecycle,
@@ -237,14 +248,15 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
237248
DestinationPortRange: fi.PtrTo("*"),
238249
})
239250
if b.Cluster.UsesLoadBalancerForKopsController() && b.Cluster.Spec.API.LoadBalancer != nil && b.Cluster.Spec.API.LoadBalancer.Type == kops.LoadBalancerTypePublic {
240-
// TODO: Limit access to necessary source address prefixes instead of "0.0.0.0/0" and "::/0"
251+
// Node traffic to the public load balancer frontend egresses through the NAT gateway, so it
252+
// arrives with the NAT gateway public IP as its source and cannot be matched by the nodes ASG.
241253
nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{
242254
Name: fi.PtrTo("AllowNodesToKubernetesAPI"),
243255
Priority: fi.PtrTo[int32](2000),
244256
Access: network.SecurityRuleAccessAllow,
245257
Direction: network.SecurityRuleDirectionInbound,
246258
Protocol: network.SecurityRuleProtocolTCP,
247-
SourceAddressPrefix: fi.PtrTo("*"),
259+
SourcePublicIPAddress: ngwPipTask,
248260
SourcePortRange: fi.PtrTo("*"),
249261
DestinationApplicationSecurityGroupNames: []*string{fi.PtrTo(b.NameForApplicationSecurityGroupControlPlane())},
250262
DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KubeAPIServer)),
@@ -255,7 +267,7 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
255267
Access: network.SecurityRuleAccessAllow,
256268
Direction: network.SecurityRuleDirectionInbound,
257269
Protocol: network.SecurityRuleProtocolTCP,
258-
SourceAddressPrefix: fi.PtrTo("*"),
270+
SourcePublicIPAddress: ngwPipTask,
259271
SourcePortRange: fi.PtrTo("*"),
260272
DestinationApplicationSecurityGroupNames: []*string{fi.PtrTo(b.NameForApplicationSecurityGroupControlPlane())},
261273
DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)),
@@ -296,16 +308,6 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
296308
})
297309
c.AddTask(nsgTask)
298310

299-
ngwPipTask := &azuretasks.PublicIPAddress{
300-
Name: fi.PtrTo(b.NameForVirtualNetwork()),
301-
Lifecycle: b.Lifecycle,
302-
ResourceGroup: b.LinkToResourceGroup(),
303-
IPVersion: network.IPVersionIPv4,
304-
AllocationMethod: network.IPAllocationMethodStatic,
305-
SKU: network.PublicIPAddressSKUNameStandard,
306-
Tags: map[string]*string{},
307-
}
308-
c.AddTask(ngwPipTask)
309311
ngwTask := &azuretasks.NatGateway{
310312
Name: fi.PtrTo(b.NameForVirtualNetwork()),
311313
Lifecycle: b.Lifecycle,

tests/integration/update_cluster/gossip-azure/kubernetes.tf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ resource "azurerm_network_security_group" "gossip-k8s-local" {
385385
name = "AllowNodesToKubernetesAPI"
386386
priority = 2000
387387
protocol = "Tcp"
388-
source_address_prefix = "*"
388+
source_address_prefix = azurerm_public_ip.gossip-k8s-local.ip_address
389389
source_port_range = "*"
390390
}
391391
security_rule {
@@ -396,7 +396,7 @@ resource "azurerm_network_security_group" "gossip-k8s-local" {
396396
name = "AllowNodesToKopsController"
397397
priority = 2001
398398
protocol = "Tcp"
399-
source_address_prefix = "*"
399+
source_address_prefix = azurerm_public_ip.gossip-k8s-local.ip_address
400400
source_port_range = "*"
401401
}
402402
security_rule {

tests/integration/update_cluster/minimal_azure/kubernetes.tf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ resource "azurerm_network_security_group" "minimal-azure-example-com" {
385385
name = "AllowNodesToKubernetesAPI"
386386
priority = 2000
387387
protocol = "Tcp"
388-
source_address_prefix = "*"
388+
source_address_prefix = azurerm_public_ip.minimal-azure-example-com.ip_address
389389
source_port_range = "*"
390390
}
391391
security_rule {
@@ -396,7 +396,7 @@ resource "azurerm_network_security_group" "minimal-azure-example-com" {
396396
name = "AllowNodesToKopsController"
397397
priority = 2001
398398
protocol = "Tcp"
399-
source_address_prefix = "*"
399+
source_address_prefix = azurerm_public_ip.minimal-azure-example-com.ip_address
400400
source_port_range = "*"
401401
}
402402
security_rule {

upup/pkg/fi/cloudup/azuretasks/networksecuritygroup.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package azuretasks
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"strings"
2223

2324
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
@@ -89,6 +90,12 @@ func (nsg *NetworkSecurityGroup) Find(c *fi.CloudupContext) (*NetworkSecurityGro
8990
// ApplicationSecurityGroups is for dependency ordering only and is not rendered to the cloud.
9091
ApplicationSecurityGroups: nsg.ApplicationSecurityGroups,
9192
}
93+
expectedRules := make(map[string]*NetworkSecurityRule)
94+
for _, rule := range nsg.SecurityRules {
95+
if rule.Name != nil {
96+
expectedRules[*rule.Name] = rule
97+
}
98+
}
9299
for _, rule := range found.Properties.SecurityRules {
93100
nsr := &NetworkSecurityRule{
94101
Name: rule.Name,
@@ -101,6 +108,14 @@ func (nsg *NetworkSecurityGroup) Find(c *fi.CloudupContext) (*NetworkSecurityGro
101108
DestinationAddressPrefix: rule.Properties.DestinationAddressPrefix,
102109
DestinationPortRange: rule.Properties.DestinationPortRange,
103110
}
111+
// Map the source address back to the referenced public IP so unchanged rules compare as equal.
112+
if expected := expectedRules[fi.ValueOf(nsr.Name)]; expected != nil && expected.SourcePublicIPAddress != nil {
113+
pipAddress := expected.SourcePublicIPAddress.IPAddress
114+
if pipAddress != nil && nsr.SourceAddressPrefix != nil && *nsr.SourceAddressPrefix == *pipAddress {
115+
nsr.SourcePublicIPAddress = expected.SourcePublicIPAddress
116+
nsr.SourceAddressPrefix = nil
117+
}
118+
}
104119
if len(rule.Properties.SourceAddressPrefixes) > 0 {
105120
nsr.SourceAddressPrefixes = rule.Properties.SourceAddressPrefixes
106121
}
@@ -181,14 +196,21 @@ func (*NetworkSecurityGroup) RenderAzure(t *azure.AzureAPITarget, a, e, changes
181196
Tags: e.Tags,
182197
}
183198
for _, nsr := range e.SecurityRules {
199+
sourceAddressPrefix := nsr.SourceAddressPrefix
200+
if nsr.SourcePublicIPAddress != nil {
201+
if nsr.SourcePublicIPAddress.IPAddress == nil {
202+
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))
203+
}
204+
sourceAddressPrefix = nsr.SourcePublicIPAddress.IPAddress
205+
}
184206
securityRule := network.SecurityRule{
185207
Name: nsr.Name,
186208
Properties: &network.SecurityRulePropertiesFormat{
187209
Priority: nsr.Priority,
188210
Access: &nsr.Access,
189211
Direction: &nsr.Direction,
190212
Protocol: &nsr.Protocol,
191-
SourceAddressPrefix: nsr.SourceAddressPrefix,
213+
SourceAddressPrefix: sourceAddressPrefix,
192214
SourceAddressPrefixes: nsr.SourceAddressPrefixes,
193215
SourcePortRange: nsr.SourcePortRange,
194216
DestinationAddressPrefix: nsr.DestinationAddressPrefix,
@@ -254,10 +276,16 @@ type NetworkSecurityRule struct {
254276
DestinationAddressPrefix *string
255277
DestinationApplicationSecurityGroupNames []*string
256278
DestinationPortRange *string
279+
280+
// SourcePublicIPAddress restricts the rule source to the referenced public IP's allocated address.
281+
SourcePublicIPAddress *PublicIPAddress
257282
}
258283

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

261286
func (e *NetworkSecurityRule) GetDependencies(tasks map[string]fi.CloudupTask) []fi.CloudupTask {
287+
if e.SourcePublicIPAddress != nil {
288+
return []fi.CloudupTask{e.SourcePublicIPAddress}
289+
}
262290
return nil
263291
}

upup/pkg/fi/cloudup/azuretasks/networksecuritygroup_terraform.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type terraformAzureNetworkSecurityRule struct {
2828
Access *string `cty:"access"`
2929
Direction *string `cty:"direction"`
3030
Protocol *string `cty:"protocol"`
31-
SourceAddressPrefix *string `cty:"source_address_prefix"`
31+
SourceAddressPrefix *terraformWriter.Literal `cty:"source_address_prefix"`
3232
SourceAddressPrefixes []string `cty:"source_address_prefixes"`
3333
SourceApplicationSecurityGroupIDs []*terraformWriter.Literal `cty:"source_application_security_group_ids"`
3434
SourcePortRange *string `cty:"source_port_range"`
@@ -69,13 +69,19 @@ func (rule *NetworkSecurityRule) toTerraform() *terraformAzureNetworkSecurityRul
6969
access := string(rule.Access)
7070
direction := string(rule.Direction)
7171
protocol := string(rule.Protocol)
72+
var sourceAddressPrefix *terraformWriter.Literal
73+
if rule.SourcePublicIPAddress != nil {
74+
sourceAddressPrefix = terraformWriter.LiteralProperty("azurerm_public_ip", fi.ValueOf(rule.SourcePublicIPAddress.Name), "ip_address")
75+
} else if rule.SourceAddressPrefix != nil {
76+
sourceAddressPrefix = terraformWriter.LiteralFromStringValue(*rule.SourceAddressPrefix)
77+
}
7278
return &terraformAzureNetworkSecurityRule{
7379
Name: rule.Name,
7480
Priority: rule.Priority,
7581
Access: &access,
7682
Direction: &direction,
7783
Protocol: &protocol,
78-
SourceAddressPrefix: rule.SourceAddressPrefix,
84+
SourceAddressPrefix: sourceAddressPrefix,
7985
SourceAddressPrefixes: stringSlice(rule.SourceAddressPrefixes),
8086
SourceApplicationSecurityGroupIDs: applicationSecurityGroupNameIDs(rule.SourceApplicationSecurityGroupNames),
8187
SourcePortRange: rule.SourcePortRange,
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
Copyright 2026 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package azuretasks
18+
19+
import (
20+
"testing"
21+
22+
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
23+
network "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"
24+
"k8s.io/kops/upup/pkg/fi"
25+
"k8s.io/kops/upup/pkg/fi/cloudup/azure"
26+
)
27+
28+
func newTestNetworkSecurityGroup(natGatewayPip *PublicIPAddress) *NetworkSecurityGroup {
29+
return &NetworkSecurityGroup{
30+
Name: to.Ptr("nsg"),
31+
Lifecycle: fi.LifecycleSync,
32+
ResourceGroup: &ResourceGroup{
33+
Name: to.Ptr("rg"),
34+
},
35+
SecurityRules: []*NetworkSecurityRule{
36+
{
37+
Name: to.Ptr("AllowSSH"),
38+
Priority: to.Ptr[int32](100),
39+
Access: network.SecurityRuleAccessAllow,
40+
Direction: network.SecurityRuleDirectionInbound,
41+
Protocol: network.SecurityRuleProtocolTCP,
42+
SourceAddressPrefix: to.Ptr("*"),
43+
SourcePortRange: to.Ptr("*"),
44+
DestinationAddressPrefix: to.Ptr("*"),
45+
DestinationPortRange: to.Ptr("22"),
46+
},
47+
{
48+
Name: to.Ptr("AllowNodesToKubernetesAPI"),
49+
Priority: to.Ptr[int32](2000),
50+
Access: network.SecurityRuleAccessAllow,
51+
Direction: network.SecurityRuleDirectionInbound,
52+
Protocol: network.SecurityRuleProtocolTCP,
53+
SourcePublicIPAddress: natGatewayPip,
54+
SourcePortRange: to.Ptr("*"),
55+
DestinationAddressPrefix: to.Ptr("*"),
56+
DestinationPortRange: to.Ptr("443"),
57+
},
58+
},
59+
Tags: map[string]*string{
60+
testTagKey: to.Ptr(testTagValue),
61+
},
62+
}
63+
}
64+
65+
func TestNetworkSecurityGroupRenderAzure(t *testing.T) {
66+
cloud := NewMockAzureCloud("eastus")
67+
apiTarget := azure.NewAzureAPITarget(cloud)
68+
nsg := &NetworkSecurityGroup{}
69+
expected := newTestNetworkSecurityGroup(&PublicIPAddress{
70+
Name: to.Ptr("natgw"),
71+
IPAddress: to.Ptr("192.0.2.1"),
72+
})
73+
if err := nsg.RenderAzure(apiTarget, nil, expected, nil); err != nil {
74+
t.Fatalf("unexpected error: %s", err)
75+
}
76+
77+
actual := cloud.NetworkSecurityGroupsClient.NSGs[*expected.Name]
78+
if a, e := *actual.Name, *expected.Name; a != e {
79+
t.Errorf("unexpected Name: expected %s, but got %s", e, a)
80+
}
81+
if a, e := *actual.Properties.SecurityRules[0].Properties.SourceAddressPrefix, "*"; a != e {
82+
t.Errorf("unexpected SourceAddressPrefix: expected %s, but got %s", e, a)
83+
}
84+
if a, e := *actual.Properties.SecurityRules[1].Properties.SourceAddressPrefix, "192.0.2.1"; a != e {
85+
t.Errorf("unexpected SourceAddressPrefix: expected %s, but got %s", e, a)
86+
}
87+
}
88+
89+
func TestNetworkSecurityGroupRenderAzureUnallocatedPublicIPAddress(t *testing.T) {
90+
cloud := NewMockAzureCloud("eastus")
91+
apiTarget := azure.NewAzureAPITarget(cloud)
92+
nsg := &NetworkSecurityGroup{}
93+
expected := newTestNetworkSecurityGroup(&PublicIPAddress{
94+
Name: to.Ptr("natgw"),
95+
})
96+
if err := nsg.RenderAzure(apiTarget, nil, expected, nil); err == nil {
97+
t.Fatalf("expected error rendering a rule whose public IP has no allocated address")
98+
}
99+
}
100+
101+
func TestNetworkSecurityGroupFind(t *testing.T) {
102+
cloud := NewMockAzureCloud("eastus")
103+
ctx := &fi.CloudupContext{
104+
T: fi.CloudupSubContext{
105+
Cloud: cloud,
106+
},
107+
}
108+
109+
natGatewayPip := &PublicIPAddress{
110+
Name: to.Ptr("natgw"),
111+
IPAddress: to.Ptr("192.0.2.1"),
112+
}
113+
nsg := newTestNetworkSecurityGroup(natGatewayPip)
114+
nsg.SecurityRules = append(nsg.SecurityRules, &NetworkSecurityRule{
115+
Name: to.Ptr("AllowNodesToKopsController"),
116+
Priority: to.Ptr[int32](2001),
117+
Access: network.SecurityRuleAccessAllow,
118+
Direction: network.SecurityRuleDirectionInbound,
119+
Protocol: network.SecurityRuleProtocolTCP,
120+
SourcePublicIPAddress: natGatewayPip,
121+
SourcePortRange: to.Ptr("*"),
122+
DestinationAddressPrefix: to.Ptr("*"),
123+
DestinationPortRange: to.Ptr("3988"),
124+
})
125+
// Find will return nothing if there is no network security group created.
126+
actual, err := nsg.Find(ctx)
127+
if err != nil {
128+
t.Fatalf("unexpected error: %s", err)
129+
}
130+
if actual != nil {
131+
t.Errorf("unexpected networkSecurityGroup found: %+v", actual)
132+
}
133+
134+
cloud.NetworkSecurityGroupsClient.NSGs[*nsg.Name] = &network.SecurityGroup{
135+
Name: nsg.Name,
136+
ID: to.Ptr("id"),
137+
Properties: &network.SecurityGroupPropertiesFormat{
138+
SecurityRules: []*network.SecurityRule{
139+
{
140+
Name: to.Ptr("AllowNodesToKubernetesAPI"),
141+
Properties: &network.SecurityRulePropertiesFormat{
142+
Priority: to.Ptr[int32](2000),
143+
Access: to.Ptr(network.SecurityRuleAccessAllow),
144+
Direction: to.Ptr(network.SecurityRuleDirectionInbound),
145+
Protocol: to.Ptr(network.SecurityRuleProtocolTCP),
146+
SourceAddressPrefix: to.Ptr("192.0.2.1"),
147+
SourcePortRange: to.Ptr("*"),
148+
DestinationAddressPrefix: to.Ptr("*"),
149+
DestinationPortRange: to.Ptr("443"),
150+
},
151+
},
152+
{
153+
Name: to.Ptr("AllowNodesToKopsController"),
154+
Properties: &network.SecurityRulePropertiesFormat{
155+
Priority: to.Ptr[int32](2001),
156+
Access: to.Ptr(network.SecurityRuleAccessAllow),
157+
Direction: to.Ptr(network.SecurityRuleDirectionInbound),
158+
Protocol: to.Ptr(network.SecurityRuleProtocolTCP),
159+
SourceAddressPrefix: to.Ptr("*"),
160+
SourcePortRange: to.Ptr("*"),
161+
DestinationAddressPrefix: to.Ptr("*"),
162+
DestinationPortRange: to.Ptr("3988"),
163+
},
164+
},
165+
},
166+
},
167+
}
168+
// Find again.
169+
actual, err = nsg.Find(ctx)
170+
if err != nil {
171+
t.Fatalf("unexpected error: %s", err)
172+
}
173+
if a, e := *actual.Name, *nsg.Name; a != e {
174+
t.Errorf("unexpected networkSecurityGroup name: expected %s, but got %s", e, a)
175+
}
176+
// A source matching the referenced public IP maps back to the task reference.
177+
if a := actual.SecurityRules[0]; a.SourcePublicIPAddress != natGatewayPip || a.SourceAddressPrefix != nil {
178+
t.Errorf("expected rule source mapped to the referenced public IP, but got %+v", a)
179+
}
180+
// A non-matching source stays literal, so pre-existing wildcard rules show up as a change.
181+
if a := actual.SecurityRules[1]; a.SourcePublicIPAddress != nil || fi.ValueOf(a.SourceAddressPrefix) != "*" {
182+
t.Errorf("expected rule source to stay literal, but got %+v", a)
183+
}
184+
}

0 commit comments

Comments
 (0)