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

Fix /etc/ lossage in CreateRootfs #2746

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Conversation

rjkroege
Copy link
Contributor

With this PR, CreateRootfs() in root_linux.go uses the NoFollowSymlinks option to correctly handle broken symlinks found in /etc when setting up the filesystems.\n

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch has no changes to coverable lines.

Files Changed Coverage
pkg/libinit/root_linux.go 0.00%

📢 Thoughts on this report? Let us know!.

Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

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

Thanks!
Can you run gofmt, then I can approve this.

@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label Aug 24, 2023
@rjkroege rjkroege force-pushed the etc-lossage branch 2 times, most recently from 2dd120f to b9ec913 Compare August 28, 2023 10:45
@rjkroege
Copy link
Contributor Author

Rebased and hopefully fixed the issues.

With this PR, CreateRootfs() in root_linux.go uses the
NoFollowSymlinks option to correctly handle broken symlinks found in
/etc when setting up the filesystems.

Signed-off-by: Robert Kroeger <rjkroege@liqui.org>
@rjkroege
Copy link
Contributor Author

oops. Now fixed. I apparently should automate this "signed off" thing.

@rjkroege
Copy link
Contributor Author

@rminnich Can you take another look. Also: the failing build_and_test is significant?

@rminnich
Copy link
Member

rminnich commented Sep 4, 2023

it certainly looks fine, I wonder if your fix found a broken test?

@rjkroege
Copy link
Contributor Author

rjkroege commented Sep 5, 2023

@rminnich with your merge, all the tests pass except the in-the-noise change to coverage. LGTM maybe or should I go write some tests? 😊

@rminnich rminnich added the automerge Applying this label auto-merges the PR when ready label Sep 7, 2023
@rminnich rminnich merged commit d09f567 into u-root:main Sep 11, 2023
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants