Feat: Add system tests for the operator

* 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 logs to tests, fix update tags function and edit logs

* change tag prefix var

Co-authored-by: Niv Raviv <[email protected]>
Co-authored-by: shaimoria <[email protected]>

Author: 3 people

.dockerignore

@@ -1,4 +1,4 @@
 # More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
 # Ignore build and test binaries.
-bin/
+# bin/
 testbin/

Makefile

@@ -167,11 +167,17 @@ delete-local-cluster:
 kind-load-controller:
 	kind load docker-image $(IMG) --name $(CLUSTER_NAME)
 
-.PHONY:run-local-aws
+.PHONY: run-local-aws
 run-local-aws:
 	docker run --rm -it -p 4566:4566 -p 4510-4559:4510-4559 localstack/localstack
 
-.PHONY:run-local-aws-on-cluster
+.PHONY: run-local-aws-on-cluster
 run-local-aws-on-cluster:
 	helm repo add localstack-repo https://helm.localstack.cloud
 	helm upgrade --install localstack localstack-repo/localstack -n $(NAMESPACE)
+	echo "applying ingress controller kong"
+	kubectl apply -f ./hack/ingress.yaml -n $(NAMESPACE)
+
+.PHONY: deploy-ingress-controller
+deploy-ingress-controller:
+	$(shell ./hack/scripts/deploy-ingress.sh)

README.md

@@ -15,7 +15,10 @@ Requirements:
 
 **Quick Start**
 
-This script will create kind cluster, build image and deploy it and will run local aws on cluster ([localstack](https://github.com/localstack/localstack))
+This script will create kind cluster,
+     build image of the controller and deploy it,
+     deploy kong ingress controller
+     and will run local aws on cluster with ingress ([localstack](https://github.com/localstack/localstack))
 ```bash
 sh ./hack/scripts/runLocalEnv.sh # you might need to run this as sudo if a regular user can't use docker
 
@@ -25,7 +28,17 @@ sh ./hack/scripts/runLocalEnv.sh # you might need to run this as sudo if a regul
 
 Todo
 
-### Development using Tilt
+### **Run system tests**
+The tests run against your local kind cluster and the [localstack](https://github.com/localstack/localstack) service that run on your cluster.
+
+run tests:
+```bash
+    1. upload local env:
+       1.1.  sh ./hack/scripts/runLocalEnv.sh
+    2. go test ./tests/systemTest/system_test.go -v # -v flag for log all tests as they are run
+```
+
+### **Development using Tilt**
 
 The recommended development flow is based on [Tilt](https://tilt.dev/) - it is used for quick iteration on code running in live containers.
 Setup based on [official docs](https://docs.tilt.dev/example_go.html) can be found in the Tiltfile.

Tiltfile

@@ -1,23 +1,50 @@
 IMG = 'controller:tilt'
-docker_build(IMG, '.')
+load("ext://restart_process", "docker_build_with_restart")
 
-def yaml():
-    return local('cd config/manager; kustomize edit set image controller=' + IMG + '; cd ../..; kustomize build config/default')
+DOCKERFILE = """FROM golang:alpine
+WORKDIR /
+COPY ./bin/manager /
+CMD ["/manager"]
+"""
 
+    
 def manifests():
-    return 'bin/controller-gen crd:trivialVersions=true rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases;'
+    return './bin/controller-gen crd rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases;'
+
 
 def generate():
-    return 'bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./...";'
+    return './bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./...";'
 
-def vetfmt():
-    return 'go vet ./...; go fmt ./...'
 
 def binary():
-    return 'CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -o bin/manager main.go'
-
-local_resource('crd', manifests() + 'kustomize build config/crd | kubectl apply -f -', deps=["api"])
-
-k8s_yaml(yaml())
-
-local_resource('recompile', generate() + binary(), deps=['controllers', 'main.go'])
+    return "CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/manager main.go"
+
+# Deploy CRD
+local_resource(
+    "CRD",
+    manifests() + "kustomize build config/crd | kubectl apply -f -",
+    deps=["api"],
+    ignore=["*/*/zz_generated.deepcopy.go"],
+)
+
+# Deploy manager
+watch_file("./config/")
+k8s_yaml(local('kustomize build config/default'))
+
+local_resource(
+    "Watch & Compile",
+    generate() + binary(),
+    deps=["controllers", "api", "main.go"],
+    ignore=["*/*/zz_generated.deepcopy.go"],
+)
+
+docker_build_with_restart(
+    IMG,
+    ".",
+    dockerfile_contents=DOCKERFILE,
+    entrypoint=["/manager"],
+    only=["./bin/manager"],
+    live_update=[
+        sync("./bin/manager", "/manager"),
+    ],
+)

controllers/aws/awsClient.go

@@ -112,50 +112,70 @@ func (a *AwsClient) HandleBucketUpdate(bucketName string, bucketSpec *s3operator
 }
 
 func (a *AwsClient) UpdateBucketTags(bucketName string, tagsToUpdate map[string]string) (bool, error) {
-	a.Log.Info("UpdateBucketTags function")
-	taggingOut, err := a.s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
+	a.Log.V(1).Info("UpdateBucketTags function")
+	if tagsToUpdate == nil {
+		tagsToUpdate = map[string]string{}
+	}
+	tagsFromAws, err := a.s3Client.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(bucketName)})
 	if err != nil {
 		a.Log.Error(err, "error from GetBucketTagging")
 		return false, err
 	}
-	diffTags := a.FindDiffTags(tagsToUpdate, taggingOut.TagSet)
-	if len(diffTags) > 0 {
+	isDiffTags, diffTags := a.FindIfDiffTags(tagsToUpdate, tagsFromAws.TagSet)
+	if isDiffTags {
 		_, err := a.s3Client.PutBucketTagging(&s3.PutBucketTaggingInput{Bucket: &bucketName, Tagging: &s3.Tagging{TagSet: diffTags}})
 		if err != nil {
 			a.Log.Error(err, "error from PutBucketTagging")
 		} else {
 			a.Log.Info("finish to update tags")
 
 		}
+	} else {
+		a.Log.Info("no tags to update")
 	}
 	return true, nil
-
 }
-func (a *AwsClient) FindDiffTags(tagsToUpdate map[string]string, tagsFromAws []*s3.Tag) []*s3.Tag {
-	a.Log.Info("FindDiffTags function")
-	var diffTags []*s3.Tag
-	mapTagsFromAws := make(map[string]struct{}, len(tagsFromAws))
+func (a *AwsClient) FindIfDiffTags(tagsToUpdate map[string]string, tagsFromAws []*s3.Tag) (bool, []*s3.Tag) {
+	a.Log.V(1).Info("FindDiffTags function")
+	isDiffTags := false
+	newTags := []*s3.Tag{}
+	mapAwsTags := map[string]string{}
+
 	for _, tag := range tagsFromAws {
-		mapTagsFromAws[tag.String()] = struct{}{}
+		tagToCheck := *tag
+		mapAwsTags[*tag.Key] = *tag.Value
+		if len(*tagToCheck.Key) < len(config.TagPrefix()) || (*tagToCheck.Key)[:len(config.TagPrefix())] != config.TagPrefix() {
+			newTags = append(newTags, &tagToCheck) //add all tags that dont have the operator prefix
+			a.Log.Info("add tag from aws", "tag", tagToCheck)
+
+		} else { //all the tags from aws that have the Tag prefix
+			tagKeyWithoutPrefix := (*tagToCheck.Key)[:len(config.TagPrefix())]
+			val, ok := tagsToUpdate[tagKeyWithoutPrefix]
+			if !ok || val != *tagToCheck.Value {
+				isDiffTags = true
+				a.Log.Info("found tags to update", "tagsToUpdate", tagToCheck)
+			}
+		}
 	}
-	for key, val := range tagsToUpdate {
-		Tagkey := key
+	for key, val := range tagsToUpdate { //add all the tags from resource to the tags array
+		Tagkey := config.TagPrefix() + key
 		Tagval := val
 		tag := s3.Tag{Key: &Tagkey, Value: &Tagval}
-		if _, found := mapTagsFromAws[tag.String()]; !found {
-			diffTags = append(diffTags, &tag)
+		a.Log.V(1).Info("add tag from spec", "tag", tag)
+		newTags = append(newTags, &tag)
+		val, ok := mapAwsTags[Tagkey]
+		if !ok || val != Tagval {
+			isDiffTags = true
+			a.Log.Info("found tags to update", "tagsToUpdate", tag)
 		}
+
 	}
-	if len(diffTags) > 0 {
-		a.Log.Info("found tags to update", "tagsToUpdate", diffTags)
-	} else {
-		a.Log.Info("no tags to update")
-	}
-	return diffTags
+	a.Log.V(1).Info("returend from find diff Tags", "isDiffTags", isDiffTags, "newTags", newTags)
+	return isDiffTags, newTags
 }
 
 func (a *AwsClient) CreateBucket(bucketInput s3.CreateBucketInput) (*s3.CreateBucketOutput, error) {
-	a.Log.Info("Starting to create S3 bucket on AWS", "bucket_name", *bucketInput.Bucket, "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
+	a.Log.Info("Starting to create S3 bucket on AWS", "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
 	res, err := a.s3Client.CreateBucket(&bucketInput)
 	if err != nil { //  cast err to awserr.Error to get the Code and
 		if aerr, ok := err.(awserr.Error); ok {
@@ -172,19 +192,19 @@ func (a *AwsClient) CreateBucket(bucketInput s3.CreateBucketInput) (*s3.CreateBu
 			}
 		} else {
 			// Message from an error.
-			a.Log.Error(err, "error in creatBucket function", "bucket_name", *bucketInput.Bucket, "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
+			a.Log.Error(err, "error in creatBucket function", "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
 			return nil, err
 		}
 	}
-	a.Log.Info("S3 bucket creation finished successfully", "bucket_name", *bucketInput.Bucket, "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
+	a.Log.Info("S3 bucket creation finished successfully", "region", *bucketInput.CreateBucketConfiguration.LocationConstraint)
 	return res, nil
 
 }
 
 func (a *AwsClient) PutBucketTagging(bucketName string, bucketTags *map[string]string) (bool, error) {
 	tags := make([]*s3.Tag, 0)
 	for key, val := range *bucketTags {
-		Tagkey := key
+		Tagkey := config.TagPrefix() + key
 		Tagval := val
 		tag := s3.Tag{
 			Key:   &Tagkey,
@@ -198,13 +218,13 @@ func (a *AwsClient) PutBucketTagging(bucketName string, bucketTags *map[string]s
 		Bucket:  aws.String(bucketName),
 		Tagging: &s3.Tagging{TagSet: tags},
 	}
-	a.Log.Info("Adding Tags to s3 bucket", "bucket_name", *input.Bucket, "bucket_tags", *input.Tagging)
+	a.Log.Info("Adding Tags to s3 bucket", "bucket_tags", *input.Tagging)
 	_, err := a.s3Client.PutBucketTagging(input)
 	if err != nil {
-		a.Log.Error(err, "error PutBucketTagging", "bucket_name", *input.Bucket)
+		a.Log.Error(err, "error PutBucketTagging")
 		return false, err
 	}
-	a.Log.Info("S3 bucket tagging finished successfully", "bucket_name", *input.Bucket, "bucket_tags", *input.Tagging)
+	a.Log.Info("S3 bucket tagging finished successfully", "bucket_tags", *input.Tagging)
 	return true, nil
 }
 
@@ -217,26 +237,26 @@ func (a *AwsClient) CreateBucketInput(bucketName string, bucketRegion string) *s
 }
 
 func (a *AwsClient) DeleteBucket(bucketName string) (*s3.DeleteBucketOutput, error) {
-	a.Log.Info("DeleteBucket function", "bucket_name", bucketName)
+	a.Log.Info("DeleteBucket function")
 	res, err := a.s3Client.DeleteBucket(&s3.DeleteBucketInput{Bucket: aws.String(bucketName)})
 	return res, err
 }
 func (a *AwsClient) CleanupsBucket(bucketName string) error {
-	a.Log.Info("CleanupsBucket function", "bucket_name", bucketName)
+	a.Log.Info("CleanupsBucket function")
 
 	iter := s3manager.NewDeleteListIterator(a.s3Client, &s3.ListObjectsInput{
 		Bucket: aws.String(bucketName),
 	})
 	if err := s3manager.NewBatchDeleteWithClient(a.s3Client).Delete(aws.BackgroundContext(), iter); err != nil {
-		a.Log.Error(err, "Unable to delete objects from bucket", "bucket_name", bucketName)
+		a.Log.Error(err, "Unable to delete objects from bucket")
 		return err
 	}
-	a.Log.Info("succeded to cleanup bucket", "bucket_name", bucketName)
+	a.Log.Info("succeded to cleanup bucket")
 	return nil
 }
 
 func (a *AwsClient) PutBucketPolicy(bucketName string, roleName string) (*s3.PutBucketPolicyOutput, error) {
-	a.Log.Info("adding bucket policy for s3 bucket", "bucket_name", bucketName, "roleName:", roleName)
+	a.Log.Info("adding bucket policy for s3 bucket", "roleName:", roleName)
 
 	// Create a policy using map interface. Filling in the bucket as the
 	// resource.
@@ -261,7 +281,7 @@ func (a *AwsClient) PutBucketPolicy(bucketName string, roleName string) (*s3.Put
 	}
 	bucketPolicy, err := json.Marshal(AllPremisionToRole)
 	if err != nil {
-		a.Log.Error(err, "error in PutBucketPolicy in Marshal", "bucket_name", bucketName, "roleName:", roleName)
+		a.Log.Error(err, "error in PutBucketPolicy in Marshal", "roleName:", roleName)
 		return nil, err
 
 	}
@@ -274,13 +294,13 @@ func (a *AwsClient) PutBucketPolicy(bucketName string, roleName string) (*s3.Put
 	if err != nil {
 		a.Log.Error(err, "error in put bucket policy", bucketName, "policy:", *input.Policy)
 	} else {
-		a.Log.Info("Attach bucket policy to s3 bucket finished successfully", "bucket_name", bucketName, "policy:", *input.Policy)
+		a.Log.Info("Attach bucket policy to s3 bucket finished successfully", "policy:", *input.Policy)
 	}
 	return res, err
 }
 
 func (a *AwsClient) PutBucketEncrypt(bucketName string) (bool, error) {
-	a.Log.Info("PutBucketEncrypt function", "bucket_name", bucketName)
+	a.Log.Info("PutBucketEncrypt function")
 	encryptRules := []*s3.ServerSideEncryptionRule{{
 		BucketKeyEnabled:                   aws.Bool(true),
 		ApplyServerSideEncryptionByDefault: &s3.ServerSideEncryptionByDefault{SSEAlgorithm: aws.String("AES256")}},
@@ -293,25 +313,25 @@ func (a *AwsClient) PutBucketEncrypt(bucketName string) (bool, error) {
 		Bucket:                            &bucketName,
 		ServerSideEncryptionConfiguration: &sSEncryptConfiguration,
 	}
-	a.Log.Info("PutBucketEncryption input", "bucket_name", bucketName, "ServerSideEncryptionConfiguration:", *input.ServerSideEncryptionConfiguration)
+	a.Log.Info("PutBucketEncryption input", "ServerSideEncryptionConfiguration:", *input.ServerSideEncryptionConfiguration)
 	_, err := a.s3Client.PutBucketEncryption(input)
 	if err != nil {
 		a.Log.Error(err, "not succsede to PutBucketEncrypt")
 		return false, err
 	}
-	a.Log.Info("succeded to encrypt bucket", "bucket_name", bucketName)
+	a.Log.Info("succeded to encrypt bucket")
 	return true, nil
 
 }
 
 func (a *AwsClient) DeleteBucketPolicy(bucketName string) (*s3.DeleteBucketPolicyOutput, error) {
-	a.Log.Info("DeleteBucketPolicy function", "bucket_name", bucketName)
+	a.Log.Info("DeleteBucketPolicy function")
 	input := &s3.DeleteBucketPolicyInput{
 		Bucket: &bucketName,
 	}
 	res, err := a.s3Client.DeleteBucketPolicy(input)
 	if err != nil {
-		a.Log.Error(err, "error in DeleteBucketPolicy", "bucket_name", bucketName)
+		a.Log.Error(err, "error in DeleteBucketPolicy")
 	}
 	return res, err
 }
@@ -340,10 +360,6 @@ func (a *AwsClient) getAllBucketsByTag(filterTag *s3.Tag) ([]*string, error) {
 	}
 	return buckets, nil
 }
-func (a *AwsClient) LogWithVal(bucketName string) *logr.Logger {
-	logger := a.Log.WithValues("bucket_name", bucketName)
-	return &logger
-}
 
 func getRoleName(bucketName string) string {
 	roleName := bucketName + "IAM-ROLE-S3Operator"
@@ -365,7 +381,7 @@ func CreateSession(Log *logr.Logger) *session.Session {
 	return ses
 }
 
-func setS3Client(Log *logr.Logger, ses *session.Session) *s3.S3 {
+func SetS3Client(Log *logr.Logger, ses *session.Session) *s3.S3 {
 	Log.Info("create s3Client wit session", "session", *ses)
 	s3Client := s3.New(ses)
 	if s3Client == nil {
@@ -396,7 +412,7 @@ func setClients(Log *logr.Logger) (*s3.S3, *resourcegroupstaggingapi.ResourceGro
 		Log.Error(err, "error in create new session")
 	} else {
 		Log.Info("session", "ses", ses)
-		return setS3Client(Log, ses), setRGTAClient(Log, ses), setIamClient(Log, ses)
+		return SetS3Client(Log, ses), setRGTAClient(Log, ses), setIamClient(Log, ses)
 	}
 	return &s3.S3{}, &resourcegroupstaggingapi.ResourceGroupsTaggingAPI{}, &iam.IAM{}
 }

controllers/config/variables.go

@@ -18,6 +18,7 @@ var resourcePerPage int64 = 100
 var awsCredentialsChainVerboseErrors bool
 var awsS3ForcePathStyle bool
 var devMode bool
+var TAG_PREFIX = "s3.operator/"
 
 func init() {
 	var err error
@@ -92,3 +93,6 @@ func AwsS3ForcePathStyle() bool {
 func DevMode() bool {
 	return devMode
 }
+func TagPrefix() string {
+	return TAG_PREFIX
+}

controllers/s3bucket_controller.go

@@ -51,7 +51,7 @@ type S3BucketReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 func (r *S3BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := r.Log.WithValues("namespace", req.Namespace, "resource_name", req.Name)
+	log := r.Log.WithValues("namespace", req.Namespace, "bucket_name", req.Name)
 	var s3Bucket s3operatorv1.S3Bucket
 
 	errToGet := r.Get(context.TODO(), req.NamespacedName, &s3Bucket)