You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It's been about a year since we moved our CI stack from Gerrit and Jenkins to GitHub but I'm still struck by how often I'm still running into "Github Gotchas". These are things that make what should be simple tasks hair-pullingly complex. This time, all I wanted to do was add a Postgres service container to our testing workflows. Right off the bat, I ran into the "you can't use service containers from a composite action" gotcha. Great. In order to keep user-irrelevent workflow stuff out of our main repos, we use a lot of composite actions from a separate, dedicated CI repo. So then I started down the path of replacing the composite actions with reusable workflows. That resulted in weeks of dredging up old memories of all the issues we encountered when we first migrated, plus a few new ones.
The fact that reusable workflows don't share the environment with their callers is documented but I don't unterstand why this restriction is in place. It would seem natural that they would share.
Not only do reusable workflows not share their environment, you can't call a reusable workflow and pass a variable from the caller's environment as an input.
results in: The workflow is not valid. ... Unrecognized named-value: 'env'. Located at position 1 within expression: env.COUNTER. That's NOT documented anywhere (at least where I could find it). You can get around this with a job/step to place the needed data into the step output that would be referenced later with needs.somestep.outputs... but it's just another ugly hack.
You also can't pass an input field defined as a number in a parent workflow's workflow_dispatch trigger to a callable workflow with an input field also defined as a number because even though workflow_dispatch defined it as a number, it's captured as a quoted string. Attempting to pass it to the child workflow results in The template is not valid. .github/workflows/TestWorkflow.yml (Line: 17, Col: 16): Unexpected value '666'.
Yes, the same ugly hack mentioned earlier can be used.
Workflows triggered by PRs submitted from forked repos are too limited in what they can do or access. I understand that some restrictions are necessary to remain secure but no read access to repo or organization action variables??? Come on! We use them extensively to control the workflows but for these PRs we have to run periodic jobs to dump the action variables to JSON then have these PRs call an action that retrieves the JSON. Even then we have to keep calling fromJSON() every time we want to use one of the variables. This also makes reusable workflows a bit problematic since sometimes a workflow may be called from a context that allows access to the variables directly and sometimes not.
Sticking with forked repo PRs... The workflows they trigger should be allowed to at least manipulate comments and labels on the PR that triggered them (maybe controlled with a setting). Right now, we have to use a separate workflow triggered by workflow_run but that comes with its own problems... The "requested" action won't re-run when the triggering job is re-run and the PR number that triggered the original workflow isn't anywhere in the context passed to the triggered workflow. We have to use the commit SHA and run a query to find the related PR. Oh, another issue is that if the original workflow is skipped, the triggered workflow is still triggered with the "requested" action with no way to tell that the original workflow was skipped. Trying to use the "in_progress" action also has issues. If there's more than 1 job in the triggering workflow and they're sequenced using "needs", multiple "in_progress" events are generated with nothing in the context that indicates which job each "in_progress" applies to.
Speaking of skipped... The pull_request and pull_request_target "labeled" action is way too coarse. If you have 4 workflows A, B, C and D, triggered by labels LA, LB, LC and LD respectively, and you add label LA to a PR, all 4 jobs run with B, C and D being marked as skipped. This clutters up both the PR checks list and the jobs list. Even worse, if workflow A added 5 checks to the PR, then you add label LB to the PR, the B workflow runs but since A also runs and is then skipped, all the checks that it added when it was legitimately run earlier are wiped out. The workflow trigger really needs to include the actual label so the workflow itself is not considered to even run. We also need an option to control whether skipped jobs even show up in the PR checks list. They're just clutter now.
And speaking of checks... We need a way to indicate whether a job should create a checks entry or not. Jobs that perform setup or cleanup actions have no business in the checks list. We also need a way to customize the checks name. When you have nested workflows the checks name can get really long and push any distinguishing bits into ... and it only gets worse if any of those jobs are from a matrix. This also affects the ruleset creation process when using "require checks to pass before merging". Being able to set custom check names will make that process much easier as well.
ActionsBuild, test, and automate your deployment pipeline with world-class CI/CDProduct Feedback
1 participant
Heading
Bold
Italic
Quote
Code
Link
Numbered list
Unordered list
Task list
Attach files
Mention
Reference
Menu
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Select Topic Area
Product Feedback
Body
It's been about a year since we moved our CI stack from Gerrit and Jenkins to GitHub but I'm still struck by how often I'm still running into "Github Gotchas". These are things that make what should be simple tasks hair-pullingly complex. This time, all I wanted to do was add a Postgres service container to our testing workflows. Right off the bat, I ran into the "you can't use service containers from a composite action" gotcha. Great. In order to keep user-irrelevent workflow stuff out of our main repos, we use a lot of composite actions from a separate, dedicated CI repo. So then I started down the path of replacing the composite actions with reusable workflows. That resulted in weeks of dredging up old memories of all the issues we encountered when we first migrated, plus a few new ones.
The fact that reusable workflows don't share the environment with their callers is documented but I don't unterstand why this restriction is in place. It would seem natural that they would share.
Not only do reusable workflows not share their environment, you can't call a reusable workflow and pass a variable from the caller's environment as an input.
results in:
The workflow is not valid. ... Unrecognized named-value: 'env'. Located at position 1 within expression: env.COUNTER
. That's NOT documented anywhere (at least where I could find it). You can get around this with a job/step to place the needed data into the step output that would be referenced later withneeds.somestep.outputs...
but it's just another ugly hack.You also can't pass an input field defined as a number in a parent workflow's workflow_dispatch trigger to a callable workflow with an input field also defined as a number because even though workflow_dispatch defined it as a number, it's captured as a quoted string. Attempting to pass it to the child workflow results in
The template is not valid. .github/workflows/TestWorkflow.yml (Line: 17, Col: 16): Unexpected value '666'
.Yes, the same ugly hack mentioned earlier can be used.
Workflows triggered by PRs submitted from forked repos are too limited in what they can do or access. I understand that some restrictions are necessary to remain secure but no read access to repo or organization action variables??? Come on! We use them extensively to control the workflows but for these PRs we have to run periodic jobs to dump the action variables to JSON then have these PRs call an action that retrieves the JSON. Even then we have to keep calling fromJSON() every time we want to use one of the variables. This also makes reusable workflows a bit problematic since sometimes a workflow may be called from a context that allows access to the variables directly and sometimes not.
Sticking with forked repo PRs... The workflows they trigger should be allowed to at least manipulate comments and labels on the PR that triggered them (maybe controlled with a setting). Right now, we have to use a separate workflow triggered by workflow_run but that comes with its own problems... The "requested" action won't re-run when the triggering job is re-run and the PR number that triggered the original workflow isn't anywhere in the context passed to the triggered workflow. We have to use the commit SHA and run a query to find the related PR. Oh, another issue is that if the original workflow is skipped, the triggered workflow is still triggered with the "requested" action with no way to tell that the original workflow was skipped. Trying to use the "in_progress" action also has issues. If there's more than 1 job in the triggering workflow and they're sequenced using "needs", multiple "in_progress" events are generated with nothing in the context that indicates which job each "in_progress" applies to.
Speaking of skipped... The pull_request and pull_request_target "labeled" action is way too coarse. If you have 4 workflows A, B, C and D, triggered by labels LA, LB, LC and LD respectively, and you add label LA to a PR, all 4 jobs run with B, C and D being marked as skipped. This clutters up both the PR checks list and the jobs list. Even worse, if workflow A added 5 checks to the PR, then you add label LB to the PR, the B workflow runs but since A also runs and is then skipped, all the checks that it added when it was legitimately run earlier are wiped out. The workflow trigger really needs to include the actual label so the workflow itself is not considered to even run. We also need an option to control whether skipped jobs even show up in the PR checks list. They're just clutter now.
And speaking of checks... We need a way to indicate whether a job should create a checks entry or not. Jobs that perform setup or cleanup actions have no business in the checks list. We also need a way to customize the checks name. When you have nested workflows the checks name can get really long and push any distinguishing bits into
...
and it only gets worse if any of those jobs are from a matrix. This also affects the ruleset creation process when using "require checks to pass before merging". Being able to set custom check names will make that process much easier as well.Beta Was this translation helpful? Give feedback.
All reactions