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

tetragon: create api go module for tooling #231

Merged
merged 2 commits into from
Jul 15, 2022
Merged

Conversation

jrfastab
Copy link
Contributor

Working on tooling and event handlers, such as hubble, its a bit ugly
to pull in the entire go mod when the user simply wants the API
definitions. Further, its useful to version the API so we can all
agree on what version is in use. Allowing us to fix core code easily
and keep the API side consistent from a versioning perspective.

To facilitate this create go module out of API code.

Working on tooling and event handlers, such as hubble, its a bit ugly
to pull in the entire go mod when the user simply wants the API
definitions. Further, its useful to version the API so we can all
agree on what version is in use. Allowing us to fix core code easily
and keep the API side consistent from a versioning perspective.

To facilitate this create go module out of API code.

Signed-off-by: John Fastabend <[email protected]>
@jrfastab jrfastab requested a review from a team as a code owner July 13, 2022 00:38
@jrfastab jrfastab requested review from tixxdz and kkourt July 13, 2022 00:38
@jrfastab jrfastab force-pushed the pr/jrfastab/gomod-api branch 3 times, most recently from 996f4ed to 3a88ca7 Compare July 14, 2022 03:19
@jrfastab jrfastab requested a review from willfindlay as a code owner July 14, 2022 17:06
The check module vendoring currently just reports an error, but does not
give any information about what the error is. This adds an echo to print
the status output so user can see what the problem is from the action
logs. This helps in the current case I'm dealing with where it doesn't
reproduce locally for some reason.

Signed-off-by: John Fastabend <[email protected]>
@jrfastab jrfastab force-pushed the pr/jrfastab/gomod-api branch from d4a1051 to ea90271 Compare July 14, 2022 17:21
Copy link
Contributor

@willfindlay willfindlay left a comment

Choose a reason for hiding this comment

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

LGTM. I just had one question and one suggestion.

Question: Will vendoring API inside of tetragon and tetragon inside of API ever create a dependency cycle in the future? I would suggest maybe having pkg/matchers be its own module or moving pkg/matchers into API to avoid this.

Suggestion: We should consider moving cmd/protoc-gen-go-tetragon into the API module to cleanly separate concerns between the two modules. It made sense to have it live in cmd/ when they were all one module but IMO not so much anymore.

I don't think either of the above points are worth blocking this on so I'll just ACK for now.

@jrfastab
Copy link
Contributor Author

the cycle doesn't block anything at the moment but does sound like it could be annoying to manage from a vendoring perspective. Anyways the pkg/matchers should just be pulled into api/ which makes it clean and then vendor is much nicer.

@jrfastab jrfastab merged commit 0c08c89 into main Jul 15, 2022
@jrfastab jrfastab deleted the pr/jrfastab/gomod-api branch July 15, 2022 02:22
@jrfastab jrfastab restored the pr/jrfastab/gomod-api branch July 15, 2022 02:22
@jrfastab jrfastab deleted the pr/jrfastab/gomod-api branch July 15, 2022 02:22
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