Skip to content

replace pkg/symlink with moby/sys/symlink#4677

Merged
AkihiroSuda merged 1 commit intocontainerd:masterfrom
thaJeztah:moby_sys_symlink
Nov 4, 2020
Merged

replace pkg/symlink with moby/sys/symlink#4677
AkihiroSuda merged 1 commit intocontainerd:masterfrom
thaJeztah:moby_sys_symlink

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Oct 29, 2020

follow-up to #4631

@thaJeztah
replace pkg/symlink with moby/sys/symlink
1146098 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 29, 2020

@dims @dmcgowan ptal

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Oct 29, 2020

Build succeeded.

estesp
estesp approved these changes Oct 29, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

Zyqsempai
Zyqsempai approved these changes Oct 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Zyqsempai Zyqsempai left a comment

Choose a reason for hiding this comment

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

LGTM

mikebrow
mikebrow approved these changes Oct 30, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM
wondering what the difference of opinion is between moby and golang https://golang.org/src/path/filepath/symlink_windows.go implementations

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 30, 2020

wondering what the difference of opinion is between moby and golang https://golang.org/src/path/filepath/symlink_windows.go implementations

If I recall correctly, this package was created to mitigated a "break out", where symlinks could be abused to traverse directories outside of the expected scope (moby/moby#3184). As to the platform-specific handling; I was actually eyeing that (didn't dig deep yet); and we should look at those, and see if they make sense to upstream (either golang stlib, or something like golang.org/x/sys/windows.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 30, 2020

@dmcgowan this LGTY?

@AkihiroSuda AkihiroSuda merged commit ef79c99 into containerd:master Nov 4, 2020
@thaJeztah thaJeztah deleted the moby_sys_symlink branch November 4, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dims dims dims approved these changes

@estesp estesp estesp approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@mikebrow mikebrow mikebrow approved these changes

+1 more reviewer

@Zyqsempai Zyqsempai Zyqsempai approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@thaJeztah @dims @estesp @Zyqsempai @AkihiroSuda @mikebrow