Skip to content

Commit 75c5665

Browse files
dkhachyandkhachyan
authored
remove nodeid duplicate validate (#193)
* remove nodeid duplicate validate * fix tests * fix tests
1 parent 6438736 commit 75c5665

2 files changed

Lines changed: 18 additions & 193 deletions

File tree

‎cmd/client/validators.go‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ type Manifest struct {
1515
APIVersion string `yaml:"apiVersion"`
1616
Kind string `yaml:"kind"`
1717
Metadata struct {
18-
Name string `yaml:"name"`
19-
Namespace string `yaml:"namespace"`
20-
Annotations map[string]string `yaml:"annotations"`
18+
Name string `yaml:"name"`
19+
Namespace string `yaml:"namespace"`
2120
} `yaml:"metadata"`
2221
}
2322

@@ -114,7 +113,6 @@ type manifestInfo struct {
114113
}
115114

116115
// checkDuplicateManifests validates that a manifest is unique within its apiVersion/kind, namespace, and name.
117-
// Special handling for Envoy resources includes node-id from annotations in the uniqueness check.
118116
// Parameters:
119117
// - m: Parsed manifest to validate
120118
// - path: File path where manifest was found
@@ -128,12 +126,6 @@ func checkDuplicateManifests(m Manifest, path string, result *ValidationResult,
128126
ns = "default"
129127
}
130128

131-
nodeID := ""
132-
// Only consider node-id annotation for VirtualService resources
133-
if m.Metadata.Annotations != nil && m.Kind == "VirtualService" {
134-
nodeID = m.Metadata.Annotations["envoy.kaasops.io/node-id"]
135-
}
136-
137129
apiKind := fmt.Sprintf("%s/%s", m.APIVersion, m.Kind)
138130
if _, exists := tracker[apiKind]; !exists {
139131
tracker[apiKind] = make(map[string]map[string]manifestInfo)
@@ -143,9 +135,6 @@ func checkDuplicateManifests(m Manifest, path string, result *ValidationResult,
143135
}
144136

145137
key := m.Metadata.Name
146-
if nodeID != "" {
147-
key = fmt.Sprintf("%s|%s", m.Metadata.Name, nodeID)
148-
}
149138

150139
if existing, exists := tracker[apiKind][ns][key]; exists {
151140
result.Valid = false

‎cmd/client/validators_test.go‎

Lines changed: 16 additions & 180 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ kind: Listener
2929
metadata:
3030
name: test-listener-http
3131
namespace: default
32-
annotations:
33-
envoy.kaasops.io/description: "HTTP listener on port 8080"
3432
spec:
3533
name: http
3634
address:
@@ -44,8 +42,6 @@ kind: Route
4442
metadata:
4543
name: test-route
4644
namespace: default
47-
annotations:
48-
envoy.kaasops.io/description: "Route returning a JSON message for path /test"
4945
spec:
5046
- name: test
5147
match:
@@ -61,8 +57,6 @@ kind: HttpFilter
6157
metadata:
6258
name: test-http-filter
6359
namespace: default
64-
annotations:
65-
envoy.kaasops.io/description: "Basic Envoy router HTTP filter"
6660
spec:
6761
- name: envoy.filters.http.router
6862
typed_config:
@@ -75,68 +69,12 @@ kind: Listener
7569
metadata:
7670
name: test-listener-http
7771
namespace: default
78-
annotations:
79-
envoy.kaasops.io/description: "Duplicate HTTP listener on port 8080"
8072
spec:
8173
name: http
8274
address:
8375
socket_address:
8476
address: 0.0.0.0
8577
port_value: 8080
86-
`
87-
// Create Listener resource with node-id (should be considered duplicate after our change)
88-
listenerYaml1 := `
89-
apiVersion: envoy.kaasops.io/v1alpha1
90-
kind: Listener
91-
metadata:
92-
name: test-listener
93-
namespace: default
94-
annotations:
95-
envoy.kaasops.io/node-id: node1
96-
spec:
97-
name: test
98-
`
99-
// Create duplicate Listener resource but with different node-id
100-
// (should be considered duplicate since node-id is ignored for non-VirtualService resources)
101-
listenerYaml2 := `
102-
apiVersion: envoy.kaasops.io/v1alpha1
103-
kind: Listener
104-
metadata:
105-
name: test-listener
106-
namespace: default
107-
annotations:
108-
envoy.kaasops.io/node-id: node2
109-
spec:
110-
name: test
111-
`
112-
// Create VirtualService resource with node-id
113-
virtualServiceYaml1 := `
114-
apiVersion: envoy.kaasops.io/v1alpha1
115-
kind: VirtualService
116-
metadata:
117-
name: test-vs
118-
namespace: default
119-
annotations:
120-
envoy.kaasops.io/node-id: node1
121-
spec:
122-
virtualHost:
123-
domains:
124-
- example.com
125-
`
126-
// Create duplicate VirtualService resource but with different node-id
127-
// (should NOT be considered duplicate since node-id is considered for VirtualService resources)
128-
virtualServiceYaml2 := `
129-
apiVersion: envoy.kaasops.io/v1alpha1
130-
kind: VirtualService
131-
metadata:
132-
name: test-vs
133-
namespace: default
134-
annotations:
135-
envoy.kaasops.io/node-id: node2
136-
spec:
137-
virtualHost:
138-
domains:
139-
- example.com
14078
`
14179
// Create a file with invalid YAML
14280
invalidYaml := `
@@ -168,28 +106,6 @@ spec:
168106
if err := os.WriteFile(filepath.Join(subDir, "valid3.yaml"), []byte(validYaml3), 0644); err != nil {
169107
t.Fatalf("Failed to write test file: %v", err)
170108
}
171-
// Only include one Listener resource to avoid duplicates
172-
if err := os.WriteFile(filepath.Join(tempDir, "listener1.yaml"), []byte(listenerYaml1), 0644); err != nil {
173-
t.Fatalf("Failed to write test file: %v", err)
174-
}
175-
// Include both VirtualService resources with different node-ids
176-
// These should not be considered duplicates with our code change
177-
if err := os.WriteFile(filepath.Join(tempDir, "vs1.yaml"), []byte(virtualServiceYaml1), 0644); err != nil {
178-
t.Fatalf("Failed to write test file: %v", err)
179-
}
180-
if err := os.WriteFile(filepath.Join(tempDir, "vs2.yaml"), []byte(virtualServiceYaml2), 0644); err != nil {
181-
t.Fatalf("Failed to write test file: %v", err)
182-
}
183-
184-
case "node-id-duplicate":
185-
// Create a directory with Listener resources that have the same name but different node-ids
186-
// After our code change, these should be considered duplicates
187-
if err := os.WriteFile(filepath.Join(tempDir, "listener1.yaml"), []byte(listenerYaml1), 0644); err != nil {
188-
t.Fatalf("Failed to write test file: %v", err)
189-
}
190-
if err := os.WriteFile(filepath.Join(tempDir, "listener2.yaml"), []byte(listenerYaml2), 0644); err != nil {
191-
t.Fatalf("Failed to write test file: %v", err)
192-
}
193109

194110
case "duplicate":
195111
// Create files with duplicates for the "duplicate" test case
@@ -304,9 +220,8 @@ func TestDuplicateValidator(t *testing.T) {
304220
APIVersion: "apps/v1",
305221
Kind: "Deployment",
306222
Metadata: struct {
307-
Name string `yaml:"name"`
308-
Namespace string `yaml:"namespace"`
309-
Annotations map[string]string `yaml:"annotations"`
223+
Name string `yaml:"name"`
224+
Namespace string `yaml:"namespace"`
310225
}{
311226
Name: "test-deployment-1",
312227
Namespace: "default",
@@ -316,9 +231,8 @@ func TestDuplicateValidator(t *testing.T) {
316231
APIVersion: "apps/v1",
317232
Kind: "Deployment",
318233
Metadata: struct {
319-
Name string `yaml:"name"`
320-
Namespace string `yaml:"namespace"`
321-
Annotations map[string]string `yaml:"annotations"`
234+
Name string `yaml:"name"`
235+
Namespace string `yaml:"namespace"`
322236
}{
323237
Name: "test-deployment-2",
324238
Namespace: "default",
@@ -334,9 +248,8 @@ func TestDuplicateValidator(t *testing.T) {
334248
APIVersion: "apps/v1",
335249
Kind: "Deployment",
336250
Metadata: struct {
337-
Name string `yaml:"name"`
338-
Namespace string `yaml:"namespace"`
339-
Annotations map[string]string `yaml:"annotations"`
251+
Name string `yaml:"name"`
252+
Namespace string `yaml:"namespace"`
340253
}{
341254
Name: "test-deployment",
342255
Namespace: "default",
@@ -346,9 +259,8 @@ func TestDuplicateValidator(t *testing.T) {
346259
APIVersion: "apps/v1",
347260
Kind: "Deployment",
348261
Metadata: struct {
349-
Name string `yaml:"name"`
350-
Namespace string `yaml:"namespace"`
351-
Annotations map[string]string `yaml:"annotations"`
262+
Name string `yaml:"name"`
263+
Namespace string `yaml:"namespace"`
352264
}{
353265
Name: "test-deployment",
354266
Namespace: "default",
@@ -364,9 +276,8 @@ func TestDuplicateValidator(t *testing.T) {
364276
APIVersion: "apps/v1",
365277
Kind: "Deployment",
366278
Metadata: struct {
367-
Name string `yaml:"name"`
368-
Namespace string `yaml:"namespace"`
369-
Annotations map[string]string `yaml:"annotations"`
279+
Name string `yaml:"name"`
280+
Namespace string `yaml:"namespace"`
370281
}{
371282
Name: "test-deployment",
372283
Namespace: "default",
@@ -376,9 +287,8 @@ func TestDuplicateValidator(t *testing.T) {
376287
APIVersion: "apps/v1",
377288
Kind: "Deployment",
378289
Metadata: struct {
379-
Name string `yaml:"name"`
380-
Namespace string `yaml:"namespace"`
381-
Annotations map[string]string `yaml:"annotations"`
290+
Name string `yaml:"name"`
291+
Namespace string `yaml:"namespace"`
382292
}{
383293
Name: "test-deployment",
384294
Namespace: "test",
@@ -394,9 +304,8 @@ func TestDuplicateValidator(t *testing.T) {
394304
APIVersion: "apps/v1",
395305
Kind: "Deployment",
396306
Metadata: struct {
397-
Name string `yaml:"name"`
398-
Namespace string `yaml:"namespace"`
399-
Annotations map[string]string `yaml:"annotations"`
307+
Name string `yaml:"name"`
308+
Namespace string `yaml:"namespace"`
400309
}{
401310
Name: "test-resource",
402311
Namespace: "default",
@@ -406,9 +315,8 @@ func TestDuplicateValidator(t *testing.T) {
406315
APIVersion: "v1",
407316
Kind: "Service",
408317
Metadata: struct {
409-
Name string `yaml:"name"`
410-
Namespace string `yaml:"namespace"`
411-
Annotations map[string]string `yaml:"annotations"`
318+
Name string `yaml:"name"`
319+
Namespace string `yaml:"namespace"`
412320
}{
413321
Name: "test-resource",
414322
Namespace: "default",
@@ -417,78 +325,6 @@ func TestDuplicateValidator(t *testing.T) {
417325
},
418326
expectDupe: false,
419327
},
420-
{
421-
name: "Same name, same kind (Listener), different node-id - should be duplicate after change",
422-
manifests: []Manifest{
423-
{
424-
APIVersion: "envoy.kaasops.io/v1alpha1",
425-
Kind: "Listener",
426-
Metadata: struct {
427-
Name string `yaml:"name"`
428-
Namespace string `yaml:"namespace"`
429-
Annotations map[string]string `yaml:"annotations"`
430-
}{
431-
Name: "test-listener",
432-
Namespace: "default",
433-
Annotations: map[string]string{
434-
"envoy.kaasops.io/node-id": "node1",
435-
},
436-
},
437-
},
438-
{
439-
APIVersion: "envoy.kaasops.io/v1alpha1",
440-
Kind: "Listener",
441-
Metadata: struct {
442-
Name string `yaml:"name"`
443-
Namespace string `yaml:"namespace"`
444-
Annotations map[string]string `yaml:"annotations"`
445-
}{
446-
Name: "test-listener",
447-
Namespace: "default",
448-
Annotations: map[string]string{
449-
"envoy.kaasops.io/node-id": "node2",
450-
},
451-
},
452-
},
453-
},
454-
expectDupe: true, // Now we expect a duplicate because node-id is ignored for non-VirtualService resources
455-
},
456-
{
457-
name: "Same name, same kind (VirtualService), different node-id - should NOT be duplicate",
458-
manifests: []Manifest{
459-
{
460-
APIVersion: "envoy.kaasops.io/v1alpha1",
461-
Kind: "VirtualService",
462-
Metadata: struct {
463-
Name string `yaml:"name"`
464-
Namespace string `yaml:"namespace"`
465-
Annotations map[string]string `yaml:"annotations"`
466-
}{
467-
Name: "test-vs",
468-
Namespace: "default",
469-
Annotations: map[string]string{
470-
"envoy.kaasops.io/node-id": "node1",
471-
},
472-
},
473-
},
474-
{
475-
APIVersion: "envoy.kaasops.io/v1alpha1",
476-
Kind: "VirtualService",
477-
Metadata: struct {
478-
Name string `yaml:"name"`
479-
Namespace string `yaml:"namespace"`
480-
Annotations map[string]string `yaml:"annotations"`
481-
}{
482-
Name: "test-vs",
483-
Namespace: "default",
484-
Annotations: map[string]string{
485-
"envoy.kaasops.io/node-id": "node2",
486-
},
487-
},
488-
},
489-
},
490-
expectDupe: false, // Not a duplicate because node-id is considered for VirtualService resources
491-
},
492328
}
493329

494330
for _, tt := range tests {

0 commit comments

Comments
 (0)