Skip to content
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.

Sign up for GitHub

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

[OTE-894] Pipeline test main replica #2547

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[OTE-894] Pipeline test main replica #2547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae2e6ce
test workflow trigger
jerryfan01234 Oct 24, 2024
0ee20a7
test trigger
jerryfan01234 Oct 28, 2024
74f627d
delete dummy file
jerryfan01234 Oct 28, 2024
8fb5385
add periodic workflow
jerryfan01234 Oct 28, 2024
4cf57b5
name change
jerryfan01234 Oct 28, 2024
f9e50ab
changes
jerryfan01234 Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/pipeline-indexer-staging-promotion.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Deploy indexer to staging

on:
pull_request:
types:
- closed

jobs:
on-merge:
if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'pipeline-main-replica'
runs-on: ubuntu-latest

Comment on lines +8 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add security configurations and environment protection

The job configuration needs security enhancements:

  1. Explicit permissions block
  2. Environment protection rules
  3. Concurrency limits to prevent parallel deployments
 jobs:
   on-merge:
     if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'pipeline-main-replica'
     runs-on: ubuntu-latest
+    environment: staging
+    concurrency: staging_deployment
+    permissions:
+      contents: read
+      deployments: write
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jobs:
on-merge:
if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'pipeline-main-replica'
runs-on: ubuntu-latest
jobs:
on-merge:
if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'pipeline-main-replica'
runs-on: ubuntu-latest
environment: staging
concurrency: staging_deployment
permissions:
contents: read
deployments: write

steps:
- name: Echo Hello World
run: echo "Hello World"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add newline at end of file

Add a newline character at the end of the file to comply with YAML best practices.

       - name: Echo Hello World
         run: echo "Hello World"
+

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 yamllint

[error] 15-15: no new line character at the end of file

(new-line-at-end-of-file)

13 changes: 13 additions & 0 deletions .github/workflows/pipeline-indexer-testnet-promotion.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: Snapshot indexer staging
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Resolve naming inconsistency between workflow name and filename

The workflow name "Snapshot indexer staging" doesn't match the filename "pipeline-indexer-testnet-promotion.yml" which mentions testnet. This could cause confusion about whether this workflow is for staging or testnet environment.

-name: Snapshot indexer staging
+name: Pipeline indexer testnet promotion
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name: Snapshot indexer staging
name: Pipeline indexer testnet promotion


on:
schedule:
# 12pm ET from Mar to Oct, 1pm ET from Nov to Feb due to daylight saving
- cron: '0 16 * * *'
Comment on lines +4 to +6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Verify cron schedule timezone calculation

The cron expression 0 16 * * * (16:00 UTC) doesn't align with the stated times:

  • March-October: 16:00 UTC = 12:00 EDT (UTC-4)
  • November-February: 16:00 UTC = 11:00 EST (UTC-5), not 13:00 EST as commented

To run at 12pm ET (March-October) and 1pm ET (November-February), use:

-    - cron: '0 16 * * *'
+    - cron: '0 17 * * *'  # 1pm ET (Nov-Feb) / 12pm ET (Mar-Oct)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
schedule:
# 12pm ET from Mar to Oct, 1pm ET from Nov to Feb due to daylight saving
- cron: '0 16 * * *'
schedule:
# 12pm ET from Mar to Oct, 1pm ET from Nov to Feb due to daylight saving
- cron: '0 17 * * *'

jobs:
deploy:
runs-on: ubuntu-latest
Comment on lines +8 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Rename job to match its current purpose

The job name 'deploy' implies deployment functionality, but the job currently only outputs "Hello World". This could be misleading for other developers.

If this is a placeholder:

-  deploy:
+  placeholder:

If this will be a deployment job, please implement proper deployment steps including:

  • Checkout code
  • Authentication
  • Deployment commands
  • Error handling
  • Status notifications

Committable suggestion was skipped due to low confidence.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix YAML formatting issues

