Skip to content

Commit 9488551

Browse files
zohar7chzohar7ch
authored
[Prep for] Auto allow webhook traffic - restrict traffic from control plane IP based on configuration & support IPv6 for control plane (#596)
1 parent 8ab57d1 commit 9488551

File tree

5 files changed

+148
-57
lines changed

5 files changed

+148
-57
lines changed

‎src/operator/controllers/webhook_traffic/network_policy_handler.go‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"k8s.io/apimachinery/pkg/runtime"
2121
"k8s.io/apimachinery/pkg/types"
2222
"k8s.io/apimachinery/pkg/util/intstr"
23+
"net"
2324
"reflect"
2425
"sigs.k8s.io/controller-runtime/pkg/client"
2526
"strings"
@@ -53,19 +54,22 @@ type NetworkPolicyHandler struct {
5354
injectablerecorder.InjectableRecorder
5455
policy automate_third_party_network_policy.Enum
5556
controlPlaneCIDRPrefixLength int
57+
allowAllIncomingTraffic bool
5658
}
5759

5860
func NewNetworkPolicyHandler(
5961
client client.Client,
6062
scheme *runtime.Scheme,
6163
policy automate_third_party_network_policy.Enum,
6264
controlPlaneCIDRPrefixLength int,
65+
allowAllIncomingTraffic bool,
6366
) *NetworkPolicyHandler {
6467
return &NetworkPolicyHandler{
6568
client: client,
6669
scheme: scheme,
6770
policy: policy,
6871
controlPlaneCIDRPrefixLength: controlPlaneCIDRPrefixLength,
72+
allowAllIncomingTraffic: allowAllIncomingTraffic,
6973
}
7074
}
7175

@@ -305,22 +309,24 @@ func (n *NetworkPolicyHandler) getWebhookService(ctx context.Context, webhookSer
305309

306310
func (n *NetworkPolicyHandler) buildNetworkPolicy(ctx context.Context, webhookName string, webhookService *admissionv1.ServiceReference, service *corev1.Service) (v1.NetworkPolicy, error) {
307311
policyName := fmt.Sprintf("webhook-%s-access-to-%s", strings.ToLower(webhookName), strings.ToLower(service.Name))
312+
rule := v1.NetworkPolicyIngressRule{}
308313

309-
controlPlaneIPs, err := n.getControlPlaneIPsAsCIDR(ctx)
310-
if err != nil {
311-
return v1.NetworkPolicy{}, errors.Wrap(err)
312-
}
313-
314-
fromControlPlaneIPs := lo.Map(controlPlaneIPs, func(controlPLaneIP string, _ int) v1.NetworkPolicyPeer {
315-
return v1.NetworkPolicyPeer{
316-
IPBlock: &v1.IPBlock{
317-
CIDR: controlPLaneIP,
318-
},
314+
if !n.allowAllIncomingTraffic {
315+
controlPlaneIPs, err := n.getControlPlaneIPsAsCIDR(ctx)
316+
if err != nil {
317+
return v1.NetworkPolicy{}, errors.Wrap(err)
319318
}
320-
})
321319

322-
rule := v1.NetworkPolicyIngressRule{}
323-
rule.From = append(rule.From, fromControlPlaneIPs...)
320+
fromControlPlaneIPs := lo.Map(controlPlaneIPs, func(controlPLaneIP string, _ int) v1.NetworkPolicyPeer {
321+
return v1.NetworkPolicyPeer{
322+
IPBlock: &v1.IPBlock{
323+
CIDR: controlPLaneIP,
324+
},
325+
}
326+
})
327+
328+
rule.From = append(rule.From, fromControlPlaneIPs...)
329+
}
324330

325331
newPolicy := v1.NetworkPolicy{
326332
ObjectMeta: metav1.ObjectMeta{
@@ -387,8 +393,11 @@ func (n *NetworkPolicyHandler) getControlPlaneIPsAsCIDR(ctx context.Context) ([]
387393
}
388394

389395
addresses := make([]string, 0)
390-
if svc.Spec.ClusterIP != "" && svc.Spec.ClusterIP != "None" {
391-
addresses = append(addresses, fmt.Sprintf("%s/32", svc.Spec.ClusterIP))
396+
for _, clusterIP := range svc.Spec.ClusterIPs {
397+
ip, isIP := n.ipAddressToCIDR(clusterIP)
398+
if isIP {
399+
addresses = append(addresses, ip)
400+
}
392401
}
393402

394403
var endpoints corev1.Endpoints
@@ -399,15 +408,30 @@ func (n *NetworkPolicyHandler) getControlPlaneIPsAsCIDR(ctx context.Context) ([]
399408

400409
for _, subset := range endpoints.Subsets {
401410
for _, endpointAddress := range subset.Addresses {
402-
if endpointAddress.IP != "" && endpointAddress.IP != "None" {
403-
addresses = append(addresses, fmt.Sprintf("%s/%d", endpointAddress.IP, n.controlPlaneCIDRPrefixLength))
411+
ip, isIP := n.ipAddressToCIDR(endpointAddress.IP)
412+
if isIP {
413+
addresses = append(addresses, ip)
404414
}
405415
}
406416
}
407417

408418
return addresses, nil
409419
}
410420

421+
func (n *NetworkPolicyHandler) ipAddressToCIDR(ipAddress string) (string, bool) {
422+
ip := net.ParseIP(ipAddress)
423+
if ip == nil {
424+
return "", false
425+
}
426+
427+
if ip.To4() != nil {
428+
return fmt.Sprintf("%s/%d", ipAddress, n.controlPlaneCIDRPrefixLength), true
429+
}
430+
// The address is IPv6, we currently support configurable CIDR prefix length only for IPv4
431+
return fmt.Sprintf("%s/128", ipAddress), true
432+
433+
}
434+
411435
func (n *NetworkPolicyHandler) policiesAreEqual(policy *v1.NetworkPolicy, otherPolicy *v1.NetworkPolicy) bool {
412436
return reflect.DeepEqual(policy.Spec, otherPolicy.Spec) &&
413437
reflect.DeepEqual(policy.Labels, otherPolicy.Labels)

‎src/operator/controllers/webhook_traffic/network_policy_handler_test.go‎

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const (
2525
TestNamespace = "test-namespace"
2626
TestWebhookName = "test-webhook"
2727
TestServicePodName = "test-service-pod"
28-
TestControlPlaneIP = "111.222.333.4"
28+
TestControlPlaneIP = "11.22.33.4"
2929
)
3030

3131
var OtterizeIngressNetpols = []v1.NetworkPolicy{
@@ -103,7 +103,7 @@ type NetworkPolicyHandlerTestSuite struct {
103103

104104
func (s *NetworkPolicyHandlerTestSuite) SetupTest() {
105105
s.MocksSuiteBase.SetupTest()
106-
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.IfBlockedByOtterize, 32)
106+
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.IfBlockedByOtterize, 32, false)
107107
s.handler.InjectRecorder(s.Recorder)
108108

109109
s.validatingWebhook = ValidatingWebhookConfiguration.DeepCopy()
@@ -154,7 +154,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
154154
s.mockGetControlPlaneIPs()
155155
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
156156

157-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
157+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, false)
158158
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
159159
err := s.handler.HandleAll(context.Background())
160160
s.Require().NoError(err)
@@ -189,7 +189,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
189189

190190
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
191191

192-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
192+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
193193
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
194194
err := s.handler.HandleAll(context.Background())
195195
s.Require().NoError(err)
@@ -225,7 +225,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
225225

226226
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
227227

228-
netpolMatcher := NewNetworkPolicyMatcher([]int32{secondPort, TestServicePort})
228+
netpolMatcher := NewNetworkPolicyMatcher([]int32{secondPort, TestServicePort}, s.handler.allowAllIncomingTraffic)
229229
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
230230
err := s.handler.HandleAll(context.Background())
231231
s.Require().NoError(err)
@@ -238,8 +238,15 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
238238
s.mockReturningWebhookService()
239239
s.mockServiceIsBlockedByOtterize(OtterizeIngressNetpols)
240240
s.mockGetControlPlaneIPs()
241-
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{*getExpectedNetpolWithPorts([]int32{TestServicePort})})
242-
s.mockGetNetworkPolicyForUpdate(*getExpectedNetpolWithPorts([]int32{TestServicePort}))
241+
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{
242+
NewNetworkPolicyBuilder(ExpectedNetpol).
243+
WithPorts([]int32{TestServicePort}).
244+
WithFromIPBlock(s.handler.allowAllIncomingTraffic).
245+
Build()})
246+
s.mockGetNetworkPolicyForUpdate(NewNetworkPolicyBuilder(ExpectedNetpol).
247+
WithPorts([]int32{TestServicePort}).
248+
WithFromIPBlock(s.handler.allowAllIncomingTraffic).
249+
Build())
243250

244251
//netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
245252
//s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
@@ -254,10 +261,18 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
254261
s.mockReturningWebhookService()
255262
s.mockServiceIsBlockedByOtterize(OtterizeIngressNetpols)
256263
s.mockGetControlPlaneIPs()
257-
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{*getExpectedNetpolWithPorts([]int32{12129})})
258-
s.mockGetNetworkPolicyForUpdate(*getExpectedNetpolWithPorts([]int32{12129}))
259-
260-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
264+
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{
265+
NewNetworkPolicyBuilder(ExpectedNetpol).
266+
WithPorts([]int32{12129}).
267+
WithFromIPBlock(s.handler.allowAllIncomingTraffic).
268+
Build()})
269+
s.mockGetNetworkPolicyForUpdate(
270+
NewNetworkPolicyBuilder(ExpectedNetpol).
271+
WithPorts([]int32{12129}).
272+
WithFromIPBlock(s.handler.allowAllIncomingTraffic).
273+
Build())
274+
275+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
261276
s.Client.EXPECT().Patch(gomock.Any(), gomock.All(netpolMatcher), gomock.Any()).Return(nil)
262277
err := s.handler.HandleAll(context.Background())
263278
s.Require().NoError(err)
@@ -281,7 +296,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleIfBlocked
281296
}
282297

283298
func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleOff_ServiceIsBlockedByOtterize_DoNothing() {
284-
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Off, 32)
299+
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Off, 32, false)
285300

286301
s.mockForReturningValidatingWebhook()
287302
//s.mockReturningWebhookService()
@@ -298,15 +313,19 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleOff_Servi
298313
}
299314

300315
func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleOff_ServiceIsBlockedByOtterize_ExistingWebhookPolicy_DeletePolicy() {
301-
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Off, 32)
316+
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Off, 32, false)
302317

303318
s.mockForReturningValidatingWebhook()
304319
//s.mockReturningWebhookService()
305320
//s.mockServiceIsBlockedByOtterize(OtterizeIngressNetpols)
306321
//s.mockGetControlPlaneIPs()
307-
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{*getExpectedNetpolWithPorts([]int32{TestServicePort})})
322+
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{
323+
NewNetworkPolicyBuilder(ExpectedNetpol).
324+
WithPorts([]int32{TestServicePort}).
325+
WithFromIPBlock(s.handler.allowAllIncomingTraffic).
326+
Build()})
308327

