Created an error value, in case the for loop fails with index not found.

handlers/pipeline.go

@@ -272,7 +272,9 @@ func PipelineDelete(c echo.Context) error {
 	}
 
 	// Remove from active pipelines
-	pipeline.GlobalActivePipelines.Remove(deletedPipelineIndex)
+	if err := pipeline.GlobalActivePipelines.Remove(deletedPipelineIndex); err != nil {
+		return c.String(http.StatusInternalServerError, err.Error())
+	}
 
 	return c.String(http.StatusOK, "Pipeline has been deleted")
 }

workers/pipeline/pipeline.go

@@ -120,19 +120,28 @@ func (ap *ActivePipelines) Append(p gaia.Pipeline) {
 }
 
 // Update updates a pipeline at the given index with the given pipeline.
-func (ap *ActivePipelines) Update(index int, p gaia.Pipeline) {
+func (ap *ActivePipelines) Update(index int, p gaia.Pipeline) error {
 	ap.Lock()
 	defer ap.Unlock()
 
+	if index >= len(ap.Pipelines) || index < 0 {
+		return fmt.Errorf("invalid index for len %d. index was: %d", len(ap.Pipelines), index)
+	}
 	ap.Pipelines[index] = p
+	return nil
 }
 
 // Remove removes a pipeline at the given index from ActivePipelines.
-func (ap *ActivePipelines) Remove(index int) {
+func (ap *ActivePipelines) Remove(index int) error {
 	ap.Lock()
 	defer ap.Unlock()
 
+	l := len(ap.Pipelines)
+	if index >= l || index+1 > l || index < 0 {
+		return fmt.Errorf("invalid index for len %d. index was: %d", len(ap.Pipelines), index)
+	}
 	ap.Pipelines = append(ap.Pipelines[:index], ap.Pipelines[index+1:]...)
+	return nil
 }
 
 // GetByName looks up the pipeline by the given name.
@@ -175,7 +184,9 @@ func (ap *ActivePipelines) Replace(p gaia.Pipeline) bool {
 func (ap *ActivePipelines) ReplaceByName(n string, p gaia.Pipeline) bool {
 	for index, pipeline := range ap.GetAll() {
 		if pipeline.Name == n {
-			ap.Update(index, p)
+			// We can safely ignore the error here, since it wouldn't even
+			// come this far if it didn't find what to update.
+			_ = ap.Update(index, p)
 			return true
 		}
 	}
@@ -220,7 +231,10 @@ func (ap *ActivePipelines) RemoveDeletedPipelines(existingPipelineNames []string
 		}
 	}
 	for _, idx := range deletedPipelineIndices {
-		ap.Remove(idx)
+		if err := ap.Remove(idx); err != nil {
+			gaia.Cfg.Logger.Error("failed to remove pipeline with index", "index", idx, "error", err.Error())
+			break
+		}
 	}
 }
 

workers/pipeline/pipeline_test.go

@@ -44,7 +44,10 @@ func TestUpdate(t *testing.T) {
 		Created: time.Now(),
 	}
 
-	ap.Update(0, p2)
+	err := ap.Update(0, p2)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	ret := ap.GetByName("Pipeline B")
 
@@ -54,6 +57,28 @@ func TestUpdate(t *testing.T) {
 
 }
 
+func TestUpdateIndexOutOfBounds(t *testing.T) {
+	ap := NewActivePipelines()
+
+	p1 := gaia.Pipeline{
+		Name:    "Pipeline A",
+		Type:    gaia.PTypeGolang,
+		Created: time.Now(),
+	}
+	ap.Append(p1)
+
+	p2 := gaia.Pipeline{
+		Name:    "Pipeline B",
+		Type:    gaia.PTypeGolang,
+		Created: time.Now(),
+	}
+
+	err := ap.Update(1, p2)
+	if err == nil {
+		t.Fatal("expected error to occur since we are out of bounds")
+	}
+}
+
 func TestRemove(t *testing.T) {
 	ap := NewActivePipelines()
 
@@ -71,7 +96,10 @@ func TestRemove(t *testing.T) {
 	}
 	ap.Append(p2)
 
-	ap.Remove(1)
+	err := ap.Remove(1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	count := 0
 	for _, pipeline := range ap.GetAll() {
@@ -86,6 +114,34 @@ func TestRemove(t *testing.T) {
 	}
 }
 
+func TestRemoveInvalidIndex(t *testing.T) {
+	ap := NewActivePipelines()
+
+	p1 := gaia.Pipeline{
+		Name:    "Pipeline A",
+		Type:    gaia.PTypeGolang,
+		Created: time.Now(),
+	}
+	ap.Append(p1)
+
+	p2 := gaia.Pipeline{
+		Name:    "Pipeline B",
+		Type:    gaia.PTypeGolang,
+		Created: time.Now(),
+	}
+	ap.Append(p2)
+
+	err := ap.Remove(2)
+	if err == nil {
+		t.Fatal("expected error when accessing something outside the length ")
+	}
+
+	err = ap.Remove(3)
+	if err == nil {
+		t.Fatal("expected error when accessing something outside the length ")
+	}
+}
+
 func TestGetByName(t *testing.T) {
 	ap := NewActivePipelines()