Merge pull request kubernetes#16978 from justinsb/strong_type_target

Support strong-typing for --target values
next-stack · Jan 19, 2025 · f925e9e · f925e9e
2 parents bd2a42c + 284b15b
commit f925e9e
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 19 deletions.
diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go
@@ -61,8 +61,11 @@ import (
 
 type CreateClusterOptions struct {
 	cloudup.NewClusterOptions
-	Yes                        bool
-	Target                     string
+	Yes bool
+
+	// Target is the type of target we will operate against (direct, dry-run, terraform)
+	Target cloudup.Target
+
 	ControlPlaneVolumeSize     int32
 	NodeVolumeSize             int32
 	ContainerRuntime           string
@@ -203,7 +206,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	}
 
 	cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Specify --yes to immediately create the cluster")
-	cmd.Flags().StringVar(&options.Target, "target", options.Target, fmt.Sprintf("Valid targets: %s, %s. Set this flag to %s if you want kOps to generate terraform", cloudup.TargetDirect, cloudup.TargetTerraform, cloudup.TargetTerraform))
+	cmd.Flags().Var(&options.Target, "target", fmt.Sprintf("Valid targets: %q, %q. Set this flag to %q if you want kOps to generate terraform", cloudup.TargetDirect, cloudup.TargetTerraform, cloudup.TargetTerraform))
 	cmd.RegisterFlagCompletionFunc("target", completeCreateClusterTarget(options))
 
 	// Configuration / state location
@@ -1010,7 +1013,7 @@ func completeNetworking(options *CreateClusterOptions) func(cmd *cobra.Command,
 
 func completeCreateClusterTarget(options *CreateClusterOptions) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		completions := []string{
+		completions := []cloudup.Target{
 			cloudup.TargetDirect,
 			cloudup.TargetDryRun,
 		}
@@ -1019,7 +1022,7 @@ func completeCreateClusterTarget(options *CreateClusterOptions) func(cmd *cobra.
 				completions = append(completions, cloudup.TargetTerraform)
 			}
 		}
-		return completions, cobra.ShellCompDirectiveNoFileComp
+		return toStringSlice(completions), cobra.ShellCompDirectiveNoFileComp
 	}
 }
 

diff --git a/cmd/kops/update_cluster.go b/cmd/kops/update_cluster.go
@@ -76,8 +76,11 @@ type UpdateClusterOptions struct {
 // which are shared with the reconcile cluster command.
 // The fields _not_ shared with the reconcile cluster command are the ones in CreateKubecfgOptions.
 type CoreUpdateClusterOptions struct {
-	Yes                bool
-	Target             string
+	Yes bool
+
+	// Target is the type of target we will operate against (direct, dry-run, terraform)
+	Target cloudup.Target
+
 	OutDir             string
 	SSHPublicKey       string
 	RunTasksOptions    fi.RunTasksOptions
@@ -123,7 +126,7 @@ func (o *UpdateClusterOptions) InitDefaults() {
 
 func (o *CoreUpdateClusterOptions) InitDefaults() {
 	o.Yes = false
-	o.Target = "direct"
+	o.Target = cloudup.TargetDirect
 	o.SSHPublicKey = ""
 	o.OutDir = ""
 	// By default we enforce the version skew between control plane and worker nodes
@@ -157,7 +160,7 @@ func NewCmdUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	}
 
 	cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Create cloud resources, without --yes update is in dry run mode")
-	cmd.Flags().StringVar(&options.Target, "target", options.Target, "Target - direct, terraform")
+	cmd.Flags().Var(&options.Target, "target", fmt.Sprintf("Target - %q, %q", cloudup.TargetDirect, cloudup.TargetTerraform))
 	cmd.RegisterFlagCompletionFunc("target", completeUpdateClusterTarget(f, &options.CoreUpdateClusterOptions))
 	cmd.Flags().StringVar(&options.SSHPublicKey, "ssh-public-key", options.SSHPublicKey, "SSH public key to use (deprecated: use kops create secret instead)")
 	cmd.Flags().StringVar(&options.OutDir, "out", options.OutDir, "Path to write any local output")
@@ -552,14 +555,14 @@ func completeUpdateClusterTarget(f commandutils.Factory, options *CoreUpdateClus
 
 		cluster, _, _, directive := GetClusterForCompletion(ctx, f, nil)
 		if cluster == nil {
-			return []string{
+			return toStringSlice([]cloudup.Target{
 				cloudup.TargetDirect,
 				cloudup.TargetDryRun,
 				cloudup.TargetTerraform,
-			}, directive
+			}), directive
 		}
 
-		completions := []string{
+		completions := []cloudup.Target{
 			cloudup.TargetDirect,
 			cloudup.TargetDryRun,
 		}
@@ -568,8 +571,16 @@ func completeUpdateClusterTarget(f commandutils.Factory, options *CoreUpdateClus
 				completions = append(completions, cloudup.TargetTerraform)
 			}
 		}
-		return completions, cobra.ShellCompDirectiveNoFileComp
+		return toStringSlice(completions), cobra.ShellCompDirectiveNoFileComp
+	}
+}
+
+func toStringSlice[T ~string](targets []T) []string {
+	strings := make([]string, len(targets))
+	for i, target := range targets {
+		strings[i] = string(target)
 	}
+	return strings
 }
 
 func completeLifecycleOverrides(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {

diff --git a/docs/cli/kops_create_cluster.md b/docs/cli/kops_create_cluster.md
diff --git a/docs/cli/kops_update_cluster.md b/docs/cli/kops_update_cluster.md
diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go
@@ -93,7 +93,7 @@ type ApplyClusterCmd struct {
 	InstanceGroups []*kops.InstanceGroup
 
 	// TargetName specifies how we are operating e.g. direct to GCE, or AWS, or dry-run, or terraform
-	TargetName string
+	TargetName Target
 
 	// Target is the fi.Target we will operate against
 	Target fi.CloudupTarget

diff --git a/upup/pkg/fi/cloudup/target.go b/upup/pkg/fi/cloudup/target.go
@@ -16,8 +16,42 @@ limitations under the License.
 
 package cloudup
 
+import (
+	"fmt"
+	"strings"
+
+	"github.com/spf13/pflag"
+)
+
+// Target is the type of target we are operating against.
+type Target string
+
 const (
-	TargetDirect    = "direct"
-	TargetDryRun    = "dryrun"
-	TargetTerraform = "terraform"
+	// TargetDirect means we will apply the changes directly to the cloud.
+	TargetDirect Target = "direct"
+	// TargetDryRun means we will not apply the changes but will print what would have been done.
+	TargetDryRun Target = "dryrun"
+	// TargetTerraform means we will generate terraform code.
+	TargetTerraform Target = "terraform"
 )
+
+// Target can be used as a flag value.
+var _ pflag.Value = (*Target)(nil)
+
+func (t *Target) String() string {
+	return string(*t)
+}
+
+func (t *Target) Set(value string) error {
+	switch strings.ToLower(value) {
+	case string(TargetDirect), string(TargetDryRun), string(TargetTerraform):
+		*t = Target(value)
+		return nil
+	default:
+		return fmt.Errorf("invalid target: %q", value)
+	}
+}
+
+func (t *Target) Type() string {
+	return "target"
+}