Skip to content

[SYSE-363 master] May template application#6270

Merged
alephnull merged 1 commit into
masterfrom
releng/master
May 13, 2024
Merged

[SYSE-363 master] May template application#6270
alephnull merged 1 commit into
masterfrom
releng/master

Conversation

@alephnull

@alephnull alephnull commented May 13, 2024

Copy link
Copy Markdown
Contributor

User description

  • Do not maintain release-4-lts anymore
  • Build faster
  • Distroless based images
  • upgrade tests have incorrect conditional

PR Type

enhancement, bug_fix


Description

  • Updated CI/CD pipeline configuration in .github/workflows/release.yml to fix incorrect metadata references and improve the build process.
  • Added new 'cache_db' outputs and updated Docker compose commands to support additional database configurations.
  • Introduced a new 'distroless' job in the CI/CD pipeline to build distroless images for the Tyk gateway, enhancing security by minimizing runtime dependencies.
  • Created a new ci/Dockerfile.distroless for building distroless images, which includes multi-stage builds and optimized configurations for different architectures.

Changes walkthrough 📝

Relevant files
Enhancement
release.yml
Enhancements and Fixes in CI/CD Pipeline Configuration     

.github/workflows/release.yml

  • Updated Docker metadata step IDs and output tags to use 'ci_metadata'
    instead of 'metadata'.
  • Modified the goreleaser command to skip signing instead of snapshot
    signing based on the branch condition.
  • Updated the container version used in the test-controller-api job from
    v1.7 to v1.8.
  • Added handling for a new 'cache_db' output and included it in Docker
    compose commands.
  • Introduced a new job 'distroless' to build distroless images of Tyk
    gateway.
  • +66/-8   
    Dockerfile.distroless
    New Dockerfile for Distroless Tyk Gateway Images                 

    ci/Dockerfile.distroless

  • Created a new Dockerfile for building distroless images of Tyk
    gateway.
  • Uses multi-stage builds starting from 'debian:bookworm-slim' and
    transitioning to 'gcr.io/distroless/base-debian12:nonroot'.
  • Copies the Tyk gateway application from the build stage to the
    distroless stage.
  • +20/-0   

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

    @alephnull
    alephnull requested a review from a team as a code owner May 13, 2024 11:14
    @alephnull
    alephnull enabled auto-merge (squash) May 13, 2024 11:14
    @alephnull alephnull changed the title [SYSE-366 master] Distroless based images [SYSE-363 master] May template application May 13, 2024
    @github-actions

    Copy link
    Copy Markdown
    Contributor

    API Changes

    no api changes detected

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Description updated to latest commit (e840e35)

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    4, due to the complexity and breadth of changes across multiple configuration files and Docker setups, which require careful validation to ensure they don't introduce regressions or configuration errors.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The condition in the goreleaser command might fail due to incorrect syntax. The logical operator might not work as expected in the shell script within the YAML configuration.

    Configuration Error: The docker compose commands have been updated to include a new file ${{ matrix.cache_db }}.yml which might not exist or be properly configured, leading to runtime errors.

    🔒 Security concerns

    No

    Code feedback:
    relevant file.github/workflows/release.yml
    suggestion      

    Consider verifying the existence and correct configuration of ${{ matrix.cache_db }}.yml before using it in docker compose commands to avoid runtime errors. [important]

    relevant linedocker compose -p auto -f pro-ha.yml -f deps_pro-ha.yml -f ${{ matrix.db }}.yml -f ${{ matrix.cache_db }}.yml --env-file versions.env --profile master-datacenter up --quiet-pull -d

    relevant file.github/workflows/release.yml
    suggestion      

    Review and test the conditional logic in the goreleaser command to ensure it executes correctly under all branch conditions. [important]

    relevant linegoreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign' || '' }}' | tee /tmp/build.sh

    relevant file.github/workflows/release.yml
    suggestion      

    Ensure that all necessary labels and tags are correctly applied in the distroless_metadata step to maintain consistency and traceability of Docker images. [medium]

    relevant lineid: distroless_metadata

    relevant fileci/Dockerfile.distroless
    suggestion      

    Validate the multi-stage Dockerfile to ensure that all dependencies and configurations are correctly copied from the DEB stage to the distroless base image to prevent runtime issues. [important]

    relevant lineCOPY --from=DEB /opt/tyk-gateway /opt/tyk-gateway

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct the flag in the goreleaser command to ensure proper functionality

    Ensure that the conditional logic for the goreleaser command is correctly formatted. The
    original command used --skip-sign while the new command uses --skip=sign. Verify which one
    is correct as they might have different implications.

    .github/workflows/release.yml [86]

    -goreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign' || '' }}' | tee /tmp/build.sh
    +goreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip-sign' || '' }}' | tee /tmp/build.sh
     
    Suggestion importance[1-10]: 10

    Why: The suggestion correctly identifies a critical typo in the goreleaser command that affects its functionality. Correcting --skip=sign to --skip-sign is crucial for the intended behavior of the command.

    10
    Enhancement
    Add validation for the new cache_db output to ensure it is properly set

    The new cache_db output and matrix variable have been added without corresponding
    validation or error handling. Consider adding checks to ensure that these values are set
    and valid before they are used in the workflow.

    .github/workflows/release.yml [171]

     cache_db: ${{ steps.params.outputs.api_cache_db }}
    +if: ${{ steps.params.outputs.api_cache_db != '' }}
     
    Suggestion importance[1-10]: 7

    Why: Adding validation for the cache_db output is a good practice to ensure it is properly set before use, enhancing the robustness of the workflow.

    7
    Best practice
    Add error handling for conditional steps in the distroless job

    The introduction of the distroless job includes multiple steps that conditionally execute
    based on the presence of a tag. Ensure that these conditions are necessary and that there
    is fallback or error handling for cases where the conditions are not met.

    .github/workflows/release.yml [374-377]

     - name: Login to DockerHub
       if: startsWith(github.ref, 'refs/tags')
       uses: docker/login-action@v3
       with:
         username: ${{ secrets.DOCKER_USERNAME }}
         password: ${{ secrets.DOCKER_PASSWORD }}
    +  else:
    +    run: echo "Skipping DockerHub login as no tag is present"
     
    Suggestion importance[1-10]: 6

    Why: The suggestion to add error handling for conditional steps is a good practice, especially for steps that depend on the presence of a tag. It improves the reliability of the workflow.

    6
    Possible issue
    Verify compatibility of ubuntu:xenial with current dependencies

    The addition of ubuntu:xenial to the distro matrix might introduce compatibility issues
    with newer software dependencies. Verify that all dependencies are supported on
    ubuntu:xenial or consider removing it if not compatible.

    .github/workflows/release.yml [426]

    -- ubuntu:xenial
    +# - ubuntu:xenial  # Uncomment if compatibility is confirmed
     
    Suggestion importance[1-10]: 5

    Why: While the suggestion to verify compatibility of ubuntu:xenial is valid, it is more of a precautionary measure rather than fixing an immediate issue. It's important but not critical unless incompatibility is proven.

    5

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @alephnull
    alephnull force-pushed the releng/master branch 2 times, most recently from 9eab246 to 49dbff0 Compare May 13, 2024 12:33
    @github-actions

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @alephnull
    alephnull force-pushed the releng/master branch 3 times, most recently from 021c353 to 87c351d Compare May 13, 2024 13:33
    @sonarqubecloud

    Copy link
    Copy Markdown

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    No data about Coverage
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    @alephnull
    alephnull merged commit 00e4d9c into master May 13, 2024
    @alephnull
    alephnull deleted the releng/master branch May 13, 2024 14:26
    nerdydread pushed a commit that referenced this pull request Sep 6, 2024
    ### **User description**
    - [x] Do not maintain release-4-lts anymore
    - [ ] Build faster
    - [ ] Distroless based images
    - [x] upgrade tests have incorrect conditional
    
    
    ___
    
    ### **PR Type**
    enhancement, bug_fix
    
    
    ___
    
    ### **Description**
    - Updated CI/CD pipeline configuration in
    `.github/workflows/release.yml` to fix incorrect metadata references and
    improve the build process.
    - Added new 'cache_db' outputs and updated Docker compose commands to
    support additional database configurations.
    - Introduced a new 'distroless' job in the CI/CD pipeline to build
    distroless images for the Tyk gateway, enhancing security by minimizing
    runtime dependencies.
    - Created a new `ci/Dockerfile.distroless` for building distroless
    images, which includes multi-stage builds and optimized configurations
    for different architectures.
    
    
    ___
    
    
    
    ### **Changes walkthrough** 📝
    <table><thead><tr><th></th><th align="left">Relevant
    files</th></tr></thead><tbody><tr><td><strong>Enhancement
    </strong></td><td><table>
    <tr>
      <td>
        <details>
    <summary><strong>release.yml</strong><dd><code>Enhancements and Fixes in
    CI/CD Pipeline Configuration</code>&nbsp; &nbsp; &nbsp; </dd></summary>
    <hr>
    
    .github/workflows/release.yml
    <li>Updated Docker metadata step IDs and output tags to use
    'ci_metadata' <br>instead of 'metadata'.<br> <li> Modified the
    goreleaser command to skip signing instead of snapshot <br>signing based
    on the branch condition.<br> <li> Updated the container version used in
    the test-controller-api job from <br>v1.7 to v1.8.<br> <li> Added
    handling for a new 'cache_db' output and included it in Docker
    <br>compose commands.<br> <li> Introduced a new job 'distroless' to
    build distroless images of Tyk <br>gateway.<br>
    
    
    </details>
        
    
      </td>
    <td><a
    href="https://github.com/TykTechnologies/tyk/pull/6270/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34">+66/-8</a>&nbsp;
    &nbsp; </td>
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>Dockerfile.distroless</strong><dd><code>New Dockerfile
    for Distroless Tyk Gateway Images</code>&nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
    <hr>
    
    ci/Dockerfile.distroless
    <li>Created a new Dockerfile for building distroless images of Tyk
    <br>gateway.<br> <li> Uses multi-stage builds starting from
    'debian:bookworm-slim' and <br>transitioning to
    'gcr.io/distroless/base-debian12:nonroot'.<br> <li> Copies the Tyk
    gateway application from the build stage to the <br>distroless
    stage.<br>
    
    
    </details>
        
    
      </td>
    <td><a
    href="https://github.com/TykTechnologies/tyk/pull/6270/files#diff-ab1e64220db8ccca1a52a505decc2beb2156d5ec3ecb7d6b8660cc3dc7e1f5bd">+20/-0</a>&nbsp;
    &nbsp; </td>
    </tr>                    
    </table></td></tr></tr></tbody></table>
    
    ___
    
    > 💡 **PR-Agent usage**:
    >Comment `/help` on the PR to get a list of all available PR-Agent tools
    and their descriptions
    
    Co-authored-by: Gromit <policy@gromit>
    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.

    2 participants