fix(webhook): stop attaching an instance's first interface twice

README.md

@@ -14,7 +14,7 @@ Services defines Kubernetes Custom Resource Definitions for virtual tenant netwo
 
 The controller runs in a POP cell beside network-services-operator, compute and the workload providers. It turns a `NetworkContext` into a `VPC` identity; when a `NetworkInterface` claim is fulfilled it creates the `VPCAttachment` and the `NetworkAttachmentDefinition`, allocates the attachment identifier, and publishes the annotations a workload must carry; and it projects what the data plane reported back onto `VPCAttachment` and `NetworkInterface` status.
 
-It also serves a mutating admission webhook that injects the Multus annotations into Pods labelled `networking.datumapis.com/inject-interfaces: "true"`, so Multus knowledge stays inside the one component that writes NetworkAttachmentDefinitions. It names the instance's interfaces in `k8s.v1.cni.cncf.io/networks`, and names the first of them in `v1.multus-cni.io/default-network` so the Pod's default network is the tenant's rather than the cluster CNI's.
+It also serves a mutating admission webhook that injects the Multus annotations into Pods labelled `networking.datumapis.com/inject-interfaces: "true"`, so Multus knowledge stays inside the one component that writes NetworkAttachmentDefinitions. It names the instance's first interface in `v1.multus-cni.io/default-network`, so the Pod's default network is the tenant's rather than the cluster CNI's, and names the remaining interfaces in `k8s.v1.cni.cncf.io/networks`. The first interface is not repeated there, or Multus would attach it twice.
 
 It requires `--attachment-mode` (`Netns` or `Hypervisor`) — how guests in the cell consume an interface. There is no default, because defaulting would hand a microVM an interface it cannot use.
 

internal/webhook/pod_webhook.go

@@ -112,8 +112,7 @@ func (i *PodInterfaceInjector) Handle(ctx context.Context, req admission.Request
 	if pod.Annotations == nil {
 		pod.Annotations = map[string]string{}
 	}
-	pod.Annotations[MultusNetworksAnnotation] = mergeNetworks(pod.Annotations[MultusNetworksAnnotation], networks)
-	pod.Annotations[MultusDefaultNetworkAnnotation] = networks[0]
+	injectNetworks(pod.Annotations, networks)
 	pod.Annotations[InjectedInterfacesAnnotation] = strings.Join(injected, ",")
 
 	patched, err := json.Marshal(pod)
@@ -186,12 +185,27 @@ func instanceOwner(pod *corev1.Pod) (string, bool) {
 	return "", false
 }
 
+// injectNetworks makes the first network the Pod's default and lists the rest as
+// additional networks. Multus attaches the default network as eth0 on its own, so
+// listing it again would attach the same definition a second time as net1.
+func injectNetworks(annotations map[string]string, networks []string) {
+	defaultNetwork := networks[0]
+	annotations[MultusDefaultNetworkAnnotation] = defaultNetwork
+
+	merged := mergeNetworks(annotations[MultusNetworksAnnotation], networks[1:], defaultNetwork)
+	if merged == "" {
+		delete(annotations, MultusNetworksAnnotation)
+		return
+	}
+	annotations[MultusNetworksAnnotation] = merged
+}
+
 // mergeNetworks appends the resolved networks to whatever the Pod already asked
-// for, without duplicating an entry.
-func mergeNetworks(existing string, networks []string) string {
+// for, without duplicating an entry or repeating the default network.
+func mergeNetworks(existing string, networks []string, defaultNetwork string) string {
 	merged := []string{}
-	for _, entry := range strings.Split(existing, ",") {
-		if entry = strings.TrimSpace(entry); entry != "" {
+	for entry := range strings.SplitSeq(existing, ",") {
+		if entry = strings.TrimSpace(entry); entry != "" && entry != defaultNetwork {
 			merged = append(merged, entry)
 		}
 	}

internal/webhook/pod_webhook_test.go

@@ -85,16 +85,50 @@ func TestMergeNetworks(t *testing.T) {
 		want     string
 	}{
 		{"empty", "", []string{"ns/a"}, "ns/a"},
+		{"nothing to add", "", nil, ""},
 		{"appends in order", "", []string{"ns/a", "ns/b"}, "ns/a,ns/b"},
 		{"preserves what the pod asked for", "ns/other", []string{"ns/a"}, "ns/other,ns/a"},
 		{"does not duplicate", "ns/a", []string{"ns/a"}, "ns/a"},
 		{"tolerates whitespace", " ns/other , ", []string{"ns/a"}, "ns/other,ns/a"},
+		{"drops the default network", "ns/default,ns/other", []string{"ns/a"}, "ns/other,ns/a"},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			if got := mergeNetworks(test.existing, test.networks); got != test.want {
+			if got := mergeNetworks(test.existing, test.networks, "ns/default"); got != test.want {
 				t.Errorf("got %q, want %q", got, test.want)
 			}
 		})
 	}
 }
+
+func TestInjectNetworks(t *testing.T) {
+	tests := []struct {
+		name        string
+		existing    map[string]string
+		networks    []string
+		wantDefault string
+		wantList    string
+		wantListSet bool
+	}{
+		{"single interface is only the default", map[string]string{},
+			[]string{"ns/a"}, "ns/a", "", false},
+		{"additional interfaces exclude the default", map[string]string{},
+			[]string{"ns/a", "ns/b", "ns/c"}, "ns/a", "ns/b,ns/c", true},
+		{"preserves what the pod asked for", map[string]string{MultusNetworksAnnotation: "ns/other"},
+			[]string{"ns/a"}, "ns/a", "ns/other", true},
+		{"drops a default the pod already listed", map[string]string{MultusNetworksAnnotation: "ns/a"},
+			[]string{"ns/a", "ns/b"}, "ns/a", "ns/b", true},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			injectNetworks(test.existing, test.networks)
+			if got := test.existing[MultusDefaultNetworkAnnotation]; got != test.wantDefault {
+				t.Errorf("default network: got %q, want %q", got, test.wantDefault)
+			}
+			got, set := test.existing[MultusNetworksAnnotation]
+			if got != test.wantList || set != test.wantListSet {
+				t.Errorf("networks: got (%q, %v), want (%q, %v)", got, set, test.wantList, test.wantListSet)
+			}
+		})
+	}
+}