Merge pull request #761 from cloudskiff/rework_s3_bucket_metric_supplier

Split s3_bucket_metrics_supplier

pkg/remote/aws/init.go

@@ -65,6 +65,8 @@ func Init(version string, alerter *alerter.Alerter,
 	remoteLibrary.AddDetailsFetcher(aws.AwsS3BucketInventoryResourceType, NewS3BucketInventoryDetailsFetcher(provider, deserializer))
 	remoteLibrary.AddEnumerator(NewS3BucketNotificationEnumerator(s3Repository, factory, provider.Config))
 	remoteLibrary.AddDetailsFetcher(aws.AwsS3BucketNotificationResourceType, NewS3BucketNotificationDetailsFetcher(provider, deserializer))
+	remoteLibrary.AddEnumerator(NewS3BucketMetricsEnumerator(s3Repository, factory, provider.Config))
+	remoteLibrary.AddDetailsFetcher(aws.AwsS3BucketMetricResourceType, NewS3BucketMetricsDetailsFetcher(provider, deserializer))
 
 	remoteLibrary.AddEnumerator(NewEC2EbsVolumeEnumerator(ec2repository, factory, provider.Config))
 	remoteLibrary.AddDetailsFetcher(aws.AwsEbsVolumeResourceType, common.NewGenericDetailsFetcher(aws.AwsEbsVolumeResourceType, provider, deserializer))
@@ -74,7 +76,6 @@ func Init(version string, alerter *alerter.Alerter,
 	remoteLibrary.AddDetailsFetcher(aws.AwsEipResourceType, common.NewGenericDetailsFetcher(aws.AwsEipResourceType, provider, deserializer))
 
 	supplierLibrary.AddSupplier(NewS3BucketAnalyticSupplier(provider, s3Repository, deserializer))
-	supplierLibrary.AddSupplier(NewS3BucketMetricSupplier(provider, s3Repository, deserializer))
 	supplierLibrary.AddSupplier(NewS3BucketPolicySupplier(provider, s3Repository, deserializer))
 	supplierLibrary.AddSupplier(NewEC2EipAssociationSupplier(provider, deserializer, ec2repository))
 	supplierLibrary.AddSupplier(NewRoute53ZoneSupplier(provider, deserializer, route53repository))

pkg/remote/aws/s3_bucket_metric_supplier_test.go

@@ -1,162 +0,0 @@
-package aws
-
-import (
-	"context"
-	"testing"
-
-	awssdk "github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/cloudskiff/driftctl/pkg/parallel"
-	"github.com/cloudskiff/driftctl/pkg/remote/aws/client"
-	"github.com/cloudskiff/driftctl/pkg/remote/aws/repository"
-	"github.com/cloudskiff/driftctl/pkg/remote/cache"
-	remoteerror "github.com/cloudskiff/driftctl/pkg/remote/error"
-	tf "github.com/cloudskiff/driftctl/pkg/remote/terraform"
-	"github.com/cloudskiff/driftctl/pkg/resource"
-	resourceaws "github.com/cloudskiff/driftctl/pkg/resource/aws"
-	testresource "github.com/cloudskiff/driftctl/test/resource"
-
-	"github.com/cloudskiff/driftctl/pkg/terraform"
-	"github.com/cloudskiff/driftctl/test"
-	"github.com/cloudskiff/driftctl/test/goldenfile"
-	"github.com/cloudskiff/driftctl/test/mocks"
-	"github.com/stretchr/testify/assert"
-)
-
-func TestS3BucketMetricSupplier_Resources(t *testing.T) {
-
-	tests := []struct {
-		test    string
-		dirName string
-		mocks   func(repository *repository.MockS3Repository)
-		wantErr error
-	}{
-		{
-			test: "multiple bucket with multiple metrics", dirName: "s3_bucket_metrics_multiple",
-			mocks: func(repository *repository.MockS3Repository) {
-				repository.On(
-					"ListAllBuckets",
-				).Return([]*s3.Bucket{
-					{Name: awssdk.String("bucket-martin-test-drift")},
-					{Name: awssdk.String("bucket-martin-test-drift2")},
-					{Name: awssdk.String("bucket-martin-test-drift3")},
-				}, nil)
-
-				repository.On(
-					"GetBucketLocation",
-					"bucket-martin-test-drift",
-				).Return(
-					"eu-west-1",
-					nil,
-				)
-
-				repository.On(
-					"GetBucketLocation",
-					"bucket-martin-test-drift2",
-				).Return(
-					"eu-west-3",
-					nil,
-				)
-
-				repository.On(
-					"GetBucketLocation",
-					"bucket-martin-test-drift3",
-				).Return(
-					"ap-northeast-1",
-					nil,
-				)
-
-				repository.On(
-					"ListBucketMetricsConfigurations",
-					&s3.Bucket{Name: awssdk.String("bucket-martin-test-drift2")},
-					"eu-west-3",
-				).Return(
-					[]*s3.MetricsConfiguration{
-						{Id: awssdk.String("Metrics_Bucket2")},
-						{Id: awssdk.String("Metrics2_Bucket2")},
-					},
-					nil,
-				)
-			},
-		},
-		{
-			test: "cannot list bucket", dirName: "s3_bucket_metrics_list_bucket",
-			mocks: func(repository *repository.MockS3Repository) {
-				repository.On("ListAllBuckets").Return(nil, awserr.NewRequestFailure(nil, 403, ""))
-			},
-			wantErr: remoteerror.NewResourceEnumerationErrorWithType(awserr.NewRequestFailure(nil, 403, ""), resourceaws.AwsS3BucketMetricResourceType, resourceaws.AwsS3BucketResourceType),
-		},
-		{
-			test: "cannot list metrics", dirName: "s3_bucket_metrics_list_metrics",
-			mocks: func(repository *repository.MockS3Repository) {
-				repository.On("ListAllBuckets").Return(
-					[]*s3.Bucket{
-						{Name: awssdk.String("bucket-martin-test-drift")},
-					},
-					nil,
-				)
-				repository.On(
-					"GetBucketLocation",
-					"bucket-martin-test-drift",
-				).Return(
-					"eu-west-3",
-					nil,
-				)
-
-				repository.On(
-					"ListBucketMetricsConfigurations",
-					&s3.Bucket{Name: awssdk.String("bucket-martin-test-drift")},
-					"eu-west-3",
-				).Return(
-					nil,
-					awserr.NewRequestFailure(nil, 403, ""),
-				)
-
-			},
-			wantErr: remoteerror.NewResourceEnumerationError(awserr.NewRequestFailure(nil, 403, ""), resourceaws.AwsS3BucketMetricResourceType),
-		},
-	}
-	for _, tt := range tests {
-		shouldUpdate := tt.dirName == *goldenfile.Update
-
-		providerLibrary := terraform.NewProviderLibrary()
-		supplierLibrary := resource.NewSupplierLibrary()
-
-		repo := testresource.InitFakeSchemaRepository("aws", "3.19.0")
-		resourceaws.InitResourcesMetadata(repo)
-		factory := terraform.NewTerraformResourceFactory(repo)
-
-		deserializer := resource.NewDeserializer(factory)
-		if shouldUpdate {
-			provider, err := InitTestAwsProvider(providerLibrary)
-			if err != nil {
-				t.Fatal(err)
-			}
-			repository := repository.NewS3Repository(client.NewAWSClientFactory(provider.session), cache.New(0))
-			supplierLibrary.AddSupplier(NewS3BucketMetricSupplier(provider, repository, deserializer))
-		}
-
-		t.Run(tt.test, func(t *testing.T) {
-
-			mock := repository.MockS3Repository{}
-			tt.mocks(&mock)
-
-			provider := mocks.NewMockedGoldenTFProvider(tt.dirName, providerLibrary.Provider(terraform.AWS), shouldUpdate)
-			s := &S3BucketMetricSupplier{
-				provider,
-				deserializer,
-				&mock,
-				terraform.NewParallelResourceReader(parallel.NewParallelRunner(context.TODO(), 10)),
-				tf.TerraformProviderConfig{
-					Name:         "test",
-					DefaultAlias: "eu-west-3",
-				},
-			}
-			got, err := s.Resources()
-			assert.Equal(t, err, tt.wantErr)
-
-			test.CtyTestDiff(got, tt.dirName, provider, deserializer, shouldUpdate, t)
-		})
-	}
-}

pkg/remote/aws/s3_bucket_metrics_detail_fetcher.go

@@ -0,0 +1,38 @@
+package aws
+
+import (
+	"github.com/cloudskiff/driftctl/pkg/resource"
+	"github.com/cloudskiff/driftctl/pkg/resource/aws"
+	"github.com/cloudskiff/driftctl/pkg/terraform"
+)
+
+type S3BucketMetricsDetailsFetcher struct {
+	reader       terraform.ResourceReader
+	deserializer *resource.Deserializer
+}
+
+func NewS3BucketMetricsDetailsFetcher(provider terraform.ResourceReader, deserializer *resource.Deserializer) *S3BucketMetricsDetailsFetcher {
+	return &S3BucketMetricsDetailsFetcher{
+		reader:       provider,
+		deserializer: deserializer,
+	}
+}
+
+func (r *S3BucketMetricsDetailsFetcher) ReadDetails(res resource.Resource) (resource.Resource, error) {
+	ctyVal, err := r.reader.ReadResource(terraform.ReadResourceArgs{
+		Ty: aws.AwsS3BucketMetricResourceType,
+		ID: res.TerraformId(),
+		Attributes: map[string]string{
+			"alias": *res.Attributes().GetString("region"),
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+	deserializedRes, err := r.deserializer.DeserializeOne(aws.AwsS3BucketMetricResourceType, *ctyVal)
+	if err != nil {
+		return nil, err
+	}
+
+	return deserializedRes, nil
+}

pkg/remote/aws/s3_bucket_metrics_enumerator.go

@@ -0,0 +1,74 @@
+package aws
+
+import (
+	"fmt"
+
+	"github.com/cloudskiff/driftctl/pkg/remote/aws/repository"
+	remoteerror "github.com/cloudskiff/driftctl/pkg/remote/error"
+	tf "github.com/cloudskiff/driftctl/pkg/remote/terraform"
+	"github.com/cloudskiff/driftctl/pkg/resource"
+	"github.com/cloudskiff/driftctl/pkg/resource/aws"
+	"github.com/sirupsen/logrus"
+)
+
+type S3BucketMetricsEnumerator struct {
+	repository     repository.S3Repository
+	factory        resource.ResourceFactory
+	providerConfig tf.TerraformProviderConfig
+}
+
+func NewS3BucketMetricsEnumerator(repo repository.S3Repository, factory resource.ResourceFactory, providerConfig tf.TerraformProviderConfig) *S3BucketMetricsEnumerator {
+	return &S3BucketMetricsEnumerator{
+		repository:     repo,
+		factory:        factory,
+		providerConfig: providerConfig,
+	}
+}
+
+func (e *S3BucketMetricsEnumerator) SupportedType() resource.ResourceType {
+	return aws.AwsS3BucketMetricResourceType
+}
+
+func (e *S3BucketMetricsEnumerator) Enumerate() ([]resource.Resource, error) {
+	buckets, err := e.repository.ListAllBuckets()
+	if err != nil {
+		return nil, remoteerror.NewResourceEnumerationErrorWithType(err, aws.AwsS3BucketMetricResourceType, aws.AwsS3BucketResourceType)
+	}
+
+	results := make([]resource.Resource, len(buckets))
+
+	for _, bucket := range buckets {
+		region, err := e.repository.GetBucketLocation(*bucket.Name)
+		if err != nil {
+			return nil, err
+		}
+		if region == "" || region != e.providerConfig.DefaultAlias {
+			logrus.WithFields(logrus.Fields{
+				"region": region,
+				"bucket": *bucket.Name,
+			}).Debug("Skipped bucket")
+			continue
+		}
+
+		metricsConfigurationList, err := e.repository.ListBucketMetricsConfigurations(bucket, region)
+		if err != nil {
+			return nil, remoteerror.NewResourceEnumerationError(err, aws.AwsS3BucketMetricResourceType)
+		}
+
+		for _, metric := range metricsConfigurationList {
+			id := fmt.Sprintf("%s:%s", *bucket.Name, *metric.Id)
+			results = append(
+				results,
+				e.factory.CreateAbstractResource(
+					string(e.SupportedType()),
+					id,
+					map[string]interface{}{
+						"region": region,
+					},
+				),
+			)
+		}
+	}
+
+	return results, err
+}

pkg/remote/aws/s3_bucket_metrics_supplier.go

@@ -1,88 +0,0 @@
-package aws
-
-import (
-	"fmt"
-
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/cloudskiff/driftctl/pkg/remote/aws/repository"
-	remoteerror "github.com/cloudskiff/driftctl/pkg/remote/error"
-	tf "github.com/cloudskiff/driftctl/pkg/remote/terraform"
-	"github.com/cloudskiff/driftctl/pkg/resource"
-	"github.com/cloudskiff/driftctl/pkg/resource/aws"
-
-	"github.com/cloudskiff/driftctl/pkg/terraform"
-	"github.com/zclconf/go-cty/cty"
-)
-
-type S3BucketMetricSupplier struct {
-	reader         terraform.ResourceReader
-	deserializer   *resource.Deserializer
-	repository     repository.S3Repository
-	runner         *terraform.ParallelResourceReader
-	providerConfig tf.TerraformProviderConfig
-}
-
-func NewS3BucketMetricSupplier(provider *AWSTerraformProvider, repository repository.S3Repository, deserializer *resource.Deserializer) *S3BucketMetricSupplier {
-	return &S3BucketMetricSupplier{
-		provider,
-		deserializer,
-		repository,
-		terraform.NewParallelResourceReader(provider.Runner().SubRunner()),
-		provider.Config,
-	}
-}
-
-func (s *S3BucketMetricSupplier) Resources() ([]resource.Resource, error) {
-	buckets, err := s.repository.ListAllBuckets()
-	if err != nil {
-		return nil, remoteerror.NewResourceEnumerationErrorWithType(err, aws.AwsS3BucketMetricResourceType, aws.AwsS3BucketResourceType)
-	}
-
-	for _, bucket := range buckets {
-		bucket := *bucket
-		region, err := s.repository.GetBucketLocation(*bucket.Name)
-		if err != nil {
-			return nil, err
-		}
-		if region == "" || region != s.providerConfig.DefaultAlias {
-			continue
-		}
-		if err := s.listBucketMetricConfiguration(&bucket, region); err != nil {
-			return nil, remoteerror.NewResourceEnumerationError(err, aws.AwsS3BucketMetricResourceType)
-		}
-	}
-	ctyVals, err := s.runner.Wait()
-	if err != nil {
-		return nil, err
-	}
-
-	return s.deserializer.Deserialize(aws.AwsS3BucketMetricResourceType, ctyVals)
-}
-
-func (s *S3BucketMetricSupplier) listBucketMetricConfiguration(bucket *s3.Bucket, region string) error {
-
-	metricsConfigurationList, err := s.repository.ListBucketMetricsConfigurations(bucket, region)
-	if err != nil {
-		return err
-	}
-
-	for _, config := range metricsConfigurationList {
-		id := fmt.Sprintf("%s:%s", *bucket.Name, *config.Id)
-		s.runner.Run(func() (cty.Value, error) {
-			s3BucketMetric, err := s.reader.ReadResource(
-				terraform.ReadResourceArgs{
-					Ty: aws.AwsS3BucketMetricResourceType,
-					ID: id,
-					Attributes: map[string]string{
-						"alias": region,
-					},
-				},
-			)
-			if err != nil {
-				return cty.NilVal, err
-			}
-			return *s3BucketMetric, err
-		})
-	}
-	return nil
-}

pkg/remote/s3_scanner_test.go

@@ -460,3 +460,154 @@ func TestS3BucketNotification(t *testing.T) {
 		})
 	}
 }
+
+func TestS3BucketMetrics(t *testing.T) {
+
+	tests := []struct {
+		test    string
+		dirName string
+		mocks   func(repository *repository.MockS3Repository)
+		wantErr error
+	}{
+		{
+			test: "multiple bucket with multiple metrics", dirName: "s3_bucket_metrics_multiple",
+			mocks: func(repository *repository.MockS3Repository) {
+				repository.On(
+					"ListAllBuckets",
+				).Return([]*s3.Bucket{
+					{Name: awssdk.String("bucket-martin-test-drift")},
+					{Name: awssdk.String("bucket-martin-test-drift2")},
+					{Name: awssdk.String("bucket-martin-test-drift3")},
+				}, nil)
+
+				repository.On(
+					"GetBucketLocation",
+					"bucket-martin-test-drift",
+				).Return(
+					"eu-west-1",
+					nil,
+				)
+
+				repository.On(
+					"GetBucketLocation",
+					"bucket-martin-test-drift2",
+				).Return(
+					"eu-west-3",
+					nil,
+				)
+
+				repository.On(
+					"GetBucketLocation",
+					"bucket-martin-test-drift3",
+				).Return(
+					"ap-northeast-1",
+					nil,
+				)
+
+				repository.On(
+					"ListBucketMetricsConfigurations",
+					&s3.Bucket{Name: awssdk.String("bucket-martin-test-drift2")},
+					"eu-west-3",
+				).Return(
+					[]*s3.MetricsConfiguration{
+						{Id: awssdk.String("Metrics_Bucket2")},
+						{Id: awssdk.String("Metrics2_Bucket2")},
+					},
+					nil,
+				)
+			},
+		},
+		{
+			test: "cannot list bucket", dirName: "s3_bucket_metrics_list_bucket",
+			mocks: func(repository *repository.MockS3Repository) {
+				repository.On("ListAllBuckets").Return(nil, awserr.NewRequestFailure(nil, 403, ""))
+			},
+			wantErr: remoteerror.NewResourceEnumerationErrorWithType(awserr.NewRequestFailure(nil, 403, ""), resourceaws.AwsS3BucketMetricResourceType, resourceaws.AwsS3BucketResourceType),
+		},
+		{
+			test: "cannot list metrics", dirName: "s3_bucket_metrics_list_metrics",
+			mocks: func(repository *repository.MockS3Repository) {
+				repository.On("ListAllBuckets").Return(
+					[]*s3.Bucket{
+						{Name: awssdk.String("bucket-martin-test-drift")},
+					},
+					nil,
+				)
+				repository.On(
+					"GetBucketLocation",
+					"bucket-martin-test-drift",
+				).Return(
+					"eu-west-3",
+					nil,
+				)
+
+				repository.On(
+					"ListBucketMetricsConfigurations",
+					&s3.Bucket{Name: awssdk.String("bucket-martin-test-drift")},
+					"eu-west-3",
+				).Return(
+					nil,
+					awserr.NewRequestFailure(nil, 403, ""),
+				)
+
+			},
+			wantErr: remoteerror.NewResourceEnumerationError(awserr.NewRequestFailure(nil, 403, ""), resourceaws.AwsS3BucketMetricResourceType),
+		},
+	}
+
+	schemaRepository := testresource.InitFakeSchemaRepository("aws", "3.19.0")
+	resourceaws.InitResourcesMetadata(schemaRepository)
+	factory := terraform.NewTerraformResourceFactory(schemaRepository)
+	deserializer := resource.NewDeserializer(factory)
+	alerter := &mocks.AlerterInterface{}
+
+	for _, c := range tests {
+		t.Run(c.test, func(tt *testing.T) {
+			shouldUpdate := c.dirName == *goldenfile.Update
+
+			session := session.Must(session.NewSessionWithOptions(session.Options{
+				SharedConfigState: session.SharedConfigEnable,
+			}))
+
+			scanOptions := ScannerOptions{Deep: true}
+			providerLibrary := terraform.NewProviderLibrary()
+			remoteLibrary := common.NewRemoteLibrary()
+
+			// Initialize mocks
+			fakeRepo := &repository.MockS3Repository{}
+			c.mocks(fakeRepo)
+			var repo repository.S3Repository = fakeRepo
+			providerVersion := "3.19.0"
+			realProvider, err := terraform2.InitTestAwsProvider(providerLibrary, providerVersion)
+			if err != nil {
+				t.Fatal(err)
+			}
+			provider := terraform2.NewFakeTerraformProvider(realProvider)
+			provider.WithResponse(c.dirName)
+
+			// Replace mock by real resources if we are in update mode
+			if shouldUpdate {
+				err := realProvider.Init()
+				if err != nil {
+					t.Fatal(err)
+				}
+				provider.ShouldUpdate()
+				repo = repository.NewS3Repository(client.NewAWSClientFactory(session), cache.New(0))
+			}
+
+			remoteLibrary.AddEnumerator(aws.NewS3BucketMetricsEnumerator(repo, factory, tf.TerraformProviderConfig{
+				Name:         "test",
+				DefaultAlias: "eu-west-3",
+			}))
+			remoteLibrary.AddDetailsFetcher(resourceaws.AwsS3BucketMetricResourceType, aws.NewS3BucketMetricsDetailsFetcher(provider, deserializer))
+
+			s := NewScanner(nil, remoteLibrary, alerter, scanOptions)
+			got, err := s.Resources()
+			assert.Equal(tt, err, c.wantErr)
+			if err != nil {
+				return
+			}
+			test.TestAgainstGoldenFile(got, resourceaws.AwsS3BucketMetricResourceType, c.dirName, provider, deserializer, shouldUpdate, tt)
+		})
+	}
+}