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

v0.14.1 dependencies updates (security, ...) and an urfave quick fix #3067

Merged
merged 4 commits into from May 8, 2023

Conversation

rafaeldtinoco
Copy link
Contributor

  • tracee 0.14.0 should have updated packages but didn't because of an urfave issue. this is solved by this PR and the dependencies should be updated (security issues, etc).

@@ -49,6 +49,7 @@ func main() {

return runner.Run(ctx)
},
SliceFlagSeparator: " ",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

needed to make the urfave to parse cmdline strings the same way it did

Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I understand, the default separator for string slice is comma - so why don't we use it as is? We can simply remove the parsing we do for comma seperation ourseleves, can't we?
Also, from what I understand, we can simply disable the separation using
urfave/cli#1588 - maybe we should use that instead of a space separator?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not against to remove our own parsing in the future since we have both urfave and cobra parsers which can achieve that, but for this issue I consider SliceFlagSeparator enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand, the default separator for string slice is comma - so why don't we use it as is? We can simply remove the parsing we do for comma seperation ourseleves, can't we?

we should do that after dropping urfave. For now we should use comma for both (after latest fix for cobra) and make sure command line is good, then we can change the parsing behavior (IMO).

Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -49,6 +49,7 @@ func main() {

return runner.Run(ctx)
},
SliceFlagSeparator: " ",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not against to remove our own parsing in the future since we have both urfave and cobra parsers which can achieve that, but for this issue I consider SliceFlagSeparator enough.

Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

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

LGTM

@rafaeldtinoco rafaeldtinoco merged commit f7c0bee into aquasecurity:main May 8, 2023
23 checks passed
@rafaeldtinoco rafaeldtinoco deleted the dependencies branch May 8, 2023 14:05
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.

golang updates: updating dependencies to latest brakes --filter event=aaa,bbb (comma usage)
3 participants