Label support #1862

justincormack · 2017-05-19T15:05:53Z

Add support for using labels to specify the image configuration yaml, as done in moby/tool#39 - use org.mobyproject.config as the label with a JSON version of the yaml image config.

First example I have done sysctl, which I also converted to modern style build, and added a test to make sure it was still working correctly. I also added the test-sysctl one and poweroff to clean up the tests.

Once we do this elsewhere this will make the config files much simpler for the linuxkit images; external images will not have the correct labels.

This means Go code can use the same base image, which now includes Go tooling. Signed-off-by: Justin Cormack <[email protected]>

Signed-off-by: Justin Cormack <[email protected]>

avsm · 2017-05-19T15:54:57Z

From a Slack convo:

@avsm: does the old style still work? it was kind of nice to be able to grep for admin caps
@avsm: a CI could still add them (along with hashes) for a fully elaborated yaml file
@justincormack: yes putting config in the yaml overrides the label
@justincormack: you can also see the generated config if you use -v on build

justincormack · 2017-05-19T18:06:44Z

cc @dave-tucker I changed the default command for poweroff to have a timeout of 3 but left overrides for the ones that were 10.

dave-tucker

One nit otherwise LGTM

dave-tucker · 2017-05-19T18:48:09Z

test/hack/test-ltp.yml

-    capabilities:
-      - CAP_SYS_BOOT
-    readonly: true
+    image: "linuxkit/poweroff:5673236900c6beae35bbceec90ded4b9add3f5ae"


Could you presvere the old behaviour by adding a 30 second delay here?

hmm, but actually I didnt change the default, I should change back the other ones that were 3

oh yes I did but I shouldnt have, going to revert. It makes little sense having command in yaml as it is already in image

justincormack · 2017-05-19T19:01:16Z

@dave-tucker they should be unchanged now

rn

A few comments, but great to see config in labels and go-compile being phased out

rn · 2017-05-19T20:23:38Z

pkg/sysctl/Dockerfile

 CMD ["/usr/bin/sysctl"]
+LABEL org.mobyproject.config='{"net": "host","pid": "host", "ipc": "host", "readonly": true, "capabilities": ["CAP_SYS_ADMIN"]}'


would it be possible to make this multi-line for better readability?

The advantage of this format is it is the only way to write something you can cut and paste as JSON, as it needs backslashes if it has line break. Hence the single quotes so the double ones don't need to be escaped.

OK, makes sense. escaping quotes makes this error prone and less readable

rn · 2017-05-19T20:26:26Z

test/cases/040_packages/group.sh

@@ -0,0 +1,37 @@
+#!/bin/sh
+# SUMMARY: LinuxKit security tests


not security tests

rn · 2017-05-19T20:28:40Z

test/pkg/sysctl/Makefile

+
+default: push
+
+hash: Dockerfile check.sh


Is there a reason this package makefile is old-style rather than the new more compact form with git tree hash etc

all the test ones need redoing, was easier to leave this old style for now

Ok, agree best to re-do all the test ones new style in one PR

Signed-off-by: Justin Cormack <[email protected]>

And remove all the config options as they are now in the label. Signed-off-by: Justin Cormack <[email protected]>

Signed-off-by: Justin Cormack <[email protected]>

rn · 2017-05-19T21:13:29Z

are you planning to convert the other packages using linuxkit/go-compile as well? Otherwise it's introducing more inconsistency while we should strive for more consistency...

fixed so unchanged now

justincormack · 2017-05-19T21:14:26Z

@rneugeba yes, but didnt want to do anything else in the same PR its got enough in...

dave-tucker

LGTM! 🎸

justincormack added 2 commits May 19, 2017 14:22

Add go-compile script to linuxkit/alpine

78728c2

This means Go code can use the same base image, which now includes Go tooling. Signed-off-by: Justin Cormack <[email protected]>

Convert sysctl to using linuxkit/alpine and nested build

e12b5a3

Signed-off-by: Justin Cormack <[email protected]>

GordonTheTurtle added the status/0-triage label May 19, 2017

justincormack force-pushed the label-support branch from f8f316d to 4f6eddd Compare May 19, 2017 15:23

justincormack changed the title ~~[WIP] Label support~~ Label support May 19, 2017

justincormack force-pushed the label-support branch from cc86dd3 to 0530481 Compare May 19, 2017 18:06

dave-tucker previously requested changes May 19, 2017

View reviewed changes

justincormack force-pushed the label-support branch from 0530481 to 13f309a Compare May 19, 2017 18:59

rn reviewed May 19, 2017

View reviewed changes

justincormack added 6 commits May 19, 2017 22:05

Add a test case for the sysctl package

345a294

Signed-off-by: Justin Cormack <[email protected]>

Add label to sysctl image

4b30328

Signed-off-by: Justin Cormack <[email protected]>

Update moby tool to support config in labels

07a8ceb

Signed-off-by: Justin Cormack <[email protected]>

Update git hashes for sysctl

702ad5d

And remove all the config options as they are now in the label. Signed-off-by: Justin Cormack <[email protected]>

Use labels for the test-sysctl image

4d4598e

Signed-off-by: Justin Cormack <[email protected]>

Add label for poweroff container to simplify tests

56121a7

Signed-off-by: Justin Cormack <[email protected]>

justincormack force-pushed the label-support branch from 13f309a to 56121a7 Compare May 19, 2017 21:09

dave-tucker approved these changes May 19, 2017

View reviewed changes

justincormack merged commit a81a486 into linuxkit:master May 20, 2017

justincormack deleted the label-support branch May 20, 2017 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Label support #1862

Label support #1862

justincormack commented May 19, 2017 •

edited

Loading

avsm commented May 19, 2017

justincormack commented May 19, 2017

dave-tucker left a comment

dave-tucker May 19, 2017

justincormack May 19, 2017

justincormack May 19, 2017

justincormack commented May 19, 2017

rn left a comment

rn May 19, 2017

justincormack May 19, 2017

rn May 19, 2017

rn May 19, 2017

justincormack May 19, 2017

rn May 19, 2017

justincormack May 19, 2017

rn May 19, 2017

rn commented May 19, 2017

justincormack commented May 19, 2017

dave-tucker left a comment

		CMD ["/usr/bin/sysctl"]
		LABEL org.mobyproject.config='{"net": "host","pid": "host", "ipc": "host", "readonly": true, "capabilities": ["CAP_SYS_ADMIN"]}'

Label support #1862

Label support #1862

Conversation

justincormack commented May 19, 2017 • edited Loading

avsm commented May 19, 2017

justincormack commented May 19, 2017

dave-tucker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justincormack commented May 19, 2017

rn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rn commented May 19, 2017

justincormack commented May 19, 2017

dave-tucker left a comment

Choose a reason for hiding this comment

justincormack commented May 19, 2017 •

edited

Loading