Skip to content

Commit 480f3e2

Browse files
LopatinDmitrLopatinDmitr
authored
fix(vm): prohibit duplicating networks in the VirtualMachine .spec (#1545)
This update enhances the validation logic for Virtual Machines' network configurations by prohibiting duplicate network names within the specification. Signed-off-by: Dmitry Lopatin <[email protected]>
1 parent 9a07947 commit 480f3e2

File tree

4 files changed

+143
-50
lines changed

4 files changed

+143
-50
lines changed

‎docs/USER_GUIDE.md‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2492,26 +2492,12 @@ spec:
24922492
name: user-net # Network name
24932493
```
24942494

2495-
It is allowed to connect a VM to the same network multiple times. Example:
2496-
2497-
```yaml
2498-
spec:
2499-
networks:
2500-
- type: Main # Must always be specified first
2501-
- type: Network
2502-
name: user-net # Network name
2503-
- type: Network
2504-
name: user-net # Network name
2505-
```
2506-
25072495
Example of connecting to the cluster network `corp-net`:
25082496

25092497
```yaml
25102498
spec:
25112499
networks:
25122500
- type: Main # Must always be specified first
2513-
- type: Network
2514-
name: user-net
25152501
- type: Network
25162502
name: user-net
25172503
- type: ClusterNetwork
@@ -2527,12 +2513,9 @@ status:
25272513
- type: Network
25282514
name: user-net
25292515
macAddress: aa:bb:cc:dd:ee:01
2530-
- type: Network
2531-
name: user-net
2532-
macAddress: aa:bb:cc:dd:ee:02
25332516
- type: ClusterNetwork
25342517
name: corp-net
2535-
macAddress: aa:bb:cc:dd:ee:03
2518+
macAddress: aa:bb:cc:dd:ee:02
25362519
```
25372520

25382521
For each additional network interface, a unique MAC address is automatically generated and reserved to avoid collisions. The following resources are used for this: `VirtualMachineMACAddress` (`vmmac`) and `VirtualMachineMACAddressLease` (`vmmacl`).

‎docs/USER_GUIDE.ru.md‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2524,26 +2524,12 @@ spec:
25242524
name: user-net # Название сети
25252525
```
25262526

2527-
Допускается подключать одну ВМ к одной и той же сети несколько раз. Пример:
2528-
2529-
```yaml
2530-
spec:
2531-
networks:
2532-
- type: Main # Обязательно указывать первым
2533-
- type: Network
2534-
name: user-net # Название сети
2535-
- type: Network
2536-
name: user-net # Название сети
2537-
```
2538-
25392527
Пример подключения кластерной сети `corp-net`:
25402528

25412529
```yaml
25422530
spec:
25432531
networks:
25442532
- type: Main # Обязательно указывать первым
2545-
- type: Network
2546-
name: user-net
25472533
- type: Network
25482534
name: user-net
25492535
- type: ClusterNetwork
@@ -2559,12 +2545,9 @@ status:
25592545
- type: Network
25602546
name: user-net
25612547
macAddress: aa:bb:cc:dd:ee:01
2562-
- type: Network
2563-
name: user-net
2564-
macAddress: aa:bb:cc:dd:ee:02
25652548
- type: ClusterNetwork
25662549
name: corp-net
2567-
macAddress: aa:bb:cc:dd:ee:03
2550+
macAddress: aa:bb:cc:dd:ee:02
25682551
```
25692552

25702553
Для каждого дополнительного сетевого интерфейса автоматически создается и резервируется уникальный MAC-адрес, что обеспечивает отсутствие коллизий MAC-адресов. Для этих целей используются ресурсы: `VirtualMachineMACAddress` (`vmmac`) и `VirtualMachineMACAddressLease` (`vmmacl`).

‎images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator.go‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222

23+
"k8s.io/apimachinery/pkg/api/equality"
2324
"k8s.io/component-base/featuregate"
2425
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2526

@@ -38,42 +39,60 @@ func NewNetworksValidator(featureGate featuregate.FeatureGate) *NetworksValidato
3839
}
3940

4041
func (v *NetworksValidator) ValidateCreate(_ context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) {
41-
return v.Validate(vm)
42-
}
42+
networksSpec := vm.Spec.Networks
43+
if len(networksSpec) == 0 {
44+
return nil, nil
45+
}
4346

