Skip to content

Conversation

@arrowplum
Copy link
Contributor

@arrowplum arrowplum commented Nov 7, 2025

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

@arrowplum arrowplum changed the base branch from feature/INFRA-240-add-nuget-signing-to-sign-artifacts-workflow to main November 7, 2025 04:23
@arrowplum arrowplum requested a review from Copilot November 7, 2025 05:05
Copy link
Contributor

Copilot AI left a 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 .nupkg files
  • 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.

Comment on lines 85 to 86
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
Copy link

Copilot AI Nov 7, 2025

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')

Suggested change
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

Copilot uses AI. Check for mistakes.
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"
Copy link

Copilot AI Nov 7, 2025

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.

Suggested change
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"

Copilot uses AI. Check for mistakes.
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.
Copy link

Copilot AI Nov 7, 2025

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
@arrowplum arrowplum changed the title Feature/INFRA-240-add-nuget-signing-to-sign-artifacts-workflow-2 feat: [INFRA-240] add NuGet to sign artifacts workflow Nov 7, 2025
@arrowplum arrowplum force-pushed the feature/INFRA-240-add-nuget-signing-to-sign-artifacts-workflow-2 branch from 4ff9722 to f72f4a7 Compare November 7, 2025 19:14
@arrowplum arrowplum closed this Nov 7, 2025
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.

2 participants