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

feature: adjust pouchd unix socket premissions #1561

Merged
merged 1 commit into from
Jun 21, 2018
Merged

feature: adjust pouchd unix socket premissions #1561

merged 1 commit into from
Jun 21, 2018

Conversation

Ace-Tang
Copy link
Contributor

@Ace-Tang Ace-Tang commented Jun 21, 2018

adjust pouchd unix socket file mode, make it ownered by group
pouch.

Ⅰ. Describe what this PR did

Before this patch, we can only use pouch with user root, with this patch, we add this permission to group pouch. We create a unix sock with pouch group, and change it mode to 0660, make other group can write

$ ls -l /run/pouchd.sock 
srw-rw---- 1 root pouch 0 6月  21 12:36 /run/pouchd.sock

$ users
ace

$ pouch images
IMAGE ID       IMAGE NAME                                          SIZE
00f017a8c2a6   reg.docker.alibaba-inc.com/base/busybox:latest      686.62 KB
8ac48589692a   registry.hub.docker.com/library/busybox:1.28        710.83 KB
e38bc07ac18e   registry.hub.docker.com/library/hello-world:linux   5.26 KB

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

if err := syscall.Unlink(path); err != nil && !os.IsNotExist(err) {
return nil, err
}
mask := syscall.Umask(0777)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use oldMask to make it clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why it need to named oldMask?

Copy link
Contributor

Choose a reason for hiding this comment

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

if err != nil {
// ignore error when group pouch not exist, group pouch should to be
// created before pouchd started, it means code not create pouch group
logrus.Warnf("failed to found group pouch")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/found/find

adjust pouchd unix socket file mode, make it ownered by group
pouch.

Signed-off-by: Ace-Tang <aceapril@126.com>
@codecov-io
Copy link

Codecov Report

Merging #1561 into master will increase coverage by 23.72%.
The diff coverage is 58.97%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1561       +/-   ##
===========================================
+ Coverage   17.89%   41.61%   +23.72%     
===========================================
  Files         223      267       +44     
  Lines       14711    17362     +2651     
===========================================
+ Hits         2633     7226     +4593     
+ Misses      11906     9246     -2660     
- Partials      172      890      +718
Impacted Files Coverage Δ
apis/server/server.go 48.75% <53.57%> (+48.75%) ⬆️
pkg/user/user.go 63.33% <72.72%> (+47.77%) ⬆️
pkg/debug/debug.go 22.22% <0%> (ø)
registry/auth_challenge.go 62.5% <0%> (ø)
storage/quota/grpquota.go 0% <0%> (ø)
cri/stream/httpstream/httpstream.go 0% <0%> (ø)
storage/volume/driver/context.go 8.69% <0%> (ø)
storage/volume/modules/local/local.go 74.24% <0%> (ø)
storage/volume/core.go 56.58% <0%> (ø)
lxcfs/lxcfs.go 55.55% <0%> (ø)
... and 120 more

@fuweid
Copy link
Contributor

fuweid commented Jun 21, 2018

LGTM

Next action: separate getListener from server package into pkg package

@fuweid fuweid merged commit 6377fbf into AliyunContainerService:master Jun 21, 2018
@Ace-Tang
Copy link
Contributor Author

Oh, do you want to help this, @fuweid

@Ace-Tang Ace-Tang deleted the unix_sock branch June 21, 2018 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants