Merge pull request #6516 from LK4D4/validate_logs_params

Validate that one of streams choosen in logs on api side

api/server/server.go

@@ -370,13 +370,23 @@ func getContainersLogs(eng *engine.Engine, version version.Version, w http.Respo
 	}
 
 	var (
-		job    = eng.Job("container_inspect", vars["name"])
-		c, err = job.Stdout.AddEnv()
+		inspectJob = eng.Job("container_inspect", vars["name"])
+		logsJob    = eng.Job("logs", vars["name"])
+		c, err     = inspectJob.Stdout.AddEnv()
 	)
 	if err != nil {
 		return err
 	}
-	if err = job.Run(); err != nil {
+	logsJob.Setenv("follow", r.Form.Get("follow"))
+	logsJob.Setenv("stdout", r.Form.Get("stdout"))
+	logsJob.Setenv("stderr", r.Form.Get("stderr"))
+	logsJob.Setenv("timestamps", r.Form.Get("timestamps"))
+	// Validate args here, because we can't return not StatusOK after job.Run() call
+	stdout, stderr := logsJob.GetenvBool("stdout"), logsJob.GetenvBool("stderr")
+	if !(stdout || stderr) {
+		return fmt.Errorf("Bad parameters: you must choose at least one stream")
+	}
+	if err = inspectJob.Run(); err != nil {
 		return err
 	}
 
@@ -390,14 +400,9 @@ func getContainersLogs(eng *engine.Engine, version version.Version, w http.Respo
 		errStream = outStream
 	}
 
-	job = eng.Job("logs", vars["name"])
-	job.Setenv("follow", r.Form.Get("follow"))
-	job.Setenv("stdout", r.Form.Get("stdout"))
-	job.Setenv("stderr", r.Form.Get("stderr"))
-	job.Setenv("timestamps", r.Form.Get("timestamps"))
-	job.Stdout.Add(outStream)
-	job.Stderr.Set(errStream)
-	if err := job.Run(); err != nil {
+	logsJob.Stdout.Add(outStream)
+	logsJob.Stderr.Set(errStream)
+	if err := logsJob.Run(); err != nil {
 		fmt.Fprintf(outStream, "Error running logs job: %s\n", err)
 	}
 	return nil

api/server/server_unit_test.go

@@ -4,12 +4,14 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"github.com/dotcloud/docker/api"
-	"github.com/dotcloud/docker/engine"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+
+	"github.com/dotcloud/docker/api"
+	"github.com/dotcloud/docker/engine"
 )
 
 func TestGetBoolParam(t *testing.T) {
@@ -151,6 +153,99 @@ func TestGetContainersByName(t *testing.T) {
 	}
 }
 
+func TestLogs(t *testing.T) {
+	eng := engine.New()
+	var inspect bool
+	var logs bool
+	eng.Register("container_inspect", func(job *engine.Job) engine.Status {
+		inspect = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		return engine.StatusOK
+	})
+	expected := "logs"
+	eng.Register("logs", func(job *engine.Job) engine.Status {
+		logs = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		follow := job.Getenv("follow")
+		if follow != "1" {
+			t.Fatalf("follow: %s, must be 1", follow)
+		}
+		stdout := job.Getenv("stdout")
+		if stdout != "1" {
+			t.Fatalf("stdout %s, must be 1", stdout)
+		}
+		stderr := job.Getenv("stderr")
+		if stderr != "" {
+			t.Fatalf("stderr %s, must be empty", stderr)
+		}
+		timestamps := job.Getenv("timestamps")
+		if timestamps != "1" {
+			t.Fatalf("timestamps %s, must be 1", timestamps)
+		}
+		job.Stdout.Write([]byte(expected))
+		return engine.StatusOK
+	})
+	r := serveRequest("GET", "/containers/test/logs?follow=1&stdout=1&timestamps=1", nil, eng, t)
+	if r.Code != http.StatusOK {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusOK)
+	}
+	if !inspect {
+		t.Fatal("container_inspect job was not called")
+	}
+	if !logs {
+		t.Fatal("logs job was not called")
+	}
+	res := r.Body.String()
+	if res != expected {
+		t.Fatalf("Output %s, expected %s", res, expected)
+	}
+}
+
+func TestLogsNoStreams(t *testing.T) {
+	eng := engine.New()
+	var inspect bool
+	var logs bool
+	eng.Register("container_inspect", func(job *engine.Job) engine.Status {
+		inspect = true
+		if len(job.Args) == 0 {
+			t.Fatal("Job arguments is empty")
+		}
+		if job.Args[0] != "test" {
+			t.Fatalf("Container name %s, must be test", job.Args[0])
+		}
+		return engine.StatusOK
+	})
+	eng.Register("logs", func(job *engine.Job) engine.Status {
+		logs = true
+		return engine.StatusOK
+	})
+	r := serveRequest("GET", "/containers/test/logs", nil, eng, t)
+	if r.Code != http.StatusBadRequest {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusBadRequest)
+	}
+	if inspect {
+		t.Fatal("container_inspect job was called, but it shouldn't")
+	}
+	if logs {
+		t.Fatal("logs job was called, but it shouldn't")
+	}
+	res := strings.TrimSpace(r.Body.String())
+	expected := "Bad parameters: you must choose at least one stream"
+	if !strings.Contains(res, expected) {
+		t.Fatalf("Output %s, expected %s in it", res, expected)
+	}
+}
+
 func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t *testing.T) *httptest.ResponseRecorder {
 	r := httptest.NewRecorder()
 	req, err := http.NewRequest(method, target, body)