atc: behaviour: unpausing archived pipelines fails

#5316

Rather than adding a concrete error type to check against, we elected to
create an interface for our domain-specific error to satisfy - see:

https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully

Following the example of trigger-job we decided that 409 Conflict was the
most logical http status to use to represent this kind of policy
violation.

We decided to add the `Conflict` interface to the `db` package because
it seems there is no more-appropriate "business logic" or "policy" layer
to place this kind of validation error. Since multiple backend members
in the web node interact with pipelines directly via the database, it
seems justified to place the business logic as close to that integration
boundary as possible.

Recent painful experiences with observability motivated the choice to
treat logging more like a feature, and have unit tests that specifically
require it.

Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
Co-authored-by: Jamie Klassen <cklassen@pivotal.io>

atc/api/pipelines_test.go

@@ -1035,7 +1035,22 @@ var _ = Describe("Pipelines API", func() {
 					})
 				})
 
-				Context("when unpausing the pipeline fails", func() {
+				Context("when unpausing the pipeline fails with a conflict", func() {
+					BeforeEach(func() {
+						fakeTeam.PipelineReturns(dbPipeline, true, nil)
+						fakeConflict := new(dbfakes.FakeConflict)
+						fakeConflict.ConflictReturns("I'm really conflicted")
+						dbPipeline.UnpauseReturns(fakeConflict)
+					})
+
+					It("returns 409", func() {
+						Expect(response.StatusCode).To(Equal(http.StatusConflict))
+						body, _ := ioutil.ReadAll(response.Body)
+						Expect(body).To(Equal([]byte("I'm really conflicted\n")))
+					})
+				})
+
+				Context("when unpausing the pipeline fails for an unknown reason", func() {
 					BeforeEach(func() {
 						fakeTeam.PipelineReturns(dbPipeline, true, nil)
 						dbPipeline.UnpauseReturns(errors.New("welp"))

atc/api/pipelineserver/unpause.go

@@ -10,6 +10,12 @@ func (s *Server) UnpausePipeline(pipelineDB db.Pipeline) http.Handler {
 	logger := s.logger.Session("unpause-pipeline")
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		err := pipelineDB.Unpause()
+
+		if conflict, ok := err.(db.Conflict); ok {
+			logger.Error("failed-to-unpause-pipeline", err)
+			http.Error(w, conflict.Conflict(), http.StatusConflict)
+			return
+		}
 		if err != nil {
 			logger.Error("failed-to-unpause-pipeline", err)
 			w.WriteHeader(http.StatusInternalServerError)

atc/api/pipelineserver/unpause_test.go

@@ -0,0 +1,88 @@
+package pipelineserver_test
+
+import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
+
+	"github.com/concourse/concourse/atc/api/pipelineserver"
+	"github.com/concourse/concourse/atc/api/pipelineserver/pipelineserverfakes"
+	"github.com/concourse/concourse/atc/db"
+	"github.com/concourse/concourse/atc/db/dbfakes"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Unpause Handler", func() {
+	var (
+		fakeLogger *pipelineserverfakes.FakeLogger
+		server     *pipelineserver.Server
+		dbPipeline *dbfakes.FakePipeline
+		handler    http.Handler
+		recorder   *httptest.ResponseRecorder
+		request    *http.Request
+	)
+
+	BeforeEach(func() {
+		fakeLogger = new(pipelineserverfakes.FakeLogger)
+		fakeLogger.SessionReturns(fakeLogger)
+		server = pipelineserver.NewServer(
+			fakeLogger,
+			new(dbfakes.FakeTeamFactory),
+			new(dbfakes.FakePipelineFactory),
+			"",
+		)
+		dbPipeline = new(dbfakes.FakePipeline)
+		handler = server.UnpausePipeline(dbPipeline)
+		recorder = httptest.NewRecorder()
+		request = httptest.NewRequest("PUT", "http://example.com", nil)
+	})
+
+	Context("when there is a conflict error", func() {
+		var expectedError db.Conflict
+
+		BeforeEach(func() {
+			expectedError = new(dbfakes.FakeConflict)
+			dbPipeline.UnpauseReturns(expectedError)
+		})
+
+		It("logs the error", func() {
+			handler.ServeHTTP(recorder, request)
+
+			Expect(fakeLogger.ErrorCallCount()).To(Equal(1))
+			action, actualError, _ := fakeLogger.ErrorArgsForCall(0)
+			Expect(action).To(Equal("failed-to-unpause-pipeline"), "wrong action name")
+			Expect(actualError).To(Equal(expectedError))
+		})
+
+		It("returns a 409 status code", func() {
+			handler.ServeHTTP(recorder, request)
+
+			Expect(recorder.Code).To(Equal(http.StatusConflict))
+		})
+	})
+
+	Context("when there is a database error", func() {
+		var expectedError error
+
+		BeforeEach(func() {
+			expectedError = errors.New("db error")
+			dbPipeline.UnpauseReturns(expectedError)
+		})
+
+		It("logs the error", func() {
+			handler.ServeHTTP(recorder, request)
+
+			Expect(fakeLogger.ErrorCallCount()).To(Equal(1))
+			action, actualError, _ := fakeLogger.ErrorArgsForCall(0)
+			Expect(action).To(Equal("failed-to-unpause-pipeline"), "wrong action name")
+			Expect(actualError).To(Equal(expectedError))
+		})
+
+		It("returns a 500 status code", func() {
+			handler.ServeHTTP(recorder, request)
+
+			Expect(recorder.Code).To(Equal(http.StatusInternalServerError))
+		})
+	})
+})

atc/db/conflict.go

@@ -0,0 +1,20 @@
+package db
+
+import "fmt"
+
+//go:generate counterfeiter . Conflict
+
+type Conflict interface {
+	error
+	Conflict() string
+}
+
+type conflict string
+
+func (c conflict) Error() string {
+	return fmt.Sprintf("conflict error: %s", string(c))
+}
+
+func (c conflict) Conflict() string {
+	return string(c)
+}

atc/db/pipeline.go

@@ -600,6 +600,10 @@ func (p *pipeline) Pause() error {
 }
 
 func (p *pipeline) Unpause() error {
+	if p.Archived() {
+		return conflict("archived pipelines cannot be unpaused")
+	}
+
 	tx, err := p.conn.Begin()
 	if err != nil {
 		return err

atc/db/pipeline_test.go

@@ -247,6 +247,20 @@ var _ = Describe("Pipeline", func() {
 			})
 		})
 
+		Context("when the pipeline is archived", func() {
+			BeforeEach(func() {
+				pipeline.Archive()
+			})
+
+			It("fails with an archive error", func() {
+				err := pipeline.Unpause()
+				Expect(err).To(HaveOccurred())
+				conflict, isConflict := err.(db.Conflict)
+				Expect(isConflict).To(BeTrue())
+				Expect(conflict.Conflict()).To(Equal("archived pipelines cannot be unpaused"))
+			})
+		})
+
 		Context("when requesting schedule for unpausing pipeline", func() {
 			var found bool
 			var err error

atc/integration/archiving_test.go

@@ -50,6 +50,15 @@ var _ = Describe("ATC Integration Test", func() {
 		Expect(pipeline.Paused).To(BeTrue(), "pipeline was not paused")
 	})
 
+	It("fails when unpausing an archived pipeline", func() {
+		client := login(atcURL, "test", "test")
+		givenAPipeline(client, "pipeline")
+		whenIArchiveIt(client, "pipeline")
+		_, err := client.Team("main").UnpausePipeline("pipeline")
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("archived pipelines cannot be unpaused"))
+	})
+
 	Context("when the archiving pipeline endpoint is not enabled", func() {
 		BeforeEach(func() {
 			cmd.EnableArchivePipeline = false