-
Notifications
You must be signed in to change notification settings - Fork 1
feat: [INFRA-240] add NuGet to sign artifacts workflow #84
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
feat: [INFRA-240] add NuGet to sign artifacts workflow #84
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds NuGet package signing support to the sign-artifacts workflow using SSL.com's code signing service. The workflow now detects .nupkg files in unsigned artifacts, signs them via SSL.com, and includes verification tests. Additionally, it updates workflow references from v2.0.1 to v2.0.2 and improves RPM build environment handling.
Key changes:
- Adds NuGet signing capability with SSL.com integration for
.nupkgfiles - Updates all workflow references to use v2.0.2
- Adds NuGet signature verification job in test workflow
- Updates test fixtures and counts to include NuGet packages
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/reusable_sign-artifacts.yaml |
Adds NuGet package detection, SSL.com signing step, and required secrets for NuGet signing |
.github/workflows/sign-artifacts/README.md |
Documents new NuGet signing support, SSL.com secrets, and updated input descriptions |
.github/workflows/test_sign-artifacts-workflow.yaml |
Adds SSL.com secrets and NuGet signature verification job |
.github/workflows/test_execute-build-workflow.yaml |
Updates workflow reference to v2.0.2 |
.github/workflows/test_deploy-artifacts-workflow.yaml |
Updates workflow reference to v2.0.2 |
.github/workflows/test_create-release-bundle-workflow.yaml |
Updates workflow reference to v2.0.2 |
.github/workflows/execute-build/test_apps/hi/Makefile |
Improves RPM build environment handling with dnf/yum detection |
.github/workflows/deploy-artifacts/create-test-fixtures.sh |
Adds NuGet package to test fixtures |
.github/workflows/deploy-artifacts/test-entrypoint.sh |
Updates upload command counts to reflect new test fixture |
.github/workflows/example_reusable-integration.yaml |
Fixes job dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Found nuget packages: $(ls unsigned-nuget-packages/)" | ||
| echo "unsigned-nuget-packages=$(ls unsigned-nuget-packages/)" >> $GITHUB_OUTPUT | ||
| echo "count=$(ls unsigned-nuget-packages/ | wc -l)" >> $GITHUB_OUTPUT |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ls in command substitution will fail with an error if the directory is empty. Since this is just logging, consider using ls -A or adding || true to handle the empty directory case gracefully: $(ls unsigned-nuget-packages/ 2>/dev/null || echo 'none')
| echo "Found nuget packages: $(ls unsigned-nuget-packages/)" | |
| echo "unsigned-nuget-packages=$(ls unsigned-nuget-packages/)" >> $GITHUB_OUTPUT | |
| echo "count=$(ls unsigned-nuget-packages/ | wc -l)" >> $GITHUB_OUTPUT | |
| echo "Found nuget packages: $(ls -A unsigned-nuget-packages/ 2>/dev/null || echo 'none')" | |
| echo "unsigned-nuget-packages=$(ls -A unsigned-nuget-packages/ 2>/dev/null || echo '')" >> $GITHUB_OUTPUT | |
| echo "count=$(ls -A unsigned-nuget-packages/ 2>/dev/null | wc -l)" >> $GITHUB_OUTPUT |
| echo " test-1.0-2.noarch.rpm not found, creating mock" | ||
| echo "test-rpm-content" > "$BUILD_ARTIFACTS_DIR/test-1.0-2.noarch.rpm" | ||
| fi | ||
| cp -v tests/some/structure/Aerospike.Client.8.0.2.nupkg "$BUILD_ARTIFACTS_DIR/Aerospike.Client.8.0.2.nupkg" |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] There are two spaces between the source path and the destination path. While this doesn't affect functionality, it's inconsistent with the rest of the file's formatting. Consider using a single space for consistency.
| cp -v tests/some/structure/Aerospike.Client.8.0.2.nupkg "$BUILD_ARTIFACTS_DIR/Aerospike.Client.8.0.2.nupkg" | |
| cp -v tests/some/structure/Aerospike.Client.8.0.2.nupkg "$BUILD_ARTIFACTS_DIR/Aerospike.Client.8.0.2.nupkg" |
| build-docker-deploy: | ||
| uses: aerospike/shared-workflows/.github/workflows/[email protected] | ||
| needs: extract-version | ||
| needs: [extract-version, package-built-artifacts] #don't really need package-built-artifacts but we need that to finish first. |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should use proper capitalization and punctuation: # Don't really need package-built-artifacts but we need that to finish first.
| needs: [extract-version, package-built-artifacts] #don't really need package-built-artifacts but we need that to finish first. | |
| needs: [extract-version, package-built-artifacts] # Don't really need package-built-artifacts, but we need that to finish first. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
4ff9722 to
f72f4a7
Compare
Brings the core of #29 up into the modern shared workflows.
Note: this was built on top of #83 which will need to be merged first.DONE