Merge pull request kubernetes-retired#34 from wlan0/master

fix rbac rules and check for errors getting bucketclass

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,50 +71,70 @@ 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
 	}
 
 	if bucketRequest.Spec.BucketInstanceName == "" {
 		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
 		}

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",

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
 	}

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 {

resources/deployment.yaml

@@ -32,3 +32,5 @@ spec:
       containers:
         - name: objectstorage-controller
           image: quay.io/containerobjectstorage/objectstorage-controller:latest
+          args:
+          - "--v=5"

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