44-
func (v *NetworksValidator) ValidateUpdate(_ context.Context, _, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) {
45-
return v.Validate(newVM)
46-
}
47+
if !v.featureGate.Enabled(featuregates.SDN) {
48+
return nil, fmt.Errorf("network configuration requires SDN to be enabled")
49+
}
4750

48-
func (v *NetworksValidator) Validate(vm *v1alpha2.VirtualMachine) (admission.Warnings, error) {
49-
networksSpec := vm.Spec.Networks
51+
return v.validateNetworksSpec(networksSpec)
52+
}
5053

51-
if len(networksSpec) == 0 {
54+
func (v *NetworksValidator) ValidateUpdate(_ context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) {
55+
newNetworksSpec := newVM.Spec.Networks
56+
if len(newNetworksSpec) == 0 {
5257
return nil, nil
5358
}
5459

5560
if !v.featureGate.Enabled(featuregates.SDN) {
5661
return nil, fmt.Errorf("network configuration requires SDN to be enabled")
5762
}
5863

64+
isChanged := !equality.Semantic.DeepEqual(newNetworksSpec, oldVM.Spec.Networks)
65+
if isChanged {
66+
return v.validateNetworksSpec(newNetworksSpec)
67+
}
68+
return nil, nil
69+
}
70+
71+
func (v *NetworksValidator) validateNetworksSpec(networksSpec []v1alpha2.NetworksSpec) (admission.Warnings, error) {
5972
if networksSpec[0].Type != v1alpha2.NetworksTypeMain {
6073
return nil, fmt.Errorf("first network in the list must be of type '%s'", v1alpha2.NetworksTypeMain)
6174
}
6275
if networksSpec[0].Name != "" {
6376
return nil, fmt.Errorf("network with type '%s' should not have a name", v1alpha2.NetworksTypeMain)
6477
}
6578

79+
namesSet := make(map[string]struct{})
80+
6681
for i, network := range networksSpec {
6782
if network.Type == v1alpha2.NetworksTypeMain {
6883
if i > 0 {
6984
return nil, fmt.Errorf("only one network of type '%s' is allowed", v1alpha2.NetworksTypeMain)
7085
}
7186
continue
7287
}
73-
7488
if network.Name == "" {
7589
return nil, fmt.Errorf("network at index %d with type '%s' must have a non-empty name", i, network.Type)
7690
}
91+
92+
if _, exists := namesSet[network.Name]; exists {
93+
return nil, fmt.Errorf("network name '%s' is duplicated", network.Name)
94+
}
95+
namesSet[network.Name] = struct{}{}
7796
}
7897

7998
return nil, nil

‎images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator_test.go‎

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"github.com/deckhouse/virtualization/api/core/v1alpha2"
2525
)
2626

27-
func TestNetworksValidate(t *testing.T) {
27+
func TestNetworksValidateCreate(t *testing.T) {
2828
tests := []struct {
2929
networks []v1alpha2.NetworksSpec
3030
sdnEnabled bool
@@ -36,12 +36,13 @@ func TestNetworksValidate(t *testing.T) {
3636
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain, Name: "main"}}, true, false},
3737
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test"}, {Type: v1alpha2.NetworksTypeMain}}, true, false},
3838
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain}, {Type: v1alpha2.NetworksTypeNetwork, Name: "test"}}, true, true},
39+
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain}, {Type: v1alpha2.NetworksTypeNetwork, Name: "test"}, {Type: v1alpha2.NetworksTypeNetwork, Name: "test"}}, true, false},
3940
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain}, {Type: v1alpha2.NetworksTypeNetwork}}, true, false},
4041
{[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain}}, false, false},
4142
}
4243

