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

Show "seccomp" in docker info (#20909). #21172

Merged
merged 1 commit into from Apr 14, 2016

Conversation

yongtang
Copy link
Member

This pull request added a SecurityOptions field in the GET /info output to show if there is apparmor, seccomp, or selinux suport.

The API changes are updated in the documentation and the update in GET /info is covered by the test case in TestInfoApi.

This pull request fixes #20909.

Note: a pull request in https://github.com/docker/engine-api will be opened separately.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@icecrime icecrime added the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 14, 2016
@justincormack
Copy link
Contributor

Sorry haven't had a chance to review this yet, will do soon.

@calavera
Copy link
Contributor

Seccomp is a security option like AppArmor and SELinux. Neither of them have a specific output in info, why should we differ with that? I think it would be more useful to have a "SecurityOptions" section that shows whatever the user configured.

@calavera
Copy link
Contributor

Also, changes in engine-api need to go in https://github.com/docker/engine-api.

@justincormack
Copy link
Contributor

For diagnosing issues it is useful to know what is configured, we did discuss in #20909 how to display it. I think a line SecurityOptions: seccomp selinux would probably be the most useful. (Especially as libseccomp-golang is not intending to provide any version information it seems).

@yongtang
Copy link
Member Author

Thanks @calavera @justincormack for the feedback. Let me update the pull request accordingly.

@yongtang yongtang force-pushed the 20909-seccomp-in-docker-info branch from 21fa2d4 to 6f4585e Compare March 18, 2016 03:34
@yongtang yongtang force-pushed the 20909-seccomp-in-docker-info branch from 6f4585e to b453c2a Compare March 19, 2016 01:47
yongtang added a commit to yongtang/docker that referenced this pull request Mar 28, 2016
This PR updates vendored engine-api to e37a82dfcea64559ca6a581776253c01d83357d9
in order to support `SecurityOptions` in `Info`.

See moby#20909, moby#21172 for details related to `SecurityOptions`.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 20909-seccomp-in-docker-info branch 2 times, most recently from 5e638d5 to e8ebb43 Compare March 29, 2016 16:56
This pull request added a `SecurityOptions` field in the `GET /info`
output to show if there is `apparmor`, `seccomp`, or `selinux` suport.

The API changes are updated in the documentation and the update in
`GET /info` is covered by the test case in `TestInfoApi`.

This pull request fixes moby#20909.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 20909-seccomp-in-docker-info branch from e8ebb43 to 190654a Compare March 30, 2016 09:23
@yongtang
Copy link
Member Author

Hi @justincormack @calavera @thaJeztah the Jenkins CI finally passed all tests for this PR (SecurityOptions - seccomp/apparmor/selinux).

Let me know if there are any changes I need to make and any comments would be appreciated.

@vdemeester vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 30, 2016
securityOptions = append(securityOptions, "apparmor")
}
if sysInfo.Seccomp {
securityOptions = append(securityOptions, "seccomp")
Copy link
Member

Choose a reason for hiding this comment

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

seccomp and apparmor (?) can have a custom profile loaded. I don't see why this is useful w/o showing what's there. This output is just basically telling me that I have seccomp|apparmor|selinux on my system :| I find this superfluous

Copy link
Member

Choose a reason for hiding this comment

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

i.e. we have a bug report mentioning it's using seccomp but we don't have the profile (if it isn't the default)

Copy link
Contributor

Choose a reason for hiding this comment

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

The main aim is so when triaging issues we know what a user has installed, which is very hard to find now.

For seccomp the profile is per container run, so the daemon cannot print it. For apparmor it could be useful, but we dont want to always print it I dont think, it is large.

@thaJeztah
Copy link
Member

LGTM

@vdemeester
Copy link
Member

LGTM 🐮

@calavera
Copy link
Contributor

LGTM. Moving to docs review.

@thaJeztah
Copy link
Member

Docs LGTM

@vdemeester
Copy link
Member

Docs LGTM 🐯

@vdemeester vdemeester merged commit bc0c882 into moby:master Apr 14, 2016
@yongtang yongtang deleted the 20909-seccomp-in-docker-info branch April 14, 2016 23:30
yongtang added a commit to yongtang/docker that referenced this pull request Jun 14, 2016
The security infomation has already been added to `GET /info` in moby#21172.
However, it is not part of the output of `docker info` yet.

This fix adds the security information to `docker info`.

Additional tests has been added to cover changes.

This fix fixes moby#23500. This fix is related to moby#20909, moby#21172.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@thaJeztah
Copy link
Member

relates to #21172

tiborvass pushed a commit to tiborvass/docker that referenced this pull request Jun 17, 2016
The security infomation has already been added to `GET /info` in moby#21172.
However, it is not part of the output of `docker info` yet.

This fix adds the security information to `docker info`.

Additional tests has been added to cover changes.

This fix fixes moby#23500. This fix is related to moby#20909, moby#21172.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
(cherry picked from commit eee20b5)
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.

Show "seccomp" in docker info
8 participants