Skip to content

Comments

fix: Modified Branch-Build to support Release#5803

Merged
sriramveeraghanta merged 1 commit intopreviewfrom
fix/branch-build-simplify
Oct 11, 2024
Merged

fix: Modified Branch-Build to support Release#5803
sriramveeraghanta merged 1 commit intopreviewfrom
fix/branch-build-simplify

Conversation

@mguptahub
Copy link
Collaborator

@mguptahub mguptahub commented Oct 11, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a reusable action for building and pushing Docker images, enhancing CI/CD capabilities.
    • Added new workflow inputs for flexible build configurations, including build type and release version.
    • Implemented new jobs for asset attachment and release publication, streamlining the release process.
  • Bug Fixes

    • Enhanced logic for environment variable management and conditional checks in build jobs.
  • Refactor

    • Overhauled workflow structure for improved organization and support of complex build scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 11, 2024

Walkthrough

The changes introduce a new GitHub Actions workflow for building and pushing Docker images, defined in a newly created action.yml file. This reusable action includes various input parameters necessary for its operation. Additionally, the existing workflow file build-branch.yml has been updated to enhance its functionality, including new input parameters, workflow triggers, and job configurations. The modifications streamline the build process and introduce new jobs for asset attachment and release publishing.

Changes

File Path Change Summary
.github/actions/build-push-action/action.yml - New action added with name and description.
- Input parameters introduced: docker-username, docker-token, docker-image-owner, docker-image-name, build-context, dockerfile-path, build-args, buildx-driver, buildx-version, buildx-platforms, buildx-endpoint, build-release, build-prerelease, release-version.
- Runs configuration added.
.github/workflows/build-branch.yml - Workflow name changed to "Branch Build CE".
- New inputs added: build_type, releaseVersion, isPrerelease.
- Updated environment variables.
- Job branch_build_setup modified to run on ubuntu-20.04.
- New jobs added: attach_assets_to_build, publish_release.
- Existing jobs updated to use common actions and streamlined conditional checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant GitHub Actions
    participant Docker Hub

    User->>GitHub Actions: Trigger workflow
    GitHub Actions->>GitHub Actions: Set up environment variables
    GitHub Actions->>Docker Hub: Log in with credentials
    GitHub Actions->>GitHub Actions: Build Docker image
    GitHub Actions->>Docker Hub: Push Docker image
    GitHub Actions->>User: Workflow complete
Loading

🐰 "In the garden of code, we build and we push,
With Docker and actions, we make quite a rush.
New inputs and jobs, all neatly aligned,
A workflow so smooth, it’s truly refined!
Hops of joy as we celebrate,
In the world of GitHub, we innovate!" 🐇


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (9)
.github/actions/build-push-action/action.yml (4)

1-60: LGTM! Well-structured action definition with comprehensive inputs.

The action name, description, and input parameters are well-defined and cover all necessary aspects for building and pushing Docker images. A few suggestions for improvement:

  1. Consider adding pattern validation for version-related inputs (e.g., buildx-version, release-version) to ensure they follow the expected format.
  2. For build-args, it might be helpful to provide an example in the description of how multiple arguments should be formatted.
  3. The buildx-endpoint input might benefit from a brief explanation of its purpose and potential values.
🧰 Tools
🪛 yamllint

[error] 29-29: trailing spaces

(trailing-spaces)


65-97: LGTM! Comprehensive Docker tag setup with proper version validation.

The "Set Docker Tag" step effectively handles various scenarios for tagging Docker images. The semver regex pattern for version validation is correct and the error message for invalid versions is informative.

One suggestion for improvement:
Consider adding a comment explaining the purpose of the FLAT_BRANCH_VERSION variable and why certain characters are being removed. This would enhance code readability and maintainability.

🧰 Tools
🪛 yamllint

[error] 96-96: trailing spaces

(trailing-spaces)


98-126: LGTM! Well-structured steps using official actions and best practices.

The steps for Docker Hub login, Buildx setup, repository checkout, and image build/push are well-implemented using official GitHub Actions. The use of Docker Buildx enables multi-platform builds, which is a great feature.

