Disable CG in non-prod branches #13214

wtgodbe · 2023-04-19T18:54:33Z

Disables Component Governance except in internal builds of branches named main or internal/release/*. Repos who want to opt in to other branches running CG can set the disableComponentGovernance parameter in their .yml file (Same with dev branches). I tested this in EFCore and all the scenarios I tried worked. Note that repos like aspnetcore that use the job.yml template instead of jobs.yml will also have to manually set the disableComponentGovernance param. We should backport this to 6 and 7 once approved

garath · 2023-04-19T19:07:49Z

Just curious, CG tools already understand the difference between "production" and "non-production" branches. Is that not covering your scenario?

dougbu · 2023-04-19T19:20:49Z

Just curious, CG tools already understand the difference between "production" and "non-production" branches. Is that not covering your scenario?

There's a long email thread w/ Tactics and more people on it about this very topic. The systems no longer work that way @garath.

eng/common/templates/jobs/jobs.yml

garath · 2023-04-19T20:05:11Z

There's a long email thread w/ Tactics and more people on it about this very topic.

Ah I had a feeling that I was missing part of the convo. Thanks for the answer!

wtgodbe · 2023-04-19T21:03:11Z

Test builds for new approach:

non-prod branch - Should skip CG (no arcade injected task, auto-injected task should be skipped due to variable)
prod branch - Should run the arcade injected task, skip the auto-injected task
non-prod branch that opts itself back in - Same behavior as above

wtgodbe · 2023-04-19T21:09:18Z

Whoops, gotta account for OS when setting the variable, another iteration incoming

wtgodbe · 2023-04-19T21:28:54Z

New builds:

non-prod branch - Should skip CG (no arcade injected task, auto-injected task should be skipped due to variable)
prod branch - Should run the arcade injected task, skip the auto-injected task
non-prod branch that opts itself back in - Same behavior as above

wtgodbe · 2023-04-19T22:03:46Z

One more iteration because I realized that at least one of the steps in component-governance.yml needed to run on every build, and previously neither would run in PR builds against internal branches. I moved all of the logic into the invocation of the template. New builds going now

wtgodbe · 2023-04-19T22:39:07Z

Alright, latest builds behaved how I'd expect. @dougbu any concerns with the current approach?

dougbu · 2023-04-19T23:09:14Z

eng/common/templates/steps/component-governance.yml

+
+steps:
+- ${{ if eq(parameters.disableComponentGovernance, 'true') }}:
+  - script: "echo ##vso[task.setvariable variable=skipComponentGovernanceDetection]true"


Could you explain how this works or link to the successful test builds❔ I expected the variable would need to be set around https://github.com/dotnet/arcade/pull/13214/files#diff-22da7e3800f8d77bccd75b181def1385123174ef4a38a7ccbd7a3d7317580c12R162 i.e., before the default injection logic runs (which is effectively the first step of any pipeline).

here is an example from a build of EFCore using the arcade files from this branch. This variable doesn't actually skip the step entirely (even when set "in time"), it only tells the step to no-op. The auto-injected step only skips entirely if CG has already ran (e.g. here)

In that build, click on Windows on the left side, open job preparation parameters, then head down to line 163. You'll see a long line that ends w/ a check of the variable:

Evaluating: and( eq(resources['repositories']['self']['ref'], resources['repositories']['self']['defaultBranch']), eq(resources['repositories']['__designer_repo']['ref'], resources['repositories']['__designer_repo']['defaultBranch']), or(eq(resources['repositories']['self']['type'], 'Git'), eq(resources['repositories']['__designer_repo']['type'], 'Git'), eq(resources['repositories']['self']['type'], 'Github'), eq(resources['repositories']['__designer_repo']['type'], 'Github'), eq(resources['repositories']['self']['type'], 'GitHubEnterprise'), eq(resources['repositories']['__designer_repo']['type'], 'GitHubEnterprise')), ne(variables['PREVENT_RUNNING_CG'], 'true'), ne(variables['skipComponentGovernanceDetection'], 'true'))

If the variable were set in job.yml's variables expansion (as we did at https://github.com/dotnet/aspnetcore/blob/bec278eabea54f63da15e10e654bdfa4168a2479/.azure/pipelines/jobs/default-build.yml#L164-L165), the step should be removed entirely. That's more efficient and avoids noise in the build display. Unfortunately, it's not working 😦

In any case and as I think about this more, I doubt there's anyone w/ a strange need to use the auto-injected task b/c it's so limited e.g., it only handles 'main' and runs in public builds too. You could simplify things and unconditionally set the variable in job.yml.

Due to the current bug around auto-injection (it worked when we changed dotnet/aspnetcore), this isn't worth holding up the PR.

wtgodbe · 2023-04-20T15:44:29Z

/backport to release/6.0

wtgodbe · 2023-04-20T15:44:34Z

/backport to release/7.0

github-actions · 2023-04-20T15:44:46Z

Started backporting to release/6.0: https://github.com/dotnet/arcade/actions/runs/4756248514

github-actions · 2023-04-20T15:44:48Z

Started backporting to release/7.0: https://github.com/dotnet/arcade/actions/runs/4756249367

github-actions · 2023-04-20T15:44:53Z

@wtgodbe backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Disable CG in non-prod branches
Applying: Use a template instead
Using index info to reconstruct a base tree...
M	eng/common/templates/job/job.yml
M	eng/common/templates/jobs/jobs.yml
Falling back to patching base and 3-way merge...
Auto-merging eng/common/templates/jobs/jobs.yml
Auto-merging eng/common/templates/job/job.yml
CONFLICT (content): Merge conflict in eng/common/templates/job/job.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Use a template instead
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-04-20T15:44:54Z

@wtgodbe backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Disable CG in non-prod branches
Applying: Use a template instead
Using index info to reconstruct a base tree...
M	eng/common/templates/job/job.yml
Falling back to patching base and 3-way merge...
Auto-merging eng/common/templates/job/job.yml
CONFLICT (content): Merge conflict in eng/common/templates/job/job.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Use a template instead
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-04-20T15:44:54Z

@wtgodbe an error occurred while backporting to release/6.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

github-actions · 2023-04-20T15:44:55Z

@wtgodbe an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

wtgodbe · 2023-04-20T15:47:44Z

Manual PRs incoming. Looks like the timeout failure is happening everywhere, @mmitche / @dougbu could I get help merging this?

JanKrivanek · 2023-04-24T09:58:17Z

eng/common/templates/job/job.yml

+  - template: /eng/common/templates/steps/component-governance.yml
+    parameters:
+      ${{ if eq(parameters.disableComponentGovernance, '') }}:
+        ${{ if and(ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest'), eq(parameters.runAsPublic, 'false'), or(contains(variables['Build.SourceBranch'], 'internal/release'), eq(variables['Build.SourceBranch'], 'main'))) }}:


Doesn't the 'Build.SourceBranch' contain the full branch name?

eq(variables['Build.SourceBranch'], 'main') ---> eq(variables['Build.SourceBranch'], 'refs/heads/main')
contains(variables['Build.SourceBranch'], 'internal/release') ---> startsWith(variables['Build.SourceBranch'], 'refs/heads/internal/release')

jakobbotsch · 2023-04-24T18:37:56Z

eng/common/templates/job/job.yml

+    parameters:
+      ${{ if eq(parameters.disableComponentGovernance, '') }}:
+        ${{ if and(ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest'), eq(parameters.runAsPublic, 'false'), or(contains(variables['Build.SourceBranch'], 'internal/release'), eq(variables['Build.SourceBranch'], 'main'))) }}:
+          disableComponentGovernance: false
+        ${{ else }}:
+          disableComponentGovernance: true
+      componentGovernanceIgnoreDirectories: ${{ parameters.componentGovernanceIgnoreDirectories }}


Shouldn't it have something like

disableComponentGovernance: ${{ parameters.disableComponentGovernance }}

in the non-empty case to pass the value through? (We are getting CG warnings in our internal superpmi-collect pipeline that tries to pass this through to job.yml.)

I put up a PR for this: #13274

Disable CG in non-prod branches

1360e60

wtgodbe requested review from a team and mmitche April 19, 2023 18:54

mmitche previously approved these changes Apr 19, 2023

View reviewed changes

dougbu reviewed Apr 19, 2023

View reviewed changes

eng/common/templates/jobs/jobs.yml Outdated Show resolved Hide resolved

dougbu suggested changes Apr 19, 2023

View reviewed changes

eng/common/templates/jobs/jobs.yml Outdated Show resolved Hide resolved

Use a template instead

dd76ae9

wtgodbe dismissed mmitche’s stale review via dd76ae9 April 19, 2023 21:00

wtgodbe added 2 commits April 19, 2023 14:11

Fixup

069108e

Fix syntax

b9a733b

wtgodbe added 2 commits April 19, 2023 14:59

Make it always run

35d7298

Fixup

4d4de93

dougbu reviewed Apr 19, 2023

View reviewed changes

dougbu previously approved these changes Apr 20, 2023

View reviewed changes

Remove unneeded param

709fb59

wtgodbe dismissed dougbu’s stale review via 709fb59 April 20, 2023 16:10

wtgodbe mentioned this pull request Apr 20, 2023

[release/7.0] Disable CG in non-prod branches #13228

Merged

Merge branch 'main' into wtgodbe/CGCondition

08b77e4

mmitche merged commit d75baf7 into main Apr 20, 2023

wtgodbe deleted the wtgodbe/CGCondition branch April 20, 2023 20:55

JanKrivanek reviewed Apr 24, 2023

View reviewed changes

JanKrivanek mentioned this pull request Apr 24, 2023

Improve the branch names syntax #13264

Merged

jakobbotsch reviewed Apr 24, 2023

View reviewed changes

jander-msft mentioned this pull request Apr 24, 2023

Allow using non-default disableComponentGovernance value #13274

Merged

1 task

Disable CG in non-prod branches #13214

Disable CG in non-prod branches #13214

Uh oh!

Conversation

wtgodbe commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garath commented Apr 19, 2023

Uh oh!

dougbu commented Apr 19, 2023

Uh oh!

Uh oh!

Uh oh!

garath commented Apr 19, 2023

Uh oh!

wtgodbe commented Apr 19, 2023

Uh oh!

wtgodbe commented Apr 19, 2023

Uh oh!

wtgodbe commented Apr 19, 2023

Uh oh!

wtgodbe commented Apr 19, 2023

Uh oh!

wtgodbe commented Apr 19, 2023

Uh oh!

dougbu Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

wtgodbe Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu Apr 20, 2023

Choose a reason for hiding this comment

Uh oh!

wtgodbe commented Apr 20, 2023

Uh oh!

wtgodbe commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

github-actions bot commented Apr 20, 2023

Uh oh!

wtgodbe commented Apr 20, 2023

Uh oh!

JanKrivanek Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

jakobbotsch Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

jander-msft Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

wtgodbe commented Apr 19, 2023 •

edited

Loading

wtgodbe Apr 19, 2023 •

edited

Loading