Enhance S3Backend (#323)

* make `configuration.spec.backend.s3.region` optional Signed-off-by: loheagn <[email protected]> * check if the bucket exists when build s3 backend Signed-off-by: loheagn <[email protected]> * fix typo Signed-off-by: loheagn <[email protected]>
kubevela · Jun 27, 2022 · 5d0ff0b · 5d0ff0b
1 parent 487809a
commit 5d0ff0b
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 80 deletions.
diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go
@@ -140,9 +140,10 @@ type KubernetesBackendConf struct {
 // S3BackendConf defines all options supported by the Terraform `s3` backend type.
 // You can refer to https://www.terraform.io/language/settings/backends/s3 for the usage of each option.
 type S3BackendConf struct {
-	Region string `json:"region" hcl:"region"`
-	Bucket string `json:"bucket" hcl:"bucket"`
-	Key    string `json:"key" hcl:"key"`
+	// Region is optional, default to the AWS_DEFAULT_REGION in the credentials of the provider
+	Region *string `json:"region,omitempty" hcl:"region"`
+	Bucket string  `json:"bucket" hcl:"bucket"`
+	Key    string  `json:"key" hcl:"key"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml
@@ -230,11 +230,12 @@ spec:
                       key:
                         type: string
                       region:
+                        description: Region is optional, default to the AWS_DEFAULT_REGION
+                          in the credentials of the provider
                         type: string
                     required:
                     - bucket
                     - key
-                    - region
                     type: object
                   secretSuffix:
                     description: 'SecretSuffix used when creating secrets. Secrets

diff --git a/controllers/configuration/backend/backend_test.go b/controllers/configuration/backend/backend_test.go
@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/oam-dev/terraform-controller/api/v1beta2"
-	"github.com/oam-dev/terraform-controller/controllers/provider"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -300,73 +299,6 @@ terraform {
 				errMsg: "it's not allowed to set `spec.backend.inline` and `spec.backend.backendType` at the same time",
 			},
 		},
-		{
-			name: "inline s3 backend",
-			args: args{
-				credentials: map[string]string{
-					provider.EnvAWSAccessKeyID:     "a",
-					provider.EnvAWSSessionToken:    "token",
-					provider.EnvAWSSecretAccessKey: "secret",
-				},
-				configuration: &v1beta2.Configuration{
-					ObjectMeta: metav1.ObjectMeta{Namespace: "a"},
-					Spec: v1beta2.ConfigurationSpec{
-						Backend: &v1beta2.Backend{
-							Inline: `
-terraform {
-  backend s3 {
-    bucket = "bucket1"
-    key    = "test.tfstate"
-    region = "us-east-1"
-  }
-}
-`,
-						},
-					},
-				},
-			},
-			want: want{
-				errMsg: "",
-				backend: &S3Backend{
-					client: nil,
-					Region: "us-east-1",
-					Key:    "test.tfstate",
-					Bucket: "bucket1",
-				},
-			},
-		},
-		{
-			name: "explicit s3 backend",
-			args: args{
-				credentials: map[string]string{
-					provider.EnvAWSAccessKeyID:     "a",
-					provider.EnvAWSSessionToken:    "token",
-					provider.EnvAWSSecretAccessKey: "secret",
-				},
-				configuration: &v1beta2.Configuration{
-					ObjectMeta: metav1.ObjectMeta{Namespace: "a"},
-					Spec: v1beta2.ConfigurationSpec{
-						Backend: &v1beta2.Backend{
-							BackendType: backendTypeS3,
-							S3: &v1beta2.S3BackendConf{
-								Region: "us-east-1",
-								Bucket: "bucket1",
-								Key:    "test.tfstate",
-							},
-						},
-					},
-				},
-			},
-			want: want{
-				errMsg: "",
-				backend: &S3Backend{
-					client: nil,
-					Region: "us-east-1",
-					Key:    "test.tfstate",
-					Bucket: "bucket1",
-				},
-			},
-		},
 	}
 
 	for _, tc := range testcases {
@@ -376,12 +308,6 @@ terraform {
 				t.Errorf("ValidConfigurationObject() error = %v, wantErr %v", err, tc.want.errMsg)
 				return
 			}
-			if got != nil {
-				if b, ok := got.(*S3Backend); ok {
-					b.client = nil
-					got.(*S3Backend).client = nil
-				}
-			}
 			if !reflect.DeepEqual(tc.want.backend, got) {
 				t.Errorf("got %#v, want %#v", got, tc.want.backend)
 			}

diff --git a/controllers/configuration/backend/s3.go b/controllers/configuration/backend/s3.go
@@ -47,8 +47,19 @@ func newS3Backend(_ client.Client, backendConf interface{}, credentials map[stri
 	if !ok || conf == nil {
 		return nil, fmt.Errorf("invalid backendConf, want *v1beta2.S3BackendConf, but got %#v", backendConf)
 	}
+
+	var region string
+	if conf.Region != nil && *conf.Region != "" {
+		region = *conf.Region
+	} else {
+		region = credentials[provider.EnvAWSDefaultRegion]
+	}
+	if region == "" {
+		return nil, errors.New("fail to get region when build s3 backend")
+	}
+
 	s3Backend := &S3Backend{
-		Region: conf.Region,
+		Region: region,
 		Key:    conf.Key,
 		Bucket: conf.Bucket,
 	}
@@ -73,9 +84,27 @@ func newS3Backend(_ client.Client, backendConf interface{}, credentials map[stri
 	}
 	s3Backend.client = s3.New(sess)
 
+	// check if the bucket exists
+	if err := s3Backend.checkBucketExists(); err != nil {
+		return nil, err
+	}
+
 	return s3Backend, nil
 }
 
+func (s *S3Backend) checkBucketExists() error {
+	bucketListOutput, err := s.client.ListBuckets(&s3.ListBucketsInput{})
+	if err != nil {
+		return fmt.Errorf("fail to list bucket when check if the bucket(%s) exists: %w", s.Bucket, err)
+	}
+	for _, bucket := range bucketListOutput.Buckets {
+		if bucket.Name != nil && *bucket.Name == s.Bucket {
+			return nil
+		}
+	}
+	return fmt.Errorf("fail to get bucket (%s), please make sure the bucket exists and the provider credentials have access to the bucket", s.Bucket)
+}
+
 func (s *S3Backend) getObject() (*s3.GetObjectOutput, error) {
 	input := &s3.GetObjectInput{
 		Key:    &s.Key,