From ce2d567dec58a572fb88212076fa3347461c9706 Mon Sep 17 00:00:00 2001
From: Jessica Yuen <im.jessicayuen@gmail.com>
Date: Mon, 6 Nov 2017 13:25:55 -0800
Subject: [PATCH] Jsonnet handling of component names with special characters
Currently, if a component name contains a special character, ex:
foo-bar, this translates to the jsonnet identifier: foo-bar, which is
invalid syntax.
This change will quote component names where there are special
characters.
---
cmd/prototype.go | 7 +-
cmd/root.go | 4 ++
cmd/root_test.go | 10 +++
metadata/params/params.go | 68 ++++++++++++++++--
metadata/params/params_test.go | 123 +++++++++++++++++++++++++++++++--
utils/strings.go | 32 +++++++++
utils/strings_test.go | 53 ++++++++++++++
7 files changed, 285 insertions(+), 12 deletions(-)
create mode 100644 utils/strings.go
create mode 100644 utils/strings_test.go
diff --git a/cmd/prototype.go b/cmd/prototype.go
index fd35f92b..17e44bf5 100644
--- a/cmd/prototype.go
+++ b/cmd/prototype.go
@@ -25,6 +25,7 @@ import (
"github.com/ksonnet/ksonnet/metadata"
"github.com/ksonnet/ksonnet/prototype"
"github.com/ksonnet/ksonnet/prototype/snippet/jsonnet"
+ "github.com/ksonnet/ksonnet/utils"
"github.com/spf13/cobra"
)
@@ -397,7 +398,11 @@ func expandPrototype(proto *prototype.SpecificationSchema, templateType prototyp
return "", err
}
if templateType == prototype.Jsonnet {
- template = append([]string{`local params = std.extVar("` + metadata.ParamsExtCodeKey + `").components.` + componentName + ";"}, template...)
+ componentsText := "components." + componentName
+ if !utils.IsASCIIIdentifier(componentName) {
+ componentsText = fmt.Sprintf(`components["%s"]`, componentName)
+ }
+ template = append([]string{`local params = std.extVar("` + metadata.ParamsExtCodeKey + `").` + componentsText + ";"}, template...)
}
return jsonnet.Parse(componentName, strings.Join(template, "\n"))
diff --git a/cmd/root.go b/cmd/root.go
index 95a42328..38be526b 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -27,6 +27,8 @@ import (
"reflect"
"strings"
+ "github.com/ksonnet/ksonnet/metadata/params"
+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
log "github.com/sirupsen/logrus"
@@ -430,6 +432,7 @@ func expandEnvCmdObjs(cmd *cobra.Command, envSpec *envSpec, cwd metadata.AbsPath
//
// {
// foo: import "components/foo.jsonnet"
+// "foo-bar": import "components/foo-bar.jsonnet"
// }
func constructBaseObj(paths []string) string {
var obj bytes.Buffer
@@ -441,6 +444,7 @@ func constructBaseObj(paths []string) string {
}
name := strings.TrimSuffix(path.Base(p), ext)
+ name = params.SanitizeComponent(name)
fmt.Fprintf(&obj, " %s: import \"%s\",\n", name, p)
}
obj.WriteString("}\n")
diff --git a/cmd/root_test.go b/cmd/root_test.go
index 2bab864d..d5539cdc 100644
--- a/cmd/root_test.go
+++ b/cmd/root_test.go
@@ -63,6 +63,16 @@ func TestConstructBaseObj(t *testing.T) {
`{
bar: import "another/fake/path/bar.jsonnet",
}
+`,
+ },
+ // test special character case
+ {
+ []string{
+ "another/fake/path/foo-bar.jsonnet",
+ },
+ `{
+ "foo-bar": import "another/fake/path/foo-bar.jsonnet",
+}
`,
},
}
diff --git a/metadata/params/params.go b/metadata/params/params.go
index 1090ee03..a99ab7a5 100644
--- a/metadata/params/params.go
+++ b/metadata/params/params.go
@@ -24,6 +24,7 @@ import (
"github.com/google/go-jsonnet/ast"
"github.com/google/go-jsonnet/parser"
+ "github.com/ksonnet/ksonnet/utils"
)
const (
@@ -39,6 +40,38 @@ func astRoot(component, snippet string) (ast.Node, error) {
return parser.Parse(tokens)
}
+// SanitizeComponent puts quotes around a component name if it contains special
+// characters.
+func SanitizeComponent(component string) string {
+ if !utils.IsASCIIIdentifier(component) {
+ return fmt.Sprintf(`"%s"`, component)
+ }
+ return component
+}
+
+func getFieldId(field ast.ObjectField) (string, error) {
+ switch field.Kind {
+ case ast.ObjectFieldStr:
+ // case "foo-bar": {...}
+ c, _ := field.Expr1.(*ast.LiteralString)
+ switch c.Kind {
+ case ast.StringSingle, ast.StringDouble:
+ return c.Value, nil
+ default:
+ return "", fmt.Errorf("Found unsupported LiteralString type %T", c)
+ }
+ case ast.ObjectFieldID:
+ // case foo: {...}
+ return string(*field.Id), nil
+ }
+ return "", fmt.Errorf("Found unsupported ObjectField.Kind, type %T", field)
+}
+
+func hasComponent(component string, field ast.ObjectField) (bool, error) {
+ id, err := getFieldId(field)
+ return id == component, err
+}
+
func visitParams(component ast.Node) (Params, *ast.LocationRange, error) {
params := make(Params)
var loc *ast.LocationRange
@@ -71,7 +104,11 @@ func visitAllParams(components ast.Object) (map[string]Params, error) {
if err != nil {
return nil, err
}
- params[string(*f.Id)] = p
+ id, err := getFieldId(f)
+ if err != nil {
+ return nil, err
+ }
+ params[id] = p
}
return params, nil
@@ -86,7 +123,12 @@ func visitParamValue(param ast.Node) (string, error) {
case *ast.LiteralBoolean:
return strconv.FormatBool(n.Value), nil
case *ast.LiteralString:
- return fmt.Sprintf(`"%s"`, n.Value), nil
+ switch n.Kind {
+ case ast.StringSingle, ast.StringDouble:
+ return fmt.Sprintf(`"%s"`, n.Value), nil
+ default:
+ return "", fmt.Errorf("Found unsupported LiteralString type %T", n)
+ }
default:
return "", fmt.Errorf("Found an unsupported param AST node type: %T", n)
}
@@ -145,6 +187,7 @@ func visitComponentsObj(component, snippet string) (*ast.Object, error) {
}
func appendComponent(component, snippet string, params Params) (string, error) {
+ component = SanitizeComponent(component)
componentsNode, err := visitComponentsObj(component, snippet)
if err != nil {
return "", err
@@ -152,7 +195,11 @@ func appendComponent(component, snippet string, params Params) (string, error) {
// Ensure that the component we are trying to create params for does not already exist.
for _, field := range componentsNode.Fields {
- if field.Id != nil && string(*field.Id) == component {
+ hasComponent, err := hasComponent(component, field)
+ if err != nil {
+ return "", err
+ }
+ if hasComponent {
return "", fmt.Errorf("Component parameters for '%s' already exists", component)
}
}
@@ -181,7 +228,11 @@ func getComponentParams(component, snippet string) (Params, *ast.LocationRange,
}
for _, field := range componentsNode.Fields {
- if field.Id != nil && string(*field.Id) == component {
+ hasComponent, err := hasComponent(component, field)
+ if err != nil {
+ return nil, nil, err
+ }
+ if hasComponent {
return visitParams(field.Expr2)
}
}
@@ -229,7 +280,7 @@ func findEnvComponentsObj(node ast.Node) (*ast.Object, error) {
return findEnvComponentsObj(n.Right)
case *ast.Object:
for _, f := range n.Fields {
- if *f.Id == "components" {
+ if *f.Id == componentsID {
c, isObj := f.Expr2.(*ast.Object)
if !isObj {
return nil, fmt.Errorf("Expected components node type to be object")
@@ -254,7 +305,11 @@ func getEnvironmentParams(component, snippet string) (Params, *ast.LocationRange
}
for _, f := range n.Fields {
- if f.Id != nil && string(*f.Id) == component {
+ hasComponent, err := hasComponent(component, f)
+ if err != nil {
+ return nil, nil, false, err
+ }
+ if hasComponent {
params, loc, err := visitParams(f.Expr2)
return params, loc, true, err
}
@@ -285,6 +340,7 @@ func getAllEnvironmentParams(snippet string) (map[string]Params, error) {
}
func setEnvironmentParams(component, snippet string, params Params) (string, error) {
+ component = SanitizeComponent(component)
currentParams, loc, hasComponent, err := getEnvironmentParams(component, snippet)
if err != nil {
return "", err
diff --git a/metadata/params/params_test.go b/metadata/params/params_test.go
index 6c2ef26a..fe15eb62 100644
--- a/metadata/params/params_test.go
+++ b/metadata/params/params_test.go
@@ -101,6 +101,37 @@ func TestAppendComponentParams(t *testing.T) {
replicas: 5,
},
},
+}`,
+ },
+ // Test appending a component with a hyphenated name
+ {
+ "foo-bar",
+ `
+{
+ global: {
+ // User-defined global parameters; accessible to all component and environments, Ex:
+ // replicas: 4,
+ },
+ components: {
+ // Component-level parameters, defined initially from 'ks prototype use ...'
+ // Each object below should correspond to a component in the components/ directory
+ },
+}`,
+ Params{"replicas": "5", "name": `"foo-bar"`},
+ `
+{
+ global: {
+ // User-defined global parameters; accessible to all component and environments, Ex:
+ // replicas: 4,
+ },
+ components: {
+ // Component-level parameters, defined initially from 'ks prototype use ...'
+ // Each object below should correspond to a component in the components/ directory
+ "foo-bar": {
+ name: "foo-bar",
+ replicas: 5,
+ },
+ },
}`,
},
}
@@ -215,6 +246,21 @@ func TestGetComponentParams(t *testing.T) {
}`,
Params{"name": `"foo"`, "replicas": "1"},
},
+ // Test getting the parameters for a component name with special characters
+ {
+ "foo-bar",
+ `
+{
+ global: {},
+ components: {
+ "foo-bar": {
+ name: "foo-bar",
+ replicas: 1,
+ },
+ },
+}`,
+ Params{"name": `"foo-bar"`, "replicas": "1"},
+ },
}
errors := []struct {
@@ -243,6 +289,30 @@ func TestGetComponentParams(t *testing.T) {
// replicas: 4,
components: {},
},
+}`,
+ },
+ // Test case where one of the component names is a block string
+ {
+ "foo",
+ `
+{
+ components: {
+ |||foo|||: {
+ name: "foo",
+ }
+ },
+}`,
+ },
+ // Test case where one of the param values is a block string
+ {
+ "foo",
+ `
+{
+ components: {
+ "foo": {
+ name: |||name is foo|||,
+ }
+ },
}`,
},
}
@@ -305,15 +375,15 @@ func TestGetAllComponentParams(t *testing.T) {
bar: {
replicas: 5
},
- foo: {
- name: "foo",
+ "foo-bar": {
+ name: "foo-bar",
replicas: 1,
},
},
}`,
map[string]Params{
- "bar": Params{"replicas": "5"},
- "foo": Params{"name": `"foo"`, "replicas": "1"},
+ "bar": Params{"replicas": "5"},
+ "foo-bar": Params{"name": `"foo-bar"`, "replicas": "1"},
},
},
}
@@ -411,6 +481,28 @@ func TestSetComponentParams(t *testing.T) {
replicas: 5,
},
},
+}`,
+ },
+ // Test setting parameter for a component with a special character in name
+ {
+ "foo-bar",
+ `
+{
+ components: {
+ "foo-bar": {
+ name: "foo-bar",
+ },
+ },
+}`,
+ Params{"replicas": "5"},
+ `
+{
+ components: {
+ "foo-bar": {
+ name: "foo-bar",
+ replicas: 5,
+ },
+ },
}`,
},
}
@@ -508,7 +600,7 @@ params + {
name: "bar",
replicas: 1,
},
- foo +: {
+ "foo" +: {
name: "foo",
replicas: 5,
},
@@ -616,6 +708,27 @@ params + {
replicas: 5,
},
},
+}`,
+ },
+ // Test setting environment param case where component isn't in the snippet
+ // and the component name has special characters
+ {
+ "foo-bar",
+ `
+local params = import "/fake/path";
+params + {
+ components +: {
+ },
+}`,
+ Params{"replicas": "5"},
+ `
+local params = import "/fake/path";
+params + {
+ components +: {
+ "foo-bar" +: {
+ replicas: 5,
+ },
+ },
}`,
},
}
diff --git a/utils/strings.go b/utils/strings.go
new file mode 100644
index 00000000..c2652f6b
--- /dev/null
+++ b/utils/strings.go
@@ -0,0 +1,32 @@
+// Copyright 2017 The kubecfg authors
+//
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+ "strings"
+)
+
+// HasSpecialCharacter takes a string and returns true if the string contains
+// any special characters.
+func IsASCIIIdentifier(s string) bool {
+ f := func(r rune) bool {
+ return r < 'A' || r > 'z'
+ }
+ if strings.IndexFunc(s, f) != -1 {
+ return false
+ }
+ return true
+}
diff --git a/utils/strings_test.go b/utils/strings_test.go
new file mode 100644
index 00000000..447d0844
--- /dev/null
+++ b/utils/strings_test.go
@@ -0,0 +1,53 @@
+// Copyright 2017 The kubecfg authors
+//
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestIsASCIIIdentifier(t *testing.T) {
+ tests := []struct {
+ input string
+ expected bool
+ }{
+ {
+ input: "HelloWorld",
+ expected: true,
+ },
+ {
+ input: "Hello World",
+ expected: false,
+ },
+ {
+ input: "helloworld",
+ expected: true,
+ },
+ {
+ input: "hello-world",
+ expected: false,
+ },
+ {
+ input: "hello世界",
+ expected: false,
+ },
+ }
+ for _, test := range tests {
+ require.EqualValues(t, test.expected, IsASCIIIdentifier(test.input))
+ }
+}
--
GitLab