Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(workflows): auto-approve labelling conditional statement always runs true #1574

Open
wants to merge 98 commits into
base: master
Choose a base branch
from

Conversation

sumupitchayan
Copy link
Contributor

@sumupitchayan sumupitchayan commented Oct 18, 2023

Follow up to #1428

We initially wanted to conditionally set the auto-approve label of PR's in this workflow based on a testingMode input parameter. This first step is supposed to conditionally set the labels output to auto-approve if testingMode was set to false:

{
  name: 'Set auto-approve label for PR if in testing mode',
  id: 'set-auto-approve-label',
  if: '${{ github.event.inputs.testingMode }} == false',
  run: 'echo labels="auto-approve" >> $GITHUB_OUTPUT',
  env: { GITHUB_TOKEN: '${{ secrets.PROJEN_GITHUB_TOKEN }}' },
  continueOnError: false,
},

And would then be used in the following step creating a PR like so:

...WorkflowActions.createPullRequest({
  ... ,
  labels: [
    '${{ steps.set-auto-approve-label.outputs.labels }}',
  ],
}),

However, there is a very annoying known bug in GitHub workflow steps where conditions are always evaluated as true when containing expression syntax inside. This causes the auto-approve label to always be added to PR's in the Kubernetes upgrade workflow, even when testingMode is set to true.

I've had a hard time finding workarounds to this problem. Right now what I'm just doing is manually commenting-out the steps that involve publishing or setting the auto-approve label of PR's created.

Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
…estingMode false is passed in as default

Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
@sumupitchayan sumupitchayan marked this pull request as ready for review October 19, 2023 05:17
@iliapolo
Copy link
Member

@sumupitchayan Can't you move the condition out of the if and inside the run directive. You would then set the step output to "auto-approve" if the condition is true, and to an empty string (or whatever can work that causes a no-op) when the condition is false.

Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
@sumupitchayan
Copy link
Contributor Author

@sumupitchayan Can't you move the condition out of the if and inside the run directive. You would then set the step output to "auto-approve" if the condition is true, and to an empty string (or whatever can work that causes a no-op) when the condition is false.

@iliapolo I tried doing that earlier with this:

        {
          name: 'Set auto-approve label for PR if in testing mode',
          id: 'set-auto-approve-label',
          run: 'if (${{ github.event.inputs.testingMode }} == false);then echo labels="auto-approve" >> $GITHUB_OUTPUT;fi',
          env: { GITHUB_TOKEN: '${{ secrets.PROJEN_GITHUB_TOKEN }}' },
          continueOnError: false,
        },
        ...WorkflowActions.createPullRequest({
          ...
          labels: [
            '${{ steps.set-auto-approve-label.outputs.labels }}',
          ],
        }),

But it doesn't seem to be working in the Create PR step of the generate-new-k8s-spec job: see the logs here -- no auto-approve label is set on the PR even when I set the testingMode default to false

Signed-off-by: Sumu <sumughan@amazon.com>
sumupitchayan and others added 22 commits October 19, 2023 16:54
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
…step after yarn install

Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
@mergify mergify bot mentioned this pull request Feb 21, 2024
sumupitchayan and others added 7 commits February 21, 2024 12:55
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: Sumu <sumughan@amazon.com>
…bsite job

Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: github-actions <github-actions@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants