Merge pull request #11761 from johngmyers/override-tweak

Adjustments to SpecOverride

cmd/kops/create_cluster.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
@@ -70,8 +71,8 @@ type CreateClusterOptions struct {
 	// SSHPublicKeys is a map of the SSH public keys we should configure; required on AWS, not required on GCE
 	SSHPublicKeys map[string][]byte
 
-	// Overrides allows setting values directly in the spec.
+	// Sets allows setting values directly in the spec.
-	Overrides []string
+	Sets []string
 	// Unsets allows unsetting values directly in the spec.
 	Unsets []string
 
@@ -302,7 +303,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().StringVarP(&options.Output, "output", "o", options.Output, "Output format. One of json|yaml. Used with the --dry-run flag.")
 
 	if featureflag.SpecOverrideFlag.Enabled() {
-		cmd.Flags().StringSliceVar(&options.Overrides, "override", options.Overrides, "Directly configure values in the spec")
+		cmd.Flags().StringSliceVar(&options.Sets, "set", options.Sets, "Directly set values in the spec")
 		cmd.Flags().StringSliceVar(&options.Unsets, "unset", options.Unsets, "Directly unset values in the spec")
 	}
 
@@ -336,6 +337,15 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().BoolVar(&options.OpenstackLBOctavia, "os-octavia", options.OpenstackLBOctavia, "If true octavia loadbalancer api will be used")
 	cmd.Flags().StringVar(&options.OpenstackDNSServers, "os-dns-servers", options.OpenstackDNSServers, "comma separated list of DNS Servers which is used in network")
 	cmd.Flags().StringVar(&options.OpenstackNetworkID, "os-network", options.OpenstackNetworkID, "The ID of the existing OpenStack network to use")
+
+	cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName {
+		switch name {
+		case "override":
+			name = "set"
+		}
+		return pflag.NormalizedName(name)
+	})
+
 	return cmd
 }
 
@@ -516,7 +526,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 	if err := commands.UnsetClusterFields(c.Unsets, cluster); err != nil {
 		return err
 	}
-	if err := commands.SetClusterFields(c.Overrides, cluster); err != nil {
+	if err := commands.SetClusterFields(c.Sets, cluster); err != nil {
 		return err
 	}
 

tests/integration/create_cluster/overrides/options.yaml

@@ -4,5 +4,5 @@ Zones:
 CloudProvider: aws
 Networking: cni
 KubernetesVersion: v1.21.0
-Overrides:
+Sets:
 - cluster.spec.nodePortAccess=1.2.3.4/32,10.20.30.0/24

util/pkg/reflectutils/access.go

@@ -95,12 +95,12 @@ func setPrimitive(v reflect.Value, newValue string) error {
 	}
 
 	if v.Type().Kind() == reflect.Slice {
-		// Because this function generally sets values, we overwrite instead of appending.
+		// To support multiple values, we split on commas.
-		// Then to support multiple values, we split on commas.
 		// We have no way to escape a comma currently; but in general we prefer having a slice in the schema,
 		// rather than having values that need to be parsed, so we may not need it.
 		tokens := strings.Split(newValue, ",")
-		valueArray := reflect.MakeSlice(v.Type(), 0, len(tokens))
+		valueArray := reflect.MakeSlice(v.Type(), 0, v.Len()+len(tokens))
+		valueArray = reflect.AppendSlice(valueArray, v)
 		for _, s := range tokens {
 			valueItem := reflect.New(v.Type().Elem())
 			if err := setPrimitive(valueItem.Elem(), s); err != nil {

util/pkg/reflectutils/access_test.go

@@ -163,6 +163,13 @@ func TestSet(t *testing.T) {
 			Path:     "spec.containers[0].enumSlice",
 			Value:    "ABC,DEF",
 		},
+		{
+			Name:     "append enum slice",
+			Input:    "{ 'spec': { 'containers': [ { 'enumSlice': [ 'ABC', 'DEF' ] } ] } }",
+			Expected: "{ 'spec': { 'containers': [ { 'enumSlice': [ 'ABC', 'DEF', 'GHI', 'JKL' ] } ] } }",
+			Path:     "spec.containers[0].enumSlice",
+			Value:    "GHI,JKL",
+		},
 		// Not sure if we should do this...
 		// {
 		// 	Name:     "creating missing array elements",