diff --git a/pkg/security/controller/namespace_scc_allocation_controller.go b/pkg/security/controller/namespace_scc_allocation_controller.go index dbb42fe6b..ca62a4151 100644 --- a/pkg/security/controller/namespace_scc_allocation_controller.go +++ b/pkg/security/controller/namespace_scc_allocation_controller.go @@ -6,19 +6,23 @@ import ( "reflect" "time" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/klog" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + runtimejson "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/wait" corev1informers "k8s.io/client-go/informers/core/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" + "k8s.io/klog" coreapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" @@ -46,6 +50,8 @@ type NamespaceSCCAllocationController struct { rangeAllocationClient securityv1client.RangeAllocationsGetter queue workqueue.RateLimitingInterface + + encoder runtime.Encoder } func NewNamespaceSCCAllocationController( @@ -55,6 +61,13 @@ func NewNamespaceSCCAllocationController( requiredUIDRange *uid.Range, mcs MCSAllocationFunc, ) *NamespaceSCCAllocationController { + + scheme := runtime.NewScheme() + utilruntime.Must(corev1.AddToScheme(scheme)) + codecs := serializer.NewCodecFactory(scheme) + jsonSerializer := runtimejson.NewSerializer(runtimejson.DefaultMetaFactory, scheme, scheme, false) + encoder := codecs.EncoderForVersion(jsonSerializer, corev1.SchemeGroupVersion) + c := &NamespaceSCCAllocationController{ requiredUIDRange: requiredUIDRange, mcsAllocator: mcs, @@ -63,6 +76,7 @@ func NewNamespaceSCCAllocationController( nsLister: namespaceInformer.Lister(), nsListerSynced: namespaceInformer.Informer().HasSynced, queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerName), + encoder: encoder, } namespaceInformer.Informer().AddEventHandlerWithResyncPeriod( @@ -178,8 +192,20 @@ func (c *NamespaceSCCAllocationController) allocate(ns *corev1.Namespace) error nsCopy.Annotations[securityv1.MCSAnnotation] = label.String() } } - - _, err = c.namespaceClient.Update(nsCopy) + nsCopyBytes, err := runtime.Encode(c.encoder, nsCopy) + if err != nil { + return err + } + nsBytes, err := runtime.Encode(c.encoder, ns) + if err != nil { + return err + } + patchBytes, err := strategicpatch.CreateTwoWayMergePatch(nsBytes, nsCopyBytes, &corev1.Namespace{}) + if err != nil { + return err + } + // use patch here not to conflict with other actors + _, err = c.namespaceClient.Patch(ns.Name, types.StrategicMergePatchType, patchBytes) if apierrors.IsNotFound(err) { return nil } diff --git a/pkg/security/controller/namespace_security_allocation_controller_test.go b/pkg/security/controller/namespace_security_allocation_controller_test.go index d196c2895..5f21fb7bb 100644 --- a/pkg/security/controller/namespace_security_allocation_controller_test.go +++ b/pkg/security/controller/namespace_security_allocation_controller_test.go @@ -1,6 +1,7 @@ package controller import ( + "encoding/json" "fmt" "math/big" "strings" @@ -8,15 +9,16 @@ import ( "github.com/davecgh/go-spew/spew" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/apitesting" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + runtimejson "k8s.io/apimachinery/pkg/runtime/serializer/json" kubefakeclient "k8s.io/client-go/kubernetes/fake" corev1listers "k8s.io/client-go/listers/core/v1" clientgotesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" securityv1 "github.com/openshift/api/security/v1" @@ -25,6 +27,10 @@ import ( "github.com/openshift/library-go/pkg/security/uid" ) +type patchData struct { + metav1.ObjectMeta `json:"metadata,omitempty"` +} + func TestController(t *testing.T) { kubeclient := kubefakeclient.NewSimpleClientset() securityclient := securityv1fakeclient.NewSimpleClientset() @@ -32,12 +38,18 @@ func TestController(t *testing.T) { uidr, _ := uid.NewRange(10, 20, 2) mcsr, _ := mcs.NewRange("s0:", 10, 2) + + scheme, codecs := apitesting.SchemeForOrDie(corev1.AddToScheme) + jsonSerializer := runtimejson.NewSerializer(runtimejson.DefaultMetaFactory, scheme, scheme, false) + encoder := codecs.EncoderForVersion(jsonSerializer, corev1.SchemeGroupVersion) + c := &NamespaceSCCAllocationController{ requiredUIDRange: uidr, mcsAllocator: DefaultMCSAllocation(uidr, mcsr, 5), namespaceClient: kubeclient.CoreV1().Namespaces(), nsLister: corev1listers.NewNamespaceLister(indexer), rangeAllocationClient: securityclient.SecurityV1(), + encoder: encoder, } err := c.Repair() if err != nil { @@ -55,7 +67,7 @@ func TestController(t *testing.T) { } securityclient.ClearActions() - err = c.allocate(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}) + err = c.allocate(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}) if err != nil { t.Fatal(err) } @@ -66,7 +78,11 @@ func TestController(t *testing.T) { } createNSAction := kubeActions[0] - got := createNSAction.(clientgotesting.CreateAction).GetObject().(*v1.Namespace) + data := createNSAction.(clientgotesting.PatchAction).GetPatch() + got := patchData{} + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("unexpected error parsing patch data: %v", err) + } if got.Annotations[securityv1.UIDRangeAnnotation] != "10/2" { t.Errorf("unexpected uid annotation: %#v", got) } @@ -99,7 +115,7 @@ func TestControllerError(t *testing.T) { actions int }{ "not found": { - err: func() error { return errors.NewNotFound(kapi.Resource("Namespace"), "test") }, + err: func() error { return errors.NewNotFound(corev1.Resource("Namespace"), "test") }, errFn: func(err error) bool { return err == nil }, actions: 1, }, @@ -112,9 +128,9 @@ func TestControllerError(t *testing.T) { actions: 1, reactFn: func(a clientgotesting.Action) (bool, runtime.Object, error) { if a.Matches("get", "namespaces") { - return true, &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}, nil + return true, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}, nil } - return true, (*v1.Namespace)(nil), errors.NewConflict(kapi.Resource("namespace"), "test", fmt.Errorf("test conflict")) + return true, (*corev1.Namespace)(nil), errors.NewConflict(corev1.Resource("namespace"), "test", fmt.Errorf("test conflict")) }, errFn: func(err error) bool { return err != nil && strings.Contains(err.Error(), "test conflict") @@ -126,7 +142,7 @@ func TestControllerError(t *testing.T) { t.Run(s, func(t *testing.T) { if testCase.reactFn == nil { testCase.reactFn = func(a clientgotesting.Action) (bool, runtime.Object, error) { - return true, (*v1.Namespace)(nil), testCase.err() + return true, (*corev1.Namespace)(nil), testCase.err() } } kubeclient := kubefakeclient.NewSimpleClientset() @@ -137,12 +153,18 @@ func TestControllerError(t *testing.T) { uidr, _ := uid.NewRange(10, 19, 2) mcsr, _ := mcs.NewRange("s0:", 10, 2) + + scheme, codecs := apitesting.SchemeForOrDie(corev1.AddToScheme) + jsonSerializer := runtimejson.NewSerializer(runtimejson.DefaultMetaFactory, scheme, scheme, false) + encoder := codecs.EncoderForVersion(jsonSerializer, corev1.SchemeGroupVersion) + c := &NamespaceSCCAllocationController{ requiredUIDRange: uidr, mcsAllocator: DefaultMCSAllocation(uidr, mcsr, 5), namespaceClient: kubeclient.CoreV1().Namespaces(), nsLister: corev1listers.NewNamespaceLister(indexer), rangeAllocationClient: securityclient.SecurityV1(), + encoder: encoder, } err := c.Repair() @@ -151,7 +173,7 @@ func TestControllerError(t *testing.T) { } securityclient.ClearActions() - err = c.allocate(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}) + err = c.allocate(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test"}}) if !testCase.errFn(err) { t.Fatal(err) }