Rework label source concept #905

tgraf · 2017-06-05T01:30:31Z

Summary:

Consolidates pkg/k8s and pkg/k8s/types
Moves label source definitions into pkg/labels
Renames cilium: label source to container:
Introduces concept of a SelectLabel which defaults to any:
Introduces concept of a unspec: label source for labels with unspecified source
Add documentation around label concept and selection

Signed-off-by: Thomas Graf <thomas@cilium.io>

It's generally a good idea to keep all valid label source names in a central place to avoid conflicts. This also helps to avoids cyclic imports. Signed-off-by: Thomas Graf <thomas@cilium.io>

- Instead of LabelSourceCilium, use LabelSourceContainer for labels which have been derived from the container runtime and LabelSourceUnspec if no source has been specified - Remove LabelSourceCilium and keep LabelSourceReserved as the identifier for reserved label keys. - Add new *SelectLabel*() functions which are used to create labels used to select other labels. The source defaults to any for these labels. Signed-off-by: Thomas Graf <thomas@cilium.io>

Signed-off-by: Thomas Graf <thomas@cilium.io>

aanm · 2017-06-05T15:38:56Z

pkg/labels/labels_test.go

@@ -179,7 +207,7 @@ func (s *LabelsSuite) TestLabelSliceSHA256Sum(c *C) {
 	}
 	sha256sum, err := LabelSliceSHA256Sum(labels)
 	c.Assert(err, IsNil)
-	c.Assert(sha256sum, DeepEquals, "180d22655dec0e78f62c3e4ac17f5994baedbdfc5c882cbbd668e2628c44f320")
+	c.Assert(sha256sum, DeepEquals, "330d57790ccf8cd0b10000d6297baa5fa27b9d6e1cb228055b45dc6e440ea4da")


Because of this I'm not sure if we can release this in 0.9. This will create conflicting SecLabelIDs across cluster for different cilium version.

Just noticed the milestone is 0.10

Why do you think this is a problem? Are you worried about users running 0.8 and 0.9 in parallel?

Yes, upgrading one node at a time.

Or even stopping cilium and restarting it with a new version.

It will assign a new security identity to the pod and will evaluate a new policy which will have the same effect. I don't think it breaks anything except that for a group of containers that previously used a single identity, they may use two identities for a while but with the same policy enforcement result.

Either way, let's merge it for 0.10 if you agree with the overall concept and we can think about a 0.9 backport separately.

aanm · 2017-06-05T15:39:54Z

pkg/labels/labels.go

-	// LabelSourceCilium is an internal Cilium label
-	LabelSourceCilium = "cilium"
+	// LabelSourceContainer is a label imported from the container runtime
+	LabelSourceContainer = "container"


Should we have a label for each container runtime?

aanm · 2017-06-05T15:42:13Z

Documentation/architecture.rst

+
+When using labels to select resources, both the key and the value must match,
+e.g. when a policy should be applied to all endpoints will label
+``my.corp.foo`` then the label ``my.corp.foo=value`` will not match the


I think it should be "my.corp.foo=bar then the label..."

Sure, I'm fine with that. Fixed

aanm · 2017-06-05T15:43:23Z

Documentation/architecture.rst

+container runtime is not aware of the orchestration system which may result in
+possibly conflicting key names.
+
+To resolve this potential conflict, Cilium attached a label source prefix to


s/attached/attaches

aanm · 2017-06-05T15:47:33Z

Documentation/architecture.rst

+label was derived from Kubernetes, it would not match if the label was derived
+from the container runtime. If the source of a label is to be ignored, the
+label can be prefixed with ``any:`` in which case all labels, regardless of the
+source will match as long as the key and value part match.


Where will the user would use any:? Can he start a container with --label any:foo=bar? Can he specify any: in the policy? Can he use the policy trace tool with the source any:?

There are lots of new concepts for a first time reader, I think providing a clear example what he should or should not do helps.

tgraf · 2017-06-05T21:32:18Z

@aanm I've improved the documentation about label source and matching on the source. Let me know if parts are still unclear.

aanm · 2017-06-05T22:16:37Z

Documentation/architecture.rst

+``k8s:role=frontend``, ``container:user=joe``, ``k8s:role=backend``. This means
+that when you run a Docker container using ``docker run [...] -l foo=bar``, the
+label ``container:foo=bar`` will appear on the Cilium endpoint representing the
+container. Similiarly, a Kubernetes pod started with the label ``foo: bar``


foo: bar -> foo=bar

I listed it as foo: bar because that is how it appears in the yaml.

ohh... I honestly for a moment thought this meant source: value

tgraf added 4 commits June 4, 2017 11:45

k8s: Move pkg/k8s/types into pkg/k8s

94c8356

Signed-off-by: Thomas Graf <thomas@cilium.io>

labels: Move label source definition into labels package

d16399a

It's generally a good idea to keep all valid label source names in a central place to avoid conflicts. This also helps to avoids cyclic imports. Signed-off-by: Thomas Graf <thomas@cilium.io>

Documentation: Provide documentation on labels

54f93a3

Signed-off-by: Thomas Graf <thomas@cilium.io>

tgraf added the kind/enhancement This would improve or streamline existing functionality. label Jun 5, 2017

tgraf added this to the 0.10 milestone Jun 5, 2017

tgraf force-pushed the rework-label-source branch 2 times, most recently from 212353b to bab943f Compare June 5, 2017 13:21

tgraf requested a review from aanm June 5, 2017 13:59

tgraf added the pending-review label Jun 5, 2017

aanm reviewed Jun 5, 2017

View reviewed changes

tgraf force-pushed the rework-label-source branch from bab943f to 54f93a3 Compare June 5, 2017 21:10

tgraf requested a review from aanm June 5, 2017 21:31

aanm requested changes Jun 5, 2017

View reviewed changes

aanm self-requested a review June 5, 2017 22:51

aanm approved these changes Jun 5, 2017

View reviewed changes

tgraf merged commit 0246820 into master Jun 5, 2017

tgraf deleted the rework-label-source branch June 5, 2017 23:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework label source concept #905

Rework label source concept #905

tgraf commented Jun 5, 2017

aanm Jun 5, 2017

aanm Jun 5, 2017

tgraf Jun 5, 2017

aanm Jun 5, 2017

aanm Jun 5, 2017

tgraf Jun 5, 2017

aanm Jun 5, 2017

aanm Jun 5, 2017

tgraf Jun 5, 2017

aanm Jun 5, 2017

tgraf Jun 5, 2017

aanm Jun 5, 2017

tgraf commented Jun 5, 2017

aanm Jun 5, 2017

tgraf Jun 5, 2017

aanm Jun 5, 2017

Rework label source concept #905

Rework label source concept #905

Conversation

tgraf commented Jun 5, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgraf commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment