From 7c924c3c4cbd4c427a867848900df7533edc278c Mon Sep 17 00:00:00 2001
From: Vivek Singh <vivek.singh@veeam.com>
Date: Thu, 26 Oct 2023 10:16:38 +0200
Subject: [PATCH] Fix the way action set name was being configured (#2430)

* Fix the way action set name was configred

`kanctl` was able to configure the actionset name using the flag called
`action-set`.
This has two problems
1. The better flag to give actionset name would be `--name` instead of
`--action-set`. Becuase there is another flag called `action`, so it
gets confusing between `--action-set` and `--action`.
2. Even though user would expect that if they have provided `--name`
the actionset would be named exactly that. But the problem was we
were using `generateName` in code that resulted in a string being
added in the name.

* Fix golint

* Fix typo

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 pkg/kanctl/actionset.go      | 15 ++++++++-------
 pkg/kanctl/actionset_test.go | 12 +++++++++---
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/pkg/kanctl/actionset.go b/pkg/kanctl/actionset.go
index 59a4c3ffb6..47849678eb 100644
--- a/pkg/kanctl/actionset.go
+++ b/pkg/kanctl/actionset.go
@@ -27,6 +27,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/yaml"
 
@@ -40,7 +41,7 @@ import (
 
 const (
 	actionFlagName                       = "action"
-	actionSetFlagName                    = "action-set"
+	actionSetFlagName                    = "name"
 	blueprintFlagName                    = "blueprint"
 	configMapsFlagName                   = "config-maps"
 	deploymentFlagName                   = "deployment"
@@ -190,7 +191,7 @@ func newActionSet(params *PerformParams) (*crv1alpha1.ActionSet, error) {
 
 	return &crv1alpha1.ActionSet{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: name,
+			Name: name,
 		},
 		Spec: &crv1alpha1.ActionSetSpec{
 			Actions: actions,
@@ -254,7 +255,7 @@ func ChildActionSet(parent *crv1alpha1.ActionSet, params *PerformParams) (*crv1a
 
 	return &crv1alpha1.ActionSet{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: name,
+			Name: name,
 		},
 		Spec: &crv1alpha1.ActionSetSpec{
 			Actions: actions,
@@ -741,19 +742,19 @@ func max(x, y int) int {
 
 func generateActionSetName(p *PerformParams) (string, error) {
 	if p.ActionSetName != "" {
-		return fmt.Sprintf("%s-", p.ActionSetName), nil
+		return p.ActionSetName, nil
 	}
 
 	if p.ActionName != "" {
 		if p.ParentName != "" {
-			return fmt.Sprintf("%s-%s-", p.ActionName, p.ParentName), nil
+			return fmt.Sprintf("%s-%s-%s", p.ActionName, p.ParentName, rand.String(5)), nil
 		}
 
-		return fmt.Sprintf("%s-", p.ActionName), nil
+		return fmt.Sprintf("%s-%s", p.ActionName, rand.String(5)), nil
 	}
 
 	if p.ParentName != "" {
-		return fmt.Sprintf("%s-", p.ParentName), nil
+		return fmt.Sprintf("%s-%s", p.ParentName, rand.String(5)), nil
 	}
 
 	return "", errMissingFieldActionName
diff --git a/pkg/kanctl/actionset_test.go b/pkg/kanctl/actionset_test.go
index 12557d9d49..3605635857 100644
--- a/pkg/kanctl/actionset_test.go
+++ b/pkg/kanctl/actionset_test.go
@@ -89,8 +89,8 @@ func (k *KanctlTestSuite) TestGenerateActionSetName(c *C) {
 		{actionName: "my-action", actionSetName: "", parentName: "", expected: "my-action-"},
 		{actionName: "my-action", actionSetName: "", parentName: "parent", expected: "my-action-parent-"},
 		{actionName: "", actionSetName: "", parentName: "parent", expected: "parent-"},
-		{actionName: "my-action", actionSetName: "my-override", parentName: "parent", expected: "my-override-"},
-		{actionName: "", actionSetName: "my-override", parentName: "", expected: "my-override-"},
+		{actionName: "my-action", actionSetName: "my-override", parentName: "parent", expected: "my-override"},
+		{actionName: "", actionSetName: "my-override", parentName: "", expected: "my-override"},
 	}
 
 	for _, tc := range testCases {
@@ -102,6 +102,12 @@ func (k *KanctlTestSuite) TestGenerateActionSetName(c *C) {
 
 		actual, err := generateActionSetName(params)
 		c.Assert(err, DeepEquals, tc.expectedErr)
-		c.Assert(actual, DeepEquals, tc.expected)
+		if tc.actionSetName != "" || tc.expected == "" {
+			// if --name is provided we just use that we dont derive name
+			c.Assert(actual, DeepEquals, tc.expected)
+		} else {
+			// random 5 chars are added at the end if name is derived by us
+			c.Assert(actual[0:len(actual)-5], DeepEquals, tc.expected)
+		}
 	}
 }