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

score/security: added optional, splitted probes as future replacemnt … #326

Merged
merged 2 commits into from
Nov 7, 2020

Conversation

markuslackner
Copy link

…for security context probe

Fixes #325

RELNOTE: container-security-context now deprecated and added optional probes container-security-context-user-group-id, container-security-context-privileged and container-security-context-readonlyrootfilesystem as replacement

@markuslackner markuslackner force-pushed the split_securitycontext branch 3 times, most recently from daa264e to 5021dea Compare November 4, 2020 22:17
@markuslackner
Copy link
Author

markuslackner commented Nov 4, 2020

@zegl Can you take a look?

Some remarks:

  • the user/group id probe and the privileged probe need an existing security context. if its missing two almost identical errors will be generated.
  • I duplicated most of the tests and activated the appropriate optional probe. It should be quite easy to delete the original securitycontext probe in a future release
  • I updated the documentation and added a new doc file for security context. Is this ok and is the content understandable?

Copy link
Owner

@zegl zegl left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! The changes looks good to me, but I'd like it to make the communication about this (future) breakage a bit clearer.

Comment on lines 17 to 18
In future releases the `container-security-context` will become *optional*
and replaced by the more detailed checks.
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like to commit to the following timeline, and think that it would be a good idea to mention it in this document.

  • v1.10: Introduce the three new checks.
  • v1.11: Make container-security-context optional, and make the three new checks run by default.
  • v1.12: Remove container-security-context.

Since this is going to be a breaking change, I'd like to be as clear as possible with what's going to happen, so that users get a chance to prepare.

Copy link
Author

Choose a reason for hiding this comment

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

I have updated the document

Copy link
Owner

@zegl zegl left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks a lot for your contribution.

bors merge

@bors
Copy link
Contributor

bors bot commented Nov 7, 2020

Build succeeded:

@bors bors bot merged commit a3ebfd1 into zegl:master Nov 7, 2020
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.

Split "Container Security Context"
2 participants