One security suggestion:
Consider using GitHub's OIDC (OpenID Connect) for Docker Hub authentication instead of using a static token. This can be more secure as it provides short-lived, automatically rotated credentials. You can implement this using the docker/login-action with the username input set to ${{ secrets.DOCKERHUB_USERNAME }} and password set to ${{ secrets.DOCKERHUB_TOKEN }}, where these secrets are configured in your GitHub repository.

🧰 Tools
🪛 yamllint

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

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


29-29: Minor formatting improvements suggested.

The static analysis tool reported a few minor formatting issues:

  1. Trailing spaces on lines 29 and 96.
  2. Missing newline character at the end of the file.

While these don't affect functionality, addressing them would improve code style consistency. Consider running a YAML linter as part of your pre-commit process to catch these issues automatically.

Also applies to: 96-96, 126-126

🧰 Tools
🪛 yamllint

[error] 29-29: trailing spaces

(trailing-spaces)

.github/workflows/build-branch.yml (5)

42-42: Consider using a more recent Ubuntu version.

The job is now set to run on ubuntu-20.04. While this is a stable version, consider using a more recent LTS version like ubuntu-22.04 for potential performance improvements and longer support.


Line range hint 71-119: Improve script quality and safety in the set_env_variables step.

The script in the set_env_variables step has several areas for improvement:

  1. Use { and } instead of ( and ) to avoid subshell overhead.
  2. Use double quotes around variables to prevent globbing and word splitting.
  3. Consider using { cmd1; cmd2; } >> $GITHUB_OUTPUT instead of individual redirects for better performance.
  4. The semver regex could be improved for more accurate version matching.

Here's an improved version of the script:

if [[ "${ARM64_BUILD}" == "true" ]] || { [[ "${BUILD_TYPE}" == "Release" ]] && [[ "${IS_PRERELEASE}" != "true" ]]; }; then
  {
    echo "BUILDX_DRIVER=cloud"
    echo "BUILDX_VERSION=lab:latest"
    echo "BUILDX_PLATFORMS=linux/amd64,linux/arm64"
    echo "BUILDX_ENDPOINT=makeplane/plane-dev"
  } >> "$GITHUB_OUTPUT"
else
  {
    echo "BUILDX_DRIVER=docker-container"
    echo "BUILDX_VERSION=latest"
    echo "BUILDX_PLATFORMS=linux/amd64"
    echo "BUILDX_ENDPOINT="
  } >> "$GITHUB_OUTPUT"
fi

BR_NAME=$(echo "${TARGET_BRANCH}" | sed 's/[^a-zA-Z0-9.-]//g')
echo "TARGET_BRANCH=${BR_NAME}" >> "$GITHUB_OUTPUT"

{
  echo "DH_IMG_WEB=plane-frontend"
  echo "DH_IMG_SPACE=plane-space"
  echo "DH_IMG_ADMIN=plane-admin"
  echo "DH_IMG_LIVE=plane-live"
  echo "DH_IMG_BACKEND=plane-backend"
  echo "DH_IMG_PROXY=plane-proxy"
} >> "$GITHUB_OUTPUT"

echo "BUILD_TYPE=${BUILD_TYPE}" >> "$GITHUB_OUTPUT"
BUILD_RELEASE=false
BUILD_PRERELEASE=false
RELVERSION="latest"

if [[ "${BUILD_TYPE}" == "Release" ]]; then
  FLAT_RELEASE_VERSION=$(echo "${RELEASE_VERSION}" | sed 's/[^a-zA-Z0-9.-]//g')
  echo "FLAT_RELEASE_VERSION=${FLAT_RELEASE_VERSION}" >> "$GITHUB_OUTPUT"

  semver_regex="^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$"
  if [[ ! $FLAT_RELEASE_VERSION =~ $semver_regex ]]; then
    echo "Invalid Release Version Format : $FLAT_RELEASE_VERSION"
    echo "Please provide a valid SemVer version"
    echo "e.g. v1.2.3 or v1.2.3-alpha-1"
    echo "Exiting the build process"
    exit 1
  fi
  BUILD_RELEASE=true
  RELVERSION=$FLAT_RELEASE_VERSION

  if [[ "${IS_PRERELEASE}" == "true" ]]; then
    BUILD_PRERELEASE=true
  fi
