search

TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.51k stars 1.07k forks source link

[SYSE-370 master] June template application #6403

Closed ermirizio closed 1 month ago

ermirizio commented 1 month ago

User description

PR Type

Bug fix, Enhancement

Description

Changes walkthrough 📝

Relevant files
Enhancement
release.yml
Fix and enhance GitHub Actions release workflow                   
.github/workflows/release.yml
  • Fixed the BASE_REF variable to use github.ref_name instead of
    github.ref.
  • Consolidated multi-line labels into a single line with newline
    characters.
  • Simplified the run command for setting up the environment and running
    tests.
    		•  +5/-28   

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

    github-actions[bot] commented 1 month ago

    PR Reviewer Guide 🔍

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

    **BASE_REF Logic:** The change from `github.ref` to `github.ref_name` in the `BASE_REF` environment variable needs careful review to ensure it does not affect other workflows or steps negatively. **Labels Formatting:** Consolidating labels into a single line with newline characters might affect readability or parsing in some Docker or Kubernetes environments. It's important to verify that this change is compatible with all systems that might consume these labels.
    github-actions[bot] commented 1 month ago

    API Changes

    no api changes detected
    github-actions[bot] commented 1 month ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Readability
    Improve the readability and manageability of the run command by formatting it into multiple lines ___ **The run command script is overly complex and hard to read due to its single-line
    format. Splitting this into multiple lines with proper indentation will improve
    readability and make it easier to manage and debug.** [.github/workflows/release.yml [284]](https://github.com/TykTechnologies/tyk/pull/6403/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R284-R284) ```diff -run: "match_tag=${{steps.ecr.outputs.registry}}/$REPO:$BASE_REF\ndocker run -q --rm -v ~/.docker/config.json:/root/.docker/config.json tykio/gromit policy match ${match_tag} 2>versions.env\n\ntags=${{ needs.goreleaser.outputs.tags }} \n\n\ntyk_image=${tags[0]}\n\necho '# alfa and beta have to come after the override\ntyk_alfa_image=$tyk_image\ntyk_beta_image=$tyk_image\nECR=${{steps.ecr.outputs.registry}}\ntyk_pump_image=${{matrix.pump}}\ntyk_sink_image=${{matrix.sink}}\nconfs_dir=./pro-ha\nenv_file=local-${{ matrix.envfiles.db }}.env' >> versions.env\necho \"::group::versions\"\ncat versions.env\necho \"::endgroup::\"\n# Add Tyk component config variations to $env_file\ncat confs/${{ matrix.envfiles.config }}.env >> local-${{ matrix.envfiles.db }}.env\n# bring up env, the project name is important\ndocker compose -p auto -f pro-ha.yml -f deps_pro-ha.yml -f ${{ matrix.envfiles.db }}.yml -f ${{ matrix.envfiles.cache }}.yml --env-file versions.env --profile master-datacenter up --quiet-pull -d\n./dash-bootstrap.sh http://localhost:3000\ndocker compose -p auto -f pro-ha.yml -f deps_pro-ha.yml -f ${{ matrix.envfiles.db }}.yml -f ${{ matrix.envfiles.cache }}.yml --env-file versions.env --profile slave-datacenter up --quiet-pull -d\n" +run: | + match_tag=${{steps.ecr.outputs.registry}}/$REPO:$BASE_REF + docker run -q --rm -v ~/.docker/config.json:/root/.docker/config.json tykio/gromit policy match ${match_tag} 2>versions.env + tags=${{ needs.goreleaser.outputs.tags }} + tyk_image=${tags[0]} + echo '# alfa and beta have to come after the override + tyk_alfa_image=$tyk_image + tyk_beta_image=$tyk_image + ECR=${{steps.ecr.outputs.registry}} + tyk_pump_image=${{matrix.pump}} + tyk_sink_image=${{matrix.sink}} + confs_dir=./pro-ha + env_file=local-${{ matrix.envfiles.db }}.env' >> versions.env + echo "::group::versions" + cat versions.env + echo "::endgroup::" + # Add Tyk component config variations to $env_file + cat confs/${{ matrix.envfiles.config }}.env >> local-${{ matrix.envfiles.db }}.env + # bring up env, the project name is important + docker compose -p auto -f pro-ha.yml -f deps_pro-ha.yml -f ${{ matrix.envfiles.db }}.yml -f ${{ matrix.envfiles.cache }}.yml --env-file versions.env --profile master-datacenter up --quiet-pull -d + ./dash-bootstrap.sh http://localhost:3000 + docker compose -p auto -f pro-ha.yml -f deps_pro-ha.yml -f ${{ matrix.envfiles.db }}.yml -f ${{ matrix.envfiles.cache }}.yml --env-file versions.env --profile slave-datacenter up --quiet-pull -d ```
    Suggestion importance[1-10]: 10 Why: Splitting the complex `run` command into multiple lines with proper indentation greatly enhances readability and makes the script easier to debug and maintain.
    		10
    Best practice
    Use a multi-line YAML string for Docker labels to enhance clarity and manageability ___ **The labels string in the Docker build-push action is quite long and includes newline
    characters, which can be error-prone and hard to manage. It's better to format these
    labels as a multi-line YAML string to enhance clarity and avoid formatting issues.** [.github/workflows/release.yml [167]](https://github.com/TykTechnologies/tyk/pull/6403/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R167-R167) ```diff -labels: "org.opencontainers.image.title=tyk-gateway (distroless) \norg.opencontainers.image.description=Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols\norg.opencontainers.image.vendor=tyk.io\norg.opencontainers.image.version=${{ github.ref_name }}\n" +labels: | + org.opencontainers.image.title=tyk-gateway (distroless) + org.opencontainers.image.description=Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols + org.opencontainers.image.vendor=tyk.io + org.opencontainers.image.version=${{ github.ref_name }} ```
    Suggestion importance[1-10]: 9 Why: Formatting the labels as a multi-line YAML string significantly improves readability and reduces the risk of formatting errors, making the code easier to maintain.
    		9
    Maintainability
    Improve readability and maintainability by using a separate step to conditionally set the BASE_REF environment variable ___ **It's recommended to use a more explicit and readable format for setting environment
    variables. Instead of using a complex inline conditional expression, consider using
    a step to set the BASE_REF environment variable conditionally based on the event
    type. This improves readability and maintainability.** [.github/workflows/release.yml [211]](https://github.com/TykTechnologies/tyk/pull/6403/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R211-R211) ```diff -BASE_REF: ${{startsWith(github.event_name, 'pull_request') && github.base_ref || github.ref_name}} +- name: Set BASE_REF + run: | + if ${{ github.event_name == 'pull_request' }}; then + echo "BASE_REF=${{ github.base_ref }}" >> $GITHUB_ENV + else + echo "BASE_REF=${{ github.ref_name }}" >> $GITHUB_ENV ```
    Suggestion importance[1-10]: 8 Why: This suggestion improves the readability and maintainability of the workflow by separating the logic for setting the `BASE_REF` environment variable into a distinct step, making it easier to understand and manage.
    		8
    Possible bug
    Add parentheses to ensure correct evaluation order in the conditional expression for BASE_REF ___ **To avoid potential issues with the evaluation of boolean expressions in YAML, it's
    recommended to explicitly use parentheses to ensure the correct precedence and
    evaluation order.** [.github/workflows/release.yml [211]](https://github.com/TykTechnologies/tyk/pull/6403/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R211-R211) ```diff -BASE_REF: ${{startsWith(github.event_name, 'pull_request') && github.base_ref || github.ref_name}} +BASE_REF: ${{ (startsWith(github.event_name, 'pull_request') && github.base_ref) || github.ref_name }} ```
    Suggestion importance[1-10]: 7 Why: Adding parentheses clarifies the precedence of operations in the conditional expression, reducing the risk of logical errors and improving code clarity.
    		7
    sonarcloud[bot] commented 1 month ago

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    0.0% Coverage on New Code
    0.0% Duplication on New Code

    See analysis details on SonarCloud