Skip to content

Support following absolute symlinks for node analysis #1014

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Nov 21, 2022
Merged

Support following absolute symlinks for node analysis #1014

merged 6 commits into from
Nov 21, 2022

Conversation

RTann
Copy link
Collaborator

@RTann RTann commented Nov 15, 2022

No description provided.

@ghost
Copy link

ghost commented Nov 15, 2022 •
edited by ghost
Loading

Images are ready for the commit at bb00e04.

To use the images, use the tag 2.27.x-9-gbb00e04ba0.

msugakov
msugakov approved these changes Nov 16, 2022
View reviewed changes
Copy link
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

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

Looks legit to me

@vikin91
Copy link
Contributor

vikin91 commented Nov 16, 2022

I will try this on Matthias'es feat branch: stackrox/stackrox#3722

@vikin91
Copy link
Contributor

vikin91 commented Nov 16, 2022

I managed to test it only locally on Colima, here it the failure obtained using the code from stackrox/stackrox#3722 (Scanner 2.26.x-32-g3bd1d35930)

stackrox/stackrox#3722 (comment)

@RTann @msugakov
Update pkg/fsutil/fileinfo/util.go
f38c1f4 
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
@openshift-ci
Copy link

openshift-ci bot commented Nov 16, 2022

@RTann: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests f38c1f4 link false /test e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@RTann RTann requested review from Maddosaurus and vikin91 November 21, 2022 14:58
vikin91
vikin91 approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@vikin91 vikin91 left a comment

Choose a reason for hiding this comment

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

LGTM assuming that we handle the panic when exploring the filesystem on Docker in a separate PR.

@RTann
Copy link
Collaborator Author

RTann commented Nov 21, 2022

LGTM assuming that we handle the panic when exploring the filesystem on Docker in a separate PR.

I'm going to merge because I think they are somewhat unrelated and can be handled in separate PRs. Plus, this is not used in prod yet

@RTann RTann merged commit db92c5c into master Nov 21, 2022
@RTann RTann deleted the ross/node-analyze-symlink branch November 21, 2022 15:27
@msugakov msugakov mentioned this pull request Dec 19, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikin91 vikin91 vikin91 approved these changes

@msugakov msugakov msugakov approved these changes

@Maddosaurus Maddosaurus Awaiting requested review from Maddosaurus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RTann @vikin91 @msugakov