Skip to content

Commit

Permalink
fix(helm): improve --set parser
Browse files Browse the repository at this point in the history 
This replaces the old set parser with a brand new one. This also changes
the internal algorithm from duplicating YAML to merging YAML, which
might solve a problem one user reported in chat, but which was never
captured in an issue.

Closes #1540
Closes #1556
  • Loading branch information
@technosophos
technosophos committed Nov 29, 2016
1 parent 7853f52 commit 1249ffd
Show file tree
Hide file tree
Showing 7 changed files with 595 additions and 174 deletions.
103 changes: 10 additions & 93 deletions cmd/helm/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strconv"
"strings"

"text/template"
Expand All @@ -35,6 +34,7 @@ import (

"k8s.io/helm/cmd/helm/downloader"
"k8s.io/helm/cmd/helm/helmpath"
"k8s.io/helm/cmd/helm/strvals"
"k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/kube"
"k8s.io/helm/pkg/proto/hapi/release"
Expand Down Expand Up @@ -95,7 +95,7 @@ type installCmd struct {
keyring string
out io.Writer
client helm.Interface
values *values
values string
nameTemplate string
version string
}
Expand All @@ -104,7 +104,6 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
inst := &installCmd{
out: out,
client: c,
values: new(values),
}

cmd := &cobra.Command{
Expand Down Expand Up @@ -133,7 +132,7 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
f.BoolVar(&inst.dryRun, "dry-run", false, "simulate an install")
f.BoolVar(&inst.disableHooks, "no-hooks", false, "prevent hooks from running during install")
f.BoolVar(&inst.replace, "replace", false, "re-use the given name, even if that name is already used. This is unsafe in production")
f.Var(inst.values, "set", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.StringVar(&inst.values, "set", "", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.StringVar(&inst.nameTemplate, "name-template", "", "specify template used to name the release")
f.BoolVar(&inst.verify, "verify", false, "verify the package before installing it")
f.StringVar(&inst.keyring, "keyring", defaultKeyring(), "location of public keys used for verification")
Expand Down Expand Up @@ -199,32 +198,25 @@ func (i *installCmd) run() error {
}

func (i *installCmd) vals() ([]byte, error) {
var buffer bytes.Buffer
base := map[string]interface{}{}

// User specified a values file via -f/--values
if i.valuesFile != "" {
bytes, err := ioutil.ReadFile(i.valuesFile)
if err != nil {
return []byte{}, err
}
buffer.Write(bytes)

// Force a new line. An extra won't matter, but a missing one can
// break things. https://github.com/kubernetes/helm/issues/1430
buffer.WriteRune('\n')
if err := yaml.Unmarshal(bytes, &base); err != nil {
return []byte{}, fmt.Errorf("failed to parse %s: %s", i.valuesFile, err)
}
}

// User specified value pairs via --set
// These override any values in the specified file
if len(i.values.pairs) > 0 {
bytes, err := i.values.yaml()
if err != nil {
return []byte{}, err
}
buffer.Write(bytes)
if err := strvals.ParseInto(i.values, base); err != nil {
return []byte{}, fmt.Errorf("failed parsing --set data: %s", err)
}

return buffer.Bytes(), nil
return yaml.Marshal(base)
}

// printRelease prints info about a release if the flagDebug is true.
Expand All @@ -243,81 +235,6 @@ func (i *installCmd) printRelease(rel *release.Release) {
}
}

// values represents the command-line value pairs
type values struct {
pairs map[string]interface{}
}

func (v *values) yaml() ([]byte, error) {
return yaml.Marshal(v.pairs)
}

func (v *values) String() string {
out, _ := v.yaml()
return string(out)
}

func (v *values) Type() string {
// Added to pflags.Value interface, but not documented there.
return "struct"
}

func (v *values) Set(data string) error {
v.pairs = map[string]interface{}{}

items := strings.Split(data, ",")
for _, item := range items {
n, val := splitPair(item)
names := strings.Split(n, ".")
ln := len(names)
current := &v.pairs
for i := 0; i < ln; i++ {
if i+1 == ln {
// We're at the last element. Set it.
(*current)[names[i]] = val
} else {
//
if e, ok := (*current)[names[i]]; !ok {
m := map[string]interface{}{}
(*current)[names[i]] = m
current = &m
} else if m, ok := e.(map[string]interface{}); ok {
current = &m
}
}
}
}
return nil
}

func typedVal(val string) interface{} {
if strings.EqualFold(val, "true") {
return true
}

if strings.EqualFold(val, "false") {
return false
}

if iv, err := strconv.ParseInt(val, 10, 64); err == nil {
return iv
}

if fv, err := strconv.ParseFloat(val, 64); err == nil {
return fv
}

return val
}

func splitPair(item string) (name string, value interface{}) {
pair := strings.SplitN(item, "=", 2)
if len(pair) == 1 {
return pair[0], true
}
return pair[0], typedVal(pair[1])
}

// locateChartPath looks for a chart directory in known places, and returns either the full path or an error.
//
// This does not ensure that the chart is well-formed; only that the requested filename exists.
Expand Down
66 changes: 0 additions & 66 deletions cmd/helm/install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package main

import (
"fmt"
"io"
"regexp"
"strings"
Expand Down Expand Up @@ -99,71 +98,6 @@ func TestInstall(t *testing.T) {
})
}

func TestValues(t *testing.T) {
args := "sailor=sinbad,good,port.source=baghdad,port.destination=basrah,success=True"
vobj := new(values)
vobj.Set(args)

if vobj.Type() != "struct" {
t.Fatalf("Expected Type to be struct, got %s", vobj.Type())
}

vals := vobj.pairs
if fmt.Sprint(vals["good"]) != "true" {
t.Errorf("Expected good to be true. Got %v", vals["good"])
}

if !vals["success"].(bool) {
t.Errorf("Expected boolean true. Got %T, %v", vals["success"], vals["success"])
}

port := vals["port"].(map[string]interface{})

if fmt.Sprint(port["source"]) != "baghdad" {
t.Errorf("Expected source to be baghdad. Got %s", port["source"])
}
if fmt.Sprint(port["destination"]) != "basrah" {
t.Errorf("Expected source to be baghdad. Got %s", port["source"])
}

y := `good: true
port:
destination: basrah
source: baghdad
sailor: sinbad
success: true
`
out, err := vobj.yaml()
if err != nil {
t.Fatal(err)
}
if string(out) != y {
t.Errorf("Expected YAML to be \n%s\nGot\n%s\n", y, out)
}

if vobj.String() != y {
t.Errorf("Expected String() to be \n%s\nGot\n%s\n", y, out)
}

// Combined case, overriding a property
vals["sailor"] = "pisti"
updatedYAML := `good: true
port:
destination: basrah
source: baghdad
sailor: pisti
success: true
`
newOut, err := vobj.yaml()
if err != nil {
t.Fatal(err)
}
if string(newOut) != updatedYAML {
t.Errorf("Expected YAML to be \n%s\nGot\n%s\n", updatedYAML, newOut)
}

}

type nameTemplateTestCase struct {
tpl string
expected string
Expand Down
32 changes: 32 additions & 0 deletions cmd/helm/strvals/doc.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.
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 strvals provides tools for working with strval lines.
Helm supports a compressed format for YAML settings which we call strvals.
The format is roughly like this:
name=value,topname.subname=value
The above is equivalent to the YAML document
name: value
topname:
subname: value
This package provides a parser and utilities for converting the strvals format
to other formats.
*/
package strvals
Loading

0 comments on commit 1249ffd

Please sign in to comment.