From 79d0754d03fc94dd33f9b76539c5770e8357ba58 Mon Sep 17 00:00:00 2001
From: Mykola Morhun <mmorhun@redhat.com>
Date: Thu, 21 Nov 2024 11:20:38 +0200
Subject: [PATCH] Prevent sequential underscores in quay robot account name
 (#156)

* Prevent sequential underscores in quay robot account name

* Use regular expressions instead of for loop to remove repetative underscores

* Fixup for case when underscores cause by other symbols replacement
---
 controllers/imagerepository_controller.go     |  8 +++
 .../imagerepository_controller_unit_test.go   | 61 +++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go
index c1435df..c9ea363 100644
--- a/controllers/imagerepository_controller.go
+++ b/controllers/imagerepository_controller.go
@@ -22,6 +22,7 @@ import (
 	"encoding/base64"
 	"encoding/hex"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -864,9 +865,16 @@ func generateQuayRobotAccountName(imageRepositoryName string, isPullOnly bool) s
 	if isPullOnly {
 		robotAccountName += "_pull"
 	}
+	robotAccountName = removeDuplicateUnderscores(robotAccountName)
 	return robotAccountName
 }
 
+// removeDuplicateUnderscores replaces sequence of underscores with only one.
+// Example: ab__cd___e => ab_cd_e
+func removeDuplicateUnderscores(s string) string {
+	return regexp.MustCompile("_+").ReplaceAllString(s, "_")
+}
+
 func getSecretName(imageRepository *imagerepositoryv1alpha1.ImageRepository, isPullOnly bool) string {
 	secretName := imageRepository.Name
 	if len(secretName) > 220 {
diff --git a/controllers/imagerepository_controller_unit_test.go b/controllers/imagerepository_controller_unit_test.go
index 54126d0..185e8dd 100644
--- a/controllers/imagerepository_controller_unit_test.go
+++ b/controllers/imagerepository_controller_unit_test.go
@@ -58,6 +58,18 @@ func TestGenerateQuayRobotAccountName(t *testing.T) {
 			isPull:                         true,
 			expectedRobotAccountNamePrefix: expectedRobotAccountLongPrefix,
 		},
+		{
+			name:                           "Should prevent multiple underscores in Quay robot account name",
+			imageRepositoryName:            "my__app_tenant_component____name",
+			isPull:                         false,
+			expectedRobotAccountNamePrefix: "my_app_tenant_component_name",
+		},
+		{
+			name:                           "Should prevent multiple underscores in Quay robot account name cause be other symbols replacement",
+			imageRepositoryName:            "my_._image_/_repository_-_name_",
+			isPull:                         true,
+			expectedRobotAccountNamePrefix: "my_image_repository_name",
+		},
 	}
 
 	for _, tc := range testCases {
@@ -79,6 +91,55 @@ func TestGenerateQuayRobotAccountName(t *testing.T) {
 	}
 }
 
+func TestRemoveDuplicateUnderscores(t *testing.T) {
+	testCases := []struct {
+		name     string
+		arg      string
+		expected string
+	}{
+		{
+			name:     "Should not modify string without repeating underscores",
+			arg:      "my_test_string",
+			expected: "my_test_string",
+		},
+		{
+			name:     "Should handle double underscores",
+			arg:      "my_test__string",
+			expected: "my_test_string",
+		},
+		{
+			name:     "Should handle multiple underscores",
+			arg:      "my_test____________string",
+			expected: "my_test_string",
+		},
+		{
+			name:     "Should handle underscores in many places",
+			arg:      "my____test__string",
+			expected: "my_test_string",
+		},
+		{
+			name:     "Should handle underscores at the beginning and end",
+			arg:      "__my_test__string__",
+			expected: "_my_test_string_",
+		},
+		{
+			name:     "Should handle empty string",
+			arg:      "",
+			expected: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := removeDuplicateUnderscores(tc.arg)
+
+			if got != tc.expected {
+				t.Errorf("Expected %s, but got %s", tc.expected, got)
+			}
+		})
+	}
+}
+
 func TestGetSecretName(t *testing.T) {
 	longImageRepositoryCrName := getRandomString(300)
 	expectedSecretLongPrefix := longImageRepositoryCrName[0:220]