Skip to content

Commit 0af3ac5

Browse files
committed
feat(netpol): refine NetworkPolicy controller and add E2E verification
- Add ate.dev/worker-pool label to NetworkPolicy metadata and remove Egress management to allow default egress behavior - Define ateSystemNamespace and atenetRouterAppName constants for Ingress peer selectors - Centralize deterministic NetworkPolicy naming logic (resources.NetworkPolicyName) and prevent trailing hyphens - Add comprehensive E2E test suite for networkpolicy covering control plane reconciliation, positive data plane verification (authorized ingress via atenet-router), and negative data plane verification (unauthorized ingress blocking)
1 parent c2ca7a6 commit 0af3ac5

6 files changed

Lines changed: 617 additions & 40 deletions

File tree

cmd/atecontroller/internal/controllers/networkpolicy_controller.go

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ package controllers
1616

1717
import (
1818
"context"
19-
"crypto/sha256"
20-
"encoding/hex"
2119
"fmt"
2220

2321
networkingv1 "k8s.io/api/networking/v1"
@@ -29,10 +27,15 @@ import (
2927
"sigs.k8s.io/controller-runtime/pkg/client"
3028
"sigs.k8s.io/controller-runtime/pkg/log"
3129

30+
"github.com/agent-substrate/substrate/internal/resources"
3231
atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1"
3332
)
3433

35-
const networkPolicyFieldOwner = "ate-networkpolicy"
34+
const (
35+
networkPolicyFieldOwner = "ate-networkpolicy"
36+
ateSystemNamespace = "ate-system"
37+
atenetRouterAppName = "atenet-router"
38+
)
3639

3740
type NetworkPolicyReconciler struct {
3841
client.Client
@@ -84,7 +87,10 @@ func (r *NetworkPolicyReconciler) reconcileImpl(ctx context.Context, wp *atev1al
8487
}
8588

8689
func buildNetworkPolicyApplyConfig(wp *atev1alpha1.WorkerPool) *networkingv1ac.NetworkPolicyApplyConfiguration {
87-
np := networkingv1ac.NetworkPolicy(npName(wp.Name), wp.Namespace).
90+
np := networkingv1ac.NetworkPolicy(resources.NetworkPolicyName(wp.Name), wp.Namespace).
91+
WithLabels(map[string]string{
92+
"ate.dev/worker-pool": wp.Name,
93+
}).
8894
WithOwnerReferences(metav1ac.OwnerReference().
8995
WithAPIVersion(atev1alpha1.GroupVersion.String()).
9096
WithKind("WorkerPool").
@@ -98,39 +104,25 @@ func buildNetworkPolicyApplyConfig(wp *atev1alpha1.WorkerPool) *networkingv1ac.N
98104
WithSpec(networkingv1ac.NetworkPolicySpec().
99105
WithPodSelector(metav1ac.LabelSelector().
100106
WithMatchLabels(map[string]string{"ate.dev/worker-pool": wp.Name})).
101-
WithPolicyTypes(networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress).
107+
WithPolicyTypes(networkingv1.PolicyTypeIngress).
102108
WithIngress(
103109
networkingv1ac.NetworkPolicyIngressRule().
104110
WithFrom(
105111
networkingv1ac.NetworkPolicyPeer().
106112
WithNamespaceSelector(metav1ac.LabelSelector().
107-
WithMatchLabels(map[string]string{"kubernetes.io/metadata.name": "ate-system"})).
113+
WithMatchLabels(map[string]string{"kubernetes.io/metadata.name": ateSystemNamespace})).
108114
WithPodSelector(metav1ac.LabelSelector().
109-
WithMatchLabels(map[string]string{"app": "atenet-router"})),
115+
WithMatchLabels(map[string]string{"app": atenetRouterAppName})),
110116
),
111-
).
112-
WithEgress(
113-
networkingv1ac.NetworkPolicyEgressRule(),
114117
))
115118

116-
// TODO: don't implement any Egress policy yet.
119+
// Egress is left unmanaged by Kubernetes NetworkPolicy for now while waiting for
120+
// further progress on Egress API designs and because some egress rules may not be
121+
// expressible at the Kubernetes layer.
117122

118123
return np
119124
}
120125

121-
func npName(wpName string) string {
122-
sum := sha256.Sum256([]byte(wpName))
123-
hash := hex.EncodeToString(sum[:])
124-
if len(hash) > 5 {
125-
hash = hash[:5]
126-
}
127-
truncated := wpName
128-
if len(truncated) > 30 {
129-
truncated = truncated[:30]
130-
}
131-
return fmt.Sprintf("substrate-%s-%s", truncated, hash)
132-
}
133-
134126
func (r *NetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
135127
return ctrl.NewControllerManagedBy(mgr).
136128
Named("networkpolicy").

cmd/atecontroller/internal/controllers/networkpolicy_controller_test.go

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ package controllers
1616

1717
import (
1818
"context"
19+
"strings"
1920
"testing"
2021

2122
networkingv1 "k8s.io/api/networking/v1"
2223
"k8s.io/apimachinery/pkg/types"
24+
25+
"github.com/agent-substrate/substrate/internal/resources"
2326
)
2427

2528
func TestWorkerPoolCreatesNetworkPolicy(t *testing.T) {
@@ -30,7 +33,7 @@ func TestWorkerPoolCreatesNetworkPolicy(t *testing.T) {
3033
t.Cleanup(func() { k8sClient.Delete(testCtx, wp) }) //nolint:errcheck
3134

3235
eventually(t, func(ctx context.Context) (bool, error) {
33-
npName := npName(wp.Name)
36+
npName := resources.NetworkPolicyName(wp.Name)
3437
np := &networkingv1.NetworkPolicy{}
3538
err := k8sClient.Get(ctx, types.NamespacedName{Name: npName, Namespace: wp.Namespace}, np)
3639
if err != nil {
@@ -42,23 +45,24 @@ func TestWorkerPoolCreatesNetworkPolicy(t *testing.T) {
4245
return false, nil
4346
}
4447

48+
// Verify metadata label matches the worker pool
49+
if np.Labels == nil || np.Labels["ate.dev/worker-pool"] != wp.Name {
50+
return false, nil
51+
}
52+
4553
// Verify PodSelector matches the worker pool
4654
if np.Spec.PodSelector.MatchLabels == nil || np.Spec.PodSelector.MatchLabels["ate.dev/worker-pool"] != wp.Name {
4755
return false, nil
4856
}
4957

50-
// Verify PolicyTypes contains Ingress and Egress
58+
// Verify PolicyTypes contains Ingress
5159
hasIngress := false
52-
hasEgress := false
5360
for _, pt := range np.Spec.PolicyTypes {
5461
if pt == networkingv1.PolicyTypeIngress {
5562
hasIngress = true
5663
}
57-
if pt == networkingv1.PolicyTypeEgress {
58-
hasEgress = true
59-
}
6064
}
61-
if !hasIngress || !hasEgress {
65+
if !hasIngress {
6266
return false, nil
6367
}
6468

@@ -71,23 +75,68 @@ func TestWorkerPoolCreatesNetworkPolicy(t *testing.T) {
7175
return false, nil
7276
}
7377
fromPeer := ingressRule.From[0]
74-
if fromPeer.NamespaceSelector == nil || fromPeer.NamespaceSelector.MatchLabels["kubernetes.io/metadata.name"] != "ate-system" {
78+
if fromPeer.NamespaceSelector == nil || fromPeer.NamespaceSelector.MatchLabels["kubernetes.io/metadata.name"] != ateSystemNamespace {
7579
return false, nil
7680
}
77-
if fromPeer.PodSelector == nil || fromPeer.PodSelector.MatchLabels["app"] != "atenet-router" {
81+
if fromPeer.PodSelector == nil || fromPeer.PodSelector.MatchLabels["app"] != atenetRouterAppName {
7882
return false, nil
7983
}
8084

81-
// Verify Egress Rules:
82-
// For now, we allow all egress.
83-
if len(np.Spec.Egress) != 1 {
84-
return false, nil
85-
}
86-
egressRule := np.Spec.Egress[0]
87-
if len(egressRule.To) != 0 || len(egressRule.Ports) != 0 {
85+
// Verify Egress Rules are unmanaged (empty)
86+
if len(np.Spec.Egress) != 0 {
8887
return false, nil
8988
}
9089

9190
return true, nil
9291
})
9392
}
93+
94+
func TestNpName(t *testing.T) {
95+
tests := []struct {
96+
name string
97+
wpName string
98+
wantPrefix string
99+
}{
100+
{
101+
name: "short workerpool name",
102+
wpName: "my-wp",
103+
wantPrefix: "substrate-my-wp-",
104+
},
105+
{
106+
name: "long workerpool name truncated cleanly",
107+
wpName: "a-very-long-workerpool-name-that-exceeds-30-chars",
108+
wantPrefix: "substrate-a-very-long-workerpool-name-th-",
109+
},
110+
{
111+
name: "long workerpool name truncating right at a hyphen",
112+
wpName: "this-is-a-long-worker-pool-n-with-hyphen-at-30",
113+
// "this-is-a-long-worker-pool-n-" is 30 chars long, ending in a hyphen.
114+
// Trimming trailing hyphens turns it into "this-is-a-long-worker-pool-n".
115+
wantPrefix: "substrate-this-is-a-long-worker-pool-n-",
116+
},
117+
{
118+
name: "workerpool name ending with hyphens",
119+
wpName: "workerpool-name---",
120+
wantPrefix: "substrate-workerpool-name-",
121+
},
122+
}
123+
124+
for _, tt := range tests {
125+
t.Run(tt.name, func(t *testing.T) {
126+
got := resources.NetworkPolicyName(tt.wpName)
127+
128+
if !strings.HasPrefix(got, tt.wantPrefix) {
129+
t.Errorf("npName(%q) = %q, want prefix %q", tt.wpName, got, tt.wantPrefix)
130+
}
131+
132+
// Ensure no double hyphens before the hash
133+
// The format is substrate-<truncated>-<hash>
134+
parts := strings.Split(got, "-")
135+
for i, part := range parts {
136+
if part == "" && i > 0 && i < len(parts)-1 {
137+
t.Errorf("npName(%q) = %q contains empty part (double hyphen)", tt.wpName, got)
138+
}
139+
}
140+
})
141+
}
142+
}

0 commit comments

Comments
 (0)