Skip to content

Address StagesController < PipelinesController inheritance

Overview

Right now the StagesController inherits directly from the PipelinesController. This can be undesirable as the PipelinesController has a set of actions and operations that are considered final which are served by the set of endpoints described by: /pipelines(/:id)(.format). When we inherit from PipelinesController, a badly written route can expose these actions inadvertently in a different context, like: /pipelines/stages/....

Proposal

If our intent is to have a controller that we can inherit from, we could create a base or abstract controller like: Projects::(Base|Abstract)PipelinesController where PipelinesController/Stages/TestsControllers and others could inherit from.

Overall, we follow a model wherein there is a Projects::ApplicationController that is serving as an abstract controller. I would assume that we could follow the same pattern for Projects::Pipelines::ApplicationController and have it be a top-level abstract controller for a given set of Pipeline functionalities that are common, like finding pipeline_id.

For example:

Projects::PipelinesController < Projects::ApplicationController
# a top-level controller for handling `/pipelines(/:id)(.format)`

Projects::Pipelines::ApplicationController < Projects::ApplicationController
# an abstract controller for all subpaths of a `/pipelines/:pipeline_id`

Projects::Pipelines::TestsController < Projects::Pipelines::ApplicationController
# `/pipelines/:pipeline_id/tests(/:id)(.format)`

Projects::Pipelines::StagesController < Projects::Pipelines::ApplicationController
# `/pipelines/:pipeline_id/stages(/:id)(.format)`

Ensure that this MR is reviewed by a reviewer and a maintainer in ~"group::continuous integration" .

Motivation

The following discussion from !33895 (closed) should be addressed:

OMG. I saw that. This is really bad idea to inherit PipelinesController that implements another actions. We are leaking a number of actions that are unrelated to a given controller.

👋 @rickywiens could you help me identify/prioritize who/when should we fix this controller ?

Edited by Ricky Wiens