4344
for i, test := range tests {
44-
t.Run(fmt.Sprintf("TestCase%d", i), func(t *testing.T) {
45+
t.Run(fmt.Sprintf("CreateTestCase%d", i), func(t *testing.T) {
4546
vm := &v1alpha2.VirtualMachine{Spec: v1alpha2.VirtualMachineSpec{Networks: test.networks}}
4647

4748
// Create feature gate with SDN
@@ -51,13 +52,120 @@ func TestNetworksValidate(t *testing.T) {
5152
}
5253
networkValidator := NewNetworksValidator(featureGate)
5354

54-
_, err := networkValidator.Validate(vm)
55+
_, err := networkValidator.ValidateCreate(t.Context(), vm)
5556
if test.valid && err != nil {
56-
t.Errorf("For spec %s expected valid, but validation failed", test.networks)
57+
t.Errorf("Validation failed for spec %s: expected valid, but got an error: %v", test.networks, err)
5758
}
59+
if !test.valid && err == nil {
60+
t.Errorf("Validation succeeded for spec %s: expected error, but got none", test.networks)
61+
}
62+
})
63+
}
64+
}
5865

66+
func TestNetworksValidateUpdate(t *testing.T) {
67+
tests := []struct {
68+
oldNetworksSpec []v1alpha2.NetworksSpec
69+
newNetworksSpec []v1alpha2.NetworksSpec
70+
sdnEnabled bool
71+
valid bool
72+
}{
73+
{
74+
oldNetworksSpec: []v1alpha2.NetworksSpec{},
75+
newNetworksSpec: []v1alpha2.NetworksSpec{},
76+
sdnEnabled: true,
77+
valid: true,
78+
},
79+
{
80+
oldNetworksSpec: []v1alpha2.NetworksSpec{},
81+
newNetworksSpec: []v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain}},
82+
sdnEnabled: true,
83+
valid: true,
84+
},
85+
{
86+
oldNetworksSpec: []v1alpha2.NetworksSpec{},
87+
newNetworksSpec: []v1alpha2.NetworksSpec{
88+
{Type: v1alpha2.NetworksTypeMain},
89+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
90+
},
91+
sdnEnabled: true,
92+
valid: true,
93+
},
94+
{
95+
oldNetworksSpec: []v1alpha2.NetworksSpec{},
96+
newNetworksSpec: []v1alpha2.NetworksSpec{
97+
{Type: v1alpha2.NetworksTypeMain},
98+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
99+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
100+
},
101+
sdnEnabled: true,
102+
valid: false,
103+
},
104+
{
105+
oldNetworksSpec: []v1alpha2.NetworksSpec{
106+
{Type: v1alpha2.NetworksTypeMain},
107+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
108+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
109+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
110+
},
111+
newNetworksSpec: []v1alpha2.NetworksSpec{
112+
{Type: v1alpha2.NetworksTypeMain},
113+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
114+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
115+
},
116+
sdnEnabled: true,
117+
valid: false,
118+
},
119+
{
120+
oldNetworksSpec: []v1alpha2.NetworksSpec{
121+
{Type: v1alpha2.NetworksTypeMain},
122+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
123+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
124+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
125+
},
126+
newNetworksSpec: []v1alpha2.NetworksSpec{
127+
{Type: v1alpha2.NetworksTypeMain},
128+
{Type: v1alpha2.NetworksTypeNetwork, Name: "test"},
129+
},
130+
sdnEnabled: true,
131+
valid: true,
132+
},
133+
}
134+
135+
for i, test := range tests {
136+
t.Run(fmt.Sprintf("UpdateTestCase%d", i), func(t *testing.T) {
137+
oldVM := &v1alpha2.VirtualMachine{
138+
Spec: v1alpha2.VirtualMachineSpec{
139+
Networks: test.oldNetworksSpec,
140+
},
141+
}
142+
newVM := &v1alpha2.VirtualMachine{
143+
Spec: v1alpha2.VirtualMachineSpec{
144+
Networks: test.newNetworksSpec,
145+
},
146+
}
147+
148+
// Create feature gate with SDN
149+
featureGate, _, setFromMap, _ := featuregates.New()
150+
if test.sdnEnabled {
151+
_ = setFromMap(map[string]bool{
152+
string(featuregates.SDN): true,
153+
})
154+
}
155+
networkValidator := NewNetworksValidator(featureGate)
156+
_, err := networkValidator.ValidateUpdate(t.Context(), oldVM, newVM)
157+
158+
if test.valid && err != nil {
159+
t.Errorf(
160+
"Validation failed for old spec %v and new spec %v: expected valid, but got an error: %v",
161+
test.oldNetworksSpec, test.newNetworksSpec, err,
162+
)
163+
}
59164
if !test.valid && err == nil {
60-
t.Errorf("For spec %s expected not valid, but validation succeeded", test.networks)
165+
t.Errorf(
166+
"Validation succeeded for old spec %v and new spec %v: expected error, but got none",
167+
test.oldNetworksSpec, test.newNetworksSpec,
168+
)
61169
}
62170
})
63171
}

0 commit comments

Comments
 (0)