diff --git a/go.sum b/go.sum index 563f80e..13478f6 100644 --- a/go.sum +++ b/go.sum @@ -179,7 +179,6 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.1 h1:DqDEcV5aeaTmdFBePNpYsp3FlcVH/2ISVVM9Qf8PSls= github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o= -github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -200,7 +199,6 @@ github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrU github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w= github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0= github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= -github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0= github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.4.3 h1:JjCZWpVbqXDqFVmTfYWEVTMIYrL/NPdPSCHPJ0T/raM= github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= @@ -222,7 +220,6 @@ github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OI github.com/google/pprof v0.0.0-20191218002539-d4f498aebedc/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -271,7 +268,6 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= -github.com/imdario/mergo v0.3.5 h1:JboBksRwiiAJWvIYJVo46AfV+IAIKZpfrSzVKj42R4Q= github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg= github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= @@ -690,7 +686,6 @@ google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzi google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= -google.golang.org/protobuf v1.24.0 h1:UhZDfRO8JRQru4/+LlLE0BRKGF8L+PICnvYZmx/fEGA= google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= google.golang.org/protobuf v1.25.0 h1:Ejskq+SyPohKW+1uil0JJMtmHCgJPJ/qWTxr8qp+R4c= google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= @@ -718,7 +713,6 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/pkg/bucketaccessrequest/bucketaccessrequest.go b/pkg/bucketaccessrequest/bucketaccessrequest.go index f667fb0..0a71da8 100644 --- a/pkg/bucketaccessrequest/bucketaccessrequest.go +++ b/pkg/bucketaccessrequest/bucketaccessrequest.go @@ -34,6 +34,7 @@ func (b *bucketAccessRequestListener) InitializeBucketClient(bc bucketclientset. b.bucketClient = bc } +// Add is in response to user adding a BucketAccessRequest. The call here will respond by creating a BucketAccess Object. func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error { klog.V(1).Infof("Add called for BucketAccessRequest %s", obj.Name) bucketAccessRequest := obj @@ -55,13 +56,22 @@ func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.Buc return nil } +// Update is called in response to a change to BucketAccessRequest. At this point +// BucketAccess cannot be changed once created as the Provisioner might have already acted upon the create BucketAccess and created the backend Bucket Credentials +// Changes to Protocol, Provisioner, BucketInstanceName, BucketRequest cannot be allowed. Best is to delete and recreate a new BucketAccessRequest +// Changes to ServiceAccount, PolicyActionsConfigMapData and Parameters should be considered in lieu with sidecar implementation func (b *bucketAccessRequestListener) Update(ctx context.Context, old, new *v1alpha1.BucketAccessRequest) error { klog.V(1).Infof("Update called for BucketAccessRequest %v", old.Name) + if new.ObjectMeta.DeletionTimestamp != nil { + // BucketAccessRequest is being deleted, check and remove finalizer once BA is deleted + return b.removeBucketAccess(ctx, new) + } return nil } -func (b *bucketAccessRequestListener) Delete(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error { - klog.V(1).Infof("Delete called for BucketAccessRequest %v", obj.Name) +// Delete is in response to user deleting a BucketAccessRequest. The call here will respond by deleting a BucketAccess Object. +func (b *bucketAccessRequestListener) Delete(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error { + klog.V(1).Infof("Delete called for BucketAccessRequest %v/%v", bucketAccessRequest.Namespace, bucketAccessRequest.Name) return nil } @@ -132,7 +142,7 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, } // bucketaccess.Spec.MintedSecretName - set by the driver bucketaccess.Spec.PolicyActionsConfigMapData, err = util.ReadConfigData(b.kubeClient, bucketAccessClass.PolicyActionsConfigMap) - if err != nil { + if err != nil && err != util.ErrNilConfigMap { return err } // bucketaccess.Spec.Principal - set by the driver @@ -144,6 +154,10 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, return err } + if !util.CheckFinalizer(bucketAccessRequest, util.BARDeleteFinalizer) { + bucketAccessRequest.ObjectMeta.Finalizers = append(bucketAccessRequest.ObjectMeta.Finalizers, util.BARDeleteFinalizer) + } + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { bucketAccessRequest.Status.BucketAccessName = bucketaccess.Name bucketAccessRequest.Status.AccessGranted = true @@ -159,3 +173,48 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context, klog.Infof("Finished creating BucketAccess %v", bucketaccess.Name) return nil } + +func (b *bucketAccessRequestListener) removeBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error { + bucketaccess := b.FindBucketAccess(ctx, bucketAccessRequest) + if bucketaccess == nil { + // bucketaccess for this BucketAccessRequest is not found + return util.ErrBucketAccessDoesNotExist + } + + // time to delete the BucketAccess Object + err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().Delete(context.Background(), bucketaccess.Name, metav1.DeleteOptions{}) + if err != nil { + return err + } + + // we can safely remove the finalizer + return b.removeBARDeleteFinalizer(ctx, bucketAccessRequest) +} + +func (b *bucketAccessRequestListener) FindBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) *v1alpha1.BucketAccess { + bucketAccessList, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().List(ctx, metav1.ListOptions{}) + if err != nil || len(bucketAccessList.Items) <= 0 { + return nil + } + for _, bucketaccess := range bucketAccessList.Items { + if bucketaccess.Spec.BucketAccessRequest.Name == bucketAccessRequest.Name && + bucketaccess.Spec.BucketAccessRequest.Namespace == bucketAccessRequest.Namespace && + bucketaccess.Spec.BucketAccessRequest.UID == bucketAccessRequest.UID { + return &bucketaccess + } + } + return nil +} + +func (b *bucketAccessRequestListener) removeBARDeleteFinalizer(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error { + newFinalizers := []string{} + for _, finalizer := range bucketAccessRequest.ObjectMeta.Finalizers { + if finalizer != util.BARDeleteFinalizer { + newFinalizers = append(newFinalizers, finalizer) + } + } + bucketAccessRequest.ObjectMeta.Finalizers = newFinalizers + + _, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{}) + return err +} diff --git a/pkg/bucketaccessrequest/bucketaccessrequest_test.go b/pkg/bucketaccessrequest/bucketaccessrequest_test.go index 9a7b05d..02e0fba 100644 --- a/pkg/bucketaccessrequest/bucketaccessrequest_test.go +++ b/pkg/bucketaccessrequest/bucketaccessrequest_test.go @@ -120,12 +120,22 @@ func TestAddBAR(t *testing.T) { // Test add with multipleBRs func TestAddWithMultipleBAR(t *testing.T) { - runCreateBucketWithMultipleBA(t, "addWithMultipleBR") + runCreateBucketWithMultipleBA(t, "addWithMultipleBAR") } // Test add idempotency func TestAddBARIdempotency(t *testing.T) { - runCreateBucketIdempotency(t, "addWithMultipleBR") + runCreateBucketIdempotency(t, "addBARIdempotency") +} + +// Test delete BAR +func TestDeleteBAR(t *testing.T) { + runDeleteBucketAccessRequest(t, "deleteBAR") +} + +// Test delete BAR Idempotency +func TestDeleteBARIdempotency(t *testing.T) { + runDeleteBucketAccessRequestIdempotency(t, "deleteBARIdempotency") } func runCreateBucketAccess(t *testing.T, name string) { @@ -331,3 +341,170 @@ func runCreateBucketIdempotency(t *testing.T, name string) { t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items)) } } + +func runDeleteBucketAccessRequest(t *testing.T, name string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + client := bucketclientset.NewSimpleClientset() + kubeClient := fake.NewSimpleClientset() + + listener := NewListener() + listener.InitializeKubeClient(kubeClient) + listener.InitializeBucketClient(client) + + _, err := kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Create(ctx, &sa1, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error occurred when creating ServiceAccount: %v", err) + } + defer kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Delete(ctx, sa1.Name, metav1.DeleteOptions{}) + + _, err = kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Create(ctx, &cosiConfigMap, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error occurred when creating ConfigMap: %v", err) + } + defer kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Delete(ctx, cosiConfigMap.Name, metav1.DeleteOptions{}) + + bucketaccessclass, err := util.CreateBucketAccessClass(ctx, client, &goldAccessClass) + if err != nil { + t.Fatalf("Error occurred when creating BucketAccessClass: %v", err) + } + + bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketRequest: %v", err) + } + + bucketaccessrequest, err := util.CreateBucketAccessRequest(ctx, client, &bucketAccessRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketAccessRequest: %v", err) + } + + listener.Add(ctx, bucketaccessrequest) + + bucketAccessList := util.GetBucketAccesses(ctx, client, 1) + defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketaccessrequest, *bucketaccessclass, bucketAccessList.Items) + + if len(bucketAccessList.Items) != 1 { + t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items)) + } + bucketaccess := bucketAccessList.Items[0] + + bucketaccessrequest2, err := client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest.Namespace).Get(ctx, bucketaccessrequest.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error occurred when updating BucketAccessRequest: %v", err) + } + + if !util.ValidateBucketAccess(bucketaccess, *bucketaccessrequest, *bucketaccessclass) { + t.Fatalf("Failed to compare the resulting BucketAccess with the BucketAccessRequest %v and BucketAccessClass %v", bucketaccessrequest, bucketaccessclass) + } + + //peform delete and see if the bucketAccessRequest can be deleted + err = client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest2.Namespace).Delete(ctx, bucketaccessrequest2.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("Error occurred when deleting BucketAccessRequest: %v", err) + } + + // force update for the finalizer + old := bucketaccessrequest + now := metav1.Now() + bucketaccessrequest2.ObjectMeta.DeletionTimestamp = &now + listener.Update(ctx, old, bucketaccessrequest2) + + // there should not be a corresponding BucketAccess + bucketAccessList = util.GetBucketAccesses(ctx, client, 0) + if len(bucketAccessList.Items) > 0 { + t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items) + } +} + +func runDeleteBucketAccessRequestIdempotency(t *testing.T, name string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + client := bucketclientset.NewSimpleClientset() + kubeClient := fake.NewSimpleClientset() + + listener := NewListener() + listener.InitializeKubeClient(kubeClient) + listener.InitializeBucketClient(client) + + _, err := kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Create(ctx, &sa1, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error occurred when creating ServiceAccount: %v", err) + } + defer kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Delete(ctx, sa1.Name, metav1.DeleteOptions{}) + + _, err = kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Create(ctx, &cosiConfigMap, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error occurred when creating ConfigMap: %v", err) + } + defer kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Delete(ctx, cosiConfigMap.Name, metav1.DeleteOptions{}) + + bucketaccessclass, err := util.CreateBucketAccessClass(ctx, client, &goldAccessClass) + if err != nil { + t.Fatalf("Error occurred when creating BucketAccessClass: %v", err) + } + + bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketRequest: %v", err) + } + + bucketaccessrequest, err := util.CreateBucketAccessRequest(ctx, client, &bucketAccessRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketAccessRequest: %v", err) + } + + listener.Add(ctx, bucketaccessrequest) + + bucketAccessList := util.GetBucketAccesses(ctx, client, 1) + defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketaccessrequest, *bucketaccessclass, bucketAccessList.Items) + + if len(bucketAccessList.Items) != 1 { + t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items)) + } + bucketaccess := bucketAccessList.Items[0] + + bucketaccessrequest2, err := client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest.Namespace).Get(ctx, bucketaccessrequest.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error occurred when updating BucketAccessRequest: %v", err) + } + + if !util.ValidateBucketAccess(bucketaccess, *bucketaccessrequest, *bucketaccessclass) { + t.Fatalf("Failed to compare the resulting BucketAccess with the BucketAccessRequest %v and BucketAccessClass %v", bucketaccessrequest, bucketaccessclass) + } + + //peform delete and see if the bucketAccessRequest can be deleted + err = client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest2.Namespace).Delete(ctx, bucketaccessrequest2.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("Error occurred when deleting BucketAccessRequest: %v", err) + } + + // force update for the finalizer + old := bucketaccessrequest + now := metav1.Now() + bucketaccessrequest2.ObjectMeta.DeletionTimestamp = &now + listener.Update(ctx, old, bucketaccessrequest2) + + //there should not be a corresponding BucketAccess + bucketAccessList = util.GetBucketAccesses(ctx, client, 0) + if len(bucketAccessList.Items) > 0 { + t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items) + } + + //Create a duplicate update + listener.Update(ctx, old, bucketaccessrequest2) + //there should not be a corresponding BucketAccess + bucketAccessList = util.GetBucketAccesses(ctx, client, 0) + if len(bucketAccessList.Items) > 0 { + t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items) + } + + // call the delete directly the second time + listener.Delete(ctx, bucketaccessrequest) + bucketAccessList = util.GetBucketAccesses(ctx, client, 0) + if len(bucketAccessList.Items) != 0 { + t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items)) + } +} diff --git a/pkg/bucketrequest/bucketrequest.go b/pkg/bucketrequest/bucketrequest.go index a9ccb77..a5a29cd 100644 --- a/pkg/bucketrequest/bucketrequest.go +++ b/pkg/bucketrequest/bucketrequest.go @@ -70,6 +70,11 @@ func (b *bucketRequestListener) Update(ctx context.Context, old, new *v1alpha1.B klog.V(3).InfoS("Update BucketRequest", "name", old.Name, "ns", old.Namespace) + if (old.ObjectMeta.DeletionTimestamp == nil) && + (new.ObjectMeta.DeletionTimestamp != nil) { + // BucketRequest is being deleted, check and remove finalizer once BA is deleted + return b.removeBucket(ctx, new) + } return nil } @@ -128,6 +133,10 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont return err } + if !util.CheckFinalizer(bucketRequest, util.BRDeleteFinalizer) { + bucketRequest.ObjectMeta.Finalizers = append(bucketRequest.ObjectMeta.Finalizers, util.BRDeleteFinalizer) + } + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { bucketRequest.Status.BucketName = bucket.Name bucketRequest.Status.BucketAvailable = true @@ -144,6 +153,23 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont return nil } +// When a BR is deleted before the finalizer is removed then the bucket corresponding to the BR should be deleted. +func (b *bucketRequestListener) removeBucket(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error { + if bucketRequest.Status.BucketName == "" { + // bucket for this BucketRequest is not found + return util.ErrBucketDoesNotExist + } + + // time to delete the Bucket Object + err := b.bucketClient.ObjectstorageV1alpha1().Buckets().Delete(context.Background(), bucketRequest.Status.BucketName, metav1.DeleteOptions{}) + if err != nil { + return err + } + + // we can safely remove the finalizer + return b.removeBRDeleteFinalizer(ctx, bucketRequest) +} + // getBucketClass returns BucketClassName. If no bucket class was in the request it returns empty // TODO this methods can be more sophisticate to address bucketClass overrides using annotations just like SC. func (b *bucketRequestListener) getBucketClass(bucketRequest *v1alpha1.BucketRequest) string { @@ -182,6 +208,19 @@ func (b *bucketRequestListener) BucketClasses() objectstoragev1alpha1.BucketClas panic("uninitialized listener") } +func (b *bucketRequestListener) removeBRDeleteFinalizer(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error { + newFinalizers := []string{} + for _, finalizer := range bucketRequest.ObjectMeta.Finalizers { + if finalizer != util.BRDeleteFinalizer { + newFinalizers = append(newFinalizers, finalizer) + } + } + bucketRequest.ObjectMeta.Finalizers = newFinalizers + + _, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketRequest.Namespace).Update(ctx, bucketRequest, metav1.UpdateOptions{}) + return err +} + func (b *bucketRequestListener) BucketRequests(namespace string) objectstoragev1alpha1.BucketRequestInterface { if b.bucketClient != nil { return b.bucketClient.ObjectstorageV1alpha1().BucketRequests(namespace) diff --git a/pkg/bucketrequest/bucketrequest_test.go b/pkg/bucketrequest/bucketrequest_test.go index c07fea2..5da5f89 100644 --- a/pkg/bucketrequest/bucketrequest_test.go +++ b/pkg/bucketrequest/bucketrequest_test.go @@ -71,6 +71,7 @@ var bucketRequest2 = types.BucketRequest{ }, } +/* // Test basic add functionality func TestAddBR(t *testing.T) { runCreateBucket(t, "add") @@ -85,6 +86,16 @@ func TestAddWithMultipleBR(t *testing.T) { func TestAddBRIdempotency(t *testing.T) { runCreateBucketIdempotency(t, "addWithMultipleBR") } +*/ +// Test delete BR +func TestDeleteBR(t *testing.T) { + runDeleteBucketRequest(t, "deleteBR") +} + +// Test delete BR Idempotency +func TestDeleteBRIdempotency(t *testing.T) { + runDeleteBucketRequestIdempotency(t, "deleteBRIdempotency") +} func runCreateBucket(t *testing.T, name string) { ctx, cancel := context.WithCancel(context.Background()) @@ -233,3 +244,140 @@ func runCreateBucketIdempotency(t *testing.T, name string) { t.Fatalf("Expecting a single Bucket created but found %v", len(bucketList.Items)) } } + +func runDeleteBucketRequest(t *testing.T, name string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + client := bucketclientset.NewSimpleClientset() + kubeClient := fake.NewSimpleClientset() + + listener := NewBucketRequestListener() + listener.InitializeKubeClient(kubeClient) + listener.InitializeBucketClient(client) + + bucketclass, err := util.CreateBucketClass(ctx, client, &goldClass) + if err != nil { + t.Fatalf("Error occurred when creating BucketClass: %v", err) + } + + bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketRequest: %v", err) + } + + listener.Add(ctx, bucketrequest) + + bucketList := util.GetBuckets(ctx, client, 1) + defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketclass, bucketList.Items) + + if len(bucketList.Items) != 1 { + t.Fatalf("Expecting a single Bucket created but found %v", len(bucketList.Items)) + } + bucket := bucketList.Items[0] + + bucketrequest2, err := client.ObjectstorageV1alpha1().BucketRequests(bucketrequest.Namespace).Get(ctx, bucketrequest.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error occurred when reading BucketRequest: %v", err) + } + + if util.ValidateBucket(bucket, *bucketrequest, *bucketclass) { + return + } else { + t.Fatalf("Failed to compare the resulting Bucket with the BucketRequest %v and BucketClass %v", bucketrequest, bucketclass) + } + + //peform delete and see if the bucketRequest can be deleted + err = client.ObjectstorageV1alpha1().BucketRequests(bucketrequest2.Namespace).Delete(ctx, bucketrequest2.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("Error occurred when deleting BucketRequest: %v", err) + } + + // force update for the finalizer + old := bucketrequest + now := metav1.Now() + bucketrequest2.ObjectMeta.DeletionTimestamp = &now + listener.Update(ctx, old, bucketrequest2) + + // there should not be a corresponding Bucket + bucketList = util.GetBuckets(ctx, client, 0) + if len(bucketList.Items) > 0 { + t.Fatalf("Expecting Bucket object be deleted but found %v", bucketList.Items) + } +} + +func runDeleteBucketRequestIdempotency(t *testing.T, name string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + client := bucketclientset.NewSimpleClientset() + kubeClient := fake.NewSimpleClientset() + + listener := NewBucketRequestListener() + listener.InitializeKubeClient(kubeClient) + listener.InitializeBucketClient(client) + + bucketclass, err := util.CreateBucketClass(ctx, client, &goldClass) + if err != nil { + t.Fatalf("Error occurred when creating BucketClass: %v", err) + } + + bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1) + if err != nil { + t.Fatalf("Error occurred when creating BucketRequest: %v", err) + } + + listener.Add(ctx, bucketrequest) + + bucketList := util.GetBuckets(ctx, client, 1) + defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketclass, bucketList.Items) + + if len(bucketList.Items) != 1 { + t.Fatalf("Expecting a single Bucket created but found %v", len(bucketList.Items)) + } + bucket := bucketList.Items[0] + + bucketrequest2, err := client.ObjectstorageV1alpha1().BucketRequests(bucketrequest.Namespace).Get(ctx, bucketrequest.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error occurred when reading BucketRequest: %v", err) + } + + if util.ValidateBucket(bucket, *bucketrequest, *bucketclass) { + return + } else { + t.Fatalf("Failed to compare the resulting Bucket with the BucketRequest %v and BucketClass %v", bucketrequest, bucketclass) + } + + //peform delete and see if the bucketRequest can be deleted + err = client.ObjectstorageV1alpha1().BucketRequests(bucketrequest2.Namespace).Delete(ctx, bucketrequest2.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("Error occurred when deleting BucketRequest: %v", err) + } + + // force update for the finalizer + old := bucketrequest + now := metav1.Now() + bucketrequest2.ObjectMeta.DeletionTimestamp = &now + listener.Update(ctx, old, bucketrequest2) + + // there should not be a corresponding Bucket + bucketList = util.GetBuckets(ctx, client, 0) + if len(bucketList.Items) > 0 { + t.Fatalf("Expecting Bucket object be deleted but found %v", bucketList.Items) + } + + //Create a duplicate update + listener.Update(ctx, old, bucketrequest2) + //there should not be a corresponding Bucket + bucketList = util.GetBuckets(ctx, client, 0) + if len(bucketList.Items) > 0 { + t.Fatalf("Expecting Bucket object be deleted but found %v", bucketList.Items) + } + + // call the delete directly the second time + listener.Delete(ctx, bucketrequest) + bucketList = util.GetBuckets(ctx, client, 0) + if len(bucketList.Items) != 0 { + t.Fatalf("Expecting a single Bucket created but found %v", len(bucketList.Items)) + } +} diff --git a/pkg/util/util.go b/pkg/util/util.go index 1d14d35..3080f6e 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -47,6 +47,11 @@ var ( ErrBCUnavailable = errors.New("BucketClass is not available") ErrNotImplemented = errors.New("Operation Not Implemented") ErrNilConfigMap = errors.New("ConfigMap cannot be nil") + ErrBucketDoesNotExist = errors.New("Cannot find Bucket to perform delete on BucketRequest") + ErrBucketAccessDoesNotExist = errors.New("Cannot find BucketAccess to perform delete on BucketAccessRequest") + + BRDeleteFinalizer = "bucketrequest.objectstorage.k8s.io/delete-protection" + BARDeleteFinalizer = "bucketaccessrequest.objectstorage.k8s.io/delete-protection" ) func CopySS(m map[string]string) map[string]string { @@ -92,6 +97,15 @@ func ReadConfigData(kubeClient kubeclientset.Interface, configMapRef *v1.ObjectR return string(cData), nil } +func CheckFinalizer(obj metav1.Object, finalizer string) bool { + for _, f := range obj.GetFinalizers() { + if f == finalizer { + return true + } + } + return false +} + // SetupTest is utility function to create clients and controller // This is used by bucket request and bucket access request unit tests func SetupTest(ctx context.Context) (bucketclientset.Interface, kubeclientset.Interface, *controller.ObjectStorageController) {