Commit cdfd99ea authored by Jessica Yuen's avatar Jessica Yuen
Browse files

Accurately read/write non-ASCII param identifiers



Currently, if we run `param set guestbook-ui foo-bar x`,

The jsonnet snippet will write: `foo-bar: "x"`.

However, this is incorrect jsonnet syntax. This commit will fix this
issue, to instead write `"foo-bar": "x"`.

Also updates the `Get...` Param functions to return non-quoted
identifiers for prettier display.
Signed-off-by: default avatarJessica Yuen <im.jessicayuen@gmail.com>
parent 37d11357
......@@ -27,8 +27,6 @@ import (
"reflect"
"strings"
"github.com/ksonnet/ksonnet/metadata/params"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
log "github.com/sirupsen/logrus"
......@@ -480,7 +478,7 @@ func constructBaseObj(componentPaths, componentNames []string) (string, error) {
// Emit object field. Sanitize the name to guarantee we generate valid
// Jsonnet.
componentName = params.SanitizeComponent(componentName)
componentName = utils.QuoteNonASCII(componentName)
fmt.Fprintf(&obj, " %s: %s,\n", componentName, importExpr)
}
......
......@@ -22,9 +22,10 @@ import (
"strconv"
"strings"
"github.com/ksonnet/ksonnet/utils"
"github.com/google/go-jsonnet/ast"
"github.com/google/go-jsonnet/parser"
"github.com/ksonnet/ksonnet/utils"
)
const (
......@@ -66,15 +67,6 @@ func findComponentsObj(node ast.Node) (*ast.Object, error) {
return nil, fmt.Errorf("Invalid params schema -- did not expect node type: %T", node)
}
// 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:
......@@ -109,14 +101,15 @@ func visitParams(component ast.Node) (Params, *ast.LocationRange, error) {
loc = n.Loc()
for _, field := range n.Fields {
if field.Id != nil {
key := string(*field.Id)
val, err := visitParamValue(field.Expr2)
if err != nil {
return nil, nil, err
}
params[key] = val
key, err := getFieldID(field)
if err != nil {
return nil, nil, err
}
val, err := visitParamValue(field.Expr2)
if err != nil {
return nil, nil, err
}
params[key] = val
}
return params, loc, nil
......@@ -179,9 +172,9 @@ func writeParams(indent int, params Params) string {
buffer.WriteString("\n")
for i, key := range keys {
param := params[key]
if strings.HasPrefix(param, "|||\n") {
println(param)
key := utils.QuoteNonASCII(key)
if strings.HasPrefix(param, "|||\n") {
// every line in a block string needs to be indented
lines := strings.Split(param, "\n")
buffer.WriteString(fmt.Sprintf("%s%s: %s\n", indentBuffer.String(), key, lines[0]))
......@@ -225,7 +218,7 @@ func appendComponent(component, snippet string, params Params) (string, error) {
// Create the jsonnet resembling the component params
var buffer bytes.Buffer
buffer.WriteString(" " + SanitizeComponent(component) + ": {")
buffer.WriteString(" " + utils.QuoteNonASCII(component) + ": {")
buffer.WriteString(writeParams(6, params))
buffer.WriteString(" },")
......@@ -254,7 +247,7 @@ func getComponentParams(component, snippet string) (Params, *ast.LocationRange,
}
}
return nil, nil, fmt.Errorf("Could not find component identifier '%s' when attempting to set params", component)
return nil, nil, fmt.Errorf("Could not find component identifier '%s'", component)
}
func getAllComponentParams(snippet string) (map[string]Params, error) {
......@@ -342,7 +335,7 @@ func setEnvironmentParams(component, snippet string, params Params) (string, err
lines := strings.Split(snippet, "\n")
if !hasComponent {
var buffer bytes.Buffer
buffer.WriteString(fmt.Sprintf("\n %s +: {", SanitizeComponent(component)))
buffer.WriteString(fmt.Sprintf("\n %s +: {", utils.QuoteNonASCII(component)))
buffer.WriteString(writeParams(6, params))
buffer.WriteString(" },\n")
paramsSnippet = buffer.String()
......
......@@ -275,11 +275,11 @@ func TestGetComponentParams(t *testing.T) {
},
foo: {
name: "foo",
replicas: 1,
"replica-count": 1,
},
},
}`,
Params{"name": `"foo"`, "replicas": "1"},
Params{"name": `"foo"`, "replica-count": "1"},
},
// Test getting the parameters for a component name with special characters
{
......@@ -521,6 +521,30 @@ func TestSetComponentParams(t *testing.T) {
replicas: 5,
},
},
}`,
},
// Test adding a parameter with special characters in the param name
{
"foo",
`
{
components: {
foo: {
"foo-bar": 5,
name: "foo",
},
},
}`,
Params{"replica-count": "5"},
`
{
components: {
foo: {
"foo-bar": 5,
name: "foo",
"replica-count": 5,
},
},
}`,
},
// Test setting parameter for a component with a special character in name
......@@ -700,13 +724,13 @@ params + {
},
"foo" +: {
name: "foo",
replicas: 5,
"replica-count": 5,
},
},
}`,
map[string]Params{
"bar": Params{"name": `"bar"`, "replicas": "1"},
"foo": Params{"name": `"foo"`, "replicas": "5"},
"foo": Params{"name": `"foo"`, "replica-count": "5"},
},
},
}
......@@ -815,6 +839,38 @@ params + {
replicas: 5,
},
},
}`,
},
// Test special character in param identifier
{
"foo",
`
local params = import "/fake/path";
params + {
components +: {
bar +: {
name: "bar",
"replica-count": 1,
},
foo +: {
name: "foo",
},
},
}`,
Params{"replica-count": "5"},
`
local params = import "/fake/path";
params + {
components +: {
bar +: {
name: "bar",
"replica-count": 1,
},
foo +: {
name: "foo",
"replica-count": 5,
},
},
}`,
},
// Test setting environment param case where component isn't in the snippet
......
......@@ -17,6 +17,7 @@ package utils
import (
"bytes"
"fmt"
"strings"
"github.com/PuerkitoBio/purell"
......@@ -34,6 +35,15 @@ func IsASCIIIdentifier(s string) bool {
return true
}
// QuoteNonASCII puts quotes around an identifier that contains non-ASCII
// characters.
func QuoteNonASCII(s string) string {
if !IsASCIIIdentifier(s) {
return fmt.Sprintf(`"%s"`, s)
}
return s
}
// NormalizeURL uses purell's "usually safe normalization" algorithm to
// normalize URLs. This includes removing dot segments, removing trailing
// slashes, removing unnecessary escapes, removing default ports, and setting
......
......@@ -53,6 +53,37 @@ func TestIsASCIIIdentifier(t *testing.T) {
}
}
func TestQuoteNonASCII(t *testing.T) {
tests := []struct {
input string
expected string
}{
{
input: "HelloWorld",
expected: "HelloWorld",
},
{
input: "Hello World",
expected: `"Hello World"`,
},
{
input: "helloworld",
expected: "helloworld",
},
{
input: "hello-world",
expected: `"hello-world"`,
},
{
input: "hello世界",
expected: `"hello世界"`,
},
}
for _, test := range tests {
require.EqualValues(t, test.expected, QuoteNonASCII(test.input))
}
}
func TestNormalizeURL(t *testing.T) {
tests := []struct {
input string
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment