handle skipped node in middle of dependency chain

handler/api/api.go

@@ -308,6 +308,7 @@ func (s Server) Handler() http.Handler {
 		r.Get("/{namespace}", globalsecrets.HandleList(s.Globals))
 		r.Post("/{namespace}", globalsecrets.HandleCreate(s.Globals))
 		r.Get("/{namespace}/{name}", globalsecrets.HandleFind(s.Globals))
+		r.Post("/{namespace}/{name}", globalsecrets.HandleUpdate(s.Globals))
 		r.Patch("/{namespace}/{name}", globalsecrets.HandleUpdate(s.Globals))
 		r.Delete("/{namespace}/{name}", globalsecrets.HandleDelete(s.Globals))
 	})

trigger/dag/dag.go

@@ -51,12 +51,35 @@ func (d *Dag) Get(name string) (*Vertex, bool) {
 	return vertex, ok
 }
 
+// Dependencies returns the direct dependencies accounting for
+// skipped dependencies.
+func (d *Dag) Dependencies(name string) []string {
+	vertex := d.graph[name]
+	return d.dependencies(vertex)
+}
+
 // Ancestors returns the acentors of the vertex.
 func (d *Dag) Ancestors(name string) []*Vertex {
 	vertex := d.graph[name]
 	return d.ancestors(vertex)
 }
 
+// DetectCycles returns true if cycles are detected in the graph.
+func (d *Dag) DetectCycles() bool {
+	visited := make(map[string]bool)
+	recStack := make(map[string]bool)
+
+	for vertex := range d.graph {
+		if !visited[vertex] {
+			if d.detectCycles(vertex, visited, recStack) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// helper function returns the list of ancestors for the vertex.
 func (d *Dag) ancestors(parent *Vertex) []*Vertex {
 	if parent == nil {
 		return nil
@@ -75,21 +98,30 @@ func (d *Dag) ancestors(parent *Vertex) []*Vertex {
 	return combined
 }
 
-// DetectCycles returns true if cycles are detected in the graph.
-func (d *Dag) DetectCycles() bool {
-	visited := make(map[string]bool)
-	recStack := make(map[string]bool)
-
-	for vertex := range d.graph {
-		if !visited[vertex] {
-			if d.detectCycles(vertex, visited, recStack) {
-				return true
-			}
+// helper function returns the list of dependencies for the,
+// vertex taking into account skipped dependencies.
+func (d *Dag) dependencies(parent *Vertex) []string {
+	if parent == nil {
+		return nil
+	}
+	var combined []string
+	for _, name := range parent.graph {
+		vertex, found := d.graph[name]
+		if !found {
+			continue
+		}
+		if vertex.Skip {
+			// if the vertex is skipped we should move up the
+			// graph and check direct ancestors.
+			combined = append(combined, d.dependencies(vertex)...)
+		} else {
+			combined = append(combined, vertex.Name)
 		}
 	}
-	return false
+	return combined
 }
 
+// helper function returns true if the vertex is cyclical.
 func (d *Dag) detectCycles(name string, visited, recStack map[string]bool) bool {
 	visited[name] = true
 	recStack[name] = true

trigger/dag/dag_test.go

@@ -7,6 +7,7 @@
 package dag
 
 import (
+	"reflect"
 	"testing"
 )
 
@@ -139,3 +140,72 @@ func TestAncestors_Complex(t *testing.T) {
 		return
 	}
 }
+
+func TestDependencies(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend")
+	dag.Add("publish", "backend", "frontend")
+
+	if deps := dag.Dependencies("backend"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies")
+	}
+	if deps := dag.Dependencies("frontend"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies")
+	}
+
+	got, want := dag.Dependencies("publish"), []string{"backend", "frontend"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Unexpected dependencies, got %v", got)
+	}
+}
+
+func TestDependencies_Skipped(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend").Skip = true
+	dag.Add("publish", "backend", "frontend")
+
+	if deps := dag.Dependencies("backend"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies")
+	}
+	if deps := dag.Dependencies("frontend"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies")
+	}
+
+	got, want := dag.Dependencies("publish"), []string{"backend"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Unexpected dependencies, got %v", got)
+	}
+}
+
+func TestDependencies_Complex(t *testing.T) {
+	dag := New()
+	dag.Add("clone")
+	dag.Add("backend")
+	dag.Add("frontend", "backend").Skip = true
+	dag.Add("publish", "frontend", "clone")
+	dag.Add("notify", "publish")
+
+	if deps := dag.Dependencies("clone"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies for clone")
+	}
+	if deps := dag.Dependencies("backend"); len(deps) != 0 {
+		t.Errorf("Expect zero dependencies for backend")
+	}
+
+	got, want := dag.Dependencies("frontend"), []string{"backend"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Unexpected dependencies for frontend, got %v", got)
+	}
+
+	got, want = dag.Dependencies("publish"), []string{"backend", "clone"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Unexpected dependencies for publish, got %v", got)
+	}
+
+	got, want = dag.Dependencies("notify"), []string{"publish"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Unexpected dependencies for notify, got %v", got)
+	}
+}

trigger/trigger.go

@@ -368,10 +368,16 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 	}
 
 	for _, stage := range stages {
-		if stage.Status != core.StatusWaiting {
-			continue
-		}
-		if deps := dag.Ancestors(stage.Name); len(deps) == 0 {
+		// here we re-work the dependencies for the stage to
+		// account for the fact that some steps may be skipped
+		// and may otherwise break the dependnecy chain.
+		stage.DependsOn = dag.Dependencies(stage.Name)
+
+		// if the stage is pending dependencies, but those
+		// dependencies are skipped, the stage can be executed
+		// immediately.
+		if stage.Status == core.StatusWaiting &&
+			len(stage.DependsOn) == 0 {
 			stage.Status = core.StatusPending
 		}
 	}