Skip to content

Support uid in WithAdditionalGIDs.#2641

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:support-uid-in-additional-group
Sep 13, 2018
Merged

Support uid in WithAdditionalGIDs.#2641
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:support-uid-in-additional-group

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

I can do another temporary mount to convert uid to username, but I think it is better to just do it in WithAdditionalGIDs reusing the same temporary mount.

Signed-off-by: Lantao Liu lantaol@google.com

Random-Liu added a commit to Random-Liu/cri-containerd that referenced this pull request Sep 13, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
Random-Liu added a commit to Random-Liu/cri-containerd that referenced this pull request Sep 13, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
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

Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu Random-Liu force-pushed the support-uid-in-additional-group branch from 35613da to 178db32 Compare September 13, 2018 17:51
return u.Uid == uid
})
if err != nil {
if os.IsNotExist(err) || err == errNoUsersFound {
Copy link
Copy Markdown
Member Author

@Random-Liu Random-Liu Sep 13, 2018

Choose a reason for hiding this comment

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

We should ignore not found error here, because it is normal that a built from scratch image doesn't have /etc/passwd.

return false
})
if err != nil {
if os.IsNotExist(err) {
Copy link
Copy Markdown
Member Author

@Random-Liu Random-Liu Sep 13, 2018

Choose a reason for hiding this comment

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

We should ignore not found error here, because it is possible that a built from scratch image has /etc/passwd (thus username is valid), but doesn't have /etc/group.

Random-Liu added a commit to Random-Liu/cri-containerd that referenced this pull request Sep 13, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
@estesp
Copy link
Copy Markdown
Member

estesp commented Sep 13, 2018

Thanks @Random-Liu; argh--should have caught that in my implementation. I remember looking over that and only thinking about /etc/passwd and then discarding because I wasn't doing string <--> id lookups.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 1950f79 into containerd:master Sep 13, 2018
@Random-Liu Random-Liu deleted the support-uid-in-additional-group branch September 13, 2018 22:51
Random-Liu added a commit to Random-Liu/containerd that referenced this pull request Sep 13, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
dmcgowan referenced this pull request Sep 14, 2018
[release/1.1] Backport: Support uid in WithAdditionalGIDs
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.

3 participants