Skip to content

Commit

Permalink
Bug 1820687: use patch when updating namespace
Browse files Browse the repository at this point in the history
  • Loading branch information
@soltysh
soltysh committed Apr 28, 2020
1 parent e8ca5f3 commit a1f3fe2
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 15 deletions.
38 changes: 32 additions & 6 deletions pkg/security/controller/namespace_scc_allocation_controller.go
View file
Expand Up @@ -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"

securityv1 "github.com/openshift/api/security/v1"
Expand All @@ -45,6 +49,8 @@ type NamespaceSCCAllocationController struct {
rangeAllocationClient securityv1client.RangeAllocationsGetter

queue workqueue.RateLimitingInterface

encoder runtime.Encoder
}

func NewNamespaceSCCAllocationController(
Expand All @@ -54,6 +60,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.WithoutConversion().EncoderForVersion(jsonSerializer, corev1.SchemeGroupVersion)

c := &NamespaceSCCAllocationController{
requiredUIDRange: requiredUIDRange,
mcsAllocator: mcs,
Expand All @@ -62,6 +75,7 @@ func NewNamespaceSCCAllocationController(
nsLister: namespaceInformer.Lister(),
nsListerSynced: namespaceInformer.Informer().HasSynced,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerName),
encoder: encoder,
}

namespaceInformer.Informer().AddEventHandlerWithResyncPeriod(
Expand Down Expand Up @@ -177,8 +191,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
}
Expand Down
40 changes: 31 additions & 9 deletions pkg/security/controller/namespace_security_allocation_controller_test.go
View file
@@ -1,22 +1,24 @@
package controller

import (
"encoding/json"
"fmt"
"math/big"
"strings"
"testing"

"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"
Expand All @@ -25,19 +27,29 @@ 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()
indexer := cache.NewIndexer(controller.KeyFunc, cache.Indexers{})

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.WithoutConversion().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 {
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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,
},
Expand All @@ -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")
Expand All @@ -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()
Expand All @@ -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.WithoutConversion().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()
Expand All @@ -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)
}
Expand Down

0 comments on commit a1f3fe2

Please sign in to comment.