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.

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

[ci] Add workflow to build and deploy FFmpeg tools image #2507

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Dec 12, 2024

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Added new GitHub Actions workflow to automate building and deploying FFmpeg tools image
  • Enhanced security by updating rclone with fixed crypto dependency version
  • Updated system dependencies for FFmpeg in both FFmpeg and Video containers
  • Added FFmpeg binary validation using ldd
  • Improved pip package installation with upgrade flag

Changes walkthrough 📝

Relevant files
Enhancement
Dockerfile
Security updates and dependency fixes for FFmpeg container

.ffmpeg/Dockerfile

  • Added GO_CRYPTO_VERSION argument and patched rclone deps to fix CVEs
  • Updated system dependencies for FFmpeg
  • Added ldd check for FFmpeg binary
  • +9/-3     
    Configuration changes
    build-ffmpeg.yml
    New CI workflow for FFmpeg image build and deployment       

    .github/workflows/build-ffmpeg.yml

  • Added new GitHub Actions workflow to build and deploy FFmpeg tools
    image
  • Configures containerd, builds multi-arch images
  • Handles Docker Hub authentication and image deployment
  • +68/-0   
    Dependencies
    Dockerfile
    Dependency updates for Video container                                     

    Video/Dockerfile

  • Updated system dependencies for FFmpeg
  • Added --upgrade flag to pip install command
  • +2/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @VietND96 VietND96 merged commit 51a7a12 into trunk Dec 12, 2024
    33 of 45 checks passed
    @VietND96 VietND96 deleted the build-ffmpeg branch December 12, 2024 18:51
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 Security concerns

    Credentials exposure:
    The workflow directly uses Docker Hub credentials in a docker login command instead of using the more secure docker/login-action. This could potentially expose credentials in logs. Consider using docker/login-action which handles credentials more securely.

    ⚡ Recommended focus areas for review

    Security Risk
    Docker Hub credentials are used directly in workflow commands instead of using DOCKER_LOGIN action which is more secure

    Error Handling
    continue_on_error is set to true for the deployment step which could silently fail without proper notification

    Dependency Version
    Using 'latest' for GO version could lead to reproducibility issues. Consider pinning to specific version

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Verify integrity of downloaded dependencies using cryptographic hash checks to prevent supply chain attacks

    Add a hash check when downloading external dependencies to prevent supply chain
    attacks. This is particularly important for security-critical components like
    FFmpeg.

    .github/workflows/build-ffmpeg.yml [44-52]

     - name: Build images
         uses: nick-invision/retry@master
         with:
           timeout_minutes: 300
           max_attempts: 2
           retry_wait_seconds: 60
           command: |
    +        echo "ffmpeg-${VERSION_FFMPEG}.tar.xz.sha256sum" | sha256sum --check
             PLATFORMS="${PLATFORMS}" make ffmpeg
             make tag_ffmpeg_latest
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding cryptographic hash verification for downloaded dependencies is a critical security measure to prevent supply chain attacks, especially for security-sensitive components like FFmpeg.

    9
    Limit workflow permissions to the minimum required set to reduce potential security risks

    Remove write-all permissions and specify only the minimum required permissions for
    the workflow to function securely.

    .github/workflows/build-ffmpeg.yml [19]

    -permissions: write-all
    +permissions:
    +  contents: read
    +  packages: write
    +  id-token: write
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Following the principle of least privilege by limiting GitHub Actions permissions is essential for security, reducing the potential impact of compromised workflows.

    9
    Scan container images for security vulnerabilities before deployment to production

    Add a step to scan the container image for vulnerabilities before deployment using a
    security scanner like Trivy.

    .github/workflows/build-ffmpeg.yml [59-68]

    +- name: Scan image for vulnerabilities
    +    run: |
    +      trivy image ${NAMESPACE}/ffmpeg:latest
     - name: Deploy new images
         if: ${{ github.event.inputs.release == 'true' }}
         uses: nick-invision/retry@master
         with:
           timeout_minutes: 20
           max_attempts: 5
           retry_wait_seconds: 300
           continue_on_error: true
           command: |
             make release_ffmpeg_latest
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding vulnerability scanning before deployment is a crucial security practice that can prevent deploying containers with known vulnerabilities to production.

    8

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant