-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Osquerybeat: Strip osqueryd binary for linux #38952
Osquerybeat: Strip osqueryd binary for linux #38952
Conversation
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
This pull request doesn't have a |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ask that this be changed to support agentbeat as well as osquerybeat. Agentbeat must also ship with osqueryd or the osquerybeat subcommand will not work.
See this PR for the changes to make it work with agentbeat - #38951
This pull request is now in conflicts. Could you fix it? 🙏
|
With agentbeat merged, lets get the conflicts resolved and then I can take a better look. |
Picked up the latest main, resolved conflicts, improved code slightly |
Adding the backport-v8.14 label, IMO not stripping osqueryd is a bug. |
Or a feature upstream ;-) |
x-pack/osquerybeat/magefile.go
Outdated
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this still here? The code has changed to not need this specifically in osquerybeat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a mistake on conflict resolution (context switching), the function is not used. Removed.
if err := stripLinuxOsqueryd(); err != nil { | ||
return err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work for agentbeat? Agentbeat doesn't import the magefile from osquerybeat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works because agent beat calls out to osquerybeat to build the extension
beats/x-pack/agentbeat/magefile.go
Line 112 in 5e16f25
return callForBeat("crossBuildExt", "osquerybeat") |
This path is also followed for stanalone osquerybeat crossbuild.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I forgot I did it that way. ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stripping the osqueryd binary goes this LGTM! but I am not familiar with AgentBeat and I can't guarantee that nothing erupts there but I trust @aleksmaus comment here
* Osquerybeat: Strip osqueryd binary for linux * Improve code comment, fix a typo * Address code review request to make it compatible with agentbeat build * Remove unused function (cherry picked from commit ddddf80)
Sweet!! |
PR elastic#38952 surfaced a bug with elastic#38366 where the parameters weren't enclosed in doublet quotes. https://buildkite.com/elastic/beats-xpack-osquerybeat/builds/1973#018f06dd-19fd-40e0-ba21-960d195f178e/131-316
* Osquerybeat: Strip osqueryd binary for linux * Improve code comment, fix a typo * Address code review request to make it compatible with agentbeat build * Remove unused function (cherry picked from commit ddddf80) Co-authored-by: Aleksandr Maus <[email protected]>
Proposed commit message
Strip osqueryd binary for linux that is unstripped in the official osquery tar.gz distro.
Size reduction
ARM: 273,418,504 -> 78,956,480 bytes
x86: 270,282,072 -> 86,097,240 bytes
Changed dev-tools code for linux crossbuilds.
Noticed that the image docker.elastic.co/beats-dev/golang-crossbuild:1.21.9-arm currently exists for both ARM and x86 archs and is being used randomly, which causes a problem for
strip
tool when arch of binaries are mismatched with the arch of the tool. This affects only linux builds.Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Ensure that everything builds as before. The size of
osqueryd
is 3x smaller than before on linux OS, currently is under 90M.Related issues
Screenshots
Before:
After: