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.

Sign up for GitHub

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

Jump to bottom

Include security options in the container created event #31557

Merged
merged 1 commit into from
Aug 30, 2016
Merged

Include security options in the container created event #31557

merged 1 commit into from
Aug 30, 2016

Conversation

timstclair
Copy link

@timstclair timstclair commented Aug 26, 2016 •
edited
Loading

New container creation events look like:

Created container with docker id /k8s_bar2.a4; Security:[seccomp=sub/subtest(md5:07c9bcb4db631f7ca191d6e0bca49f76)]

Created container with docker id /k8s_bar2.a4; Security:[seccomp=unconfined apparmor=foo-profile]

The goal is to provide enough information to confirm that the requseted security constraints were honored.

For #31284

/cc @dchen1107 @thockin @jfrazelle @pweil- @pmorie

Justification for v1.4:

  • Risk: low. This appends some additional information to a human readable message. A bug here would probably not break any functionality
  • Roll-back: I don't anticipate any more changes to this area of the code. No functionality depends on this change.
  • Cost of not including: Users don't get any (positive) confirmation that the AppArmor or Seccomp profile they requested were actually enabled.

This change is Reviewable

@timstclair timstclair added this to the v1.4 milestone Aug 26, 2016
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Aug 26, 2016
@dchen1107
Copy link
Member

LGTM

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2016
@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 27, 2016
@timstclair timstclair added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 29, 2016
@k8s-bot
Copy link

k8s-bot commented Aug 29, 2016

GCE e2e build/test passed for commit 785c83c.

@timstclair
Copy link
Author

@k8s-bot node e2e test this, issue: #31633

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Aug 30, 2016

GCE e2e build/test passed for commit 785c83c.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 17787eb into kubernetes:master Aug 30, 2016
@timstclair timstclair mentioned this pull request Aug 31, 2016
Open
19 tasks
@dchen1107 dchen1107 mentioned this pull request Sep 6, 2016
Closed
@smarterclayton
Copy link
Contributor

I'm kind of confused why I, as a user, would care about this? It shows up in events for every user in the system - is it supposed to be actionable? What is it telling me that I need to know? The referenced issue doesn't actually clearly articulate why this message is important enough that everyone needs to see it.

@smarterclayton
Copy link
Contributor

As a clarification - events are primarily to help users. I know this helps some users... it just doesn't help many of them. Maybe 2. Or 3. Can we move this somewhere more relevant, like very deeply buried in status where only someone who cares can see it?

@smarterclayton
Copy link
Contributor

Or as an event annotation?

@timstclair
Copy link
Author

Or as an event annotation?

I like that suggestion. Eventually this should be moved out to the monitoring pipeline anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dchen1107 dchen1107

Labels
area/security lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
7 participants
@timstclair @dchen1107 @k8s-bot @k8s-github-robot @smarterclayton @googlebot @tallclair