Aws bugs fixes and enhancements (fix 4+m initialization of aws shell/scans) (#6454)

* Race condition fix

* Filter propagation

* Skip child discovery

* S3 Optimization

* Retry logic to reduce bad connection time

* Documentation

* fix unintended bug

* rds cluster / instance description

* simplify nil handling for child filters

* address review s3 optimization

* add tests for discovery and filter propagation

* propagation adressed by review

* fix and add tests for parser handeling

* fix s3 client side initalization

Author: LittleSalkin1806

providers-sdk/v1/inventory/inventory.go

@@ -4,6 +4,7 @@
 package inventory
 
 import (
+	"maps"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -455,9 +456,7 @@ func (cfg *Config) Clone(opts ...CloneOption) *Config {
 			clonedObject.Discover = &Discovery{}
 		}
 		clonedObject.Discover.Filter = make(map[string]string)
-		for k, v := range cfg.Discover.Filter {
-			clonedObject.Discover.Filter[k] = v
-		}
+		maps.Copy(clonedObject.Discover.Filter, cfg.Discover.GetFilter())
 	}
 
 	return clonedObject

providers/aws/config/config.go

@@ -46,7 +46,7 @@ Examples:
 	cnspec scan aws ec2 instance-connect <user@host>
 	cnspec scan aws ec2 instance-connect <user@host> --identity-file <path>
 	cnspec scan aws ec2 ebs <snapshot-id>
-	cnspec scan aws --filters region=ap-south-1
+	cnspec scan aws --filters regions=ap-south-1,us-east-1
 
 Notes:
   If you set the AWS_PROFILE environment variable, you can omit the profile flag.
@@ -131,7 +131,7 @@ Notes:
 					Long:    "filters",
 					Type:    plugin.FlagType_KeyValue,
 					Default: "",
-					Desc:    "Filter options, e.g., --filters region=us-east-2, --filters ec2:instance-ids=i-093439483935",
+					Desc:    "Filter options, e.g., --filters regions=us-east-2,us-east-1 , --filters ec2:instance-ids=i-093439483935",
 				},
 			},
 		},

providers/aws/connection/connection.go

@@ -11,6 +11,7 @@ import (
 	"maps"
 	"net/http"
 	"slices"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
@@ -75,9 +76,11 @@ func NewAwsConnection(id uint32, asset *inventory.Asset, conf *inventory.Config)
 	for _, opt := range opts {
 		opt(c)
 	}
-	// custom retry client
+	// custom retry client with reduced retries and shorter backoff
+	// to avoid excessive delays when regions are unreachable
 	retryClient := retryablehttp.NewClient()
-	retryClient.RetryMax = 5
+	retryClient.RetryMax = 2              // reduced from 5 to avoid long delays on unreachable regions
+	retryClient.RetryWaitMax = 10 * time.Second        // cap at 10s instead of 30s
 	retryClient.Logger = zerologadapter.New(log.Logger)
 	c.awsConfigOptions = append(c.awsConfigOptions, config.WithHTTPClient(retryClient.StandardClient()))
 
@@ -328,6 +331,10 @@ func (h *AwsConnection) Regions() ([]string, error) {
 	svc := h.Ec2(h.cfg.Region)
 	ctx := context.Background()
 
+	// DescribeRegions works to get the list of enabled regions for the account ( each account of organization)
+	// but this does not mean the respective service endpoint is available in that region. They will timeout instead of failing fast
+	// (e.g. EKS,KMS,Sagemaker is for example not available in ap-southeast-1 etc)
+	// This also does not cover SCPs that might block access to certain regions.
 	res, err := svc.DescribeRegions(ctx, &ec2.DescribeRegionsInput{})
 	if err != nil {
 		log.Warn().Err(err).Msg("unable to describe regions")

providers/aws/provider/provider.go

@@ -54,9 +54,6 @@ func (s *Service) ParseCLI(req *plugin.ParseCLIReq) (*plugin.ParseCLIRes, error)
 		return &plugin.ParseCLIRes{Asset: asset}, nil
 	}
 
-	inventoryConfig := &inventory.Config{
-		Type: req.Connector,
-	}
 	// discovery flags
 	discoverTargets := []string{}
 	if x, ok := flags["discover"]; ok && len(x.Array) != 0 {
@@ -67,7 +64,14 @@ func (s *Service) ParseCLI(req *plugin.ParseCLIReq) (*plugin.ParseCLIRes, error)
 	}
 	filterOpts := parseFlagsToFiltersOpts(flags)
 
-	inventoryConfig.Discover = &inventory.Discovery{Targets: discoverTargets, Filter: filterOpts}
+	if len(discoverTargets) == 0 {
+		discoverTargets = []string{resources.DiscoveryAuto}
+	}
+
+	inventoryConfig := &inventory.Config{
+		Type:     req.Connector,
+		Discover: &inventory.Discovery{Targets: discoverTargets, Filter: filterOpts},
+	}
 	asset := inventory.Asset{
 		Connections: []*inventory.Config{inventoryConfig},
 		Options:     opts,
@@ -289,7 +293,7 @@ func (s *Service) detect(asset *inventory.Asset, conn plugin.Connection) error {
 }
 
 func (s *Service) discover(conn *connection.AwsConnection) (*inventory.Inventory, error) {
-	if conn.Conf.Discover == nil {
+	if len(conn.Conf.GetDiscover().GetTargets()) == 0 {
 		return nil, nil
 	}
 

providers/aws/resources/aws.lr

@@ -2739,7 +2739,7 @@ aws.rds.dbcluster @defaults("id region engine engineVersion") {
   engine string
   // Version of the database engine for this DB cluster
   engineVersion string
-  // Whether the cluster is publicly accessible
+  // Whether the cluster is publicly accessible. Note: This will only return a value for non-Aurora Multi-AZ DB clusters
   publiclyAccessible bool
   // Whether the cluster is a Multi-AZ deployment
   multiAZ bool
@@ -2847,7 +2847,7 @@ aws.rds.dbinstance @defaults("id region engine engineVersion") {
   region string
   // Availability zone where the instance exists
   availabilityZone string
-  // Whether the instance is publicly accessible. Note: This will only return a value for non-Aurora Multi-AZ DB clusters
+  // Whether the instance is publicly accessible
   publiclyAccessible bool
   // List of log types the instance is configured to export to CloudWatch logs
   enabledCloudwatchLogsExports []string

providers/aws/resources/aws_s3.go

@@ -57,51 +57,48 @@ func (a *mqlAwsS3) id() (string, error) {
 
 func (a *mqlAwsS3) buckets() ([]any, error) {
 	conn := a.MqlRuntime.Connection.(*connection.AwsConnection)
-
-	svc := conn.S3("")
 	ctx := context.Background()
 
-	totalBuckets := make([]s3types.Bucket, 0)
-	params := &s3.ListBucketsInput{}
-	paginator := s3.NewListBucketsPaginator(svc, params, func(o *s3.ListBucketsPaginatorOptions) {
-		o.Limit = 100
-	})
-	for paginator.HasMorePages() {
-		output, err := paginator.NextPage(ctx)
-		if err != nil {
-			return nil, err
-		}
-		totalBuckets = append(totalBuckets, output.Buckets...)
+	configuredRegions, err := conn.Regions()
+	if err != nil {
+		return nil, err
 	}
 
-	res := []any{}
-	for _, bucket := range totalBuckets {
-		location, err := svc.GetBucketLocation(ctx, &s3.GetBucketLocationInput{
-			Bucket: bucket.Name,
+	type bucketWithRegion struct {
+		bucket s3types.Bucket
+		region string
+	}
+	var bucketsWithRegions []bucketWithRegion
+
+	for _, region := range configuredRegions {
+		svc := conn.S3(region)
+		log.Debug().Str("region", region).Msg("listing S3 buckets in region")
+		params := &s3.ListBucketsInput{BucketRegion: aws.String(region)}
+		paginator := s3.NewListBucketsPaginator(svc, params, func(o *s3.ListBucketsPaginatorOptions) {
+			o.Limit = 100
 		})
-		if err != nil {
-			log.Error().Err(err).Str("bucket", *bucket.Name).Msg("Could not get bucket location")
-			continue
-		}
-		if location == nil {
-			log.Error().Err(err).Str("bucket", *bucket.Name).Msg("Could not get bucket location (returned null)")
-			continue
+		for paginator.HasMorePages() {
+			output, err := paginator.NextPage(ctx)
+			if err != nil {
+				log.Warn().Err(err).Str("region", region).Msg("could not list S3 buckets in region")
+				break
+			}
+			for _, bucket := range output.Buckets {
+				bucketsWithRegions = append(bucketsWithRegions, bucketWithRegion{bucket: bucket, region: region})
+			}
 		}
+	}
 
-		region := string(location.LocationConstraint)
-		// us-east-1 returns "" therefore we set it explicitly
-		// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLocation.html#API_GetBucketLocation_ResponseSyntax
-		if region == "" {
-			region = "us-east-1"
-		}
+	res := []any{}
+	for _, bwr := range bucketsWithRegions {
 		mqlS3Bucket, err := CreateResource(a.MqlRuntime, ResourceAwsS3Bucket,
 			map[string]*llx.RawData{
-				"name":        llx.StringDataPtr(bucket.Name),
-				"arn":         llx.StringData(fmt.Sprintf(s3ArnPattern, convert.ToValue(bucket.Name))),
+				"name":        llx.StringDataPtr(bwr.bucket.Name),
+				"arn":         llx.StringData(fmt.Sprintf(s3ArnPattern, convert.ToValue(bwr.bucket.Name))),
 				"exists":      llx.BoolData(true),
-				"location":    llx.StringData(region),
-				"createdTime": llx.TimeDataPtr(bucket.CreationDate),
-				"createdAt":   llx.TimeDataPtr(bucket.CreationDate),
+				"location":    llx.StringData(bwr.region),
+				"createdTime": llx.TimeDataPtr(bwr.bucket.CreationDate),
+				"createdAt":   llx.TimeDataPtr(bwr.bucket.CreationDate),
 			})
 		if err != nil {
 			return nil, err
@@ -115,7 +112,6 @@ func (a *mqlAwsS3) buckets() ([]any, error) {
 			}
 
 			if conn.Filters.General.IsFilteredOutByTags(mapStringInterfaceToStringString(tags)) {
-				log.Debug().Interface("log_group", bucket.Name).Msg("excluding log group due to filters")
 				continue
 			}
 		}

providers/aws/resources/discovery.go

@@ -151,11 +151,7 @@ func Discover(runtime *plugin.Runtime) (*inventory.Inventory, error) {
 }
 
 func getDiscoveryTargets(config *inventory.Config) []string {
-	targets := config.Discover.Targets
-
-	if len(targets) == 0 {
-		return Auto
-	}
+	targets := config.GetDiscover().GetTargets()
 
 	if stringx.Contains(targets, DiscoveryAll) {
 		// return the All list + All Api Resources list

providers/aws/resources/discovery_conversion.go

@@ -75,16 +75,17 @@ func MqlObjectToAsset(account string, mqlObject mqlObject, conn *connection.AwsC
 		return nil
 	}
 	platformid := MondooObjectID(mqlObject.awsObject)
-	t := conn.Conf
-	t.PlatformId = platformid
+	// Clone first to avoid mutating the parent connection's config (race condition)
+	clonedConfig := conn.Conf.Clone(inventory.WithoutDiscovery(), inventory.WithParentConnectionId(conn.Conf.Id), inventory.WithFilters())
+	clonedConfig.PlatformId = platformid
 	platformIds := []string{platformid, mqlObject.awsObject.arn}
 	platformIds = append(platformIds, mqlObject.platformIds...)
 	return &inventory.Asset{
 		PlatformIds: platformIds,
 		Name:        mqlObject.name,
 		Platform:    connection.GetPlatformForObject(platformName, account),
 		Labels:      mqlObject.labels,
-		Connections: []*inventory.Config{t.Clone(inventory.WithoutDiscovery(), inventory.WithParentConnectionId(t.Id))},
+		Connections: []*inventory.Config{clonedConfig},
 		Options:     conn.ConnectionOptions(),
 	}
 }
@@ -226,11 +227,14 @@ func accountAsset(conn *connection.AwsConnection, awsAccount *mqlAwsAccount) *in
 
 	id := "//platformid.api.mondoo.app/runtime/aws/accounts/" + accountId
 	accountArn := "arn:aws:sts::" + accountId
+	// Clone first to avoid mutating the parent connection's config
+	clonedConfig := conn.Conf.Clone(inventory.WithoutDiscovery(), inventory.WithParentConnectionId(conn.Conf.Id), inventory.WithFilters())
+	clonedConfig.PlatformId = id
 	return &inventory.Asset{
 		PlatformIds: []string{id, accountArn},
 		Name:        name,
 		Platform:    connection.GetPlatformForObject("", accountId),
-		Connections: []*inventory.Config{conn.Conf.Clone(inventory.WithoutDiscovery(), inventory.WithParentConnectionId(conn.Conf.Id), inventory.WithFilters())},
+		Connections: []*inventory.Config{clonedConfig},
 		Options:     conn.ConnectionOptions(),
 	}
 }