From 2e79df04d2b5bdc4f36e38f69f0bf02fd21f58d7 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Tue, 17 Oct 2017 15:21:03 +0200 Subject: [PATCH] SecurityContextConstraints: avoid unnecessary mutation of container capabilities. --- .../capabilities/mustrunas.go | 18 ++++---- .../capabilities/mustrunas_test.go | 42 +++++++++++++++---- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/pkg/security/securitycontextconstraints/capabilities/mustrunas.go b/pkg/security/securitycontextconstraints/capabilities/mustrunas.go index 075a4117be5c..6948dc30954b 100644 --- a/pkg/security/securitycontextconstraints/capabilities/mustrunas.go +++ b/pkg/security/securitycontextconstraints/capabilities/mustrunas.go @@ -33,13 +33,17 @@ func NewDefaultCapabilities(defaultAddCapabilities, requiredDropCapabilities, al // 1. a capabilities.Add set containing all the required adds (unless the // container specifically is dropping the cap) and container requested adds // 2. a capabilities.Drop set containing all the required drops and container requested drops +// +// Returns the original container capabilities if no changes are required. func (s *defaultCapabilities) Generate(pod *api.Pod, container *api.Container) (*api.Capabilities, error) { defaultAdd := makeCapSet(s.defaultAddCapabilities) requiredDrop := makeCapSet(s.requiredDropCapabilities) containerAdd := sets.NewString() containerDrop := sets.NewString() + var containerCapabilities *api.Capabilities if container.SecurityContext != nil && container.SecurityContext.Capabilities != nil { + containerCapabilities = container.SecurityContext.Capabilities containerAdd = makeCapSet(container.SecurityContext.Capabilities.Add) containerDrop = makeCapSet(container.SecurityContext.Capabilities.Drop) } @@ -47,17 +51,17 @@ func (s *defaultCapabilities) Generate(pod *api.Pod, container *api.Container) ( // remove any default adds that the container is specifically dropping defaultAdd = defaultAdd.Difference(containerDrop) - combinedAdd := defaultAdd.Union(containerAdd).List() - combinedDrop := requiredDrop.Union(containerDrop).List() + combinedAdd := defaultAdd.Union(containerAdd) + combinedDrop := requiredDrop.Union(containerDrop) - // nothing generated? return nil - if len(combinedAdd) == 0 && len(combinedDrop) == 0 { - return nil, nil + // no changes? return the original capabilities + if (len(combinedAdd) == len(containerAdd)) && (len(combinedDrop) == len(containerDrop)) { + return containerCapabilities, nil } return &api.Capabilities{ - Add: capabilityFromStringSlice(combinedAdd), - Drop: capabilityFromStringSlice(combinedDrop), + Add: capabilityFromStringSlice(combinedAdd.List()), + Drop: capabilityFromStringSlice(combinedDrop.List()), }, nil } diff --git a/pkg/security/securitycontextconstraints/capabilities/mustrunas_test.go b/pkg/security/securitycontextconstraints/capabilities/mustrunas_test.go index 5521228b04ec..47984bb50fe7 100644 --- a/pkg/security/securitycontextconstraints/capabilities/mustrunas_test.go +++ b/pkg/security/securitycontextconstraints/capabilities/mustrunas_test.go @@ -19,6 +19,10 @@ func TestGenerateAdds(t *testing.T) { "no required, no container requests": { expectedCaps: nil, }, + "no required, no container requests, non-nil": { + containerCaps: &api.Capabilities{}, + expectedCaps: &api.Capabilities{}, + }, "required, no container requests": { defaultAddCaps: []api.Capability{"foo"}, expectedCaps: &api.Capabilities{ @@ -52,13 +56,22 @@ func TestGenerateAdds(t *testing.T) { Add: []api.Capability{"bar", "foo"}, }, }, + "generation does not mutate unnecessarily": { + defaultAddCaps: []api.Capability{"foo", "bar"}, + containerCaps: &api.Capabilities{ + Add: []api.Capability{"foo", "foo", "bar", "baz"}, + }, + expectedCaps: &api.Capabilities{ + Add: []api.Capability{"foo", "foo", "bar", "baz"}, + }, + }, "generation dedupes": { - defaultAddCaps: []api.Capability{"foo", "foo", "foo", "foo"}, + defaultAddCaps: []api.Capability{"foo", "bar"}, containerCaps: &api.Capabilities{ - Add: []api.Capability{"foo", "foo", "foo"}, + Add: []api.Capability{"foo", "baz"}, }, expectedCaps: &api.Capabilities{ - Add: []api.Capability{"foo"}, + Add: []api.Capability{"bar", "baz", "foo"}, }, }, "generation is case sensitive - will not dedupe": { @@ -109,6 +122,10 @@ func TestGenerateDrops(t *testing.T) { "no required, no container requests": { expectedCaps: nil, }, + "no required, no container requests, non-nil": { + containerCaps: &api.Capabilities{}, + expectedCaps: &api.Capabilities{}, + }, "required drops are defaulted": { requiredDropCaps: []api.Capability{"foo"}, expectedCaps: &api.Capabilities{ @@ -116,12 +133,21 @@ func TestGenerateDrops(t *testing.T) { }, }, "required drops are defaulted when making container requests": { - requiredDropCaps: []api.Capability{"foo"}, + requiredDropCaps: []api.Capability{"baz"}, containerCaps: &api.Capabilities{ Drop: []api.Capability{"foo", "bar"}, }, expectedCaps: &api.Capabilities{ - Drop: []api.Capability{"bar", "foo"}, + Drop: []api.Capability{"bar", "baz", "foo"}, + }, + }, + "required drops do not mutate unnecessarily": { + requiredDropCaps: []api.Capability{"baz"}, + containerCaps: &api.Capabilities{ + Drop: []api.Capability{"foo", "bar", "baz"}, + }, + expectedCaps: &api.Capabilities{ + Drop: []api.Capability{"foo", "bar", "baz"}, }, }, "can drop a required add": { @@ -155,12 +181,12 @@ func TestGenerateDrops(t *testing.T) { }, }, "generation dedupes": { - requiredDropCaps: []api.Capability{"bar", "bar", "bar", "bar"}, + requiredDropCaps: []api.Capability{"baz", "foo"}, containerCaps: &api.Capabilities{ - Drop: []api.Capability{"bar", "bar", "bar"}, + Drop: []api.Capability{"bar", "foo"}, }, expectedCaps: &api.Capabilities{ - Drop: []api.Capability{"bar"}, + Drop: []api.Capability{"bar", "baz", "foo"}, }, }, "generation is case sensitive - will not dedupe": {