-
-
Notifications
You must be signed in to change notification settings - Fork 294
Cloud Runner Improvements - S3 Locking, Aws Local Stack (Pipelines), Testing Improvements, Rclone storage support, Provider plugin system #731
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
base: main
Are you sure you want to change the base?
Changes from all commits
a1f3d9e
db9fc17
10fc07a
1815c3c
01bbef7
36503e3
9ed94b2
b662a6f
92eaa73
a0833df
12e5985
5b34e4d
fa6440d
6e13713
e9a60d4
0650d1d
eb8b92c
c8f881a
f2dbcdf
c3e0ee6
ec93ad5
8acf3cc
962603b
a4a3612
6c4a85a
4b178e0
0166925
12b6aaa
5d0450d
1e57879
bad80a4
e9af764
52b79b2
afdc987
9b205ac
70fcc1a
f00d7c8
0c57572
d8ad8f9
4f5155d
3de8cac
1e2fa05
277dcab
8206043
4c3d97d
a04f7d8
c676d1d
f06dd86
91872a2
16d1156
9e6d69f
2d7374b
3570d40
c28831c
b8c3ad1
c5f2078
f7725a7
af988e6
f7df350
26fcfce
f6f813b
71895ac
dda7de4
32265f4
c62465a
0876bd4
d3e23a8
8f66ff2
ce848c7
4e3546c
d800b10
d58c3d6
43c11e7
c2a7091
364f9a7
f50fd8e
431a471
3f8fbb9
ee01652
94daf5a
73de3d4
5d667ab
1d2d904
f4d4612
831b913
df65063
307a2aa
85bb3d9
7779839
edc1df7
a0cb4ff
fd74d25
98963da
bd1be2e
d6cc453
464a9d1
38b7286
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,83 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: cloud-runner-integrity-localstack | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
on: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
workflow_call: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
inputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
runGithubIntegrationTests: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
description: 'Run GitHub Checks integration tests' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
required: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default: 'false' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
permissions: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
checks: write | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contents: read | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
actions: write | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
packages: read | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pull-requests: write | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
statuses: write | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id-token: write | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+12
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Restrict permissions to least-privilege. Mirror the tightened set used in the other workflow. permissions:
- checks: write
- contents: read
- actions: write
- packages: read
- pull-requests: write
- statuses: write
- id-token: write
+ contents: read
+ checks: write
+ statuses: write 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussion: Since this is localstack, might be less necessary to reduce this but on the other side, it mentions mirroring the other workflow which is probably a good idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainI'll help you analyze the permission requirements for this LocalStack workflow and compare it with other workflows to determine the best approach. @GabLeRoux You raise a good point about the LocalStack context, but I'd recommend adopting the restrictive permissions for several reasons:
The workflow essentially:
The suggested minimal permissions ( I'd go with the CodeRabbit suggestion - better to start restrictive and add permissions when proven necessary than to grant excessive permissions "just in case." 🏁 Script executed:
Length of output: 2216 Perfect! The script output confirms that the restrictive permissions are absolutely the right approach. Here's what I found: Analysis of Current vs. Required PermissionsCurrent permissions (excessive):
Actually required permissions:
Key Findings
RecommendationDefinitely adopt the restrictive permissions. The workflow performs no operations that require the additional permissions. The current permissions appear to be copied from a template rather than tailored to actual needs. You should apply this same permission reduction to This is a textbook example of why least-privilege permissions matter - even in LocalStack environments, good security practices should be maintained. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_REGION: us-east-1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_DEFAULT_REGION: us-east-1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_STACK_NAME: game-ci-local | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_ENDPOINT: http://localhost:4566 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_ENDPOINT_URL: http://localhost:4566 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_ACCESS_KEY_ID: test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_SECRET_ACCESS_KEY: test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CLOUD_RUNNER_BRANCH: ${{ github.ref }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DEBUG: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PROJECT_PATH: test-project | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
USE_IL2CPP: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tests: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: Cloud Runner Tests (LocalStack) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
services: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
localstack: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
image: localstack/localstack | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ports: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 4566:4566 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SERVICES: cloudformation,ecs,kinesis,cloudwatch,s3,logs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
strategy: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fail-fast: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
matrix: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-end2end-locking' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-end2end-caching' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-end2end-retaining' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-caching' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-environment' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-image' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-hooks' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-local-persistence' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-locking-core' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- 'cloud-runner-locking-get-locked' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lfs: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- uses: actions/setup-node@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
node-version: 20 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cache: 'yarn' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- run: yarn install --frozen-lockfile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- run: yarn run test "${{ matrix.test }}" --detectOpenHandles --forceExit --runInBand | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
timeout-minutes: 60 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PROJECT_PATH: test-project | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TARGET_PLATFORM: StandaloneWindows64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cloudRunnerTests: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
versioning: None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
KUBE_STORAGE_CLASS: local-path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PROVIDER_STRATEGY: aws | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_ACCESS_KEY_ID: test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AWS_SECRET_ACCESS_KEY: test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GIT_PRIVATE_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GITHUB_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+70
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Clean up test env and avoid overriding GITHUB_TOKEN. KUBE_STORAGE_CLASS isn’t relevant here; add missing S3/IMDS flags for consistency. versioning: None
- KUBE_STORAGE_CLASS: local-path
PROVIDER_STRATEGY: aws
AWS_ACCESS_KEY_ID: test
AWS_SECRET_ACCESS_KEY: test
- GIT_PRIVATE_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN }}
- GITHUB_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN }}
+ AWS_S3_ENDPOINT: http://localhost:4566
+ AWS_S3_FORCE_PATH_STYLE: 'true'
+ AWS_EC2_METADATA_DISABLED: 'true'
+ GITHUB_TOKEN: ${{ github.token }} 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it the case? If so, looks like a good suggestion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain@GabLeRoux Yes, the suggestion is correct! Let me break down why each change makes sense: ✅ Remove
✅ Add
✅ Add
✅ Add
✅ Fix
The suggestion improves LocalStack test reliability and follows AWS local development best practices. 🏁 Script executed:
Length of output: 325 🏁 Script executed:
Length of output: 294599 @GabLeRoux Yes—this suggestion is correct for the LocalStack job. Grounding from the repo:
Here’s the step env block to apply in .github/workflows/cloud-runner-integrity-localstack.yml: env:
UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }}
UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }}
PROJECT_PATH: test-project
TARGET_PLATFORM: StandaloneWindows64
cloudRunnerTests: true
versioning: None
PROVIDER_STRATEGY: aws
AWS_ACCESS_KEY_ID: test
AWS_SECRET_ACCESS_KEY: test
AWS_S3_ENDPOINT: http://localhost:4566
AWS_S3_FORCE_PATH_STYLE: 'true'
AWS_EC2_METADATA_DISABLED: 'true'
GITHUB_TOKEN: ${{ github.token }} If you’d like, I can also submit a follow-up patch to keep KUBE_STORAGE_CLASS only in the k8s job. |
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.
🛠️ Refactor suggestion
Extend override to cover Jest config files and fix CI lint errors.
CI reports import/no-commonjs and filename rule violations in jest.ci.config.js. Add those files to the override and disable the filename rule for them.
Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents