Skip to content

Commit aeeed89

Browse files
nivr4Niv RavivshaiMoria
authored
feat: Add Operator bucket validation
* add kubebuild init files, titfile and few command to makefile for running locally * add kind config yaml for local k8s kind cluster * modify the make file , deploy working * define s3 bucket type * add readme and script for local running * add input validition and RetryPeriod * edit titfile * add aws client functions * insert create and delete bucket functionality , modify the tiltfile * add gettter to aws client , and helm commands to localstack * support encrypt bucket * add resource map to manage k8s resource agianst aws resource * fix delete resource * fix run loclal script, add logs to delete flow * fix deletion of bucket in aws * support deletion of bucket policy and creation and deletion of iam role * support deletion of bucket policy and creation and deletion of iam role * add bucket name validation * bucketname validation * syntax * refactor: remove unused files in bin folder + update git ignore * fix: change runLocalenv.sh script variable defenitions * add tests folder * update git ignore * add varieble file to config * remove region from s3 resource * move cred var to deploy yaml * fix: makefile controller-gen * add devmode var * add to readme: golang version * add cleanup bucket function to delete flow * move config varibels under controllers folder * edit logs * test * change delete logic * edit logs * chang looger logic, fix put tgs function * add logs for update flow * remove comment * a * change kind cluster * add system test , edit run local script to deploy ingress controller * change port to 4566, fix putting tags * edit: readme, add update test * crate k8s client function * crate k8s client function * fix scripts * edit delete bucket test * add k8s manger * add logs to tests, fix update tags function and edit logs * add CRUD service account function * add service account functionality, create small app to test conectivety to s3 * merge brunch * add yaml files for testing, add integration test, add test app * fix integration test * change busy wait function, add valisation to edit flow * fix system test, add to local env empty app, fix iam role function * edit readme , add script for upload test app * add service account functions * code refactor, add verible to config * change file and folder name * fix bug in generate rbac * refactor aws client * add mount volume for token * fix package-lock.json * add configmap for define body, add cluster role for app * refactor k8s client * add yaml file for deploying srvices * add tests for testing response from auth server * add example for github action * add create kind cluster to pipline * add create kind cluster to pipline * add create kind cluster to pipline * add create kind cluster to pipline * ci * fix bug in create service account flow , add tests * add to test pods check * change tests to eventually check * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add ci * add greeting workflow * add ci for create local env and run tests on it * add branch name to ci * add branch name to ci * add unit test * change check metch app , add tests to other pod controllers * remove unuse code * add unit test * add status for bucket * change findPodsController function * rename file * change bucket status to string, add tests * fix system test * change unit tests * add unit test to readme * add unit test to readme * add retry for system tests * add sleep time between tests * add function to check operator owner on the bucket, add tests for it * add update status function to reflact bucket status * add tests for check status and non manage buckets * add isBucketManagedByOperator function * add step in github action for checking health of localstack * change func isBucketManagedByOperator * change func isBucketManagedByOperator * refactor code * fix link in readme Co-authored-by: Niv Raviv <[email protected]> Co-authored-by: shaimoria <[email protected]>
1 parent cb20df6 commit aeeed89

File tree

4 files changed

+141
-45
lines changed

4 files changed

+141
-45
lines changed

.github/workflows/github-actions-testing.yml

+8
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ jobs:
5353
- name: Run make deploy system app
5454
run: make deploy-system-test-app
5555

56+
- name: wait until localstack is up
57+
uses: nev7n/wait_for_response@v1
58+
with:
59+
url: "http://localhost:4566/localstack/_localstack/health"
60+
responseCode: 200
61+
timeout: 300000
62+
interval: 5000
63+
5664
- name: run system tests
5765
run: go test ./tests/systemTest/system_test.go -v
5866

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ The integretion tests test the functionality of integration between deploying/up
4747

4848
run tests:
4949

