OCM-8058 | fix: Ensure --name option is required for working with Kub…

…eletConfigs on HCP clusters
openshift · May 16, 2024 · 89ab77e · 89ab77e
1 parent 0e00dea
commit 89ab77e
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 19 deletions.
diff --git a/cmd/create/kubeletconfig/cmd.go b/cmd/create/kubeletconfig/cmd.go
@@ -81,14 +81,19 @@ func CreateKubeletConfigRunner(options *KubeletConfigOptions) rosa.CommandRunner
 				return fmt.Errorf("A KubeletConfig for cluster '%s' already exists. "+
 					"You should edit it via 'rosa edit kubeletconfig'", clusterKey)
 			}
-		}
+		} else {
+			options.Name, err = PromptForName(options.Name)
+			if err != nil {
+				return err
+			}
 
-		name, err := PromptForName(options.Name, false)
-		if err != nil {
-			return nil
+			err = options.ValidateForHypershift()
+			if err != nil {
+				return err
+			}
 		}
 
-		requestedPids, err := ValidateOrPromptForRequestedPidsLimit(options.PodPidsLimit, clusterKey, nil, r)
+		options.PodPidsLimit, err = ValidateOrPromptForRequestedPidsLimit(options.PodPidsLimit, clusterKey, nil, r)
 		if err != nil {
 			return err
 		}
@@ -102,7 +107,7 @@ func CreateKubeletConfigRunner(options *KubeletConfigOptions) rosa.CommandRunner
 		}
 
 		r.Reporter.Debugf("Creating KubeletConfig for cluster '%s'", clusterKey)
