Automate Jaeger Demo Deployment to OKE Using GitHub Actions #7334
Conversation
Signed-off-by: chahat sagar <109112505+chahatsagarmain@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7334 +/- ##
==========================================
+ Coverage 96.44% 96.47% +0.03%
==========================================
Files 375 375
Lines 22951 22951
==========================================
+ Hits 22135 22143 +8
+ Misses 617 611 -6
+ Partials 199 197 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
.github/workflows/ci-deploy-demo.yml
Outdated
| - name: Clone Jaeger repository | ||
| run: git clone https://github.com/jaegertracing/jaeger.git |
There was a problem hiding this comment.
The current approach clones the main branch of the Jaeger repository regardless of which branch or PR triggered the workflow. For PR and push events, this means you're not testing the actual code changes being proposed.
Consider replacing this with the standard GitHub checkout action:
- name: Checkout code
uses: actions/checkout@v3This will automatically check out the correct branch or PR that triggered the workflow, ensuring you're testing the actual changes under review rather than the main branch.
| - name: Clone Jaeger repository | |
| run: git clone https://github.com/jaegertracing/jaeger.git | |
| - name: Checkout code | |
| uses: actions/checkout@v3 |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
.github/workflows/ci-deploy-demo.yml
Outdated
| - name: Clone Jaeger repository | ||
| run: git clone https://github.com/jaegertracing/jaeger.git | ||
|
|
||
| - name: Deploy using Jaeger's deploy-all.sh | ||
| run: | | ||
| cd ./jaeger/examples/oci | ||
| bash ./deploy-all.sh |
There was a problem hiding this comment.
The workflow should use actions/checkout@v3 instead of manually cloning the repository. The current approach creates a fresh clone rather than using the repository context where the workflow is running, which can cause path inconsistencies and doesn't reflect the actual code being submitted in the PR.
Consider replacing:
- name: Clone Jaeger repository
run: git clone https://github.com/jaegertracing/jaeger.git
- name: Deploy using Jaeger's deploy-all.sh
run: |
cd ./jaeger/examples/oci
bash ./deploy-all.shWith:
- name: Checkout repository
uses: actions/checkout@v3
- name: Deploy using Jaeger's deploy-all.sh
run: |
cd ./examples/oci
bash ./deploy-all.shThis ensures the workflow operates on the correct version of the code and simplifies the path references.
| - name: Clone Jaeger repository | |
| run: git clone https://github.com/jaegertracing/jaeger.git | |
| - name: Deploy using Jaeger's deploy-all.sh | |
| run: | | |
| cd ./jaeger/examples/oci | |
| bash ./deploy-all.sh | |
| - name: Checkout repository | |
| uses: actions/checkout@v3 | |
| - name: Deploy using Jaeger's deploy-all.sh | |
| run: | | |
| cd ./examples/oci | |
| bash ./deploy-all.sh |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
.github/workflows/ci-deploy-demo.yml
Outdated
| - name: Debug Secrets | ||
| run: | | ||
| echo "OCI_CLI_USER: ${OCI_CLI_USER}" | ||
| echo "OCI_CLI_TENANCY: ${OCI_CLI_TENANCY}" | ||
| echo "OCI_CLI_FINGERPRINT: ${OCI_CLI_FINGERPRINT}" | ||
| echo "OCI_CLI_KEY_CONTENT: ${OCI_CLI_KEY_CONTENT}" | ||
| echo "OCI_CLI_REGION: ${OCI_CLI_REGION}" |
There was a problem hiding this comment.
Security Concern: The Debug Secrets step is printing credential variables to the logs. While GitHub Actions will mask the actual secret values in the output, explicitly printing credentials is a security anti-pattern that should be avoided.
If verification of environment variables is needed, consider:
- name: Verify OCI credentials are set
run: |
[[ -n "$OCI_CLI_USER" ]] && echo "OCI_CLI_USER is set" || echo "OCI_CLI_USER is not set"
[[ -n "$OCI_CLI_TENANCY" ]] && echo "OCI_CLI_TENANCY is set" || echo "OCI_CLI_TENANCY is not set"
# Similar checks for other variablesFor a production workflow, this debug step should be removed entirely once the deployment is working correctly.
| - name: Debug Secrets | |
| run: | | |
| echo "OCI_CLI_USER: ${OCI_CLI_USER}" | |
| echo "OCI_CLI_TENANCY: ${OCI_CLI_TENANCY}" | |
| echo "OCI_CLI_FINGERPRINT: ${OCI_CLI_FINGERPRINT}" | |
| echo "OCI_CLI_KEY_CONTENT: ${OCI_CLI_KEY_CONTENT}" | |
| echo "OCI_CLI_REGION: ${OCI_CLI_REGION}" | |
| - name: Verify OCI credentials are set | |
| run: | | |
| [[ -n "$OCI_CLI_USER" ]] && echo "OCI_CLI_USER is set" || echo "OCI_CLI_USER is not set" | |
| [[ -n "$OCI_CLI_TENANCY" ]] && echo "OCI_CLI_TENANCY is set" || echo "OCI_CLI_TENANCY is not set" | |
| [[ -n "$OCI_CLI_FINGERPRINT" ]] && echo "OCI_CLI_FINGERPRINT is set" || echo "OCI_CLI_FINGERPRINT is not set" | |
| [[ -n "$OCI_CLI_KEY_CONTENT" ]] && echo "OCI_CLI_KEY_CONTENT is set" || echo "OCI_CLI_KEY_CONTENT is not set" | |
| [[ -n "$OCI_CLI_REGION" ]] && echo "OCI_CLI_REGION is set" || echo "OCI_CLI_REGION is not set" | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
.github/workflows/ci-deploy-demo.yml
Outdated
| OCI_CLI_REGION: ${{ secrets.OCI_CLI_REGION }} | ||
|
|
||
| steps: | ||
| - name: Debug Secrets |
There was a problem hiding this comment.
You will not have access to secrets from a PR workflow, only when it's triggered from main.
.github/workflows/ci-deploy-demo.yml
Outdated
|
|
||
| on: | ||
| schedule: | ||
| - cron: '0 5 * * *' # Daily at 5 AM UTC |
There was a problem hiding this comment.
If the deploy fails I would rather it happens on US ET time, not at midnight .
.github/workflows/ci-deploy-demo.yml
Outdated
| - name: Debug Secrets | ||
| env: # Correctly placed at the same level as 'run' | ||
| OCI_CLI_USER: ${{ secrets.OCI_CLI_USER }} | ||
| OCI_CLI_TENANCY: ${{ secrets.OCI_CLI_TENANCY }} | ||
| OCI_CLI_FINGERPRINT: ${{ secrets.OCI_CLI_FINGERPRINT }} | ||
| OCI_CLI_KEY_CONTENT: ${{ secrets.OCI_CLI_KEY_CONTENT }} | ||
| OCI_CLI_REGION: ${{ secrets.OCI_CLI_REGION }} | ||
| run: | | ||
| echo "OCI_CLI_USER: ${OCI_CLI_USER}" | ||
| echo "OCI_CLI_TENANCY: ${OCI_CLI_TENANCY}" | ||
| echo "OCI_CLI_FINGERPRINT: ${OCI_CLI_FINGERPRINT}" | ||
| echo "OCI_CLI_KEY_CONTENT: ${OCI_CLI_KEY_CONTENT}" | ||
| echo "OCI_CLI_REGION: ${OCI_CLI_REGION}" |
There was a problem hiding this comment.
Security Concern: The debug step is printing sensitive authentication credentials to the console logs. This exposes secrets like API keys and fingerprints that could be compromised if logs are accessible to unauthorized users.
Recommendation: Remove this debug step entirely for production workflows. If verification is needed, consider:
- name: Verify Secrets Exist
run: |
[[ -n "$OCI_CLI_USER" ]] && echo "OCI_CLI_USER is set" || echo "OCI_CLI_USER is not set"
[[ -n "$OCI_CLI_TENANCY" ]] && echo "OCI_CLI_TENANCY is set" || echo "OCI_CLI_TENANCY is not set"
# Similar checks for other secretsThis approach confirms secrets are available without exposing their actual values.
| - name: Debug Secrets | |
| env: # Correctly placed at the same level as 'run' | |
| OCI_CLI_USER: ${{ secrets.OCI_CLI_USER }} | |
| OCI_CLI_TENANCY: ${{ secrets.OCI_CLI_TENANCY }} | |
| OCI_CLI_FINGERPRINT: ${{ secrets.OCI_CLI_FINGERPRINT }} | |
| OCI_CLI_KEY_CONTENT: ${{ secrets.OCI_CLI_KEY_CONTENT }} | |
| OCI_CLI_REGION: ${{ secrets.OCI_CLI_REGION }} | |
| run: | | |
| echo "OCI_CLI_USER: ${OCI_CLI_USER}" | |
| echo "OCI_CLI_TENANCY: ${OCI_CLI_TENANCY}" | |
| echo "OCI_CLI_FINGERPRINT: ${OCI_CLI_FINGERPRINT}" | |
| echo "OCI_CLI_KEY_CONTENT: ${OCI_CLI_KEY_CONTENT}" | |
| echo "OCI_CLI_REGION: ${OCI_CLI_REGION}" | |
| - name: Verify Secrets Exist | |
| env: # Correctly placed at the same level as 'run' | |
| OCI_CLI_USER: ${{ secrets.OCI_CLI_USER }} | |
| OCI_CLI_TENANCY: ${{ secrets.OCI_CLI_TENANCY }} | |
| OCI_CLI_FINGERPRINT: ${{ secrets.OCI_CLI_FINGERPRINT }} | |
| OCI_CLI_KEY_CONTENT: ${{ secrets.OCI_CLI_KEY_CONTENT }} | |
| OCI_CLI_REGION: ${{ secrets.OCI_CLI_REGION }} | |
| run: | | |
| [[ -n "$OCI_CLI_USER" ]] && echo "OCI_CLI_USER is set" || echo "OCI_CLI_USER is not set" | |
| [[ -n "$OCI_CLI_TENANCY" ]] && echo "OCI_CLI_TENANCY is set" || echo "OCI_CLI_TENANCY is not set" | |
| [[ -n "$OCI_CLI_FINGERPRINT" ]] && echo "OCI_CLI_FINGERPRINT is set" || echo "OCI_CLI_FINGERPRINT is not set" | |
| [[ -n "$OCI_CLI_KEY_CONTENT" ]] && echo "OCI_CLI_KEY_CONTENT is set" || echo "OCI_CLI_KEY_CONTENT is not set" | |
| [[ -n "$OCI_CLI_REGION" ]] && echo "OCI_CLI_REGION is set" || echo "OCI_CLI_REGION is not set" |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Signed-off-by: chahat sagar <109112505+chahatsagarmain@users.noreply.github.com>
2ab9ba5
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test