Merge pull request #2681 from drone/issue/2634

Fix for issue #2634

go.sum

@@ -126,8 +126,6 @@ github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uP
 github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
 github.com/hashicorp/go-retryablehttp v0.0.0-20180718195005-e651d75abec6 h1:qCv4319q2q7XKn0MQbi8p37hsJ+9Xo8e6yojA73JVxk=
 github.com/hashicorp/go-retryablehttp v0.0.0-20180718195005-e651d75abec6/go.mod h1:fXcdFsQoipQa7mwORhKad5jmDCeSy/RCGzWA08PO0lM=
-github.com/hashicorp/go-rootcerts v1.0.0 h1:Rqb66Oo1X/eSV1x66xbDccZjhJigjg0+e82kpwzSwCI=
-github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
 github.com/hashicorp/golang-lru v0.5.0 h1:CL2msUPvZTLb5O648aiLNJw3hnBxN2+1Jq8rCOH9wdo=
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/nomad v0.0.0-20190125003214-134391155854 h1:L7WhLZt2ory/kQWxqkMwOiBpIoa4BWoadN7yx8LHEtk=

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

@@ -0,0 +1,148 @@
+// Copyright 2019 Drone IO, Inc.
+// Copyright 2018 natessilva
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dag
+
+// Dag is a directed acyclic graph.
+type Dag struct {
+	graph map[string]*Vertex
+}
+
+// Vertex is a vetex in the graph.
+type Vertex struct {
+	Name  string
+	Skip  bool
+	graph []string
+}
+
+// New creates a new directed acyclic graph (dag) that can
+// determinte if a stage has dependencies.
+func New() *Dag {
+	return &Dag{
+		graph: make(map[string]*Vertex),
+	}
+}
+
+// Add establishes a dependency between two vertices in the graph.
+func (d *Dag) Add(from string, to ...string) *Vertex {
+	vertex := new(Vertex)
+	vertex.Name = from
+	vertex.Skip = false
+	vertex.graph = to
+	d.graph[from] = vertex
+	return vertex
+}
+
+// Get returns the vertex from the graph.
+func (d *Dag) Get(name string) (*Vertex, bool) {
+	vertex, ok := d.graph[name]
+	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
+	}
+	var combined []*Vertex
+	for _, name := range parent.graph {
+		vertex, found := d.graph[name]
+		if !found {
+			continue
+		}
+		if !vertex.Skip {
+			combined = append(combined, vertex)
+		}
+		combined = append(combined, d.ancestors(vertex)...)
+	}
+	return combined
+}
+
+// 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 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
+
+	vertex, ok := d.graph[name]
+	if !ok {
+		return false
+	}
+	for _, v := range vertex.graph {
+		// only check cycles on a vertex one time
+		if !visited[v] {
+			if d.detectCycles(v, visited, recStack) {
+				return true
+			}
+			// if we've visited this vertex in this recursion
+			// stack, then we have a cycle
+		} else if recStack[v] {
+			return true
+		}
+
+	}
+	recStack[name] = false
+	return false
+}

trigger/dag/dag_test.go

