feat: add GitHub Actions integration test workflow#135
feat: add GitHub Actions integration test workflow#135reyhankoyun wants to merge 4 commits intoaws:mainfrom reyhankoyun:feature/github-actions-workflow
Conversation
- Add workflow to run integration tests on PRs and pushes - Use credential-based authentication temporarily - Prepared for future OIDC role-based authentication - Tests run with --test-threads=1 for proper isolation
- Replace dtolnay/rust-toolchain with actions-rs/toolchain - actions-rs/* is in the allowed patterns for aws organization
- Isengard provides temporary credentials requiring session token - Add session token parameter to aws-actions/configure-aws-credentials
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
=======================================
Coverage 91.72% 91.72%
=======================================
Files 14 14
Lines 2418 2418
Branches 2418 2418
=======================================
Hits 2218 2218
Misses 150 150
Partials 50 50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| on: | ||
| workflow_dispatch: | ||
| pull_request: |
There was a problem hiding this comment.
pull_request tasks are not going to be able to retrieve credentials using OIDC from forks. We need to use pull_request_target but should wait until we have a working test workflow using workflow_dispatch and have added a way to gate running integration tests on a tag.
There was a problem hiding this comment.
I'll remove pull_request for now. So:
- Current state: workflow_dispatch + push to main only
- Next step: Get OIDC working with IAM roles
- Future step: Add
pull_request_targetwith proper label/tag gating
| - name: Run integration tests | ||
| run: | | ||
| cd integration-tests | ||
| cargo test -- --test-threads=1 --ignored No newline at end of file |
There was a problem hiding this comment.
IIRC cargo test uses the debug target, I'm not sure building using --release above does much for you.
There was a problem hiding this comment.
I can remove the release build since cargo test will build the debug binary that the tests expect. So cargo test will:
- Build the agent binary in debug mode (
target/debug/aws_secretsmanager_agent) - Build and run the integration tests
- The tests will find the debug binary and use it
| ~/.cargo/registry | ||
| ~/.cargo/git | ||
| target | ||
| key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} |
There was a problem hiding this comment.
Does the cache get invalidated when the Rust stable version changes?
There was a problem hiding this comment.
No, the current cache key doesn't include the Rust version, so it won't invalidate when Rust stable updates. I guess this could cause issues with stale compiled artifacts. Would including the Rust toolchain in the cache key fix it?
| # - name: Determine IAM role | ||
| # id: role | ||
| # run: | | ||
| # if [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; then |
There was a problem hiding this comment.
I don't think we need to do this on a pull_request_target
There was a problem hiding this comment.
With pull_request_target, don't we just need to add proper gating (like requiring a specific label) before we enable it, since it runs with elevated permissions? Wouldn't the rest be the same? I can make it more explicit like:
# if [[ ("${{ github.event_name }}" == "pull_request" || "${{ github.event_name }}" == "pull_request_target") && "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; then
2 participants
Issue #, if available:
Description of changes: This PR adds integration test workflow that will run on all PRs. Currently fails in main repo due to missing AWS IAM roles (infrastructure in progress). Workflow tested successfully in fork with temporary credentials.
--test-threads=1for proper isolationBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.