-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add CLI flags for Swarm Service seccomp, AppArmor, and no-new-privileges #5698
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5698 +/- ##
==========================================
+ Coverage 59.51% 59.59% +0.07%
==========================================
Files 346 346
Lines 29379 29451 +72
==========================================
+ Hits 17486 17551 +65
- Misses 10923 10928 +5
- Partials 970 972 +2 |
cli/command/service/opts_test.go
Outdated
const testJson = `{ | ||
"json": "you betcha" | ||
} | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the linter complains on this one;
68.89 cli/command/service/opts_test.go:376:7: ST1003: const testJson should be testJSON (stylecheck)
68.89 const testJson = `{
68.89 ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this const is only used in the test below, so perhaps consider defining the const inside that test. Give that it's a very small JSON, perhaps put it on one line?
const testJSON = `{"json": "you betcha"}`
Oh; I guess that also means changing the file to be the same , well 😂
7ce478b
to
c1cec5b
Compare
Adds CLI flags for setting some security options on services: * --seccomp to set seccomp mode or custom profile * --apparmor to default or disable apparmor * --no-new-privileges, same as with containers Signed-off-by: Drew Erny <derny@mirantis.com>
DO NOT MERGE. Also I'll probably forget to redo this commit message even after this is merge ready but still DO NOT MERGE until I fix it. Adds seccomp, apparmor, and no-new-privileges flags to docker compose for docker stack command Signed-off-by: Drew Erny <derny@mirantis.com>
c1cec5b
to
b10de33
Compare
What is done:
What needs to be done still:
What I have up now is ready for review, even in its incomplete state, but not ready for merging. |
- What I did
Add 3 flags to the
docker service create
anddocker service update
CLI commands to support the security options in moby/moby#46386.--apparmor
allows setting AppArmor todefault
ordisabled
.--no-new-privileges
does what it says on the tin--seccomp
allows eitherdefault
,unconfined
, or a file name of a JSON file with a custom seccomp profile.- How I did it
Added CLI flags in the standard way. Mostly boilerplate.
- How to verify it
Added tests for the flags.
- Description for the changelog