fi

{
  echo "BUILD_RELEASE=${BUILD_RELEASE}"
  echo "BUILD_PRERELEASE=${BUILD_PRERELEASE}"
  echo "RELEASE_VERSION=${RELVERSION}"
} >> "$GITHUB_OUTPUT"

These changes improve the script's safety and efficiency.


163-317: LGTM: Well-structured build and push jobs

The new jobs for building and pushing Docker images for various components (admin, web, space, live, apiserver, and proxy) are well-structured and use a common action. This approach promotes code reuse and consistency across different builds.

Consider using a matrix strategy to reduce repetition

While the current structure is clear and functional, you might consider using a matrix strategy to reduce repetition in the workflow. This could make the workflow more maintainable and easier to update in the future.

Here's an example of how you could refactor this using a matrix:

jobs:
  build_and_push:
    strategy:
      matrix:
        component: [admin, web, space, live, apiserver, proxy]
        include:
          - component: admin
            image_name: ${{ needs.branch_build_setup.outputs.dh_img_admin }}
            dockerfile: ./admin/Dockerfile.admin
            build_context: .
          - component: web
            image_name: ${{ needs.branch_build_setup.outputs.dh_img_web }}
            dockerfile: ./web/Dockerfile.web
            build_context: .
          # ... include similar entries for other components

    name: Build-Push ${{ matrix.component }} Docker Image
    runs-on: ubuntu-20.04
    needs: [branch_build_setup]
    if: ${{ needs.branch_build_setup.outputs.build_${{ matrix.component }} == 'true' || github.event_name == 'workflow_dispatch' || needs.branch_build_setup.outputs.gh_branch_name == 'master' }}
    steps:
      - uses: actions/checkout@v4
      - name: ${{ matrix.component }} Build and Push
        uses: ./.github/actions/build-push-action
        with:
          build-release: ${{ needs.branch_build_setup.outputs.build_release }}
          build-prerelease: ${{ needs.branch_build_setup.outputs.build_prerelease }}
          release-version: ${{ needs.branch_build_setup.outputs.release_version }}
          docker-username: ${{ secrets.DOCKERHUB_USERNAME }}
          docker-token: ${{ secrets.DOCKERHUB_TOKEN }}
          docker-image-owner: makeplane
          docker-image-name: ${{ matrix.image_name }}
          build-context: ${{ matrix.build_context }}
          dockerfile-path: ${{ matrix.dockerfile }}
          buildx-driver: ${{ needs.branch_build_setup.outputs.gh_buildx_driver }}
          buildx-version: ${{ needs.branch_build_setup.outputs.gh_buildx_version }}
          buildx-platforms: ${{ needs.branch_build_setup.outputs.gh_buildx_platforms }}
          buildx-endpoint: ${{ needs.branch_build_setup.outputs.gh_buildx_endpoint }}

This approach would significantly reduce the amount of repeated code in your workflow.


319-343: LGTM: New job for attaching assets to build

The new attach_assets_to_build job is a good addition to the workflow. It properly handles the preparation of necessary files (copying the install script) and uploads relevant files as artifacts. This ensures that important assets are available for subsequent steps or for download after the workflow completes.

However, there's a minor formatting issue:

- 343~
+ 343~

Remove the trailing space at the end of line 343.

🧰 Tools
🪛 yamllint

[error] 343-343: trailing spaces

(trailing-spaces)


344-383: LGTM: New job for publishing releases

The new publish_release job is an excellent addition to the workflow. It automates the process of creating GitHub releases, ensuring consistency and including the necessary files. The conditional execution based on the build type is also appropriate.

Consider adding a changelog generation step

To further improve the release process, consider adding a step to automatically generate a changelog based on the commits since the last release. This could be done using a tool like github-changelog-generator or by parsing commit messages.

Here's an example of how you might add this:

