Skip to content

Commit

Permalink
atc: behaviour: add ArchivePipeline endpoint
Browse files Browse the repository at this point in the history
#5315

Driving this change with tests had some pain points. We decided to work
outside-in, starting with an ATC integration test. It was difficult to
move from the outer TDD loop to the inner - I was surprised by the
number of seemingly-far-flung changes that were required to move from
one failure to the next. These included:

* adding an entry to the auditor to overcome a panic
* modifying the `atc/api/present` package to link the DB entity to the
  API entity

both of these things feel so easy to forget, and it might be nice if
they could be described by a change in the same part of the codebase.

Signed-off-by: Jamie Klassen <cklassen@pivotal.io>
Co-authored-by: Bishoy Youssef <byoussef@pivotal.io>
Co-authored-by: Aidan Oldershaw <aoldershaw@pivotal.io>
  • Loading branch information
3 people committed Mar 23, 2020
1 parent 9308f14 commit 617853e
Show file tree
Hide file tree
Showing 20 changed files with 841 additions and 2 deletions.
5 changes: 5 additions & 0 deletions atc/api/accessor/accessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,11 @@ var _ = Describe("Accessor", func() {
Entry("pipeline-operator :: "+atc.PausePipeline, atc.PausePipeline, accessor.OperatorRole, true),
Entry("viewer :: "+atc.PausePipeline, atc.PausePipeline, accessor.ViewerRole, false),

Entry("owner :: "+atc.ArchivePipeline, atc.ArchivePipeline, accessor.OwnerRole, true),
Entry("member :: "+atc.ArchivePipeline, atc.ArchivePipeline, accessor.MemberRole, false),
Entry("pipeline-operator :: "+atc.ArchivePipeline, atc.ArchivePipeline, accessor.OperatorRole, false),
Entry("viewer :: "+atc.ArchivePipeline, atc.ArchivePipeline, accessor.ViewerRole, false),

Entry("owner :: "+atc.UnpausePipeline, atc.UnpausePipeline, accessor.OwnerRole, true),
Entry("member :: "+atc.UnpausePipeline, atc.UnpausePipeline, accessor.MemberRole, true),
Entry("pipeline-operator :: "+atc.UnpausePipeline, atc.UnpausePipeline, accessor.OperatorRole, true),
Expand Down
1 change: 1 addition & 0 deletions atc/api/accessor/role_action_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ var requiredRoles = map[string]string{
atc.DeletePipeline: MemberRole,
atc.OrderPipelines: MemberRole,
atc.PausePipeline: OperatorRole,
atc.ArchivePipeline: OwnerRole,
atc.UnpausePipeline: OperatorRole,
atc.ExposePipeline: MemberRole,
atc.HidePipeline: MemberRole,
Expand Down
1 change: 1 addition & 0 deletions atc/api/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func NewHandler(
atc.DeletePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.DeletePipeline),
atc.OrderPipelines: http.HandlerFunc(pipelineServer.OrderPipelines),
atc.PausePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.PausePipeline),
atc.ArchivePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.ArchivePipeline),
atc.UnpausePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.UnpausePipeline),
atc.ExposePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.ExposePipeline),
atc.HidePipeline: pipelineHandlerFactory.HandlerFor(pipelineServer.HidePipeline),
Expand Down
39 changes: 38 additions & 1 deletion atc/api/pipelines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var _ = Describe("Pipelines API", func() {
privatePipeline.IDReturns(3)
privatePipeline.PausedReturns(false)
privatePipeline.PublicReturns(false)
privatePipeline.ArchivedReturns(true)
privatePipeline.TeamNameReturns("main")
privatePipeline.NameReturns("private-pipeline")
privatePipeline.GroupsReturns(atc.GroupConfigs{
Expand Down Expand Up @@ -273,7 +274,7 @@ var _ = Describe("Pipelines API", func() {
"name": "private-pipeline",
"paused": false,
"public": false,
"archived": false,
"archived": true,
"team_name": "main",
"last_updated": 1,
"groups": [
Expand Down Expand Up @@ -935,6 +936,42 @@ var _ = Describe("Pipelines API", func() {
})
})

Describe("PUT /api/v1/teams/:team_name/pipelines/:pipeline_name/archive", func() {
var response *http.Response

BeforeEach(func() {
fakeaccess.IsAuthenticatedReturns(true)
fakeaccess.IsAuthorizedReturns(true)
dbTeamFactory.FindTeamReturns(fakeTeam, true, nil)
fakeTeam.PipelineReturns(dbPipeline, true, nil)
})

JustBeforeEach(func() {
request, _ := http.NewRequest("PUT", server.URL+"/api/v1/teams/a-team/pipelines/a-pipeline/archive", nil)
var err error
response, err = client.Do(request)
Expect(err).NotTo(HaveOccurred())
})

It("returns 200", func() {
Expect(response.StatusCode).To(Equal(http.StatusOK))
})

It("archives the pipeline", func() {
Expect(dbPipeline.ArchiveCallCount()).To(Equal(1), "Archive() called the wrong number of times")
})

Context("when archiving the pipeline fails due to the DB", func() {
BeforeEach(func() {
dbPipeline.ArchiveReturns(errors.New("pq: a db error"))
})

It("gives a server error", func() {
Expect(response.StatusCode).To(Equal(http.StatusInternalServerError))
})
})
})

Describe("PUT /api/v1/teams/:team_name/pipelines/:pipeline_name/unpause", func() {
var response *http.Response

Expand Down
18 changes: 18 additions & 0 deletions atc/api/pipelineserver/archive.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package pipelineserver

import (
"net/http"

"github.com/concourse/concourse/atc/db"
)

func (s *Server) ArchivePipeline(pipelineDB db.Pipeline) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
s.logger.Debug("archive-pipeline")
err := pipelineDB.Archive()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
s.logger.Error("archive-pipeline", err)
}
})
}
68 changes: 68 additions & 0 deletions atc/api/pipelineserver/archive_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
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/dbfakes"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

//go:generate counterfeiter code.cloudfoundry.org/lager.Logger

var _ = Describe("Archive 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)
server = pipelineserver.NewServer(
fakeLogger,
new(dbfakes.FakeTeamFactory),
new(dbfakes.FakePipelineFactory),
"",
)
dbPipeline = new(dbfakes.FakePipeline)
handler = server.ArchivePipeline(dbPipeline)
recorder = httptest.NewRecorder()
request = httptest.NewRequest("PUT", "http://example.com", nil)
})

It("logs database errors", func() {
expectedError := errors.New("db error")
dbPipeline.ArchiveReturns(expectedError)

handler.ServeHTTP(recorder, request)

Expect(fakeLogger.ErrorCallCount()).To(Equal(1))
action, actualError, _ := fakeLogger.ErrorArgsForCall(0)
Expect(action).To(Equal("archive-pipeline"), "wrong action name")
Expect(actualError).To(Equal(expectedError))
})

It("write a debug log on every request", func() {
handler.ServeHTTP(recorder, request)

Expect(fakeLogger.DebugCallCount()).To(Equal(1))
action, _ := fakeLogger.DebugArgsForCall(0)
Expect(action).To(Equal("archive-pipeline"), "wrong action name")
})

It("logs no errors if everything works", func() {
dbPipeline.ArchiveReturns(nil)

handler.ServeHTTP(recorder, request)

Expect(fakeLogger.ErrorCallCount()).To(Equal(0))
})
})

0 comments on commit 617853e

Please sign in to comment.