Add check to not private builds to public channels #15041
Conversation
src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
Outdated
Show resolved
Hide resolved
| $isInternalBuild = $false | ||
| if ([string]::IsNullOrEmpty($buildInfo.gitHubRepository) -and $buildInfo.azureDevOpsBranch.Contains("internal/release")) { | ||
| $isInternalBuild = $true | ||
| } |
There was a problem hiding this comment.
Would it be a bit safer to start with $true and set to $false? Then we'd have a false positive the other way and worst case we block a public build but not push an internal one to public feeds by mistake (e.g. this condition fails or the build info gets changed..)
| @@ -60,6 +60,11 @@ try { | |||
| $buildNumberName = $buildNumberName.Substring(0, 255) | |||
| } | |||
|
|
|||
| $isInternalBuild = $false | |||
| if ([string]::IsNullOrEmpty($buildInfo.gitHubRepository) -and $buildInfo.azureDevOpsBranch.Contains("internal/release")) { | |||
There was a problem hiding this comment.
Alternatively, this could attempt to anonymous access the commit in GH if the repo GH repo URI is available. That gets rid of the branch name check.
The downside of this is that you do need to implement safety check skipping if this happens. Some dev builds as well as non-public repos (wpf-int) that produce public assets would need to pass this switch.
Thoughts?
There was a problem hiding this comment.
Alternatively, this could attempt to anonymous access the commit in GH if the repo GH repo URI is available. That gets rid of the branch name check.
I don't mind this if you think it'd be a more reliable way of doing the check
The downside of this is that you do need to implement safety check skipping if this happens. Some dev builds as well as non-public repos (wpf-int) that produce public assets would need to pass this switch.
Yes, this is true for both cases. I think we'd have to:
- Get a list of all default-channels that publish from non-public to public
- Add a parameter to Darc to skip this check
- Add the skip check parameter to the
post-build.yaml, and update the branches from the list above - Update the publishing pipeline yaml
There was a problem hiding this comment.
Yeah. I think for completeness that's probably a better solution.
There was a problem hiding this comment.
it might be a good idea to split the SkipSafetyChecks property in two, one for publishing to stable feeds, and one for publishing builds from internal repos to public feeds?
I'd hardcode the publishing one to true, until I go and update all internal repos that publish to public, and then I'd let it be configured by the darc call as a parameter
To double check:
Add new private channel publishing guardrails #14167