50-
1. deploy local env -> see ([Quick-Start](##Quick-Start))
50+
1. deploy local env -> see ([Quick-Start](#quick-start))
5151
2. Run
5252
```bash
5353
sh ./tests/integrationTests/testApp/uploadApp.sh

controllers/aws/s3-bucket.go

+30-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/aws/aws-sdk-go/service/s3/s3manager"
1919
)
2020

21-
2221
func (a *AwsClient) ValidateBucketName(name string) error {
2322
if len(name) > 4 && name[:4] == "xn--" {
2423
return errors.New("bucket name can't start with xn--")
@@ -60,7 +59,11 @@ func (a *AwsClient) HandleBucketDeletion(bucketToDelete string) (bool, error) {
6059
a.Log.Info(" Start to delete s3 bucket from aws")
6160
isBucketExists, err := a.IsBucketExists(bucketToDelete)
6261
if isBucketExists {
63-
err := a.cleanupsBucketContent(bucketToDelete)
62+
isOwner, err := a.isBucketManagedByOperator(bucketToDelete)
63+
if !isOwner {
64+
return false, err
65+
}
66+
err = a.cleanupsBucketContent(bucketToDelete)
6467
if err != nil {
6568
a.Log.Error(err, "err to cleanup bucket")
6669
return false, err
@@ -78,13 +81,16 @@ func (a *AwsClient) HandleBucketDeletion(bucketToDelete string) (bool, error) {
7881

7982
func (a *AwsClient) HandleBucketUpdate(bucketName string, bucketSpec *s3operatorv1.S3BucketSpec) error {
8083
a.Log.V(1).Info("HandleBucketUpdate function")
81-
_, err := a.updateBucketTags(bucketName, bucketSpec.Tags)
82-
84+
isOwner, err := a.isBucketManagedByOperator(bucketName)
85+
if isOwner {
86+
_, err = a.updateBucketTags(bucketName, bucketSpec.Tags)
87+
} else if err == nil {
88+
err = errors.New("cant update bucket that not manage by operator")
89+
}
8390
a.Log.Info("finish to HandleBucketUpdate")
8491
return err
8592
}
8693

87-
8894
func (a *AwsClient) IsBucketExists(name string) (bool, error) {
8995
_, err := a.s3Client.GetBucketLocation(&s3.GetBucketLocationInput{Bucket: aws.String(name)})
9096
if err != nil {
@@ -158,6 +164,22 @@ func (a *AwsClient) findIfDiffTags(tagsToUpdate map[string]string, tagsFromAws [
158164
a.Log.V(1).Info("returend from find diff Tags", "isDiffTags", isDiffTags, "newTags", newTags)
159165
return isDiffTags, newTags
160166
}
167+
func (a *AwsClient) isBucketManagedByOperator(bucketName string) (bool, error) {
168+
tagsFromAws, err := a.s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
169+
if err != nil {
170+
a.Log.Error(err, "error from GetBucketTagging in checkIfOwnerBucketByTag")
171+
return false, err
172+
}
173+
defaultTag := config.DefaultTag()
174+
for _, tag := range tagsFromAws.TagSet {
175+
if defaultTag.GoString() == tag.GoString() {
176+
return true, nil
177+
}
178+
}
179+
a.Log.Info("bucket is not manage by the operator")
180+
return false, nil
181+
182+
}
161183

162184
func (a *AwsClient) createBucket(bucketInput s3.CreateBucketInput) (*s3.CreateBucketOutput, error) {
163185
a.Log.Info("Starting to create S3 bucket on AWS", "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
@@ -226,7 +248,8 @@ func (a *AwsClient) deleteBucket(bucketName string) (*s3.DeleteBucketOutput, err
226248
res, err := a.s3Client.DeleteBucket(&s3.DeleteBucketInput{Bucket: aws.String(bucketName)})
227249
return res, err
228250
}
229-
//cleanupsBucketContent function - delete all the object that inside the bucket (required for deleting bucket)
251+
252+
// cleanupsBucketContent function - delete all the object that inside the bucket (required for deleting bucket)
230253
func (a *AwsClient) cleanupsBucketContent(bucketName string) error {
231254
a.Log.V(1).Info("CleanupsBucket function")
232255

@@ -319,4 +342,4 @@ func SetS3Client(Log *logr.Logger, ses *session.Session) *s3.S3 {
319342
Log.Info(" succeded create s3Client", "client", *s3Client)
320343
}
321344
return s3Client
322-
}
345+
}

tests/systemTest/system_test.go

+102-37
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ import (
2626

2727
var s3Client *s3.S3
2828
var bucketName = "bucket-test-operator"
29+
var notManageBucketName = "not-manage-bucket-test-operator"
2930
var namespace = "k8s-s3-operator-system"
3031
var logger logr.Logger
3132
var k8sClient client.Client
3233
var s3Bucket s3operatorv1.S3Bucket
34+
var s3BucketNotManage s3operatorv1.S3Bucket
3335
var graceTime = time.Duration(7)
3436
var serviceAccountName = "system-test-serviceaccount"
3537
var appName = "system-test-app"
@@ -51,63 +53,44 @@ func TestMain(m *testing.M) {
5153

5254
s3Bucket = s3operatorv1.S3Bucket{ObjectMeta: metav1.ObjectMeta{Name: bucketName, Namespace: namespace},
5355
Spec: s3operatorv1.S3BucketSpec{Serviceaccount: serviceAccountName, Selector: map[string]string{"app": appName}}}
56+
s3BucketNotManage = s3operatorv1.S3Bucket{ObjectMeta: metav1.ObjectMeta{Name: notManageBucketName, Namespace: namespace},
57+
Spec: s3operatorv1.S3BucketSpec{Serviceaccount: serviceAccountName, Selector: map[string]string{"app": appName}}}
5458
var exitVal int
5559
for i := 1; i <= 3; i++ { // retry to pass tests
5660
logger.Info("Run system tests", "tryNumber", i)
5761
exitVal = m.Run()
5862
if exitVal == 0 {
5963
logger.Info("pass all test", "tryNumber", i)
6064
break
61-
}else{
65+
} else {
66+
cleanup()
6267
time.Sleep(graceTime * time.Second)
6368
}
6469
}
6570
logger.Info("finish to run all tests")
66-
71+
cleanup()
6772
os.Exit(exitVal)
6873
}
6974

7075
func TestCreateBucket(t *testing.T) {
7176
t.Log("start TestCreateBucket")
72-
g := NewWithT(t)
73-
t.Log("check bucket not exsits")
74-
_, err := s3Client.GetBucketLocation(&s3.GetBucketLocationInput{Bucket: aws.String(bucketName)})
75-
g.Expect(err).To(HaveOccurred())
77+
checkBucketExsist(t,bucketName,false)
7678

7779
t.Log("create new bucket resource", "bucket_name", bucketName)
7880

79-
err = k8sClient.Create(context.Background(), &s3Bucket)
80-
g.Expect(err).NotTo(HaveOccurred())
81-
time.Sleep(graceTime * time.Second)
81+
createBucketResource(t,&s3Bucket)
8282

83-
t.Log("check bucket exsits")
84-
_, err = s3Client.GetBucketLocation(&s3.GetBucketLocationInput{Bucket: aws.String(bucketName)})
85-
g.Expect(err).NotTo(HaveOccurred())
83+
checkBucketExsist(t,bucketName,true)
8684

8785
t.Log("finish TestCreateBucket")
8886

8987
}
9088
func TestBucketUpdateTag(t *testing.T) {
9189
t.Log("start TestBucketUpdateTag")
92-
g := NewWithT(t)
9390

94-
t.Log("get bucket tagging expect not to have error and the only tag is the default tag ")
95-
tags, err := s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
96-
g.Expect(err).NotTo(HaveOccurred())
97-
t.Log("got tags from aws", tags.TagSet)
98-
g.Expect(len(tags.TagSet)).Should(Equal(1))
99-
100-
t.Log("update bucket tags expect not to have error and to add the new tag")
101-
k8sClient.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: bucketName}, &s3Bucket)
102-
s3Bucket.Spec.Tags = map[string]string{"testKey": "testValue"}
103-
err = k8sClient.Update(context.TODO(), &s3Bucket)
104-
g.Expect(err).NotTo(HaveOccurred())
105-
time.Sleep(graceTime * time.Second)
106-
tags, err = s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
107-
t.Log("got tags from aws", tags.TagSet)
108-
109-
g.Expect(err).NotTo(HaveOccurred())
110-
g.Expect(len(tags.TagSet)).Should(Equal(2))
91+
checkBucketTags(t,bucketName,1)
92+
updateBucketTags(t,bucketName,&s3Bucket)
93+
checkBucketTags(t,bucketName,2)
11194

11295
t.Log("finish TestBucketUpdateTag")
11396

@@ -153,21 +136,103 @@ func TestDeleteBucketData(t *testing.T) {
153136

154137
func TestDeleteBucket(t *testing.T) {
155138
t.Log("start TestDeleteBucket")
139+
deleteBucket(t,bucketName,&s3Bucket)
140+
141+
checkBucketExsist(t, bucketName, false)
142+
143+
t.Log("finish TestDeleteBucket")
144+
145+
}
146+
func TestTryCreateBucketExsistNotManage(t *testing.T) {
147+
t.Log("start TestTryCreateBucketExsistNotManage")
156148
g := NewWithT(t)
157149

158-
t.Log("check bucket exsist expect not to have error", bucketName)
150+
_,err := s3Client.CreateBucket(&s3.CreateBucketInput{Bucket: aws.String(notManageBucketName)})
151+
g.Expect(err).NotTo(HaveOccurred())
152+
checkBucketExsist(t, notManageBucketName, true)
153+
154+
createBucketResource(t,&s3BucketNotManage)
155+
checkBucketStatus(t,notManageBucketName,&s3BucketNotManage,"failed")
156+
157+
}
158+
func TestValidateUpdateOnlyManageBuckets(t *testing.T) {
159+
t.Log("start TestValidateUpdateOnlyManageBuckets")
160+
checkBucketExsist(t, notManageBucketName, true)
161+
updateBucketTags(t, notManageBucketName,&s3BucketNotManage)
162+
checkBucketStatus(t,notManageBucketName,&s3BucketNotManage,"failed")
163+
164+
}
165+
166+
func TestValidateDeleteOnlyManageBuckets(t *testing.T) {
167+
t.Log("start TestValidateDeleteOnlyManageBuckets")
168+
deleteBucket(t,notManageBucketName,&s3BucketNotManage)
169+
170+
checkBucketExsist(t, notManageBucketName, true)
171+
172+
t.Log("finish TestValidateDeleteOnlyManageBuckets")
173+
174+
}
175+
176+
func checkBucketExsist(t *testing.T, bucketName string, expectBucket bool) {
177+
g := NewWithT(t)
159178
_, err := s3Client.GetBucketLocation(&s3.GetBucketLocationInput{Bucket: aws.String(bucketName)})
179+
if expectBucket {
180+
t.Log("check bucket exsist expect not to have error", bucketName)
181+
g.Expect(err).NotTo(HaveOccurred())
182+
} else {
183+
t.Log("check bucket exsist expect to get error", bucketName)
184+
g.Expect(err).To(HaveOccurred())
185+
186+
}
187+
}
188+
func createBucketResource(t *testing.T,s3Bucket *s3operatorv1.S3Bucket){
189+
g := NewWithT(t)
190+
err := k8sClient.Create(context.Background(), s3Bucket)
191+
g.Expect(err).NotTo(HaveOccurred())
192+
time.Sleep(graceTime * time.Second)
193+
}
194+
func updateBucketTags(t *testing.T, bucketName string, s3Bucket *s3operatorv1.S3Bucket) {
195+
g := NewWithT(t)
196+
t.Log("update bucket tags expect not to have error and to add the new tag")
197+
err := k8sClient.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: bucketName}, s3Bucket)
160198
g.Expect(err).NotTo(HaveOccurred())
161199

162-
t.Log("delete bucket resource expect not to have error")
163-
err = k8sClient.Delete(context.Background(), &s3Bucket)
200+
s3Bucket.Spec.Tags = map[string]string{"testKey": "testValue"}
201+
err = k8sClient.Update(context.TODO(), s3Bucket)
164202
g.Expect(err).NotTo(HaveOccurred())
165203
time.Sleep(graceTime * time.Second)
204+
}
166205

167-
t.Log("check bucket exsist expect to get error", bucketName)
168-
_, err = s3Client.GetBucketLocation(&s3.GetBucketLocationInput{Bucket: aws.String(bucketName)})
169-
g.Expect(err).To(HaveOccurred())
206+
func deleteBucket(t *testing.T, bucketName string,s3Bucket *s3operatorv1.S3Bucket){
207+
g := NewWithT(t)
208+
checkBucketExsist(t, bucketName, true)
170209

171-
t.Log("finish TestDeleteBucket")
210+
t.Log("delete bucket resource expect not to have error")
211+
err := k8sClient.Delete(context.Background(), s3Bucket)
212+
g.Expect(err).NotTo(HaveOccurred())
213+
time.Sleep(graceTime * time.Second)
214+
}
215+
func checkBucketTags(t *testing.T,bucketName string, expectedTags int){
216+
g := NewWithT(t)
217+
t.Log("get bucket tagging expect not to have error ")
218+
tags, err := s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
219+
g.Expect(err).NotTo(HaveOccurred())
220+
t.Log("got tags from aws", tags.TagSet)
221+
g.Expect(len(tags.TagSet)).Should(Equal(expectedTags))
222+
}
223+
func checkBucketStatus(t *testing.T, bucketName string,s3Bucket *s3operatorv1.S3Bucket, expectedSatus string){
224+
g := NewWithT(t)
225+
err := k8sClient.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: bucketName}, s3Bucket)
226+
g.Expect(err).NotTo(HaveOccurred())
227+
g.Expect(s3Bucket.Status.Status).Should(Equal(expectedSatus))
172228

173229
}
230+
func cleanup(){
231+
s3Client.DeleteBucket(&s3.DeleteBucketInput{Bucket: aws.String(bucketName)})
232+
s3Client.DeleteBucket(&s3.DeleteBucketInput{Bucket: aws.String(notManageBucketName)})
233+
234+
k8sClient.Delete(context.Background(),&s3Bucket)
235+
k8sClient.Delete(context.Background(),&s3BucketNotManage)
236+
237+
238+
}

0 commit comments

Comments
 (0)