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

Update check-vulnerabilities.ps1 to error out if dotnet command fails and update build project dependencies #3786

Open
wants to merge 2 commits into
base: v4.x
Choose a base branch
from

Conversation

Francisco-Gamino
Copy link
Contributor

@Francisco-Gamino Francisco-Gamino commented Aug 9, 2024

Issue describing the changes in this PR

This PR contains the following changes:

  • Fixes If restore fails, CI script should exit with an error #3783
  • Updated check-vulnerabilities.ps1 to validate the packages for the following projects are up-to-date:
    "$PSScriptRoot/src/Azure.Functions.Cli/Azure.Functions.Cli.csproj" "$PSScriptRoot/test/Azure.Functions.Cli.Tests/Azure.Functions.Cli.Tests.csproj"
    "$PSScriptRoot/build/Build.csproj"
  • Updated project dependencies for Build.csproj

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

@Francisco-Gamino Francisco-Gamino requested a review from a team as a code owner August 9, 2024 05:53
@Francisco-Gamino
Copy link
Contributor Author

Francisco-Gamino commented Aug 9, 2024

/cc @mattchenderson @FinVamp1

@Francisco-Gamino
Copy link
Contributor Author

Hello @kshyju @mattchenderson -- Could you please review this PR? Thank you.

@mattchenderson
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@mattchenderson mattchenderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment about next work that should be tracked, but that does not need to block this specific PR.

}

cd $projectPath
$logFilePath = "$PSScriptRoot/build.log"
foreach ($projectFilePath in $CsprojFilePath) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking aloud, I wonder if we would want to parallelize these upstream of the call into the script, I suppose that's supported by the parameter being included here. But we should probably ensure that gets tracked, as it will be much more efficient.

Copy link
Contributor

@jviau jviau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with these changes, but I do want to point out with .NET8 SDK this is built into the restore process (configurable via msbuild properties). Additionally, we have component governance running for this repo which will flag these CVEs.

I'm not a big fan of hard failures for CVEs in the pipeline. I prefer being given some time to address the CVEs before all our pipelines are blocked (thus impacting ongoing work).

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.

If restore fails, CI script should exit with an error
5 participants