Follow-up: Move Security Report ingestion worker logic out of state machine transitions
The following discussion from !143046 (merged) should be addressed:
-
@mbobin started a discussion: (+4 comments) @bala.kumar adding all these checks to transitions seem like a lot of work when we're saying that we don't want to to add more logic to the states
I think it would be better to just enqueue the workers and add the guards conditions in there. This will make the state machine easier to understand and debug. wdyt?
Background
Broadly speaking, the folks over in devopsverify are trying to move logic out of these state machine transition blocks because they're difficult to understand and modify in a reliable way. Lots of functionality related to Pipelines, e.g. "gather and digest security reports" functionally becomes the responsibility of the Pipeline class, when Pipelines are really just a triggering event and don't want or need to know anything about the security reports. Lots of functionality does this, I'm not picking on security reports for any particular reason, it's just what was in front of us in !143046 (comment 1770373615)
Any error in these transition blocks can prevent the state transition of the Pipeline itself, which has widespread implications for the rest of CI processing and the other features that rely on the state. The more ancillary features using these transitions, the more they become interdependent on each other. This is also not great for the PipelineProcessWorker, which is the de facto execution home for non-CI "Pipeline" logic.
So for these Security Reports async trigger blocks, what?
The immediately preferable option would be to move logical guards out of the transition block and into the worker class, which could execute as (close to) no-op job if the condition isn't met. With CI Events not fully available, this is a good start on separating the responsibilities of Pipelines and features triggered by Pipeline state transitions. From here, there should be a non-huge jump to triggering a worker based on the creation of a CI Event.