Skip to content

Commit

Permalink
Enhances how we're handling slice processing for param/credential/out…
Browse files Browse the repository at this point in the history
…put sourcing

This PR updates how we handle slice elements for doing parameter/credential/ouput sourcing.
We probably should think about revisitng the entire process for this in the future.

In summary, we are using reflectwalk to handle walking the manifest. This involves handling a couple
call backs, notably "SliceElement" and "MapElement". The Slice Element assumed that it would see something
of the form source: <x.y.z> as a string. In reality, if that's not wrapped in quote, Go and YAML see that
as a Map, but the map callback doesn't get called to handle that. This was resulting in the block not getting
rewritten and the exec mixin was exploding (probably any would have). This PR updates the logic of the slice elem
processing to check and see if it's a map and handle it in a similar manner to a top level map entry in the yaml.

Closes: #158
  • Loading branch information
jeremyrickard committed Feb 13, 2019
1 parent 4d5fbb8 commit 0597f8e
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 0 deletions.
49 changes: 49 additions & 0 deletions pkg/config/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,24 @@ func (m *Manifest) SliceElem(index int, val reflect.Value) error {
return nil
}

// There are two cases possible in the YAML. The first is when the element in the slice is a string. That might look like:
// install:
// - description: "Install Hello World"
// exec:
// command: bash
// arguments:
// - -c
// - "source: bundle.parameters.command"
// The second case is when the declaration interpreted to represent a Map. This is the case when there are no quotes:
// install:
// - description: "Install Hello World"
// exec:
// command: bash
// arguments:
// - -c
// - source: bundle.parameters.command
// Branching logic below first checks the easy case (a string) then falls through into the case where we have a map with a single key.
// This is fairly similar to how we process map elements, but the replacement is done differently.
v, ok := val.Interface().(string)
if ok {
//if the array entry is a string that matches source:...., we should replace it
Expand All @@ -559,6 +577,37 @@ func (m *Manifest) SliceElem(index int, val reflect.Value) error {
}
val.Set(reflect.ValueOf(r))
}
} else {
v := val
if val.Kind() == reflect.Interface {
val = val.Elem()
}
if kind := val.Kind(); kind == reflect.Map {
if len(val.MapKeys()) == 1 {
sk := val.MapKeys()[0]
if sk.Kind() == reflect.Interface {
sk = sk.Elem()
}
//if the key is a string, and the string is source, then we should try
//and replace this
if sk.Kind() == reflect.String && sk.String() == "source" {
kv := val.MapIndex(sk)
if kv.Kind() == reflect.Interface {
kv = kv.Elem()
value := kv.String()
replacement, err := m.resolveValue(value)
if err != nil {
errors.Wrap(err, "unable to resolve value for map")
}
v.Set(reflect.ValueOf(replacement))
}
}
}
} else {
fmt.Println("not sure what this is...")
fmt.Printf("eh? %s", reflect.TypeOf(val).String())
}

}
return nil
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/config/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,30 @@ func TestResolveArray(t *testing.T) {
assert.Equal(t, "Ralpha", args[0])
}

func TestResolveSliceWithAMap(t *testing.T) {
c := NewTestConfig(t)
c.SetupPorterHome()

c.TestContext.AddTestFile("testdata/slice-test.yaml", Name)

require.NoError(t, c.LoadManifest())

installStep := c.Manifest.Install[0]

os.Setenv("COMMAND", "echo hello world")
err := c.Manifest.ResolveStep(installStep)
assert.NoError(t, err)

assert.NotNil(t, installStep.Data)
t.Logf("install data %v", installStep.Data)
exec := installStep.Data["exec"].(map[interface{}]interface{})
assert.NotNil(t, exec)
args := exec["arguments"].([]interface{})
assert.Len(t, args, 2)
assert.Equal(t, "echo hello world", args[1])
assert.NotNil(t, args)
}

func TestDependency_Validate_NameRequired(t *testing.T) {
c := NewTestConfig(t)
c.SetupPorterHome()
Expand Down
29 changes: 29 additions & 0 deletions pkg/config/testdata/slice-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
mixins:
- exec

name: HELLO
version: 0.1.0
description: "An example Porter configuration"
invocationImage: jeremyrickard/porter-hello:latest

parameters:
- name: command
type: string
default: "echo Hello World"

install:
- description: "Install Hello World"
exec:
command: bash
arguments:
- -c
- source: bundle.parameters.command


uninstall:
- description: "Uninstall Hello World"
exec:
command: bash
arguments:
- -c
- echo Goodbye World

0 comments on commit 0597f8e

Please sign in to comment.