- name: Generate Changelog
  run: |
    # Install github-changelog-generator if not already available
    gem install github_changelog_generator

    # Generate the changelog
    github_changelog_generator \
      --user ${{ github.repository_owner }} \
      --project ${{ github.event.repository.name }} \
      --token ${{ secrets.GITHUB_TOKEN }} \
      --since-tag $(git describe --tags --abbrev=0) \
      --output CHANGELOG.md

- name: Create Release
  id: create_release
  uses: softprops/action-gh-release@v2.0.8
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
  with:
    tag_name: ${{ env.REL_VERSION }}
    name: ${{ env.REL_VERSION }}
    body_path: CHANGELOG.md  # Use the generated changelog
    draft: false
    prerelease: ${{ env.IS_PRERELEASE }}
    files: |
      ${{ github.workspace }}/deploy/selfhost/setup.sh
      ${{ github.workspace }}/deploy/selfhost/restore.sh
      ${{ github.workspace }}/deploy/selfhost/docker-compose.yml
      ${{ github.workspace }}/deploy/selfhost/variables.env

This addition would provide users with a clear list of changes in each release.

🧰 Tools
🪛 yamllint

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

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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3011ef9 and c763093.

📒 Files selected for processing (2)
  • .github/actions/build-push-action/action.yml (1 hunks)
  • .github/workflows/build-branch.yml (5 hunks)
🧰 Additional context used
🪛 yamllint
.github/actions/build-push-action/action.yml

[error] 29-29: trailing spaces

(trailing-spaces)


[error] 96-96: trailing spaces

(trailing-spaces)


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

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

.github/workflows/build-branch.yml

[error] 343-343: trailing spaces

(trailing-spaces)


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

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

🪛 actionlint
.github/workflows/build-branch.yml

71-71: shellcheck reported issue in this script: SC2235:style:1:46: Use { ..; } instead of (..) to avoid subshell overhead

(shellcheck)


71-71: shellcheck reported issue in this script: SC2129:style:2:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:2:33: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:3:39: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:4:54: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:5:49: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2129:style:7:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:7:44: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:8:35: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:9:42: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:10:30: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2129:style:13:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:13:34: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:15:37: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:16:36: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:17:36: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:18:34: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:19:40: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:20:36: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:22:42: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:29:58: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2129:style:46:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:46:42: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:47:48: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: shellcheck reported issue in this script: SC2086:info:48:41: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)
.github/workflows/build-branch.yml (4)

33-37: LGTM: Environment variables setup

The new environment variables (BUILD_TYPE, RELEASE_VERSION, IS_PRERELEASE) are well-aligned with the added input parameters. This change provides more flexibility in the build process and allows for better control over different types of builds and releases.


56-61: LGTM: New outputs for Docker image names

The addition of new outputs for various Docker image names (web, space, admin, live, backend, proxy) is a good practice. It centralizes the configuration of image names and makes it easier to reference them in subsequent jobs.


Line range hint 1-383: Overall, excellent improvements to the workflow

The changes to this workflow file represent a significant enhancement in functionality and flexibility. Key improvements include:

  1. Support for different build types and release processes.
  2. New jobs for building various components (admin, web, space, live, apiserver, proxy).
  3. Automated release publishing process.

These changes will greatly improve the CI/CD pipeline for the project. However, there are a few minor areas for potential improvement:

  1. Consider using a matrix strategy for the build jobs to reduce repetition.
  2. Add a changelog generation step to the release process.
  3. Update to a more recent Ubuntu version (e.g., ubuntu-22.04) for the job runners.
  4. Improve the script in the set_env_variables step for better safety and efficiency.

Additionally, ensure that you add a newline character at the end of the file (line 383) to comply with POSIX standards.

Great work on enhancing this workflow!

🧰 Tools
🪛 yamllint

[error] 343-343: trailing spaces

(trailing-spaces)


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

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


28-30: Consider the implications of commenting out the push trigger.

The push trigger for the master branch has been commented out. This change means that the workflow will no longer automatically run when changes are pushed to the master branch. Consider whether this is the intended behavior or if it might negatively impact your continuous integration process.

If automatic builds on push are still desired, you may want to uncomment these lines or implement a more specific push trigger configuration.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants