ADFA-3269 Share nightly release build with private Telegram channel#1073
Conversation
…telegram channel; remove obsolete Greengeeks ssh setup
hal-eisen-adfa
changed the title
📝 WalkthroughRelease Notes - ADFA-3269Key Changes
Technical Details
Risks & Best Practices Notes
✓ Security Improvement: Removing SSH-based uploads in favor of Cloudflare R2 API authentication is a security best practice—eliminates SSH key management burden and reduces attack surface. WalkthroughThe release workflow replaces SSH-based artifact uploads with Cloudflare R2 uploads via a new Python script, removes SSH key setup logic, and adds Telegram notifications after the upload completes. The workflow maintains existing post-upload messaging while adding new cloud storage and messaging integrations. Changes
Sequence DiagramsequenceDiagram
participant GA as GitHub Actions
participant Script as cloudflare-r2-upload.py
participant R2 as Cloudflare R2
participant Telegram as Telegram API
GA->>GA: Build APK
GA->>Script: Execute with file path & variant
Script->>Script: Validate environment variables
Script->>Script: Initialize boto3 S3 client
Script->>R2: Upload APK with progress tracking
R2-->>Script: Upload complete
Script->>GA: Print completion status
GA->>Telegram: Send notification with APK link
Telegram-->>GA: Message delivered
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
588-594: Consider storingCLOUDFLARE_KEY_IDas a secret rather than a variable.While the Key ID is less sensitive than the Secret Access Key, it's still part of the credential pair. Storing it as a
secretinstead of avarwould provide additional protection (masked in logs, not visible in repository settings to users with lower permissions).env: CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} - CLOUDFLARE_KEY_ID: ${{ vars.CLOUDFLARE_KEY_ID }} + CLOUDFLARE_KEY_ID: ${{ secrets.CLOUDFLARE_KEY_ID }} CLOUDFLARE_SECRET_ACCESS_KEY: ${{ secrets.CLOUDFLARE_SECRET_ACCESS_KEY }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 588 - 594, The workflow step "Upload APK to Cloudflare R2" exposes CLOUDFLARE_KEY_ID as a repo variable; change the env binding to use a secret (secrets.CLOUDFLARE_KEY_ID) instead of vars.CLOUDFLARE_KEY_ID and add that Key ID to the repository secrets, leaving CLOUDFLARE_SECRET_ACCESS_KEY as secrets.CLOUDFLARE_SECRET_ACCESS_KEY; ensure the step still passes steps.find_apk.outputs.APK_PATH and matrix.variant into scripts/cloudflare-r2-upload.py unchanged.scripts/cloudflare-r2-upload.py (1)
73-79: Consider adding error handling around the upload operation.The
s3.upload_file()call can raise various boto3/botocore exceptions (e.g.,ClientError,NoCredentialsError, network errors). Currently, failures will produce raw tracebacks. Catching and logging a cleaner error message would improve debuggability in CI logs.Proposed enhancement
+from botocore.exceptions import ClientError + print(f"Uploading {file_name} ({file_size / (1024*1024):.1f} MB) to R2...", flush=True) -s3.upload_file(file_path, BUCKET_NAME, file_name, **upload_kwargs) -print("Upload complete.", flush=True) +try: + s3.upload_file(file_path, BUCKET_NAME, file_name, **upload_kwargs) + print("Upload complete.", flush=True) +except ClientError as e: + print(f"ERROR: Upload failed: {e}", file=sys.stderr) + sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/cloudflare-r2-upload.py` around lines 73 - 79, Wrap the s3.upload_file call in a try/except block to catch boto3/botocore exceptions (e.g., botocore.exceptions.ClientError, NoCredentialsError, EndpointConnectionError, and a generic Exception fallback); reference the existing upload_kwargs, progress_callback, s3.upload_file, BUCKET_NAME, file_path and file_name to locate the call, log/print a clear error message including the exception details, and exit with a non-zero status (or re-raise) so CI sees the failure instead of a raw traceback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 711-713: The Telegram POST currently silences failures (curl -s)
so the workflow continues on HTTP errors; update the step that calls curl (the
command using TELEGRAM_TOKEN, TELEGRAM_EARLY_ACCESS_CHAT_ID, and MESSAGE) to
detect and surface non-2xx responses by either using curl's --fail/-f and -S
flags or capturing the HTTP status and response body and then failing the job
when the status is not 200/201; ensure errors are logged (include response body
or status) and the step exits non-zero so the workflow does not continue
silently on rate limits, invalid tokens, or network issues.
In `@scripts/cloudflare-r2-upload.py`:
- Around line 41-51: Check that the provided file_path exists before calling
os.path.getsize to avoid FileNotFoundError; specifically, after assigning
file_path from sys.argv and before file_size = os.path.getsize(file_path), add a
guard using os.path.exists(file_path) (or os.path.isfile) and if it does not
exist print a clear error to stderr mentioning the missing file_path and exit
with non-zero status so the script fails cleanly; keep the subsequent logic that
computes base_name, name_root, ext, and file_name unchanged.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 588-594: The workflow step "Upload APK to Cloudflare R2" exposes
CLOUDFLARE_KEY_ID as a repo variable; change the env binding to use a secret
(secrets.CLOUDFLARE_KEY_ID) instead of vars.CLOUDFLARE_KEY_ID and add that Key
ID to the repository secrets, leaving CLOUDFLARE_SECRET_ACCESS_KEY as
secrets.CLOUDFLARE_SECRET_ACCESS_KEY; ensure the step still passes
steps.find_apk.outputs.APK_PATH and matrix.variant into
scripts/cloudflare-r2-upload.py unchanged.
In `@scripts/cloudflare-r2-upload.py`:
- Around line 73-79: Wrap the s3.upload_file call in a try/except block to catch
boto3/botocore exceptions (e.g., botocore.exceptions.ClientError,
NoCredentialsError, EndpointConnectionError, and a generic Exception fallback);
reference the existing upload_kwargs, progress_callback, s3.upload_file,
BUCKET_NAME, file_path and file_name to locate the call, log/print a clear error
message including the exception details, and exit with a non-zero status (or
re-raise) so CI sees the failure instead of a raw traceback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01f4a27f-5053-4881-8818-44fba31093e4
📒 Files selected for processing (2)
.github/workflows/release.ymlscripts/cloudflare-r2-upload.py
hal-eisen-adfa
deleted the
ADFA-3269-Share-nightly-release-to-private-Telegram
branch
3 participants
Upload nightly release to Cloudflare R2 and announce to private telegram channel
Remove obsolete Greengeeks ssh setup