There are two minor formatting issues:

  1. Line 10: Remove trailing spaces
  2. Line 13: Add newline at end of file
    runs-on: ubuntu-latest
-        
+
    steps:
      - name: Echo Hello World
-        run: echo "Hello World"
+        run: echo "Hello World"
+

Also applies to: 13-13

🧰 Tools
🪛 yamllint

[error] 10-10: trailing spaces

(trailing-spaces)

steps:
- name: Echo Hello World
run: echo "Hello World"
Comment on lines +1 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Add environment, permissions, and manual trigger to the promotion workflow

The workflow requires several critical security and usability improvements:

  1. Missing environment configuration unlike other deployment workflows that use environment protection
  2. No defined permissions scope which is a security best practice
  3. Only scheduled trigger without manual option which limits operational flexibility

Required changes:

  • Add environment: staging to the job
  • Add permissions block to limit scope
  • Add workflow_dispatch trigger for manual runs
  • Consider using the reusable workflow pattern like other deployment workflows
🔗 Analysis chain

Verify workflow triggers and permissions

Since this is a promotion pipeline that likely involves deployments, we should verify:

  1. If the workflow has appropriate permissions
  2. If environment protection rules are in place
  3. If the schedule trigger is sufficient (consider adding manual trigger)

Let me check the content of the related workflows to better understand the environment configurations and permissions.

Let me check the content of the deployment workflows to understand the complete context.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for environment configurations and other workflow permissions
echo "Checking for environment configurations..."
rg -l "environment:" .github/workflows/
echo "Checking for workflow permissions..."
rg -l "permissions:" .github/workflows/

Length of output: 444

Script:

#!/bin/bash
# Check content of workflows with environment configurations
echo "Content of indexer-reusable-build-and-push-ecs-service.yml:"
cat .github/workflows/indexer-reusable-build-and-push-ecs-service.yml

echo -e "\nContent of indexer-build-and-push-lambda.yml:"
cat .github/workflows/indexer-build-and-push-lambda.yml

# Check if there are any other promotion or deployment workflows
echo -e "\nChecking for other promotion workflows:"
rg -l "promotion|deploy" .github/workflows/

Length of output: 4509

Script:

#!/bin/bash
# Check content of deployment workflows
echo "Content of deploy-to-dev.yml:"
cat .github/workflows/deploy-to-dev.yml

echo -e "\nContent of pipeline-indexer-testnet-promotion.yml:"
cat .github/workflows/pipeline-indexer-testnet-promotion.yml

# Check if there are any environment protection rules in other workflows
echo -e "\nChecking for environment protection rules:"
rg "environment:" -A 5 -B 5 .github/workflows/

Length of output: 10312

🧰 Tools
🪛 yamllint

[error] 10-10: trailing spaces

(trailing-spaces)

[error] 13-13: no new line character at the end of file

(new-line-at-end-of-file)

Comment on lines +11 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement required promotion pipeline steps

The current implementation appears to be a placeholder. A promotion pipeline typically requires several essential steps:

Consider implementing these critical steps:

    steps:
      - uses: actions/checkout@v4
      
      - name: Configure AWS credentials
        uses: aws-actions/configure-aws-credentials@v4
        with:
          role-to-assume: ${{ secrets.AWS_ROLE_ARN }}
          aws-region: ${{ secrets.AWS_REGION }}
      
      - name: Promote indexer
        run: |
          # Add promotion logic here
          set -e
          # Example:
          # - Verify current deployment
          # - Update deployment tags
          # - Deploy to target environment
          # - Run health checks
      
      - name: Notify status
        if: always()
        uses: slackapi/slack-github-action@v1.24.0
        with:
          channel-id: ${{ secrets.SLACK_CHANNEL_ID }}
          slack-message: "Indexer promotion to testnet ${{ job.status }}"

Would you like me to help create a complete implementation of the promotion pipeline?

🧰 Tools
🪛 yamllint

[error] 13-13: no new line character at the end of file

(new-line-at-end-of-file)

Loading