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 path_symlink only works when dir mounted to / #2229

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

yagehu
Copy link
Contributor

@yagehu yagehu commented Jun 5, 2024

This commit fixes path_symlink not working when -mount is not supplied with a mapped directory.

fixes #2228

This commit fixes `path_symlink` not working when `-mount` is not
supplied with a mapped directory.

fixes tetratelabs#2228

Signed-off-by: Yage Hu <me@huyage.dev>
@yagehu yagehu requested a review from mathetake as a code owner June 5, 2024 05:42
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

Please add the test case that ensures the behavior and previously failed.

Signed-off-by: Yage Hu <me@huyage.dev>
@@ -4941,7 +4973,7 @@ func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte,
if readOnly {
fsConfig = fsConfig.WithReadOnlyDirMount(tmpDir, "/")
} else {
fsConfig = fsConfig.WithDirMount(tmpDir, "/")
fsConfig = fsConfig.WithDirMount(tmpDir, "preopen")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary to expose the problem because the original path join would discard the /, masking the bug. Preopening a directory and mounting it at a non-root location would cause the bug.

Signed-off-by: Yage Hu <me@huyage.dev>
@mathetake mathetake requested review from evacchi and ncruces June 5, 2024 16:48
Signed-off-by: Yage Hu <me@huyage.dev>
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

looks good to me, waiting for others to review!

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Thanks!

@mathetake mathetake merged commit 0c5d971 into tetratelabs:main Jun 6, 2024
64 checks passed
@yagehu yagehu deleted the path-symlink branch June 17, 2024 05:55
@yagehu yagehu restored the path-symlink branch June 17, 2024 05:55
@yagehu yagehu deleted the path-symlink branch June 17, 2024 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

path_symlink doesn't work
3 participants