Skip to content

Commit 12af74b

Browse files
mtuliomtulio
committed
svc/sg/tags: introduce owner check for delete SG flow on BYO SG
1 parent 948e8f8 commit 12af74b

File tree

2 files changed

+289
-0
lines changed

2 files changed

+289
-0
lines changed

‎pkg/providers/v1/tags.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,31 @@ func (t *awsTagging) hasClusterTag(tags []ec2types.Tag) bool {
154154
return false
155155
}
156156

157+
// hasClusterTagOwned checks if the resource has the cluster tag with the value "owned"
158+
// This is used to determine if the resource is owned by the controller.
159+
// It checks both legacy and new tags.
160+
func (t *awsTagging) hasClusterTagOwned(tags []ec2types.Tag) bool {
161+
if len(t.ClusterID) == 0 {
162+
return false
163+
}
164+
clusterTagKey := t.clusterTagKey()
165+
for _, tag := range tags {
166+
tagKey := aws.ToString(tag.Key)
167+
tagValue := aws.ToString(tag.Value)
168+
169+
// For legacy tags, the cluster ID is the value, not "owned"
170+
if (tagKey == TagNameKubernetesClusterLegacy) && (tagValue == t.ClusterID) {
171+
return true
172+
}
173+
174+
// For new tags, check if it's the cluster tag with "owned" value
175+
if tagKey == clusterTagKey && tagValue == string(ResourceLifecycleOwned) {
176+
return true
177+
}
178+
}
179+
return false
180+
}
181+
157182
func (t *awsTagging) hasNoClusterPrefixTag(tags []ec2types.Tag) bool {
158183
for _, tag := range tags {
159184
if strings.HasPrefix(aws.ToString(tag.Key), TagNameKubernetesClusterPrefix) {

‎pkg/providers/v1/tags_test.go‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,3 +343,267 @@ func TestUntagResource(t *testing.T) {
343343
})
344344
}
345345
}
346+
347+
func TestHasClusterTagOwned(t *testing.T) {
348+
tests := []struct {
349+
name string
350+
clusterID string
351+
tags []ec2types.Tag
352+
expected bool
353+
}{
354+
{
355+
name: "empty cluster ID returns false",
356+
clusterID: "",
357+
tags: []ec2types.Tag{
358+
{
359+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
360+
Value: aws.String("owned"),
361+
},
362+
},
363+
expected: false,
364+
},
365+
{
366+
name: "legacy tag with matching cluster ID returns true",
367+
clusterID: "test-cluster",
368+
tags: []ec2types.Tag{
369+
{
370+
Key: aws.String("KubernetesCluster"),
371+
Value: aws.String("test-cluster"),
372+
},
373+
},
374+
expected: true,
375+
},
376+
{
377+
name: "legacy tag with non-matching cluster ID returns false",
378+
clusterID: "test-cluster",
379+
tags: []ec2types.Tag{
380+
{
381+
Key: aws.String("KubernetesCluster"),
382+
Value: aws.String("other-cluster"),
383+
},
384+
},
385+
expected: false,
386+
},
387+
{
388+
name: "new tag with owned value returns true",
389+
clusterID: "test-cluster",
390+
tags: []ec2types.Tag{
391+
{
392+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
393+
Value: aws.String("owned"),
394+
},
395+
},
396+
expected: true,
397+
},
398+
{
399+
name: "new tag with shared value returns false",
400+
clusterID: "test-cluster",
401+
tags: []ec2types.Tag{
402+
{
403+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
404+
Value: aws.String("shared"),
405+
},
406+
},
407+
expected: false,
408+
},
409+
{
410+
name: "new tag with wrong cluster ID returns false",
411+
clusterID: "test-cluster",
412+
tags: []ec2types.Tag{
413+
{
414+
Key: aws.String("kubernetes.io/cluster/other-cluster"),
415+
Value: aws.String("owned"),
416+
},
417+
},
418+
expected: false,
419+
},
420+
{
421+
name: "no matching tags returns false",
422+
clusterID: "test-cluster",
423+
tags: []ec2types.Tag{
424+
{
425+
Key: aws.String("some-other-tag"),
426+
Value: aws.String("some-value"),
427+
},
428+
},
429+
expected: false,
430+
},
431+
{
432+
name: "empty tags list returns false",
433+
clusterID: "test-cluster",
434+
tags: []ec2types.Tag{},
435+
expected: false,
436+
},
437+
{
438+
name: "both legacy and new tags present - legacy matches",
439+
clusterID: "test-cluster",
440+
tags: []ec2types.Tag{
441+
{
442+
Key: aws.String("KubernetesCluster"),
443+
Value: aws.String("test-cluster"),
444+
},
445+
{
446+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
447+
Value: aws.String("shared"),
448+
},
449+
},
450+
expected: true,
451+
},
452+
{
453+
name: "both legacy and new tags present - new matches",
454+
clusterID: "test-cluster",
455+
tags: []ec2types.Tag{
456+
{
457+
Key: aws.String("KubernetesCluster"),
458+
Value: aws.String("other-cluster"),
459+
},
460+
{
461+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
462+
Value: aws.String("owned"),
463+
},
464+
},
465+
expected: true,
466+
},
467+
{
468+
name: "both legacy and new tags present - neither matches",
469+
clusterID: "test-cluster",
470+
tags: []ec2types.Tag{
471+
{
472+
Key: aws.String("KubernetesCluster"),
473+
Value: aws.String("other-cluster"),
474+
},
475+
{
476+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
477+
Value: aws.String("shared"),
478+
},
479+
},
480+
expected: false,
481+
},
482+
{
483+
name: "tags with nil key returns false",
484+
clusterID: "test-cluster",
485+
tags: []ec2types.Tag{
486+
{
487+
Key: nil,
488+
Value: aws.String("test-cluster"),
489+
},
490+
},
491+
expected: false,
492+
},
493+
{
494+
name: "tags with nil value returns false",
495+
clusterID: "test-cluster",
496+
tags: []ec2types.Tag{
497+
{
498+
Key: aws.String("KubernetesCluster"),
499+
Value: nil,
500+
},
501+
},
502+
expected: false,
503+
},
504+
{
505+
name: "legacy tag with empty value returns false",
506+
clusterID: "test-cluster",
507+
tags: []ec2types.Tag{
508+
{
509+
Key: aws.String("KubernetesCluster"),
510+
Value: aws.String(""),
511+
},
512+
},
513+
expected: false,
514+
},
515+
{
516+
name: "new tag with empty value returns false",
517+
clusterID: "test-cluster",
518+
tags: []ec2types.Tag{
519+
{
520+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
521+
Value: aws.String(""),
522+
},
523+
},
524+
expected: false,
525+
},
526+
{
527+
name: "multiple tags with one legacy match",
528+
clusterID: "test-cluster",
529+
tags: []ec2types.Tag{
530+
{
531+
Key: aws.String("some-tag"),
532+
Value: aws.String("some-value"),
533+
},
534+
{
535+
Key: aws.String("KubernetesCluster"),
536+
Value: aws.String("test-cluster"),
537+
},
538+
{
539+
Key: aws.String("other-tag"),
540+
Value: aws.String("other-value"),
541+
},
542+
},
543+
expected: true,
544+
},
545+
{
546+
name: "multiple tags with one new match",
547+
clusterID: "test-cluster",
548+
tags: []ec2types.Tag{
549+
{
550+
Key: aws.String("some-tag"),
551+
Value: aws.String("some-value"),
552+
},
553+
{
554+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
555+
Value: aws.String("owned"),
556+
},
557+
{
558+
Key: aws.String("other-tag"),
559+
Value: aws.String("other-value"),
560+
},
561+
},
562+
expected: true,
563+
},
564+
{
565+
name: "case sensitivity - legacy tag key case mismatch",
566+
clusterID: "test-cluster",
567+
tags: []ec2types.Tag{
568+
{
569+
Key: aws.String("kubernetescluster"),
570+
Value: aws.String("test-cluster"),
571+
},
572+
},
573+
expected: false,
574+
},
575+
{
576+
name: "case sensitivity - new tag key case mismatch",
577+
clusterID: "test-cluster",
578+
tags: []ec2types.Tag{
579+
{
580+
Key: aws.String("Kubernetes.io/cluster/test-cluster"),
581+
Value: aws.String("owned"),
582+
},
583+
},
584+
expected: false,
585+
},
586+
{
587+
name: "case sensitivity - new tag value case mismatch",
588+
clusterID: "test-cluster",
589+
tags: []ec2types.Tag{
590+
{
591+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
592+
Value: aws.String("Owned"),
593+
},
594+
},
595+
expected: false,
596+
},
597+
}
598+
599+
for _, tt := range tests {
600+
t.Run(tt.name, func(t *testing.T) {
601+
tagging := awsTagging{
602+
ClusterID: tt.clusterID,
603+
}
604+
605+
result := tagging.hasClusterTagOwned(tt.tags)
606+
assert.Equal(t, tt.expected, result, "hasClusterTagOwned returned unexpected result")
607+
})
608+
}
609+
}

0 commit comments

Comments
 (0)