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

fix: warning LegacyKeyValueFormat in Dockerfile #2314

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jul 19, 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

fix: warning LegacyKeyValueFormat in Dockerfile

Motivation and Context

  • Clear the warning raised during docker build build

WARN: LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format

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

Bug fix, Enhancement


Description

  • Updated dependency versions for OPENTELEMETRY, GRPC, and NETTY in Base/Dockerfile
  • Fixed ENV format to key=value across multiple Dockerfiles to clear warnings
  • Removed deprecated apt-key usage in Base/Dockerfile
  • Adjusted paths for JRE installation in Base/Dockerfile
  • Added LANGUAGE argument in NodeBase/Dockerfile
  • Added HOME argument in Video/Dockerfile

Changes walkthrough 📝

Relevant files
Bug fix
14 files
Dockerfile
Update dependencies and fix Dockerfile warnings                   

Base/Dockerfile

  • Updated dependency versions for OPENTELEMETRY, GRPC, and NETTY
  • Fixed ENV format to key=value
  • Removed deprecated apt-key usage
  • Adjusted paths for JRE installation
  • +10/-14 
    Dockerfile
    Fix ENV format in Distributor Dockerfile                                 

    Distributor/Dockerfile

    • Fixed ENV format to key=value
    +5/-6     
    Dockerfile
    Fix ENV format in EventBus Dockerfile                                       

    EventBus/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in Hub Dockerfile                                                 

    Hub/Dockerfile

    • Fixed ENV format to key=value
    +6/-7     
    Dockerfile
    Fix ENV format in NodeChrome Dockerfile                                   

    NodeChrome/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in NodeChromium Dockerfile                               

    NodeChromium/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in NodeDocker Dockerfile                                   

    NodeDocker/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in NodeEdge Dockerfile                                       

    NodeEdge/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in NodeFirefox Dockerfile                                 

    NodeFirefox/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in Router Dockerfile                                           

    Router/Dockerfile

    • Fixed ENV format to key=value
    +4/-5     
    Dockerfile
    Fix ENV format in SessionQueue Dockerfile                               

    SessionQueue/Dockerfile

    • Fixed ENV format to key=value
    +4/-5     
    Dockerfile
    Fix ENV format in Sessions Dockerfile                                       

    Sessions/Dockerfile

    • Fixed ENV format to key=value
    +2/-2     
    Dockerfile
    Fix ENV format in Standalone Dockerfile                                   

    Standalone/Dockerfile

    • Fixed ENV format to key=value
    +6/-7     
    Dockerfile
    Fix ENV format in StandaloneDocker Dockerfile                       

    StandaloneDocker/Dockerfile

    • Fixed ENV format to key=value
    +5/-6     
    Enhancement
    2 files
    Dockerfile
    Add LANGUAGE argument and fix ENV format                                 

    NodeBase/Dockerfile

    • Added LANGUAGE argument
    • Fixed ENV format to key=value
    +3/-2     
    Dockerfile
    Add HOME argument and fix ENV format                                         

    Video/Dockerfile

    • Added HOME argument
    • Fixed ENV format to key=value
    +25/-24 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Set the HOME environment variable immediately after its ARG definition to avoid misconfigurations

    Ensure that the HOME environment variable is set before it is used to define other
    variables or directory paths to prevent potential misconfigurations.

    Base/Dockerfile [18]

     ARG HOME=/home/${SEL_USER}
    +ENV HOME=${HOME}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Ensuring that the HOME environment variable is set before it is used to define other variables or directory paths prevents potential misconfigurations, which is crucial for the correct functioning of the Docker image.

    9
    Verify architecture compatibility for the OpenJDK package installation

    Consider verifying the architecture and version compatibility of the OpenJDK package
    to prevent runtime issues on different architectures.

    Base/Dockerfile [62]

    -openjdk-${JRE_VERSION}-jre-headless
    +RUN dpkg --add-architecture ${ARCH} && apt-get update && apt-get install -y openjdk-${JRE_VERSION}-jre-headless
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Verifying the architecture and version compatibility of the OpenJDK package can prevent runtime issues on different architectures, although this suggestion could be more context-specific and may not always be necessary.

    6
    Best practice
    Use a specific version tag for the OpenJDK package to ensure consistent Docker builds

    It's recommended to use a more specific tag than jre-headless for the OpenJDK
    package to ensure that the Docker image is built consistently across different
    environments. Using a specific version helps in maintaining consistent behavior and
    avoiding unexpected changes due to updates in the base images.

    Base/Dockerfile [62]

    -openjdk-${JRE_VERSION}-jre-headless
    +openjdk-${JRE_VERSION}-jre-headless=8u275-b01
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using a specific version tag for the OpenJDK package helps to ensure consistent behavior and avoid unexpected changes due to updates in the base images, which is a best practice for Docker builds.

    8
    Maintainability
    Use a custom Java security file instead of modifying it with sed for better maintainability

    Replace the use of sed for modifying the Java security file with a more robust
    method such as using a custom security file. This approach reduces the risk of
    syntax errors and improves maintainability.

    Base/Dockerfile [68]

    -sed -i 's/securerandom\.source=file:\/dev\/random/securerandom\.source=file:\/dev\/urandom/' /usr/lib/jvm/java-${JRE_VERSION}-openjdk-${ARCH}/conf/security/java.security
    +COPY custom_java.security /usr/lib/jvm/java-${JRE_VERSION}-openjdk-${ARCH}/conf/security/java.security
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Replacing the use of sed with a custom security file reduces the risk of syntax errors and improves maintainability, which is beneficial for long-term code management.

    7

    @VietND96 VietND96 merged commit 066ee10 into SeleniumHQ:trunk Jul 19, 2024
    2 of 16 checks passed
    @VietND96 VietND96 added this to the 4.23 milestone Jul 19, 2024
    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