Implemented pipeline name already in use check (#153)

* Implemented pipeline name already in use check

* Removed break from iterate

* Implemented pipeline name validation also in create pipeline API

* Corrected one line to short if

Author: michelvocks

handlers/handler.go

@@ -7,7 +7,7 @@ import (
 	"net/http"
 	"strings"
 
-	"github.com/GeertJohan/go.rice"
+	rice "github.com/GeertJohan/go.rice"
 	jwt "github.com/dgrijalva/jwt-go"
 	"github.com/gaia-pipeline/gaia"
 	"github.com/labstack/echo"
@@ -18,9 +18,6 @@ var (
 	// errNotAuthorized is thrown when user wants to access resource which is protected
 	errNotAuthorized = errors.New("no or invalid jwt token provided. You are not authorized")
 
-	// errPathLength is a validation error during pipeline name input
-	errPathLength = errors.New("name of pipeline is empty or one of the path elements length exceeds 50 characters")
-
 	// errPipelineNotFound is thrown when a pipeline was not found with the given id
 	errPipelineNotFound = errors.New("pipeline not found with the given id")
 

handlers/pipeline.go

@@ -3,7 +3,6 @@ package handlers
 import (
 	"net/http"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/gaia-pipeline/gaia"
@@ -14,11 +13,6 @@ import (
 	uuid "github.com/satori/go.uuid"
 )
 
-const (
-	// Split char to separate path from pipeline and name
-	pipelinePathSplitChar = "/"
-)
-
 // PipelineGitLSRemote checks for available git remote branches.
 // This is the perfect way to check if we have access to a given repo.
 func PipelineGitLSRemote(c echo.Context) error {
@@ -46,6 +40,11 @@ func CreatePipeline(c echo.Context) error {
 		return c.String(http.StatusBadRequest, err.Error())
 	}
 
+	// Validate pipeline name
+	if err := pipeline.ValidatePipelineName(p.Pipeline.Name); err != nil {
+		return c.String(http.StatusBadRequest, err.Error())
+	}
+
 	// Set initial value
 	p.Created = time.Now()
 	p.StatusType = gaia.CreatePipelineRunning
@@ -84,18 +83,8 @@ func CreatePipelineGetAll(c echo.Context) error {
 // available and valid.
 func PipelineNameAvailable(c echo.Context) error {
 	pName := c.QueryParam("name")
-
-	// The name could contain a path. Split it up
-	path := strings.Split(pName, pipelinePathSplitChar)
-
-	// Iterate all objects
-	for _, s := range path {
-		// Length should be correct
-		if len(s) < 1 || len(s) > 50 {
-			return c.String(http.StatusBadRequest, errPathLength.Error())
-		}
-
-		// TODO check if pipeline name is already in use
+	if err := pipeline.ValidatePipelineName(pName); err != nil {
+		return c.String(http.StatusBadRequest, err.Error())
 	}
 
 	return nil

handlers/pipeline_test.go

@@ -468,3 +468,110 @@ func TestPipelineCheckPeriodicSchedules(t *testing.T) {
 		}
 	})
 }
+
+func TestPipelineNameAvailable(t *testing.T) {
+	tmp, _ := ioutil.TempDir("", "TestPipelineNameAvailable")
+	dataDir := tmp
+
+	gaia.Cfg = &gaia.Config{
+		Logger:       hclog.NewNullLogger(),
+		HomePath:     dataDir,
+		DataPath:     dataDir,
+		PipelinePath: dataDir,
+	}
+
+	// Initialize store
+	dataStore, _ := services.StorageService()
+	dataStore.Init()
+	defer func() { services.MockStorageService(nil) }()
+
+	// Initialize global active pipelines
+	ap := pipeline.NewActivePipelines()
+	pipeline.GlobalActivePipelines = ap
+
+	// Initialize echo
+	e := echo.New()
+	InitHandlers(e)
+
+	p := gaia.Pipeline{
+		ID:      1,
+		Name:    "Pipeline A",
+		Type:    gaia.PTypeGolang,
+		Created: time.Now(),
+	}
+
+	// Add to store
+	err := dataStore.PipelinePut(&p)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Add to active pipelines
+	ap.Append(p)
+
+	t.Run("fails for pipeline name already in use", func(t *testing.T) {
+		req := httptest.NewRequest(echo.GET, "/", nil)
+		req.Header.Set("Content-Type", "application/json")
+		rec := httptest.NewRecorder()
+		c := e.NewContext(req, rec)
+		c.SetPath("/api/" + gaia.APIVersion + "/pipeline/name")
+		q := req.URL.Query()
+		q.Add("name", "pipeline a")
+		req.URL.RawQuery = q.Encode()
+
+		PipelineNameAvailable(c)
+
+		if rec.Code != http.StatusBadRequest {
+			t.Fatalf("expected response code %v got %v", http.StatusBadRequest, rec.Code)
+		}
+		bodyBytes, err := ioutil.ReadAll(rec.Body)
+		if err != nil {
+			t.Fatalf("cannot read response body: %s", err.Error())
+		}
+		nameAlreadyInUseMessage := "pipeline name is already in use"
+		if string(bodyBytes[:]) != nameAlreadyInUseMessage {
+			t.Fatalf("error message should be '%s' but was '%s'", nameAlreadyInUseMessage, string(bodyBytes[:]))
+		}
+	})
+
+	t.Run("fails for pipeline name is too long", func(t *testing.T) {
+		req := httptest.NewRequest(echo.GET, "/", nil)
+		req.Header.Set("Content-Type", "application/json")
+		rec := httptest.NewRecorder()
+		c := e.NewContext(req, rec)
+		c.SetPath("/api/" + gaia.APIVersion + "/pipeline/name")
+		q := req.URL.Query()
+		q.Add("name", "pipeline a pipeline a pipeline a pipeline a pipeline a pipeline a pipeline a pipeline a pipeline a pipeline a")
+		req.URL.RawQuery = q.Encode()
+
+		PipelineNameAvailable(c)
+
+		if rec.Code != http.StatusBadRequest {
+			t.Fatalf("expected response code %v got %v", http.StatusBadRequest, rec.Code)
+		}
+		bodyBytes, err := ioutil.ReadAll(rec.Body)
+		if err != nil {
+			t.Fatalf("cannot read response body: %s", err.Error())
+		}
+		nameTooLongMessage := "name of pipeline is empty or one of the path elements length exceeds 50 characters"
+		if string(bodyBytes[:]) != nameTooLongMessage {
+			t.Fatalf("error message should be '%s' but was '%s'", nameTooLongMessage, string(bodyBytes[:]))
+		}
+	})
+
+	t.Run("works for pipeline with different name", func(t *testing.T) {
+		req := httptest.NewRequest(echo.GET, "/", nil)
+		req.Header.Set("Content-Type", "application/json")
+		rec := httptest.NewRecorder()
+		c := e.NewContext(req, rec)
+		c.SetPath("/api/" + gaia.APIVersion + "/pipeline/name")
+		q := req.URL.Query()
+		q.Add("name", "pipeline b")
+		req.URL.RawQuery = q.Encode()
+
+		PipelineNameAvailable(c)
+
+		if rec.Code != http.StatusOK {
+			t.Fatalf("expected response code %v got %v", http.StatusOK, rec.Code)
+		}
+	})
+}

workers/pipeline/create_pipeline.go

@@ -1,7 +1,9 @@
 package pipeline
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/gaia-pipeline/gaia"
 	"github.com/gaia-pipeline/gaia/services"
@@ -19,6 +21,17 @@ const (
 
 	// Completed percent progress
 	pipelineCompleteStatus = 100
+
+	// Split char to separate path from pipeline and name
+	pipelinePathSplitChar = "/"
+)
+
+var (
+	// errPathLength is a validation error during pipeline name input
+	errPathLength = errors.New("name of pipeline is empty or one of the path elements length exceeds 50 characters")
+
+	// errPipelineNameInUse is thrown when a pipelines name is already in use
+	errPipelineNameInUse = errors.New("pipeline name is already in use")
 )
 
 // CreatePipeline is the main function which executes step by step the creation
@@ -145,3 +158,32 @@ func CreatePipeline(p *gaia.CreatePipeline) {
 		}
 	}
 }
+
+// ValidatePipelineName validates a given pipeline name and
+// returns the correct error back.
+func ValidatePipelineName(pName string) error {
+	// The name could contain a path. Split it up.
+	path := strings.Split(pName, pipelinePathSplitChar)
+
+	// Iterate all objects.
+	for _, s := range path {
+		// Length should be correct.
+		if len(s) < 1 || len(s) > 50 {
+			return errPathLength
+		}
+
+		// Check if pipeline name is already in use.
+		alreadyInUse := false
+		for activePipeline := range GlobalActivePipelines.Iter() {
+			if strings.ToLower(s) == strings.ToLower(activePipeline.Name) {
+				alreadyInUse = true
+			}
+		}
+
+		// Throw error because it's already in use.
+		if alreadyInUse {
+			return errPipelineNameInUse
+		}
+	}
+	return nil
+}