From 11dd1df2bdf22f82da0ee39048adba03763aed01 Mon Sep 17 00:00:00 2001 From: wlan0 <sidharthamn@gmail.com> Date: Tue, 12 Jan 2021 21:24:07 -0800 Subject: [PATCH] fix rbac rules and check for errors getting bucketclass --- .../bucketaccessrequest.go | 73 ++++++++++++------- .../bucketaccessrequest_test.go | 3 + pkg/bucketrequest/bucketrequest.go | 13 ++-- pkg/util/util.go | 19 ++--- resources/deployment.yaml | 2 + resources/rbac.yaml | 10 ++- 6 files changed, 77 insertions(+), 43 deletions(-) diff --git a/pkg/bucketaccessrequest/bucketaccessrequest.go b/pkg/bucketaccessrequest/bucketaccessrequest.go index 48cebc99..36ba7852 100644 --- a/pkg/bucketaccessrequest/bucketaccessrequest.go +++ b/pkg/bucketaccessrequest/bucketaccessrequest.go @@ -4,14 +4,15 @@ import ( "context" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubeclientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/retry" "github.com/kubernetes-sigs/container-object-storage-interface-api/apis/objectstorage.k8s.io/v1alpha1" bucketclientset "github.com/kubernetes-sigs/container-object-storage-interface-api/clientset" bucketcontroller "github.com/kubernetes-sigs/container-object-storage-interface-api/controller" "github.com/kubernetes-sigs/container-object-storage-interface-controller/pkg/util" - kubeclientset "k8s.io/client-go/kubernetes" - "k8s.io/client-go/util/retry" "github.com/golang/glog" ) @@ -41,9 +42,6 @@ func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.Buc if err != nil { // Provisioning is 100% finished / not in progress. switch err { - case util.ErrInvalidBucketAccessClass: - glog.V(1).Infof("BucketAccessClass specified does not exist while processing BucketAccessRequest %v.", bucketAccessRequest.Name) - err = nil case util.ErrBucketAccessAlreadyExists: glog.V(1).Infof("BucketAccess already exist for this BucketAccessRequest %v.", bucketAccessRequest.Name) err = nil @@ -73,26 +71,40 @@ func (b *bucketAccessRequestListener) Delete(ctx context.Context, obj *v1alpha1. // or a special error errBucketAccessAlreadyExists, errInvalidBucketAccessClass is returned when provisioning was impossible and // no further attempts to provision should be tried. func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error { - bucketAccessClassName := bucketAccessRequest.Spec.BucketAccessClassName - - bucketaccess := b.FindBucketAccess(ctx, bucketAccessRequest) - if bucketaccess != nil { - // bucketaccess has provisioned, nothing to do. - return util.ErrBucketAccessAlreadyExists + baClient := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses() + bacClient := b.bucketClient.ObjectstorageV1alpha1().BucketAccessClasses() + brClient := b.bucketClient.ObjectstorageV1alpha1().BucketRequests + barClient := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests + coreClient := b.kubeClient.CoreV1() + + name := string(bucketAccessRequest.GetUID()) + _, err := baClient.Get(ctx, name, metav1.GetOptions{}) + if err != nil { + // anything other than 404 + if !errors.IsNotFound(err) { + glog.Errorf("error fetching bucketaccess: %v", err) + return err + } + } else { // if bucket found + return nil } - bucketAccessClass, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessClasses().Get(ctx, bucketAccessClassName, metav1.GetOptions{}) - if bucketAccessClass == nil { + bucketAccessClassName := bucketAccessRequest.Spec.BucketAccessClassName + bucketAccessClass, err := bacClient.Get(ctx, bucketAccessClassName, metav1.GetOptions{}) + if err != nil { // bucket access class is invalid or not specified, cannot continue with provisioning. + glog.Errorf("error fetching bucketaccessclass [%v]: %v", bucketAccessClassName, err) return util.ErrInvalidBucketAccessClass } - bucketRequest, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketAccessRequest.Namespace).Get(ctx, bucketAccessRequest.Spec.BucketRequestName, metav1.GetOptions{}) - if bucketRequest == nil { - // bucket request does not exist, we have to reject this provision. - return util.ErrInvalidBucketRequest + brName := bucketAccessRequest.Spec.BucketRequestName + // TODO: catch this in a admission controller + if brName == "" { + return util.ErrInvalidBucketAccessRequest } + bucketRequest, err := brClient(bucketAccessRequest.Namespace).Get(ctx, brName, metav1.GetOptions{}) if err != nil { + glog.Errorf("error fetching bucket request [%v]: %v", brName, err) return err } @@ -100,23 +112,29 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, return util.ErrWaitForBucketProvisioning } - sa, err := b.kubeClient.CoreV1().ServiceAccounts(bucketAccessRequest.Namespace).Get(ctx, bucketAccessRequest.Spec.ServiceAccountName, metav1.GetOptions{}) - if err != nil { - return err + saName := bucketAccessRequest.Spec.ServiceAccountName + sa := &v1.ServiceAccount{} + if saName != "" { + sa, err = coreClient.ServiceAccounts(bucketAccessRequest.Namespace).Get(ctx, saName, metav1.GetOptions{}) + if err != nil { + return err + } } - bucketaccess = &v1alpha1.BucketAccess{} - bucketaccess.Name = util.GetUUID() + bucketaccess := &v1alpha1.BucketAccess{} + bucketaccess.Name = name bucketaccess.Spec.BucketInstanceName = bucketRequest.Spec.BucketInstanceName bucketaccess.Spec.BucketAccessRequest = &v1.ObjectReference{ Name: bucketAccessRequest.Name, Namespace: bucketAccessRequest.Namespace, - UID: bucketAccessRequest.ObjectMeta.UID} + UID: bucketAccessRequest.ObjectMeta.UID, + } bucketaccess.Spec.ServiceAccount = &v1.ObjectReference{ Name: sa.Name, Namespace: sa.Namespace, - UID: sa.ObjectMeta.UID} + UID: sa.ObjectMeta.UID, + } // bucketaccess.Spec.MintedSecretName - set by the driver bucketaccess.Spec.PolicyActionsConfigMapData, err = util.ReadConfigData(b.kubeClient, bucketAccessClass.PolicyActionsConfigMap) if err != nil { @@ -126,14 +144,17 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, bucketaccess.Spec.Provisioner = bucketAccessClass.Provisioner bucketaccess.Spec.Parameters = util.CopySS(bucketAccessClass.Parameters) - bucketaccess, err = b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().Create(context.Background(), bucketaccess, metav1.CreateOptions{}) + bucketaccess, err = baClient.Create(context.Background(), bucketaccess, metav1.CreateOptions{}) if err != nil { + if errors.IsAlreadyExists(err) { + return nil + } return err } err = retry.RetryOnConflict(retry.DefaultRetry, func() error { bucketAccessRequest.Spec.BucketAccessName = bucketaccess.Name - _, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{}) + _, err := barClient(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{}) if err != nil { return err } diff --git a/pkg/bucketaccessrequest/bucketaccessrequest_test.go b/pkg/bucketaccessrequest/bucketaccessrequest_test.go index 73834949..d9b77f12 100644 --- a/pkg/bucketaccessrequest/bucketaccessrequest_test.go +++ b/pkg/bucketaccessrequest/bucketaccessrequest_test.go @@ -68,6 +68,7 @@ var bucketRequest1 = types.BucketRequest{ ObjectMeta: metav1.ObjectMeta{ Name: "bucketrequest1", Namespace: "default", + UID: "br-12345", }, Spec: types.BucketRequestSpec{ BucketPrefix: "cosi", @@ -88,6 +89,7 @@ var bucketAccessRequest1 = types.BucketAccessRequest{ ObjectMeta: metav1.ObjectMeta{ Name: "bucketaccessrequest1", Namespace: "default", + UID: "bar-12345", }, Spec: types.BucketAccessRequestSpec{ ServiceAccountName: "sa1", @@ -104,6 +106,7 @@ var bucketAccessRequest2 = types.BucketAccessRequest{ ObjectMeta: metav1.ObjectMeta{ Name: "bucketaccessrequest2", Namespace: "default", + UID: "bar-67890", }, Spec: types.BucketAccessRequestSpec{ ServiceAccountName: "sa2", diff --git a/pkg/bucketrequest/bucketrequest.go b/pkg/bucketrequest/bucketrequest.go index f14898ec..12268455 100644 --- a/pkg/bucketrequest/bucketrequest.go +++ b/pkg/bucketrequest/bucketrequest.go @@ -37,7 +37,7 @@ func (b *bucketRequestListener) InitializeBucketClient(bc bucketclientset.Interf // Add creates a bucket in response to a bucketrequest func (b *bucketRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketRequest) error { - glog.V(1).Infof("Add called for BucketRequest %s", obj.Name) + glog.V(3).Infof("Add called for BucketRequest %s", obj.Name) bucketRequest := obj err := b.provisionBucketRequestOperation(ctx, bucketRequest) if err != nil { @@ -61,13 +61,13 @@ func (b *bucketRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketReq // update processes any updates made to the bucket request func (b *bucketRequestListener) Update(ctx context.Context, old, new *v1alpha1.BucketRequest) error { - glog.V(1).Infof("Update called for BucketRequest %v", old.Name) + glog.V(3).Infof("Update called for BucketRequest %v", old.Name) return nil } // Delete processes a bucket for which bucket request is deleted func (b *bucketRequestListener) Delete(ctx context.Context, obj *v1alpha1.BucketRequest) error { - glog.V(1).Infof("Delete called for BucketRequest %v", obj.Name) + glog.V(3).Infof("Delete called for BucketRequest %v", obj.Name) return nil } @@ -81,8 +81,8 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont bucketClassName := b.GetBucketClass(bucketRequest) bucketClass, err := b.bucketClient.ObjectstorageV1alpha1().BucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) - if bucketClass == nil { - // bucketclass does not exist in order to create a bucket + if err != nil { + glog.Errorf("error getting bucketclass: [%v] %v", bucketClassName, err) return util.ErrInvalidBucketClass } @@ -123,6 +123,9 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont bucket, err = b.bucketClient.ObjectstorageV1alpha1().Buckets().Create(context.Background(), bucket, metav1.CreateOptions{}) if err != nil { + if errors.IsAlreadyExists(err) { + return nil + } glog.V(5).Infof("Error occurred when creating Bucket %v", err) return err } diff --git a/pkg/util/util.go b/pkg/util/util.go index 14a8cede..28edfeac 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -37,15 +37,16 @@ import ( var ( // Error codes that the central controller will return - ErrBucketAlreadyExists = errors.New("A bucket already existing that matches the bucket request") - ErrInvalidBucketClass = errors.New("Cannot find bucket class with the name specified in the bucket request") - ErrBucketAccessAlreadyExists = errors.New("A bucket access already existing that matches the bucket access request") - ErrInvalidBucketAccessClass = errors.New("Cannot find bucket access class with the name specified in the bucket access request") - ErrInvalidBucketRequest = errors.New("Invalid bucket request specified in the bucket access request") - ErrWaitForBucketProvisioning = errors.New("Bucket instance specified in the bucket request is not available to provision bucket access") - ErrBCUnavailable = errors.New("BucketClass is not available") - ErrNotImplemented = errors.New("Operation Not Implemented") - ErrNilConfigMap = errors.New("ConfigMap cannot be nil") + ErrBucketAlreadyExists = errors.New("A bucket already existing that matches the bucket request") + ErrInvalidBucketClass = errors.New("Cannot find bucket class with the name specified in the bucket request") + ErrBucketAccessAlreadyExists = errors.New("A bucket access already existing that matches the bucket access request") + ErrInvalidBucketAccessClass = errors.New("Cannot find bucket access class with the name specified in the bucket access request") + ErrInvalidBucketRequest = errors.New("Invalid bucket request specified in the bucket access request") + ErrInvalidBucketAccessRequest = errors.New("Invalid bucket access request specified") + ErrWaitForBucketProvisioning = errors.New("Bucket instance specified in the bucket request is not available to provision bucket access") + ErrBCUnavailable = errors.New("BucketClass is not available") + ErrNotImplemented = errors.New("Operation Not Implemented") + ErrNilConfigMap = errors.New("ConfigMap cannot be nil") ) func CopySS(m map[string]string) map[string]string { diff --git a/resources/deployment.yaml b/resources/deployment.yaml index ac8fc2a7..c48e7487 100644 --- a/resources/deployment.yaml +++ b/resources/deployment.yaml @@ -32,3 +32,5 @@ spec: containers: - name: objectstorage-controller image: quay.io/containerobjectstorage/objectstorage-controller:latest + args: + - "--v=5" diff --git a/resources/rbac.yaml b/resources/rbac.yaml index 7ebebfe4..9b554c3c 100644 --- a/resources/rbac.yaml +++ b/resources/rbac.yaml @@ -12,16 +12,20 @@ metadata: rules: - apiGroups: ["objectstorage.k8s.io"] resources: ["bucketrequests", "bucketaccessrequests"] - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "update"] - apiGroups: ["objectstorage.k8s.io"] - resources: ["buckets", "bucketaccess"] + resources: ["buckets", "bucketaccesses"] verbs: ["get", "list", "watch", "update", "create", "delete"] - apiGroups: ["objectstorage.k8s.io"] - resources: ["bucketclass","bucketaccessclass"] + resources: ["bucketclasses","bucketaccessclasses"] verbs: ["get", "list"] - apiGroups: [""] resources: ["events"] verbs: ["list", "watch", "create", "update", "patch"] +- apiGroups: [""] + resources: ["configmaps", "serviceaccounts"] + verbs: ["list", "get"] + --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1