Skip to content

Commit

Permalink
Merge pull request #280 from carolynvs/fix-stdcopy-panic
Browse files Browse the repository at this point in the history
Fix panic when op.Err is unset in Docker driver
  • Loading branch information
carolynvs committed Feb 25, 2022
2 parents fa5637f + 152b0db commit df6ed99
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
9 changes: 7 additions & 2 deletions driver/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"io/ioutil"
"os"
unix_path "path"
"strconv"
"strings"
Expand Down Expand Up @@ -244,14 +245,18 @@ func (d *Driver) exec(op *driver.Operation) (driver.OperationResult, error) {
return driver.OperationResult{}, fmt.Errorf("unable to retrieve logs: %v", err)
}
var (
stdout = op.Out
stderr = op.Err
stdout io.Writer = os.Stdout
stderr io.Writer = os.Stderr
)
if d.containerOut != nil {
stdout = d.containerOut
} else if op.Out != nil {
stdout = op.Out
}
if d.containerErr != nil {
stderr = d.containerErr
} else if op.Err != nil {
stderr = op.Err
}
go func() {
defer attach.Close()
Expand Down
36 changes: 31 additions & 5 deletions driver/docker/docker_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package docker
import (
"bytes"
"fmt"
"io"
"os"
"testing"

Expand Down Expand Up @@ -39,7 +40,7 @@ func TestDockerDriver_Run(t *testing.T) {
}
}

runDriverTest(t, image)
runDriverTest(t, image, false)
}

// Test running a bundle where the invocation image runs as a nonroot user
Expand All @@ -51,10 +52,10 @@ func TestDockerDriver_RunNonrootInvocationImage(t *testing.T) {
},
}

runDriverTest(t, image)
runDriverTest(t, image, false)
}

func runDriverTest(t *testing.T, image bundle.InvocationImage) {
func runDriverTest(t *testing.T, image bundle.InvocationImage, skipValidations bool) {
op := &driver.Operation{
Installation: "example",
Action: "install",
Expand Down Expand Up @@ -90,10 +91,12 @@ func runDriverTest(t *testing.T, image bundle.InvocationImage) {
}

docker := &Driver{}
docker.SetContainerOut(op.Out) // Docker driver writes container stdout to driver.containerOut.
docker.SetContainerOut(op.Err)
opResult, err := docker.Run(op)

if skipValidations {
return
}

require.NoError(t, err)
assert.Equal(t, "Install action\n\nListing inputs\ninput1\n\nGenerating outputs\nAction install complete for example\n", output.String())
assert.Equal(t, 2, len(opResult.Outputs), "Expecting two output files")
Expand All @@ -103,6 +106,29 @@ func runDriverTest(t *testing.T, image bundle.InvocationImage) {
}, opResult.Outputs)
}

func TestDockerDriver_NoOpErrSet(t *testing.T) {
// Validate that when op.Err is not set that we print to stderr
r, w, err := os.Pipe()
require.NoError(t, err)
origStderr := os.Stderr
defer func() {
os.Stderr = origStderr
}()
os.Stderr = w

image := bundle.InvocationImage{
BaseImage: defaultBaseImage,
}
image.Image += "-oops-this-does-not-exist"
runDriverTest(t, image, true)
w.Close()

var buf bytes.Buffer
_, err = io.Copy(&buf, r)
require.NoError(t, err)
assert.Contains(t, buf.String(), "Unable to find image 'carolynvs/example-outputs:v1.0.0-oops-this-does-not-exist'")
}

func TestDriver_Run_CaptureOutput(t *testing.T) {
image := bundle.InvocationImage{
BaseImage: bundle.BaseImage{
Expand Down

0 comments on commit df6ed99

Please sign in to comment.