-		kubeletConfigArgs := ocm.KubeletConfigArgs{PodPidsLimit: requestedPids, Name: name}
+		kubeletConfigArgs := ocm.KubeletConfigArgs{PodPidsLimit: options.PodPidsLimit, Name: options.Name}
 
 		_, err = r.OCMClient.CreateKubeletConfig(cluster.ID(), kubeletConfigArgs)
 		if err != nil {

diff --git a/cmd/create/kubeletconfig/cmd_test.go b/cmd/create/kubeletconfig/cmd_test.go
@@ -9,6 +9,7 @@ import (
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/openshift-online/ocm-sdk-go/testing"
 
+	"github.com/openshift/rosa/pkg/interactive"
 	. "github.com/openshift/rosa/pkg/kubeletconfig"
 	"github.com/openshift/rosa/pkg/output"
 	. "github.com/openshift/rosa/pkg/test"
@@ -39,10 +40,12 @@ var _ = Describe("create kubeletconfig", func() {
 		BeforeEach(func() {
 			t = NewTestRuntime()
 			output.SetOutput("")
+			interactive.SetEnabled(false)
 		})
 
 		AfterEach(func() {
 			output.SetOutput("")
+			interactive.SetEnabled(false)
 		})
 
 		It("Returns an error if the cluster does not exist", func() {
@@ -121,6 +124,30 @@ var _ = Describe("create kubeletconfig", func() {
 				ContainSubstring("Failed getting KubeletConfig for cluster 'cluster'"))
 		})
 
+		It("Returns an error if no name specified for HCP KubeletConfig", func() {
+			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
+				c.State(cmv1.ClusterStateReady)
+				b := cmv1.HypershiftBuilder{}
+				b.Enabled(true)
+				c.Hypershift(&b)
+
+			})
+
+			t.ApiServer.AppendHandlers(
+				testing.RespondWithJSON(
+					http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster})))
+			t.SetCluster("cluster", cluster)
+
+			options := NewKubeletConfigOptions()
+			options.PodPidsLimit = 10000
+
+			runner := CreateKubeletConfigRunner(options)
+
+			err := runner(context.Background(), t.RosaRuntime, nil, nil)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("The --name flag is required for Hosted Control Plane clusters."))
+		})
+
 		It("Creates the KubeletConfig for HCP clusters", func() {
 			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
 				c.State(cmv1.ClusterStateReady)
@@ -143,6 +170,7 @@ var _ = Describe("create kubeletconfig", func() {
 
 			options := NewKubeletConfigOptions()
 			options.PodPidsLimit = 10000
+			options.Name = "test"
 
 			runner := CreateKubeletConfigRunner(options)
 			t.StdOutReader.Record()
@@ -176,6 +204,7 @@ var _ = Describe("create kubeletconfig", func() {
 
 			options := NewKubeletConfigOptions()
 			options.PodPidsLimit = 10000
+			options.Name = "test"
 
 			runner := CreateKubeletConfigRunner(options)
 

diff --git a/cmd/describe/kubeletconfig/cmd.go b/cmd/describe/kubeletconfig/cmd.go
@@ -73,7 +73,7 @@ func DescribeKubeletConfigRunner(options *KubeletConfigOptions) rosa.CommandRunn
 		var exists bool
 
 		if cluster.Hypershift().Enabled() {
-			options.Name, err = PromptForName(options.Name, true)
+			options.Name, err = PromptForName(options.Name)
 			if err != nil {
 				return err
 			}

diff --git a/cmd/describe/kubeletconfig/cmd_test.go b/cmd/describe/kubeletconfig/cmd_test.go
@@ -129,6 +129,30 @@ var _ = Describe("Describe KubeletConfig", func() {
 				Equal("The KubeletConfig specified does not exist for cluster 'cluster'"))
 		})
 
+		It("Returns an error if no name specified for HCP KubeletConfig", func() {
+			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
+				c.State(cmv1.ClusterStateReady)
+				b := cmv1.HypershiftBuilder{}
+				b.Enabled(true)
+				c.Hypershift(&b)
+
+			})
+
+			t.ApiServer.AppendHandlers(
+				RespondWithJSON(
+					http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster})))
+			t.SetCluster("cluster", cluster)
+
+			options := NewKubeletConfigOptions()
+			options.PodPidsLimit = 10000
+
+			runner := DescribeKubeletConfigRunner(options)
+
+			err := runner(context.Background(), t.RosaRuntime, nil, nil)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("The --name flag is required for Hosted Control Plane clusters."))
+		})
+
 		It("Returns an error if failing to read HCP KubeletConfig", func() {
 			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
 				c.State(cmv1.ClusterStateReady)

diff --git a/cmd/dlt/kubeletconfig/cmd.go b/cmd/dlt/kubeletconfig/cmd.go
@@ -75,11 +75,14 @@ func DeleteKubeletConfigRunner(options *KubeletConfigOptions) rosa.CommandRunner
 		}
 
 		if cluster.Hypershift().Enabled() {
-			options.Name, err = PromptForName(options.Name, true)
+			options.Name, err = PromptForName(options.Name)
+			if err != nil {
+				return err
+			}
+			err = options.ValidateForHypershift()
 			if err != nil {
 				return err
 			}
-			options.ValidateForHypershift()
 			err = r.OCMClient.DeleteKubeletConfigByName(ctx, cluster.ID(), options.Name)
 		} else {
 			err = r.OCMClient.DeleteKubeletConfig(ctx, cluster.ID())

diff --git a/cmd/dlt/kubeletconfig/cmd_test.go b/cmd/dlt/kubeletconfig/cmd_test.go
@@ -10,6 +10,7 @@ import (
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	. "github.com/openshift-online/ocm-sdk-go/testing"
 
+	"github.com/openshift/rosa/pkg/interactive"
 	. "github.com/openshift/rosa/pkg/kubeletconfig"
 	"github.com/openshift/rosa/pkg/output"
 	. "github.com/openshift/rosa/pkg/test"
@@ -39,10 +40,12 @@ var _ = Describe("delete kubeletconfig", func() {
 		BeforeEach(func() {
 			t = NewTestRuntime()
 			output.SetOutput("")
+			interactive.SetEnabled(false)
 		})
 
 		AfterEach(func() {
 			output.SetOutput("")
+			interactive.SetEnabled(false)
 		})
 
 		It("Returns an error if the cluster does not exist", func() {
@@ -56,6 +59,30 @@ var _ = Describe("delete kubeletconfig", func() {
 				Equal("There is no cluster with identifier or name 'cluster'"))
 		})
 
+		It("Returns an error if no name specified for HCP KubeletConfig", func() {
+			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
+				c.State(cmv1.ClusterStateReady)
+				b := cmv1.HypershiftBuilder{}
+				b.Enabled(true)
+				c.Hypershift(&b)
+
+			})
+
+			t.ApiServer.AppendHandlers(
+				RespondWithJSON(
+					http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster})))
+			t.SetCluster("cluster", cluster)
+
+			options := NewKubeletConfigOptions()
+			options.PodPidsLimit = 10000
+
+			runner := DeleteKubeletConfigRunner(options)
+
+			err := runner(context.Background(), t.RosaRuntime, nil, nil)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("The --name flag is required for Hosted Control Plane clusters."))
+		})
+
 		It("Deletes KubeletConfig by name for HCP Clusters", func() {
 
 			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {

diff --git a/cmd/edit/kubeletconfig/cmd.go b/cmd/edit/kubeletconfig/cmd.go
@@ -81,12 +81,15 @@ func EditKubeletConfigRunner(options *KubeletConfigOptions) rosa.CommandRunner {
 		var exists bool
 
 		if cluster.Hypershift().Enabled() {
-			options.Name, err = PromptForName(options.Name, true)
+			options.Name, err = PromptForName(options.Name)
 			if err != nil {
 				return err
 			}
 
-			options.ValidateForHypershift()
+			err = options.ValidateForHypershift()
+			if err != nil {
+				return err
+			}
 			kubeletconfig, exists, err = r.OCMClient.FindKubeletConfigByName(ctx, cluster.ID(), options.Name)
 		} else {
 			kubeletconfig, exists, err = r.OCMClient.GetClusterKubeletConfig(cluster.ID())

diff --git a/cmd/edit/kubeletconfig/cmd_test.go b/cmd/edit/kubeletconfig/cmd_test.go
@@ -57,6 +57,30 @@ var _ = Describe("edit kubeletconfig", func() {
 				Equal("There is no cluster with identifier or name 'cluster'"))
 		})
 
+		It("Returns an error if no name specified for HCP KubeletConfig", func() {
+			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {
+				c.State(cmv1.ClusterStateReady)
+				b := cmv1.HypershiftBuilder{}
+				b.Enabled(true)
+				c.Hypershift(&b)
+
+			})
+
+			t.ApiServer.AppendHandlers(
+				RespondWithJSON(
+					http.StatusOK, FormatClusterList([]*cmv1.Cluster{cluster})))
+			t.SetCluster("cluster", cluster)
+
+			options := NewKubeletConfigOptions()
+			options.PodPidsLimit = 10000
+
+			runner := EditKubeletConfigRunner(options)
+
+			err := runner(context.Background(), t.RosaRuntime, nil, nil)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("The --name flag is required for Hosted Control Plane clusters."))
+		})
+
 		It("Returns an error if the cluster is not ready", func() {
 
 			cluster := MockCluster(func(c *cmv1.ClusterBuilder) {

diff --git a/pkg/kubeletconfig/config.go b/pkg/kubeletconfig/config.go
@@ -53,19 +53,15 @@ func GetInteractiveInput(maxPidsLimit int, kubeletConfig *v1.KubeletConfig) inte
 	}
 }
 
-func PromptForName(requestedName string, nameRequired bool) (string, error) {
+func PromptForName(requestedName string) (string, error) {
 
-	if requestedName == "" && nameRequired {
-		interactive.Enable()
-	}
-
-	if interactive.Enabled() {
+	if requestedName == "" && interactive.Enabled() {
 		return interactive.GetString(interactive.Input{
 			Question: InteractiveNameHelpPrompt,
 			Help:     InteractiveNameHelp,
 			Options:  nil,
 			Default:  requestedName,
-			Required: false,
+			Required: true,
 		})
 	}
 

diff --git a/pkg/kubeletconfig/consts.go b/pkg/kubeletconfig/consts.go
@@ -6,7 +6,7 @@ const (
 	MaxUnsafePodPidsLimit          = 3694303
 	NameOption                     = "name"
 	NameOptionDefaultValue         = ""
-	NameOptionUsage                = "Name of the KubeletConfig"
+	NameOptionUsage                = "Name of the KubeletConfig (required for Hosted Control Plane clusters)"
 	PodPidsLimitOption             = "pod-pids-limit"
 	PodPidsLimitOptionUsage        = "Sets the requested pod_pids_limit for this KubeletConfig."
 	PodPidsLimitOptionDefaultValue = 0