@@ -0,0 +1,211 @@
+// Copyright 2019 Drone.IO Inc. All rights reserved.
+// Use of this source code is governed by the Drone Non-Commercial License
+// that can be found in the LICENSE file.
+
+// +build !oss
+
+package dag
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestDag(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend")
+	dag.Add("notify", "backend", "frontend")
+	if dag.DetectCycles() {
+		t.Errorf("cycles detected")
+	}
+
+	dag = New()
+	dag.Add("notify", "backend", "frontend")
+	if dag.DetectCycles() {
+		t.Errorf("cycles detected")
+	}
+
+	dag = New()
+	dag.Add("backend", "frontend")
+	dag.Add("frontend", "backend")
+	dag.Add("notify", "backend", "frontend")
+	if dag.DetectCycles() == false {
+		t.Errorf("Expect cycles detected")
+	}
+
+	dag = New()
+	dag.Add("backend", "backend")
+	dag.Add("frontend", "backend")
+	dag.Add("notify", "backend", "frontend")
+	if dag.DetectCycles() == false {
+		t.Errorf("Expect cycles detected")
+	}
+
+	dag = New()
+	dag.Add("backend")
+	dag.Add("frontend")
+	dag.Add("notify", "backend", "frontend", "notify")
+	if dag.DetectCycles() == false {
+		t.Errorf("Expect cycles detected")
+	}
+}
+
+func TestAncestors(t *testing.T) {
+	dag := New()
+	v := dag.Add("backend")
+	dag.Add("frontend", "backend")
+	dag.Add("notify", "frontend")
+
+	ancestors := dag.Ancestors("frontend")
+	if got, want := len(ancestors), 1; got != want {
+		t.Errorf("Want %d ancestors, got %d", want, got)
+	}
+	if ancestors[0] != v {
+		t.Errorf("Unexpected ancestor")
+	}
+
+	if v := dag.Ancestors("backend"); len(v) != 0 {
+		t.Errorf("Expect vertexes with no dependences has zero ancestors")
+	}
+}
+
+func TestAncestors_Skipped(t *testing.T) {
+	dag := New()
+	dag.Add("backend").Skip = true
+	dag.Add("frontend", "backend").Skip = true
+	dag.Add("notify", "frontend")
+
+	if v := dag.Ancestors("frontend"); len(v) != 0 {
+		t.Errorf("Expect skipped vertexes excluded")
+	}
+	if v := dag.Ancestors("notify"); len(v) != 0 {
+		t.Errorf("Expect skipped vertexes excluded")
+	}
+}
+
+func TestAncestors_NotFound(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend", "backend")
+	dag.Add("notify", "frontend")
+	if dag.DetectCycles() {
+		t.Errorf("cycles detected")
+	}
+	if v := dag.Ancestors("does-not-exist"); len(v) != 0 {
+		t.Errorf("Expect vertex not found does not panic")
+	}
+}
+
+func TestAncestors_Malformed(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend", "does-not-exist")
+	dag.Add("notify", "frontend")
+	if dag.DetectCycles() {
+		t.Errorf("cycles detected")
+	}
+	if v := dag.Ancestors("frontend"); len(v) != 0 {
+		t.Errorf("Expect invalid dependency does not panic")
+	}
+}
+
+func TestAncestors_Complex(t *testing.T) {
+	dag := New()
+	dag.Add("backend")
+	dag.Add("frontend")
+	dag.Add("publish", "backend", "frontend")
+	dag.Add("deploy", "publish")
+	last := dag.Add("notify", "deploy")
+	if dag.DetectCycles() {
+		t.Errorf("cycles detected")
+	}
+
+	ancestors := dag.Ancestors("notify")
+	if got, want := len(ancestors), 4; got != want {
+		t.Errorf("Want %d ancestors, got %d", want, got)
+		return
+	}
+	for _, ancestor := range ancestors {
+		if ancestor == last {
+			t.Errorf("Unexpected ancestor")
+		}
+	}
+
+	v, _ := dag.Get("publish")
+	v.Skip = true
+	ancestors = dag.Ancestors("notify")
+	if got, want := len(ancestors), 3; got != want {
+		t.Errorf("Want %d ancestors, got %d", want, got)
+		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

@@ -26,6 +26,7 @@ import (
 	"github.com/drone/drone-yaml/yaml/signer"
 
 	"github.com/drone/drone/core"
+	"github.com/drone/drone/trigger/dag"
 
 	"github.com/sirupsen/logrus"
 )
@@ -234,6 +235,7 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 	// }
 
 	var matched []*yaml.Pipeline
+	var dag = dag.New()
 	for _, document := range manifest.Resources {
 		pipeline, ok := document.(*yaml.Pipeline)
 		if !ok {
@@ -243,41 +245,41 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 		// TODO add instance
 		// TODO add target
 		// TODO add ref
+		name := pipeline.Name
+		if name == "" {
+			name = "default"
+		}
+		node := dag.Add(pipeline.Name, pipeline.DependsOn...)
+		node.Skip = true
+
 		if skipBranch(pipeline, base.Target) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match branch")
-			continue
 		} else if skipEvent(pipeline, base.Event) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match event")
-			continue
-			// } else if skipPaths(pipeline, paths) {
-			// 	logger.Debug().
-			// 		Str("branch", base.Target).
-			// 		Str("pipeline", pipeline.Name).
-			// 		Msg("skipping pipeline. does not match changed paths")
-			// 	continue
 		} else if skipRef(pipeline, base.Ref) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match ref")
-			continue
 		} else if skipRepo(pipeline, repo.Slug) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match repo")
-			continue
 		} else if skipTarget(pipeline, base.Deployment) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match deploy target")
-			continue
 		} else if skipCron(pipeline, base.Cron) {
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match cron job")
-			continue
 		} else {
 			matched = append(matched, pipeline)
+			node.Skip = false
 		}
 	}
 
+	if dag.DetectCycles() {
+		return t.createBuildError(ctx, repo, base, "Error: Dependency cycle detected in Pipeline")
+	}
+
 	if len(matched) == 0 {
 		logger.Infoln("trigger: skipping build, no matching pipelines")
 		return nil, nil
@@ -365,6 +367,21 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 		stages[i] = stage
 	}
 
+	for _, stage := range stages {
+		// 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
+		}
+	}
+
 	err = t.builds.Create(ctx, build, stages)
 	if err != nil {
 		logger = logger.WithError(err)
@@ -382,10 +399,7 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 	}
 
 	for _, stage := range stages {
-		if len(stage.DependsOn) != 0 {
+		if stage.Status != core.StatusPending {
-			continue
-		}
-		if stage.Status == core.StatusBlocked {
 			continue
 		}
 		err = t.sched.Schedule(ctx, stage)