Merge pull request #310 from maxsokolovsky/properly_unmarshal_rke_con…

…fig_settings

Transform all keys to JSON format for RKE config values

cmd/cluster.go

@@ -250,6 +250,9 @@ func clusterCreate(ctx *cli.Context) error {
 	if ctx.NArg() == 0 {
 		return cli.ShowSubcommandHelp(ctx)
 	}
+	if ctx.Bool("import") {
+		return clusterImport(ctx)
+	}
 	c, err := GetClient(ctx)
 	if err != nil {
 		return err
@@ -266,23 +269,12 @@ func clusterCreate(ctx *cli.Context) error {
 		}
 	}
 
-	rkeConfig, err := getRKEConfig(ctx)
+	config, err := getClusterConfig(ctx)
 	if err != nil {
 		return err
 	}
 
-	clusterConfig := &managementClient.Cluster{
-		Name:                          ctx.Args().First(),
-		Description:                   ctx.String("description"),
-		RancherKubernetesEngineConfig: rkeConfig,
-	}
-
-	if ctx.String("psp-default-policy") != "" {
-		clusterConfig.DefaultPodSecurityPolicyTemplateID = ctx.String("psp-default-policy")
-	}
-
-	createdCluster, err := c.ManagementClient.Cluster.Create(clusterConfig)
-
+	createdCluster, err := c.ManagementClient.Cluster.Create(config)
 	if err != nil {
 		return err
 	}
@@ -752,48 +744,61 @@ func getClusterK8sOptions(c *cliclient.MasterClient) []string {
 	return options
 }
 
-func getRKEConfig(ctx *cli.Context) (*managementClient.RancherKubernetesEngineConfig, error) {
-	if ctx.Bool("import") {
-		return nil, nil
+func getClusterConfig(ctx *cli.Context) (*managementClient.Cluster, error) {
+	config := managementClient.Cluster{
+		RancherKubernetesEngineConfig: new(managementClient.RancherKubernetesEngineConfig),
 	}
 
-	rkeConfig := &managementClient.RancherKubernetesEngineConfig{}
-
 	if ctx.String("rke-config") != "" {
 		bytes, err := readFileReturnJSON(ctx.String("rke-config"))
 		if err != nil {
 			return nil, err
 		}
-		bytes, err = fixTopLevelKeys(bytes)
-		if err != nil {
+
+		var jsonObject map[string]interface{}
+		if err = json.Unmarshal(bytes, &jsonObject); err != nil {
 			return nil, err
 		}
-		err = json.Unmarshal(bytes, &rkeConfig)
+
+		// Most values in RancherKubernetesEngineConfig are defined with struct tags for both JSON and YAML in camelCase.
+		// Changing the tags will be a breaking change. For proper deserialization, we must convert all keys to camelCase.
+		// Note that we ignore kebab-case keys. Users themselves should ensure any relevant keys
+		// (especially top-level keys in `services`, like `kube-api` or `kube-controller`) are camelCase or snake-case in cluster config.
+		convertSnakeCaseKeysToCamelCase(jsonObject)
+
+		marshalled, err := json.Marshal(jsonObject)
 		if err != nil {
 			return nil, err
 		}
+		if err = json.Unmarshal(marshalled, &config); err != nil {
+			return nil, err
+		}
 	}
 
+	config.Name = ctx.Args().First()
+	config.Description = ctx.String("description")
+
 	ignoreDockerVersion := ctx.BoolT("disable-docker-version")
-	rkeConfig.IgnoreDockerVersion = &ignoreDockerVersion
+	config.RancherKubernetesEngineConfig.IgnoreDockerVersion = &ignoreDockerVersion
 
 	if ctx.String("k8s-version") != "" {
-		rkeConfig.Version = ctx.String("k8s-version")
+		config.RancherKubernetesEngineConfig.Version = ctx.String("k8s-version")
 	}
 
 	if ctx.String("network-provider") != "" {
-		rkeConfig.Network = &managementClient.NetworkConfig{
+		config.RancherKubernetesEngineConfig.Network = &managementClient.NetworkConfig{
 			Plugin: ctx.String("network-provider"),
 		}
 	}
 
 	if ctx.String("psp-default-policy") != "" {
-		rkeConfig.Services = &managementClient.RKEConfigServices{
+		config.DefaultPodSecurityPolicyTemplateID = ctx.String("psp-default-policy")
+		config.RancherKubernetesEngineConfig.Services = &managementClient.RKEConfigServices{
 			KubeAPI: &managementClient.KubeAPIService{
 				PodSecurityPolicy: true,
 			},
 		}
 	}
 
-	return rkeConfig, nil
+	return &config, nil
 }

cmd/common.go

@@ -554,23 +554,6 @@ func parseClusterAndProjectID(id string) (string, string, error) {
 	return "", "", fmt.Errorf("Unable to extract clusterid and projectid from [%s]", id)
 }
 
-func fixTopLevelKeys(bytes []byte) ([]byte, error) {
-	old := map[string]interface{}{}
-	new := map[string]interface{}{}
-
-	err := json.Unmarshal(bytes, &old)
-	if err != nil {
-		return nil, fmt.Errorf("error unmarshalling: %v", err)
-	}
-
-	for key, val := range old {
-		newKey := convert.ToJSONKey(key)
-		new[newKey] = val
-	}
-
-	return json.Marshal(new)
-}
-
 // Return a JSON blob of the file at path
 func readFileReturnJSON(path string) ([]byte, error) {
 	file, err := ioutil.ReadFile(path)
@@ -584,6 +567,23 @@ func readFileReturnJSON(path string) ([]byte, error) {
 	return yaml.YAMLToJSON(file)
 }
 
+// renameKeys renames the keys in a given map of arbitrary depth with a provided function for string keys.
+func renameKeys(input map[string]interface{}, f func(string) string) {
+	for k, v := range input {
+		delete(input, k)
+		newKey := f(k)
+		input[newKey] = v
+		if innerMap, ok := v.(map[string]interface{}); ok {
+			renameKeys(innerMap, f)
+		}
+	}
+}
+
+// convertSnakeCaseKeysToCamelCase takes a map and recursively transforms all snake_case keys into camelCase keys.
+func convertSnakeCaseKeysToCamelCase(input map[string]interface{}) {
+	renameKeys(input, convert.ToJSONKey)
+}
+
 // Return true if the first non-whitespace bytes in buf is prefix.
 func hasPrefix(buf []byte, prefix []byte) bool {
 	trim := bytes.TrimLeftFunc(buf, unicode.IsSpace)

cmd/common_test.go

@@ -30,6 +30,35 @@ func (s *CommonTestSuite) TestParseClusterAndProjectID(c *check.C) {
 	testParse(c, "c-m-123:p-12345", "", "", true)
 }
 
+func (s *CommonTestSuite) TestConvertSnakeCaseKeysToCamelCase(c *check.C) {
+	cases := []struct {
+		input   map[string]interface{}
+		renamed map[string]interface{}
+	}{
+		{
+			map[string]interface{}{"foo_bar": "hello"},
+			map[string]interface{}{"fooBar": "hello"},
+		},
+		{
+			map[string]interface{}{"fooBar": "hello"},
+			map[string]interface{}{"fooBar": "hello"},
+		},
+		{
+			map[string]interface{}{"foobar": "hello", "some_key": "valueUnmodified", "bar-baz": "bar-baz"},
+			map[string]interface{}{"foobar": "hello", "someKey": "valueUnmodified", "bar-baz": "bar-baz"},
+		},
+		{
+			map[string]interface{}{"foo_bar": "hello", "backup_config": map[string]interface{}{"hello_world": true}, "config_id": 123},
+			map[string]interface{}{"fooBar": "hello", "backupConfig": map[string]interface{}{"helloWorld": true}, "configId": 123},
+		},
+	}
+
+	for _, tc := range cases {
+		convertSnakeCaseKeysToCamelCase(tc.input)
+		c.Assert(tc.input, check.DeepEquals, tc.renamed)
+	}
+}
+
 func testParse(c *check.C, testID, expectedCluster, expectedProject string, errorExpected bool) {
 	actualCluster, actualProject, actualErr := parseClusterAndProjectID(testID)
 	c.Assert(actualCluster, check.Equals, expectedCluster)