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

feat: Publish kargo to brew #3011

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

fykaa
Copy link
Contributor

@fykaa fykaa commented Nov 27, 2024

closes #2514

Added the Kargo CLI to Homebrew. The formula supports macOS (Intel and ARM) and Linux (Intel and ARM) with architecture-specific binaries for version v1.0.3 currently.
Formula can be found here: https://github.com/akuity/homebrew-tap

TODO: Give Permission to access akuity/homebrew-tap.git (for creating a pull request) to github-actions[bot] (GITHUB_TOKEN)

@fykaa fykaa self-assigned this Nov 27, 2024
@fykaa fykaa requested review from a team as code owners November 27, 2024 17:32
Copy link

netlify bot commented Nov 27, 2024

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit ea3c16e
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-io/deploys/6752bbe30ff20000085f4be1
😎 Deploy Preview https://deploy-preview-3011.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.20%. Comparing base (59c2db9) to head (ea3c16e).
Report is 24 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3011      +/-   ##
==========================================
+ Coverage   51.08%   51.20%   +0.11%     
==========================================
  Files         283      283              
  Lines       25373    25466      +93     
==========================================
+ Hits        12963    13041      +78     
- Misses      11712    11729      +17     
+ Partials      698      696       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fykaa fykaa force-pushed the publish-kargo-to-brew branch from 15b45e4 to a3b357d Compare November 29, 2024 14:08
@fykaa fykaa marked this pull request as draft November 29, 2024 14:09
@fykaa fykaa force-pushed the publish-kargo-to-brew branch from f566485 to f126736 Compare November 29, 2024 15:44
@fykaa fykaa marked this pull request as ready for review November 29, 2024 15:47
.github/workflows/release.yaml Outdated Show resolved Hide resolved
@krancour
Copy link
Member

krancour commented Dec 2, 2024

Only very minor feedback. Overall, looks great.

if: github.event_name == 'release'
permissions:
contents: write # Needed to push changes
pull-requests: write # Needed to create a pull request
Copy link
Member

Choose a reason for hiding this comment

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

Something to look out for. I think these are permissions WRT to the current repository. This may not be sufficient to open a PR in a different repository. We'll find out when we try this.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct, for permissions to another repository you need to use a token that has access to this repository. This also means we do not have to elevate the permissions for the step here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially thought of 2 possible approaches to address this:

  1. Move the formula file into the kargo repository so that the workflow can directly create a pull request within this same repository. This eliminates the need for cross-repository access.

  2. To interact with a different repository (akuity/homebrew-tap), we'll need to generate a pat with repo scope and save it as a secretHOME_BREW_TOKEN then I can update the workflow to use it for authentication when interacting with the akuity/homebrew-tap repository

If we may want to proceed with the second approach (having separate repo for brew formula), I will remove these permissions for the step here, and use the PAT token you generate.

Copy link
Member

@krancour krancour Dec 3, 2024

Choose a reason for hiding this comment

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

The tap absolutely needs to be a separate repo. Having played with this quite a bit recently, I'm pretty confident that things work most seamlessly when the tap repo is named <org>/homebrew-tap.

(Semi-related: akuity/homebrew-tap is not currently following the conventional layout for a tap, but it does work and we can deal with that separately later.)

Here, you can use secrets.TAP_PAT and I'll ensure that secret is configured with a PAT that can write to akuity/homebrew-tap.

docs/docs/20-quickstart.md Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
@fykaa fykaa force-pushed the publish-kargo-to-brew branch from 0faca10 to 583fda7 Compare December 3, 2024 11:01
Comment on lines 339 to 342
github.event_name == 'release' &&
startsWith(github.ref, 'refs/tags/v') &&
!contains(github.ref, '-') &&
github.ref == format('refs/tags/v{0}', steps.latest_release.outputs.version)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find much help filtering for stable builds by checking nightly specs, but this if statement here ensures the tag doesn’t contain a -, effectively excluding alpha, RC, or other unstable releases. I also added a last statemet as a safeguard to validate the tag further. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can do something like github.event_name == 'release' && github.event.action == 'released', which would filter out prereleases. As our release candidates are typically marked as a pre-release, see e.g. https://github.com/akuity/kargo/releases/tag/v1.1.0-rc.3

Ref: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#release

Copy link
Contributor

Choose a reason for hiding this comment

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

The only thing my suggestion did not take into account is @krancour's

[..] and patch releases not cut from the latest release-1.x branch

For this, I think we still need to pull off some version comparisons.


However, I also want to note that it's common for Homebrow to e.g. provide minor version taps. I.e., kargo.rb (latest), [email protected] (1.1.x releases), [email protected] (1.0.x releases), etc.

In the future, we should offer support for this as well. Without this, it would be impossible to update to a patch release for a previous minor version if it's released after a newer minor range.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing it!

I have added support for updating minor version taps.
I will see how I can update the action file to run the script with minor version tag whenever a patch version is released.

.github/workflows/release.yaml Outdated Show resolved Hide resolved
Signed-off-by: Faeka Ansari <[email protected]>
@fykaa fykaa force-pushed the publish-kargo-to-brew branch from d5ae825 to 03897db Compare December 4, 2024 02:13
@krancour krancour added this to the v1.2.0 milestone Dec 5, 2024
@fykaa fykaa force-pushed the publish-kargo-to-brew branch from 56ff2a3 to a2a6659 Compare December 6, 2024 07:18
@krancour krancour modified the milestones: v1.2.0, v1.3.0 Dec 16, 2024
git add kargo.rb
fi

./update.sh kargo ${{ github.ref_name }} ${{ env.VERSION_SUFFIX }}
Copy link
Member

Choose a reason for hiding this comment

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

I have some concerns about this script. It seems a bit brittle and also isn't producing formulae that pass the default CI tests included in a new tap (which I added into that repo not long ago since they were absent).

There's been previous mention of using goreleaser to publish the CLI to our homebrew tap. I've played with it a bit and it works extremely well. I am highly in favor of pursuing that further.

The one catch is that the free/open source version of goreleaser cannot publish versioned formulae, but a pro subscription isn't very expensive and I will sort out how we can get one.

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.

Publish the kargo package to brew
3 participants