Monorepo for Tangled tangled.org

lexicons,appview,spindle: add workflow cancel #889

merged opened by boltless.me targeting master from sl/okmkyytolvko
Labels

None yet.

assignee

None yet.

Participants 2
AT URI
at://did:plc:xasnlahkri4ewmbuzly2rlc5/sh.tangled.repo.pull/3m7zmleup7g22
+25 -20
Interdiff #1 #2
api/tangled/pipelinecancelPipeline.go

This file has not been changed.

+15 -12
appview/pages/templates/repo/pipelines/workflow.html
··· 11 11 <div class="col-span-1"> 12 12 {{ block "sidebar" . }} {{ end }} 13 13 </div> 14 - <div class="col-span-1 md:col-span-3"> 15 - <div class="flex justify-end mb-2"> 16 - <button 17 - class="btn" 18 - hx-post="/{{ $.RepoInfo.FullName }}/pipelines/{{ .Pipeline.Id }}/workflow/{{ .Workflow }}/cancel" 19 - hx-swap="none" 20 - {{ if (index .Pipeline.Statuses .Workflow).Latest.Status.IsFinish -}} 21 - disabled 22 - {{- end }} 23 - >Cancel</button> 14 + <!-- TODO(boltless): explictly check for pipeline cancel permission --> 15 + {{ if not $.RepoInfo.Roles.IsOwner }} 16 + <div class="col-span-1 md:col-span-3"> 17 + <div class="flex justify-end mb-2"> 18 + <button 19 + class="btn" 20 + hx-post="/{{ $.RepoInfo.FullName }}/pipelines/{{ .Pipeline.Id }}/workflow/{{ .Workflow }}/cancel" 21 + hx-swap="none" 22 + {{ if (index .Pipeline.Statuses .Workflow).Latest.Status.IsFinish -}} 23 + disabled 24 + {{- end }} 25 + >Cancel</button> 26 + </div> 27 + {{ block "logs" . }} {{ end }} 24 28 </div> 25 - {{ block "logs" . }} {{ end }} 26 - </div> 29 + {{ end }} 27 30 </section> 28 31 {{ template "fragments/workflow-timers" }} 29 32 {{ end }}
+7 -5
appview/pipelines/pipelines.go
··· 13 13 "tangled.org/core/api/tangled" 14 14 "tangled.org/core/appview/config" 15 15 "tangled.org/core/appview/db" 16 + "tangled.org/core/appview/middleware" 16 17 "tangled.org/core/appview/models" 17 18 "tangled.org/core/appview/oauth" 18 19 "tangled.org/core/appview/pages" ··· 39 40 logger *slog.Logger 40 41 } 41 42 42 - func (p *Pipelines) Router() http.Handler { 43 + func (p *Pipelines) Router(mw *middleware.Middleware) http.Handler { 43 44 r := chi.NewRouter() 44 45 r.Get("/", p.Index) 45 46 r.Get("/{pipeline}/workflow/{workflow}", p.Workflow) 46 47 r.Get("/{pipeline}/workflow/{workflow}/logs", p.Logs) 47 - r.Post("/{pipeline}/workflow/{workflow}/cancel", p.Cancel) 48 + r. 49 + With(mw.RepoPermissionMiddleware("repo:owner")). 50 + Post("/{pipeline}/workflow/{workflow}/cancel", p.Cancel) 48 51 49 52 return r 50 53 } ··· 375 378 r, 376 379 oauth.WithService(f.Spindle), 377 380 oauth.WithLxm(tangled.PipelineCancelPipelineNSID), 378 - oauth.WithExp(60), 379 - oauth.WithDev(false), 381 + oauth.WithDev(p.config.Core.Dev), 380 382 oauth.WithTimeout(time.Second*30), // workflow cleanup usually takes time 381 383 ) 382 384 ··· 392 394 errorId := "pipeline-action" 393 395 if err != nil { 394 396 l.Error("failed to cancel pipeline", "err", err) 395 - p.pages.Notice(w, errorId, "Failed to add secret.") 397 + p.pages.Notice(w, errorId, "Failed to cancel pipeline") 396 398 return 397 399 } 398 400 l.Debug("canceled pipeline", "uri", pipeline.AtUri())
lexicons/pipeline/cancelPipeline.json

This file has not been changed.

spindle/db/events.go

This file has not been changed.

spindle/server.go

This file has not been changed.

spindle/xrpc/pipeline_cancelPipeline.go

This file has not been changed.

spindle/xrpc/xrpc.go

This file has not been changed.

+3 -3
appview/state/router.go
··· 96 96 r.Mount("/", s.RepoRouter(mw)) 97 97 r.Mount("/issues", s.IssuesRouter(mw)) 98 98 r.Mount("/pulls", s.PullsRouter(mw)) 99 - r.Mount("/pipelines", s.PipelinesRouter()) 99 + r.Mount("/pipelines", s.PipelinesRouter(mw)) 100 100 r.Mount("/labels", s.LabelsRouter()) 101 101 102 102 // These routes get proxied to the knot ··· 313 313 return repo.Router(mw) 314 314 } 315 315 316 - func (s *State) PipelinesRouter() http.Handler { 316 + func (s *State) PipelinesRouter(mw *middleware.Middleware) http.Handler { 317 317 pipes := pipelines.New( 318 318 s.oauth, 319 319 s.repoResolver, ··· 325 325 s.enforcer, 326 326 log.SubLogger(s.logger, "pipelines"), 327 327 ) 328 - return pipes.Router() 328 + return pipes.Router(mw) 329 329 } 330 330 331 331 func (s *State) LabelsRouter() http.Handler {

History

7 rounds 7 comments
sign up or login to add to the discussion
1 commit
expand
lexicons,appview,spindle: add workflow cancel
3/3 success
expand
expand 0 comments
pull request successfully merged
1 commit
expand
lexicons,appview,spindle: add workflow cancel
1/3 failed, 2/3 success
expand
expand 1 comment
  • here, failedis spelled asdailed`
  • here, err := fmt.Errorf(...) should probably be removed.
1 commit
expand
lexicons,appview,spindle: add workflow cancel
3/3 success
expand
expand 0 comments
1 commit
expand
lexicons,appview,spindle: add workflow cancel
expand 0 comments
1 commit
expand
lexicons,appview,spindle: add workflow cancel
expand 0 comments
1 commit
expand
lexicons,appview,spindle: add workflow cancel
expand 6 comments

requesting a review @oppi.li

can we allow owners to also cancel pipelines?

sorry, i mean collaborators.

@oppi.li I'm not sure how should I implement that. We don't have repo:pipeline action defined for repo owners and contributors.

For example, setting access is controlled by repo:settings.

Should I just check for both repo:owner and repo:contributor as a dirty fix?

can we allow collaborators to also cancel pipelines?

@oppi.li I think we can merge this for now. Spindle's collaborator ingesting logic is currently broken so allowing them to cancel pipelines will reveal the bug. I'll make it possible on upcoming PR I'm working on sl/spindle-rewrite branch.

reviews:

  • here, we should use config.Core.Dev
  • here, don't set expiry, the default is already 60s into the future
  • there is no auth filter for the cancel button on the appview side, the button should only be visible to repo owners + collaborators, and the /workflow/cancel endpoint should return 403 for non-collaborators and non-owners.
  • here, the error message needs to be "Failed to cancel workflow", not "Failed to add secret"

the rest of the code is brilliant, will give this a test locally as well!

1 commit
expand
lexicons,appview,spindle: add workflow cancel
expand 0 comments