-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add kolide_brew_upgradeable
table
#1634
Conversation
b10e044
to
9cf35d9
Compare
kolide_brew_upgradeable
table
kolide_brew_upgradeable
tablekolide_brew_upgradeable
table
return data, nil | ||
} | ||
|
||
func runAsUser(ctx context.Context, uid string, cmd *exec.Cmd) error { |
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.
We use this in enough places now that we should probably just make this available/reusable somewhere. I don't think it's necessary to address in this PR, but wanted to flag in case you (or James/Zack/seph) have a good suggestion for where we should move this in the future.
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.
Agreed. I feel fried this week, so I too didn't want to address it in this PR, but I might take a look at that next week if I can.
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.
Agree it feels like we should put it into a helper. We have some exec helpers. Which reminds me, this isn't using them. So whether we expand the exec helper, or make another 🤷
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.
LGTM!
@@ -20,6 +20,10 @@ func Bputil(ctx context.Context, arg ...string) (*exec.Cmd, error) { | |||
return validatedCommand(ctx, "/usr/bin/bputil", arg...) | |||
} | |||
|
|||
func Brew(ctx context.Context, arg ...string) (*exec.Cmd, error) { | |||
return validatedCommand(ctx, "/opt/homebrew/bin/brew", arg...) |
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 is only correct for m1 machines, not for the intel x86 ones
|
||
type Table struct { | ||
slogger *slog.Logger | ||
execCC allowedcmd.AllowedCommand |
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.
You don't need the execCC
thing. It should only get used when we want to override commands in the test suite. Otherwise, just skip it
return data, nil | ||
} | ||
|
||
func runAsUser(ctx context.Context, uid string, cmd *exec.Cmd) error { |
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.
Agree it feels like we should put it into a helper. We have some exec helpers. Which reminds me, this isn't using them. So whether we expand the exec helper, or make another 🤷
Add a Kolide table on both Linux and macOS for
homebrew
outdated packages. My initial concerns of being unable to run assudo
were correct ashomebrew
panics if an attempt is made to run it assudo
. This is by design inhomebrew
.