309-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
328+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
310329
s.Client.EXPECT().Delete(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
311330
err := s.handler.HandleAll(context.Background())
312331
s.Require().NoError(err)
@@ -315,7 +334,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleOff_Servi
315334
}
316335

317336
func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleAlways_ServiceIsNotBlockedByOtterize_CreatePolicy() {
318-
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Always, 32)
337+
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Always, 32, false)
319338
s.handler.InjectRecorder(s.Recorder)
320339

321340
s.mockForReturningValidatingWebhook()
@@ -324,7 +343,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleAlways_Se
324343
s.mockGetControlPlaneIPs()
325344
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
326345

327-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
346+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
328347
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
329348
err := s.handler.HandleAll(context.Background())
330349
s.Require().NoError(err)
@@ -339,7 +358,7 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_MutatingWebhook
339358
s.mockGetControlPlaneIPs()
340359
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
341360

342-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
361+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
343362
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
344363
err := s.handler.HandleAll(context.Background())
345364
s.Require().NoError(err)
@@ -354,7 +373,25 @@ func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_CRDsWebhooks_Ha
354373
s.mockGetControlPlaneIPs()
355374
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
356375

357-
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort})
376+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
377+
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
378+
err := s.handler.HandleAll(context.Background())
379+
s.Require().NoError(err)
380+
s.ExpectEvent(ReasonCreatingWebhookTrafficNetpol)
381+
s.ExpectEvent(ReasonCreatingWebhookTrafficNetpolSuccess)
382+
}
383+
384+
func (s *NetworkPolicyHandlerTestSuite) TestNetworkPolicyHandler_HandleAlways_AllowAllIncomingTraffic_CreatingWebhookPolicy() {
385+
s.handler = NewNetworkPolicyHandler(s.Client, &runtime.Scheme{}, automate_third_party_network_policy.Always, 32, true)
386+
s.handler.InjectRecorder(s.Recorder)
387+
388+
s.mockForReturningValidatingWebhook()
389+
s.mockReturningWebhookService()
390+
//s.mockServiceIsBlockedByOtterize(OtterizeIngressNetpols)
391+
//s.mockGetControlPlaneIPs()
392+
s.mockGetExistingOtterizeWebhooksNetpols([]v1.NetworkPolicy{})
393+
394+
netpolMatcher := NewNetworkPolicyMatcher([]int32{TestServicePort}, s.handler.allowAllIncomingTraffic)
358395
s.Client.EXPECT().Create(gomock.Any(), gomock.All(netpolMatcher)).Return(nil)
359396
err := s.handler.HandleAll(context.Background())
360397
s.Require().NoError(err)
@@ -499,7 +536,7 @@ func (s *NetworkPolicyHandlerTestSuite) mockGetControlPlaneIPs() {
499536
gomock.Any(), gomock.Eq(types.NamespacedName{Name: "kubernetes", Namespace: "default"}), gomock.Eq(&corev1.Service{}),
500537
).DoAndReturn(
501538
func(_ any, _ any, svc *corev1.Service, _ ...any) error {
502-
svc.Spec.ClusterIP = TestControlPlaneIP
539+
svc.Spec.ClusterIPs = []string{TestControlPlaneIP}
503540
svc.Name = "kubernetes"
504541
svc.Namespace = "default"
505542
return nil

‎src/operator/controllers/webhook_traffic/network_policy_handler_utils_test.go‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,28 @@ var ExpectedNetpol = v1.NetworkPolicy{
2424
},
2525
Ingress: []v1.NetworkPolicyIngressRule{
2626
{
27-
Ports: []v1.NetworkPolicyPort{},
28-
From: []v1.NetworkPolicyPeer{
29-
{
30-
IPBlock: &v1.IPBlock{
31-
CIDR: fmt.Sprintf("%s/32", TestControlPlaneIP),
32-
},
33-
},
34-
},
27+
Ports: nil,
28+
From: nil,
3529
},
3630
},
3731
PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress},
3832
},
3933
}
4034

