grim/convey

Fix infinite environment mapping loop.
env-loop
2017-09-20, Eric Fritz
0e0c2d936684
Parents 80dcf627accf
Children bd5dcbe1bf33955b9c1d8976
Fix infinite environment mapping loop.
12 files changed, 166 insertions(+), 54 deletions(-)
  • +18 -3
    docker/build.go
  • +8 -2
    docker/export.go
  • +7 -3
    docker/import.go
  • +6 -1
    docker/pull.go
  • +6 -1
    docker/push.go
  • +6 -1
    docker/remove.go
  • +50 -9
    docker/run.go
  • +12 -2
    docker/tag.go
  • +2 -16
    docker/workspace_test.go
  • +17 -6
    environment/mapper.go
  • +30 -9
    environment/mapper_test.go
  • +4 -1
    intrinsic/clean.go
    • --- a/docker/build.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/build.go Wed Sep 20 20:07:30 2017 -0500
    @@ -60,7 +60,12 @@
    // export the files to it
    for _, src := range b.Files {
    - src, dest := tasks.ParseFilePath(base, environment.Mapper(src, fullEnv))
    + src, err := environment.Mapper(src, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + src, dest := tasks.ParseFilePath(base, src)
    cleanDest := filepath.Clean(filepath.Join(tmpDir, dest))
    if err = exportFile(name, st.Workspace.Name(), src, cleanDest, st); err != nil {
    @@ -80,12 +85,22 @@
    return err
    }
    + tag, err := environment.Mapper(b.Tag, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + labels, err := environment.SliceMapper(b.Labels, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    // now run the build
    params = map[string]interface{}{
    "dockerfile": filepath.Join(tmpDir, filepath.Base(b.Dockerfile)),
    - "tag": environment.Mapper(b.Tag, fullEnv),
    + "tag": tag,
    "buildContext": tmpDir,
    - "Labels": environment.SliceMapper(b.Labels, fullEnv),
    + "Labels": labels,
    "Arguments": b.Arguments,
    }
    --- a/docker/export.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/export.go Wed Sep 20 20:07:30 2017 -0500
    @@ -87,7 +87,10 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, file := range e.Files {
    - file = environment.Mapper(file, fullEnv)
    + file, err := environment.Mapper(file, fullEnv)
    + if err != nil {
    + return err
    + }
    if err := checkFilePattern(file); err != nil {
    return err
    @@ -96,7 +99,10 @@
    dockerWorkspace := st.Workspace.(*Workspace)
    if strings.ContainsRune(file, '*') {
    - mountPoint := environment.Mapper(dockerWorkspace.mountPoint, fullEnv)
    + mountPoint, err := environment.Mapper(dockerWorkspace.mountPoint, fullEnv)
    + if err != nil {
    + return err
    + }
    if err := exportGlob(name, st.Workspace.Name(), mountPoint, file, st); err != nil {
    return err
    --- a/docker/import.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/import.go Wed Sep 20 20:07:30 2017 -0500
    @@ -36,7 +36,12 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, file := range i.Files {
    - src, dest := tasks.ParseFilePath("", environment.Mapper(file, fullEnv))
    + file, err := environment.Mapper(file, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + src, dest := tasks.ParseFilePath("", file)
    params := map[string]interface{}{
    "source": src,
    @@ -44,8 +49,7 @@
    "workspaceID": st.Workspace.Name(),
    }
    - err := Docker(name, importTemplate, params, st)
    - if err != nil {
    + if err := Docker(name, importTemplate, params, st); err != nil {
    return err
    }
    }
    --- a/docker/pull.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/pull.go Wed Sep 20 20:07:30 2017 -0500
    @@ -37,8 +37,13 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, image := range p.Images {
    + image, err := environment.Mapper(image, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    params := map[string]interface{}{
    - "image": environment.Mapper(image, fullEnv),
    + "image": image,
    }
    if err := Docker(name, pullTemplate, params, st); err != nil {
    --- a/docker/push.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/push.go Wed Sep 20 20:07:30 2017 -0500
    @@ -37,8 +37,13 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, image := range p.Images {
    + image, err := environment.Mapper(image, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    params := map[string]interface{}{
    - "image": environment.Mapper(image, fullEnv),
    + "image": image,
    }
    if err := Docker(name, pushTemplate, params, st); err != nil {
    --- a/docker/remove.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/remove.go Wed Sep 20 20:07:30 2017 -0500
    @@ -37,8 +37,13 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, image := range r.Images {
    + image, err := environment.Mapper(image, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    params := map[string]interface{}{
    - "image": environment.Mapper(image, fullEnv),
    + "image": image,
    }
    if err := Docker(name, removeTemplate, params, st); err != nil {
    --- a/docker/run.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/run.go Wed Sep 20 20:07:30 2017 -0500
    @@ -120,7 +120,12 @@
    // iterate the script and run the environment variable exapansion on each line
    for idx, item := range r.Script {
    - r.Script[idx] = environment.Mapper(item, fullEnv)
    + item, err := environment.Mapper(item, fullEnv)
    + if err != nil {
    + return "", "", "", err
    + }
    +
    + r.Script[idx] = item
    }
    // write the script to the file
    @@ -145,7 +150,12 @@
    // now expand the environment
    for idx, v := range fullEnv {
    - fullEnv[idx] = environment.Mapper(v, fullEnv)
    + v, err := environment.Mapper(v, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + fullEnv[idx] = v
    }
    // assign a default workspace location
    @@ -155,9 +165,12 @@
    }
    // initialize some variables
    + scriptFile := ""
    entryPoint := r.EntryPoint
    - commandArg := environment.Mapper(r.Command, fullEnv)
    - scriptFile := ""
    + commandArg, err := environment.Mapper(r.Command, fullEnv)
    + if err != nil {
    + return err
    + }
    // if we're using a script defined in the yaml, create it and override
    // some variables
    @@ -177,6 +190,34 @@
    dockerWorkspace := st.Workspace.(*Workspace)
    + //
    + // Map variables
    +
    + image, err := environment.Mapper(r.Image, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + labels, err := environment.SliceMapper(r.Labels, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + workdir, err := environment.Mapper(r.WorkDir, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + workspacePath, err := environment.Mapper(dockerWorkspace.mountPoint, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + workspaceMount, err := environment.Mapper(workSpace, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    // build the dict for the template
    params := map[string]interface{}{
    "Command": commandArg,
    @@ -186,17 +227,17 @@
    "EntryPoint": entryPoint,
    "GID": user.Gid,
    "HealthCheck": r.HealthCheck,
    - "Image": environment.Mapper(r.Image, fullEnv),
    - "Labels": environment.SliceMapper(r.Labels, fullEnv),
    + "Image": image,
    + "Labels": labels,
    "Memory": st.Memory,
    "Network": st.Network.Name(),
    "ScriptFile": scriptFile,
    "SSHAgent": st.EnableSSHAgent,
    "SSHAuthSock": os.Getenv("SSH_AUTH_SOCK"),
    "UID": user.Uid,
    - "WorkDir": environment.Mapper(r.WorkDir, fullEnv),
    - "WorkspacePath": environment.Mapper(dockerWorkspace.mountPoint, fullEnv),
    - "WorkspaceMount": environment.Mapper(workSpace, fullEnv),
    + "WorkDir": workdir,
    + "WorkspacePath": workspacePath,
    + "WorkspaceMount": workspaceMount,
    "TaskLabel": taskLabel,
    }
    --- a/docker/tag.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/tag.go Wed Sep 20 20:07:30 2017 -0500
    @@ -38,9 +38,19 @@
    fullEnv := environment.Merge(env, st.Environment)
    for _, destination := range t.Destinations {
    + source, err := environment.Mapper(t.Source, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    + destination, err := environment.Mapper(destination, fullEnv)
    + if err != nil {
    + return err
    + }
    +
    params := map[string]interface{}{
    - "source": environment.Mapper(t.Source, fullEnv),
    - "destination": environment.Mapper(destination, fullEnv),
    + "source": source,
    + "destination": destination,
    }
    if err := Docker(name, tagTemplate, params, st); err != nil {
    --- a/docker/workspace_test.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/docker/workspace_test.go Wed Sep 20 20:07:30 2017 -0500
    @@ -17,22 +17,8 @@
    */
    package docker
    -import (
    - "fmt"
    -
    - "github.com/aphistic/sweet"
    - . "github.com/onsi/gomega"
    -
    - "bitbucket.org/rw_grim/convey/workspace"
    -)
    +import "bitbucket.org/rw_grim/convey/workspace"
    type workspaceSuite struct{}
    -func (s *workspaceSuite) TestImplementsWorkspace(t sweet.T) {
    - var ws workspace.Workspace
    -
    - fmt.Printf("%#v %v", ws, ws)
    -
    - ws = &Workspace{}
    - Expect(ws).To(Not(BeNil()))
    -}
    +var _ workspace.Workspace = &Workspace{}
    --- a/environment/mapper.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/environment/mapper.go Wed Sep 20 20:07:30 2017 -0500
    @@ -18,6 +18,7 @@
    package environment
    import (
    + "fmt"
    "os"
    "strings"
    )
    @@ -43,28 +44,38 @@
    // Mapper will replace ${TEMPLATE} patterns in the string with the KEY=VAL pairs
    // in the given environment. This function will replace patterns recursively, so
    // if VAL has the form ${TEMPLATE}, it will be replaced again.
    -func Mapper(str string, env []string) string {
    +func Mapper(str string, env []string) (string, error) {
    mapper := envMapper{env}
    last := str
    next := os.Expand(last, mapper.Map)
    + prev := map[string]struct{}{}
    - // TODO - add a max-depth or a list to detect replacement loops
    for last != next {
    + if _, ok := prev[next]; ok {
    + return "", fmt.Errorf("infinite environment mapping loop while expanding '%s'", next)
    + }
    +
    last = next
    next = os.Expand(last, mapper.Map)
    + prev[last] = struct{}{}
    }
    - return next
    + return next, nil
    }
    // SliceMapper calls Mapper for each item in a slice and returns a new slice.
    -func SliceMapper(slice []string, env []string) []string {
    +func SliceMapper(slice []string, env []string) ([]string, error) {
    mapped := []string{}
    for _, template := range slice {
    - mapped = append(mapped, Mapper(template, env))
    + expanded, err := Mapper(template, env)
    + if err != nil {
    + return nil, err
    + }
    +
    + mapped = append(mapped, expanded)
    }
    - return mapped
    + return mapped, nil
    }
    --- a/environment/mapper_test.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/environment/mapper_test.go Wed Sep 20 20:07:30 2017 -0500
    @@ -25,36 +25,57 @@
    )
    func (e *environmentSuite) TestMapperNoOSWithValue(t sweet.T) {
    - Expect(Mapper("${FOO}", []string{"FOO=bar"})).To(Equal("bar"))
    + result, err := Mapper("${FOO}", []string{"FOO=bar"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("bar"))
    }
    func (e *environmentSuite) TestMapperNoOSWithoutValue(t sweet.T) {
    - Expect(Mapper("${FOO}", []string{"FOO="})).To(Equal(""))
    + result, err := Mapper("${FOO}", []string{"FOO="})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal(""))
    }
    func (e *environmentSuite) TestMultipleEquals(t sweet.T) {
    - Expect(Mapper("${FOO}", []string{"FOO=bar=baz"})).To(Equal("bar=baz"))
    + result, err := Mapper("${FOO}", []string{"FOO=bar=baz"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("bar=baz"))
    }
    func (e *environmentSuite) TestMapperOSWithValue(t sweet.T) {
    os.Setenv("FOO", "bar")
    - Expect(Mapper("${FOO}", []string{"FOO"})).To(Equal("bar"))
    + result, err := Mapper("${FOO}", []string{"FOO"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("bar"))
    os.Unsetenv("FOO")
    }
    func (e *environmentSuite) TestMapperOSWithoutValue(t sweet.T) {
    - Expect(Mapper("${FOO}", []string{"FOO"})).To(Equal(""))
    + result, err := Mapper("${FOO}", []string{"FOO"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal(""))
    }
    func (e *environmentSuite) TestPartialMatches(t sweet.T) {
    os.Setenv("FOO", "bar")
    os.Setenv("BAR", "bonk")
    - Expect(Mapper("${FOO}", []string{"FOOBAR"})).To(Equal("$FOO"))
    - Expect(Mapper("${BARBAZ}", []string{"BAR"})).To(Equal("$BARBAZ"))
    +
    + result, err := Mapper("${FOO}", []string{"FOOBAR"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("$FOO"))
    +
    + result, err = Mapper("${BARBAZ}", []string{"BAR"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("$BARBAZ"))
    }
    func (e *environmentSuite) TestRecursiveMatch(t sweet.T) {
    - Expect(Mapper("1${FOO}a", []string{"FOO=2${BAR}b", "BAR=3${BAZ}c", "BAZ=-ohhai!-"})).To(Equal("123-ohhai!-cba"))
    + result, err := Mapper("1${FOO}a", []string{"FOO=2${BAR}b", "BAR=3${BAZ}c", "BAZ=-ohhai!-"})
    + Expect(err).To(BeNil())
    + Expect(result).To(Equal("123-ohhai!-cba"))
    }
    -// TODO - infinite expansion
    +func (e *environmentSuite) TestInfiniteExpansion(t sweet.T) {
    + _, err := Mapper("${FOO}", []string{"FOO=$BAR", "BAR=$FOO"})
    + Expect(err).To(MatchError("infinite environment mapping loop while expanding '$BAR'"))
    +}
    --- a/intrinsic/clean.go Thu Sep 21 00:52:39 2017 +0000
    +++ b/intrinsic/clean.go Wed Sep 20 20:07:30 2017 -0500
    @@ -36,7 +36,10 @@
    }
    func sanitizeFile(base, file string, env []string) (string, error) {
    - filename := environment.Mapper(file, env)
    + filename, err := environment.Mapper(file, env)
    + if err != nil {
    + return "", err
    + }
    pathname, err := filepath.Abs(filename)
    if err != nil {