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

Package manager support: homebrew #1014

Closed
craigbox opened this issue Jan 9, 2023 · 14 comments
Closed

Package manager support: homebrew #1014

craigbox opened this issue Jan 9, 2023 · 14 comments
Assignees
Labels
Install Kubescape install

Comments

@craigbox
Copy link
Contributor

craigbox commented Jan 9, 2023

Homebrew support was disabled last August due to a fixed git2go dependency, requiring an older version of libgit2 than is available in Homebrew.

(We tried to fix this, but they weren't keen on our approach, and it wasn't returned to after libgit2 1.5 came out.)

If #1001 lands, then we should be able to trivially fix the brew support.

@dwertent
Copy link
Contributor

dwertent commented Jan 9, 2023

We first tried working with go-git but it took an average of 2 seconds to extract the commit information from each file. This led us to import the git2lib instead.
For this reason, we will not merge #1001.

Thank being said, we can build without importing git2lib as introduced in #969. This means the last commit information (e.g. Auther, committer, hash, etc.) will not be available.
What do you think?

@craigbox
Copy link
Contributor Author

If we're sticking with git2go, then we could just move to the latest version (with libgit 1.5.0) and we will be back to useful again!

@dwertent
Copy link
Contributor

/cc @amirmalka Please take a look into this

@dwertent dwertent added the Install Kubescape install label Jan 10, 2023
@amirmalka
Copy link
Contributor

my 2 cents -

Removing the git2go (go wrapper for libgit2) dependency would make a lot of things easier. The homebrew core project only works with upstream versions (Homebrew/homebrew-core#106523) and the git2go is not well maintained and sometimes a bit behind libgit2 upstream.

Why did we use git2go instead of go-git? Because of performance. As part of scanning a repository, we get the latest commit information per file and this functionality took a lot of time using go-git (more than a few seconds per file), compared to git2go which was instant (similar to running git log <file>).
There are several issues opened on this performance issue:

The impact to our build process is huge with this library, both in time and tools dependencies needed to be installed on the build machine. One of the main reasons we use a go-git / git2go is to make minimal assumptions about the environment of the end user - not having git installed. However, we trade it for the headache of using these libraries - specifically git2go. It makes build process much "heavier", it is only relevant for a specific functionality of Kubescape.

I assume that running the git log shell command per file (+parsing output) would be the best approach (see comparison made here: go-git/go-git#67 (comment) )

Given the above, in my opinion Kubescape should be changed as follows:

  1. Remove git2go dependecy
  2. Repo scanning logic should be updated such that - if git is installed on the end user's machine, we use it to get commit information per file. Otherwise, we just skip it and present a warning during repo scanning (or we can decide it is required and exit with error).

I believe that except for out-of-the-box Windows and Linux alpine distributions git is either part of the OS or that the user had already have it installed.

@HollowMan6
Copy link
Contributor

HollowMan6 commented Mar 13, 2023

Hi! Just want to learn more about the latest situation of this issue. Looks like it's buildable now: Homebrew/homebrew-core#107812 (comment) Everything should be OK: Homebrew/homebrew-core#125509

So I'm wondering if this issue has already been resolved or not? Is there anything that I still need to do?

@craigbox
Copy link
Contributor Author

@amirmalka and @matthyx can fill in the latest. I think it boils down to "we build kubescape with and without a C git library, and if we build it with, then we couldn't support Homebrew".

@chenrui333 has done a bunch of work to get this up to date (thank you!). It would be great to document how it works now, and perhaps confirm if it's possible to support the version that builds with the better library?

@HollowMan6
Copy link
Contributor

I just checked and it looks like the kubescape now works with homebrew by disabling the C git scan. I don't know if this is causing any trouble or not, is there any specific things that I can do to help with this issue?

Screenshot from 2023-03-29 20-37-25

@craigbox
Copy link
Contributor Author

I wondered if a library update in homebrew meant we could now depend on/build with the C git library.

If not, or not feasible, we can document the existing integration and close the issue.

@HollowMan6
Copy link
Contributor

I don't think it's now feasible to enable git support in homebrew official repo. However, I noticed that the homebrew-tap one is actually supporting the git build. If user wants to have the kubescape with git enabled, they can use that homebrew-tap to have it. So maybe we can close this issue now.

In addition, I have reopened the PR for the workflow that enables to auto bump the kubescape version: kubescape/homebrew-tap#7 Once this get merged, we can add a step for the main repo release workflow which will trigger this one to auto bump version, thus no human maintenance is needed.

@craigbox
Copy link
Contributor Author

SGTM. Please make sure that we have documentation for (a) the publishing process (b) the distinction on which version you can install through which method. That could be as simple as a wiki page entry or a Google doc, and we'll get it onto the right place. We can then close this issue!

@HollowMan6
Copy link
Contributor

HollowMan6 commented Apr 11, 2023

Wiki is available now:

I will continue documenting other methods of installation and publishing process in the following days.

@craigbox
Copy link
Contributor Author

@matthyx @amirmalka Could you please check you're happy with/understand the process here, and then we'll close the issue!

@amirmalka
Copy link
Contributor

amirmalka commented Apr 16, 2023

@HollowMan6
Overall looks good to me. A few comments:

  1. We could have RC versions of Kubescape for which we don't want to run the auto bump. The version tag regex should be more precise (https://github.com/kubescape/homebrew-tap/blob/main/.github/workflows/release.yml#L8)

  2. We have duplicate installation procedures - one in the wiki and one in the README (https://github.com/kubescape/kubescape/blob/master/docs/getting-started.md#install-on-macos). I suggest removing the part in the README and having a link to the wiki.

@HollowMan6
Copy link
Contributor

Thank you @amirmalka !

  1. Actually the auto bump gets triggered by workflow-dispatch instead of tags for the release workflow, but anyway I have updated that.
  2. PR opened at Move building instructions to wiki, add more installation instructions #1196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Install Kubescape install
Projects
None yet
Development

No branches or pull requests

4 participants