35+
type NetworkPolicyBuilder struct {
36+
policy *v1.NetworkPolicy
37+
}
38+
4139
type NetworkPolicyMatcher struct {
42-
ports []int32
40+
ports []int32
41+
allowAllIncomingTraffic bool
4342
}
4443

45-
func NewNetworkPolicyMatcher(ports []int32) *NetworkPolicyMatcher {
46-
return &NetworkPolicyMatcher{ports: ports}
44+
func NewNetworkPolicyMatcher(ports []int32, allowAllIncomingTraffic bool) *NetworkPolicyMatcher {
45+
return &NetworkPolicyMatcher{
46+
ports: ports,
47+
allowAllIncomingTraffic: allowAllIncomingTraffic,
48+
}
4749
}
4850

4951
func (m *NetworkPolicyMatcher) String() string {
@@ -56,21 +58,47 @@ func (m *NetworkPolicyMatcher) Matches(other interface{}) bool {
5658
return false
5759
}
5860

59-
expectedNetpol := getExpectedNetpolWithPorts(m.ports)
61+
expectedNetpol := NewNetworkPolicyBuilder(ExpectedNetpol).
62+
WithPorts(m.ports).
63+
WithFromIPBlock(m.allowAllIncomingTraffic).
64+
Build()
6065

6166
return otherAsNetpol.Namespace == TestNamespace &&
6267
otherAsNetpol.Name == expectedNetpol.Name &&
6368
reflect.DeepEqual(otherAsNetpol.Labels, expectedNetpol.Labels) &&
6469
reflect.DeepEqual(otherAsNetpol.Spec, expectedNetpol.Spec)
6570
}
6671

67-
func getExpectedNetpolWithPorts(ports []int32) *v1.NetworkPolicy {
68-
expectedNetpol := ExpectedNetpol.DeepCopy()
69-
expectedNetpol.Spec.Ingress[0].Ports = lo.Map(ports, func(port int32, _ int) v1.NetworkPolicyPort {
72+
func NewNetworkPolicyBuilder(base v1.NetworkPolicy) *NetworkPolicyBuilder {
73+
return &NetworkPolicyBuilder{policy: base.DeepCopy()}
74+
}
75+
76+
func (b *NetworkPolicyBuilder) WithPorts(ports []int32) *NetworkPolicyBuilder {
77+
b.policy.Spec.Ingress[0].Ports = lo.Map(ports, func(port int32, _ int) v1.NetworkPolicyPort {
7078
return v1.NetworkPolicyPort{
7179
Protocol: lo.ToPtr(corev1.ProtocolTCP),
7280
Port: lo.ToPtr(intstr.IntOrString{Type: intstr.Int, IntVal: port}),
7381
}
7482
})
75-
return expectedNetpol
83+
return b
84+
}
85+
86+
func (b *NetworkPolicyBuilder) WithFromIPBlock(allowAll bool) *NetworkPolicyBuilder {
87+
if allowAll {
88+
// leave .From empty
89+
return b
90+
}
91+
92+
b.policy.Spec.Ingress[0].From = []v1.NetworkPolicyPeer{
93+
{
94+
IPBlock: &v1.IPBlock{
95+
CIDR: fmt.Sprintf("%s/32", TestControlPlaneIP),
96+
},
97+
},
98+
}
99+
return b
100+
}
101+
102+
func (b *NetworkPolicyBuilder) Build() v1.NetworkPolicy {
103+
return *b.policy
76104
}

0 commit comments

Comments
 (0)