From 284b15be1988e45d0d7ff3889303b5c6d5b8fdd2 Mon Sep 17 00:00:00 2001 From: justinsb Date: Thu, 5 Dec 2024 07:54:57 -0500 Subject: [PATCH] Support strong-typing for --target values A small cleanup that makes our code a little more robust. --- cmd/kops/create_cluster.go | 13 +++++---- cmd/kops/update_cluster.go | 27 +++++++++++++------ docs/cli/kops_create_cluster.md | 2 +- docs/cli/kops_update_cluster.md | 2 +- upup/pkg/fi/cloudup/apply_cluster.go | 2 +- upup/pkg/fi/cloudup/target.go | 40 +++++++++++++++++++++++++--- 6 files changed, 67 insertions(+), 19 deletions(-) diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index cbd2ca4498345..9ec32ea81199f 100644 --- 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 index d2e3423e7f148..98ecab0229b0c 100644 --- 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 index 13bae47971217..7d61382e55847 100644 --- a/docs/cli/kops_create_cluster.md +++ b/docs/cli/kops_create_cluster.md @@ -122,7 +122,7 @@ kops create cluster [CLUSTER] [flags] --ssh-access strings Restrict SSH access to this CIDR. If not set, uses the value of the admin-access flag. --ssh-public-key string SSH public key to use --subnets strings Shared subnets to use - --target string Valid targets: direct, terraform. Set this flag to terraform if you want kOps to generate terraform (default "direct") + --target target Valid targets: "direct", "terraform". Set this flag to "terraform" if you want kOps to generate terraform (default direct) -t, --topology string Network topology for the cluster: 'public' or 'private'. Defaults to 'public' for IPv4 clusters and 'private' for IPv6 clusters. --unset strings Directly unset values in the spec --utility-subnets strings Shared utility subnets to use diff --git a/docs/cli/kops_update_cluster.md b/docs/cli/kops_update_cluster.md index ab62d7862a93e..7847a09a39feb 100644 --- a/docs/cli/kops_update_cluster.md +++ b/docs/cli/kops_update_cluster.md @@ -39,7 +39,7 @@ kops update cluster [CLUSTER] [flags] --prune Delete old revisions of cloud resources that were needed during an upgrade --reconcile Reconcile the cluster by rolling the control plane and nodes sequentially --ssh-public-key string SSH public key to use (deprecated: use kops create secret instead) - --target string Target - direct, terraform (default "direct") + --target target Target - "direct", "terraform" (default direct) --user string Existing user in kubeconfig file to use. Implies --create-kube-config -y, --yes Create cloud resources, without --yes update is in dry run mode ``` diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index 83ba8afcd52a7..6557f460a2049 100644 --- 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 index b4d3d576bea76..609fa60e2b7f6 100644 --- 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" +}