From 948d2e819b36b734e96e23dd913cc8d897f2eef0 Mon Sep 17 00:00:00 2001 From: Evan Nemerson Date: Wed, 18 Oct 2023 09:48:33 -0400 Subject: [PATCH] fixes ported from Fugue --- internal/service/account/alternate_contact.go | 4 +- internal/service/apigateway/domain_name.go | 2 - internal/service/cloudfront/forge.go | 4 +- internal/service/cloudtrail/cloudtrail.go | 7 +- internal/service/cognitoidp/user_pool.go | 8 ++ internal/service/ds/directory.go | 3 +- internal/service/ec2/ec2_instance.go | 93 +++++++++++++- .../service/ec2/ec2_spot_fleet_request.go | 20 +-- internal/service/ec2/vpc_.go | 5 + internal/service/ec2/vpc_endpoint_service.go | 2 +- internal/service/ec2/vpc_network_interface.go | 2 +- .../service/organizations/organization.go | 96 +++++++-------- internal/service/s3/bucket.go | 115 +++++++++++++++++- .../s3/bucket_analytics_configuration.go | 6 + internal/service/s3/bucket_inventory.go | 6 + internal/service/s3/bucket_metric.go | 6 + internal/service/s3/bucket_notification.go | 6 + internal/service/s3/bucket_policy.go | 6 + .../service/s3/bucket_public_access_block.go | 6 + internal/service/ssm/parameter.go | 2 +- 20 files changed, 326 insertions(+), 73 deletions(-) diff --git a/internal/service/account/alternate_contact.go b/internal/service/account/alternate_contact.go index ea0f59edeef..f053b9d71fc 100644 --- a/internal/service/account/alternate_contact.go +++ b/internal/service/account/alternate_contact.go @@ -135,7 +135,9 @@ func resourceAlternateContactRead(ctx context.Context, d *schema.ResourceData, m return sdkdiag.AppendFromErr(diags, err) } - output, err := findAlternateContactByTwoPartKey(ctx, conn, accountID, contactType) + // AccountID is replaced by empty string because it must be a member account in the org if used + // See https://docs.aws.amazon.com/accounts/latest/reference/API_GetAlternateContact.html#API_GetAlternateContact_RequestSyntax + output, err := findAlternateContactByTwoPartKey(ctx, conn, "", contactType) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Account Alternate Contact (%s) not found, removing from state", d.Id()) diff --git a/internal/service/apigateway/domain_name.go b/internal/service/apigateway/domain_name.go index 78232f222be..e67cf79e0c1 100644 --- a/internal/service/apigateway/domain_name.go +++ b/internal/service/apigateway/domain_name.go @@ -107,8 +107,6 @@ func resourceDomainName() *schema.Resource { Type: schema.TypeList, Required: true, MinItems: 1, - // BadRequestException: Cannot create an api with multiple Endpoint Types - MaxItems: 1, Elem: &schema.Schema{ Type: schema.TypeString, ValidateFunc: validation.StringInSlice(enum.Slice(types.EndpointTypeEdge, types.EndpointTypeRegional), false), diff --git a/internal/service/cloudfront/forge.go b/internal/service/cloudfront/forge.go index 4343b4143bb..c81e2a07390 100644 --- a/internal/service/cloudfront/forge.go +++ b/internal/service/cloudfront/forge.go @@ -13,7 +13,9 @@ func sortInterfaceSlice(in []interface{}) []interface{} { a := []string{} b := []interface{}{} for _, v := range in { - a = append(a, v.(string)) + if v != nil { + a = append(a, v.(string)) + } } sort.Strings(a) diff --git a/internal/service/cloudtrail/cloudtrail.go b/internal/service/cloudtrail/cloudtrail.go index e932e48b536..b0455401e23 100644 --- a/internal/service/cloudtrail/cloudtrail.go +++ b/internal/service/cloudtrail/cloudtrail.go @@ -706,10 +706,11 @@ func expandEventSelectorDataResource(configured []interface{}) []types.DataResou func flattenEventSelector(configured []types.EventSelector) []map[string]interface{} { eventSelectors := make([]map[string]interface{}, 0, len(configured)) + // We want to output all the selectors (note for advanced event selectors this is empty) // Prevent default configurations shows differences - if len(configured) == 1 && len(configured[0].DataResources) == 0 && configured[0].ReadWriteType == types.ReadWriteTypeAll && len(configured[0].ExcludeManagementEventSources) == 0 { - return eventSelectors - } + // if len(configured) == 1 && len(configured[0].DataResources) == 0 && configured[0].ReadWriteType == types.ReadWriteTypeAll && len(configured[0].ExcludeManagementEventSources) == 0 { + // return eventSelectors + // } for _, raw := range configured { item := make(map[string]interface{}) diff --git a/internal/service/cognitoidp/user_pool.go b/internal/service/cognitoidp/user_pool.go index 838d127d009..2b492db099c 100644 --- a/internal/service/cognitoidp/user_pool.go +++ b/internal/service/cognitoidp/user_pool.go @@ -896,6 +896,14 @@ func resourceUserPoolRead(ctx context.Context, d *schema.ResourceData, meta inte setTagsOut(ctx, userPool.UserPoolTags) + // Set mfa_configuration the existing way if available + if userPool.MfaConfiguration != nil { + d.Set("mfa_configuration", *userPool.MfaConfiguration) + } + + // Try to fetch it via the new operation and set mfa_configuration / software_token_mfa_configuration + // This requires new permissions to succeed + input := &cognitoidentityprovider.GetUserPoolMfaConfigInput{ UserPoolId: aws.String(d.Id()), } diff --git a/internal/service/ds/directory.go b/internal/service/ds/directory.go index 33252ddb7db..14ecb7df4a4 100644 --- a/internal/service/ds/directory.go +++ b/internal/service/ds/directory.go @@ -143,7 +143,8 @@ func ResourceDirectory() *schema.Resource { }, "password": { Type: schema.TypeString, - Required: true, + Optional: true, + Computed: true, ForceNew: true, Sensitive: true, }, diff --git a/internal/service/ec2/ec2_instance.go b/internal/service/ec2/ec2_instance.go index 59c0704a22d..1621904ccc2 100644 --- a/internal/service/ec2/ec2_instance.go +++ b/internal/service/ec2/ec2_instance.go @@ -71,6 +71,31 @@ func ResourceInstance() *schema.Resource { Optional: true, AtLeastOneOf: []string{"ami", "launch_template"}, }, + + // Snyk: custom attributes begin + + "ami_owner_id": { + Type: schema.TypeString, + Computed: true, + }, + + "ami_creation_date": { + Type: schema.TypeString, + Computed: true, + }, + + "ami_platform_details": { + Type: schema.TypeString, + Computed: true, + }, + + "launch_time": { + Type: schema.TypeString, + Computed: true, + }, + + // Snyk: custom attributes end + "arn": { Type: schema.TypeString, Computed: true, @@ -1211,6 +1236,13 @@ func resourceInstanceRead(ctx context.Context, d *schema.ResourceData, meta inte d.Set("iam_instance_profile", nil) } + if err := readImageAttributes(d, conn); err != nil { + return sdkdiag.AppendErrorf(diags, "unable to read image attributes: %s", err) + } + if instance.LaunchTime != nil { + d.Set("launch_time", instance.LaunchTime.Format(time.RFC3339)) + } + { launchTemplate, err := flattenInstanceLaunchTemplate(ctx, conn, d.Id(), d.Get("launch_template.0.version").(string)) @@ -2192,12 +2224,64 @@ func disableInstanceAPIStop(ctx context.Context, conn *ec2.EC2, id string, disab return nil } +func readImageAttributes(d *schema.ResourceData, conn *ec2.EC2) error { + + imageID := d.Get("ami").(string) + var image *ec2.Image + + err := retry.Retry(1*time.Minute, func() *retry.RetryError { + res, err := conn.DescribeImages(&ec2.DescribeImagesInput{ + ImageIds: []*string{aws.String(imageID)}, + }) + if isResourceTimeoutError(err) { + return retry.RetryableError(err) + } + if tfawserr.ErrCodeEquals(err, "InvalidAMIID.Unavailable") || tfawserr.ErrCodeEquals(err, "InvalidAMIID.NotFound") { + return nil + } + if err != nil { + return retry.NonRetryableError(err) + } + if len(res.Images) == 0 { + return nil + } + image = res.Images[0] + return nil + }) + if err != nil { + return fmt.Errorf("Unable to describe AMI after retries: %s", err) + } + // Don't fail the refresh if the AMI was not found + if image == nil { + return nil + } + + if image.OwnerId != nil { + d.Set("ami_owner_id", image.OwnerId) + } + if image.PlatformDetails != nil { + d.Set("ami_platform_details", image.PlatformDetails) + } + if image.CreationDate != nil { + d.Set("ami_creation_date", image.CreationDate) + } + return nil +} + +func isResourceTimeoutError(err error) bool { + timeoutErr, ok := err.(*retry.TimeoutError) + return ok && timeoutErr.LastError == nil +} + func disableInstanceAPITermination(ctx context.Context, conn *ec2.EC2, id string, disableAPITermination bool) error { + // false = enable api termination + // true = disable api termination (protected) + input := &ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(id), DisableApiTermination: &ec2.AttributeBooleanValue{ Value: aws.Bool(disableAPITermination), }, - InstanceId: aws.String(id), } _, err := conn.ModifyInstanceAttributeWithContext(ctx, input) @@ -2317,6 +2401,10 @@ func readBlockDevicesFromInstance(ctx context.Context, d *schema.ResourceData, m VolumeIds: volIDs, }) if err != nil { + if tfawserr.ErrMessageContains(err, errCodeInvalidVolumeNotFound, "does not exist") { + log.Print("[WARN] Unable to describe volumes attached to instance") + return blockDevices, nil + } return nil, err } @@ -2433,6 +2521,9 @@ func FetchRootDeviceName(ctx context.Context, conn *ec2.EC2, amiID string) (*str image, err := FindImageByID(ctx, conn, amiID) + if tfawserr.ErrCodeEquals(err, errCodeInvalidAMIIDUnavailable) || tfawserr.ErrCodeEquals(err, "InvalidAMIID.NotFound") { + return nil, nil + } if err != nil { return nil, err } diff --git a/internal/service/ec2/ec2_spot_fleet_request.go b/internal/service/ec2/ec2_spot_fleet_request.go index 2aa806f1550..3618ffc31c1 100644 --- a/internal/service/ec2/ec2_spot_fleet_request.go +++ b/internal/service/ec2/ec2_spot_fleet_request.go @@ -2092,15 +2092,21 @@ func hashRootBlockDevice(v interface{}) int { func hashLaunchSpecification(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%s-", m["ami"].(string))) - if v, ok := m["availability_zone"].(string); ok && v != "" { - buf.WriteString(fmt.Sprintf("%s-", v)) + if _, ok := m["ami"]; ok { + buf.WriteString(fmt.Sprintf("%s-", m["ami"].(string))) } - if v, ok := m["subnet_id"].(string); ok && v != "" { - buf.WriteString(fmt.Sprintf("%s-", v)) + if _, ok := m["availability_zone"]; ok { + buf.WriteString(fmt.Sprintf("%s-", m["availability_zone"].(string))) + } + if _, ok := m["subnet_id"]; ok { + buf.WriteString(fmt.Sprintf("%s-", m["subnet_id"].(string))) + } + if _, ok := m["instance_type"]; ok { + buf.WriteString(fmt.Sprintf("%s-", m["instance_type"].(string))) + } + if _, ok := m["spot_price"]; ok { + buf.WriteString(fmt.Sprintf("%s-", m["spot_price"].(string))) } - buf.WriteString(fmt.Sprintf("%s-", m["instance_type"].(string))) - buf.WriteString(fmt.Sprintf("%s-", m["spot_price"].(string))) return create.StringHashcode(buf.String()) } diff --git a/internal/service/ec2/vpc_.go b/internal/service/ec2/vpc_.go index a6aac4071a4..34fbcff55b6 100644 --- a/internal/service/ec2/vpc_.go +++ b/internal/service/ec2/vpc_.go @@ -161,6 +161,10 @@ func ResourceVPC() *schema.Resource { ConflictsWith: []string{"ipv6_cidr_block"}, RequiredWith: []string{"ipv6_ipam_pool_id"}, }, + "is_default": { + Type: schema.TypeBool, + Computed: true, + }, "main_route_table_id": { Type: schema.TypeString, Computed: true, @@ -287,6 +291,7 @@ func resourceVPCRead(ctx context.Context, d *schema.ResourceData, meta interface d.Set("dhcp_options_id", vpc.DhcpOptionsId) d.Set("instance_tenancy", vpc.InstanceTenancy) d.Set("owner_id", ownerID) + d.Set("is_default", vpc.IsDefault) if v, err := tfresource.RetryWhenNewResourceNotFound(ctx, ec2PropagationTimeout, func() (interface{}, error) { return findVPCAttributeV2(ctx, conn, d.Id(), types.VpcAttributeNameEnableDnsHostnames) diff --git a/internal/service/ec2/vpc_endpoint_service.go b/internal/service/ec2/vpc_endpoint_service.go index 70595104a03..07fd876a5ab 100644 --- a/internal/service/ec2/vpc_endpoint_service.go +++ b/internal/service/ec2/vpc_endpoint_service.go @@ -80,7 +80,7 @@ func ResourceVPCEndpointService() *schema.Resource { "network_load_balancer_arns": { Type: schema.TypeSet, Optional: true, - MinItems: 1, + // MinItems: 1, Elem: &schema.Schema{ Type: schema.TypeString, ValidateFunc: verify.ValidARN, diff --git a/internal/service/ec2/vpc_network_interface.go b/internal/service/ec2/vpc_network_interface.go index c65303df69c..21e91aed590 100644 --- a/internal/service/ec2/vpc_network_interface.go +++ b/internal/service/ec2/vpc_network_interface.go @@ -517,7 +517,7 @@ func resourceNetworkInterfaceRead(ctx context.Context, d *schema.ResourceData, m Resource: "network-interface/" + d.Id(), }.String() d.Set("arn", arn) - if eni.Attachment != nil { + if eni.Attachment != nil && eni.Attachment.DeviceIndex != nil && eni.Attachment.AttachmentId != nil { if err := d.Set("attachment", []interface{}{flattenNetworkInterfaceAttachment(eni.Attachment)}); err != nil { return sdkdiag.AppendErrorf(diags, "setting attachment: %s", err) } diff --git a/internal/service/organizations/organization.go b/internal/service/organizations/organization.go index df30494cc0b..11a2cf9e647 100644 --- a/internal/service/organizations/organization.go +++ b/internal/service/organizations/organization.go @@ -253,67 +253,67 @@ func resourceOrganizationRead(ctx context.Context, d *schema.ResourceData, meta return sdkdiag.AppendErrorf(diags, "reading Organization: %s", err) } - accounts, err := findAccounts(ctx, conn) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Organization (%s) accounts: %s", d.Id(), err) - } - - managementAccountID := aws.StringValue(org.MasterAccountId) - managementAccountName := "" - for _, v := range accounts { - if aws.StringValue(v.Id) == managementAccountID { - managementAccountName = aws.StringValue(v.Name) - } - } - nonManagementAccounts := tfslices.Filter(accounts, func(v *organizations.Account) bool { - return aws.StringValue(v.Id) != managementAccountID - }) - - roots, err := findRoots(ctx, conn) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Organization (%s) roots: %s", d.Id(), err) - } - - if err := d.Set("accounts", flattenAccounts(accounts)); err != nil { - return sdkdiag.AppendErrorf(diags, "setting accounts: %s", err) - } + // accounts, err := findAccounts(ctx, conn) + + // if err != nil { + // return sdkdiag.AppendErrorf(diags, "reading Organization (%s) accounts: %s", d.Id(), err) + // } + + // managementAccountID := aws.StringValue(org.MasterAccountId) + // managementAccountName := "" + // for _, v := range accounts { + // if aws.StringValue(v.Id) == managementAccountID { + // managementAccountName = aws.StringValue(v.Name) + // } + // } + // nonManagementAccounts := tfslices.Filter(accounts, func(v *organizations.Account) bool { + // return aws.StringValue(v.Id) != managementAccountID + // }) + + // roots, err := findRoots(ctx, conn) + + // if err != nil { + // return sdkdiag.AppendErrorf(diags, "reading Organization (%s) roots: %s", d.Id(), err) + // } + + // if err := d.Set("accounts", flattenAccounts(accounts)); err != nil { + // return sdkdiag.AppendErrorf(diags, "setting accounts: %s", err) + // } d.Set("arn", org.Arn) d.Set("feature_set", org.FeatureSet) d.Set("master_account_arn", org.MasterAccountArn) d.Set("master_account_email", org.MasterAccountEmail) d.Set("master_account_id", org.MasterAccountId) - d.Set("master_account_name", managementAccountName) - if err := d.Set("non_master_accounts", flattenAccounts(nonManagementAccounts)); err != nil { - return sdkdiag.AppendErrorf(diags, "setting non_master_accounts: %s", err) - } - if err := d.Set("roots", flattenRoots(roots)); err != nil { - return sdkdiag.AppendErrorf(diags, "setting roots: %s", err) - } + // d.Set("master_account_name", managementAccountName) + // if err := d.Set("non_master_accounts", flattenAccounts(nonManagementAccounts)); err != nil { + // return sdkdiag.AppendErrorf(diags, "setting non_master_accounts: %s", err) + // } + // if err := d.Set("roots", flattenRoots(roots)); err != nil { + // return sdkdiag.AppendErrorf(diags, "setting roots: %s", err) + // } - var awsServiceAccessPrincipals []string + // var awsServiceAccessPrincipals []string // ConstraintViolationException: The request failed because the organization does not have all features enabled. Please enable all features in your organization and then retry. - if aws.StringValue(org.FeatureSet) == organizations.OrganizationFeatureSetAll { - awsServiceAccessPrincipals, err = FindEnabledServicePrincipalNames(ctx, conn) + // if aws.StringValue(org.FeatureSet) == organizations.OrganizationFeatureSetAll { + // awsServiceAccessPrincipals, err = FindEnabledServicePrincipalNames(ctx, conn) - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Organization (%s) service principals: %s", d.Id(), err) - } - } + // if err != nil { + // return sdkdiag.AppendErrorf(diags, "reading Organization (%s) service principals: %s", d.Id(), err) + // } + // } - d.Set("aws_service_access_principals", awsServiceAccessPrincipals) + // d.Set("aws_service_access_principals", awsServiceAccessPrincipals) - var enabledPolicyTypes []string + // var enabledPolicyTypes []string - for _, policyType := range roots[0].PolicyTypes { - if aws.StringValue(policyType.Status) == organizations.PolicyTypeStatusEnabled { - enabledPolicyTypes = append(enabledPolicyTypes, aws.StringValue(policyType.Type)) - } - } + // for _, policyType := range roots[0].PolicyTypes { + // if aws.StringValue(policyType.Status) == organizations.PolicyTypeStatusEnabled { + // enabledPolicyTypes = append(enabledPolicyTypes, aws.StringValue(policyType.Type)) + // } + // } - d.Set("enabled_policy_types", enabledPolicyTypes) + // d.Set("enabled_policy_types", enabledPolicyTypes) return diags } diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 50b88c53bef..bf0f6d0e66c 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -814,6 +814,11 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findBucketPolicy(ctx, conn, d.Id()) }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("permissions error on S3 Bucket (%s) while getting encryption configuration: %s", d.Id(), err) + } + // The call to HeadBucket above can occasionally return no error (i.e. NoSuchBucket) // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls // such as GetBucketPolicy, the error should be caught for non-new buckets as follows. @@ -844,6 +849,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findBucketACL(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting grant ACL configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAcl, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -868,12 +881,29 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findCORSRules(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting CORS configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketCors, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") return diags } + // RM-4400 - more descriptive 403 errors + if err != nil && !tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + return sdkdiag.AppendErrorf(diags, "permissions error on S3 Bucket (%s) while getting CORS configuration: %s", d.Id(), err) + } + + if err != nil && !tfawserr.ErrCodeEquals(err, errCodeNoSuchCORSConfiguration, errCodeNotImplemented, errCodeXNotImplemented) { + return sdkdiag.AppendErrorf(diags, "getting S3 Bucket CORS configuration: %s", err) + } + switch { case err == nil: if err := d.Set("cors_rule", flattenBucketCORSRules(corsRules)); err != nil { @@ -897,7 +927,10 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf d.SetId("") return diags } - + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting website endpoint: %s", d.Id(), err) + } switch { case err == nil: website, err := flattenBucketWebsite(bucketWebsite) @@ -920,6 +953,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findBucketVersioning(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting versioning configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketVersioning, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -944,6 +985,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findBucketAccelerateConfiguration(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting acceleration status configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAccelerateConfiguration, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -955,6 +1004,8 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf d.Set("acceleration_status", bucketAccelerate.Status) case tfresource.NotFound(err), tfawserr.ErrCodeEquals(err, errCodeMethodNotAllowed, errCodeNotImplemented, errCodeXNotImplemented, errCodeUnsupportedArgument): d.Set("acceleration_status", nil) + case tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden): + return diag.Errorf("permissions error on S3 Bucket (%s) while getting acceleration status configuration: %s", d.Id(), err) default: return diag.Errorf("reading S3 Bucket (%s) accelerate configuration: %s", d.Id(), err) } @@ -966,6 +1017,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findBucketRequestPayment(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting request payer configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketRequestPayment, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -988,6 +1047,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findLoggingEnabled(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting logging configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLogging, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -1012,6 +1079,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findLifecycleRules(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting lifecycle configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLifecycleConfiguration, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -1036,6 +1111,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findReplicationConfiguration(ctx, conn, d.Id()) }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting bucket replication configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketReplication, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -1060,6 +1143,14 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return findServerSideEncryptionConfiguration(ctx, conn, d.Id(), "") }) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting SSE encryption configuration: %s", d.Id(), err) + } + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketEncryption, the error should be caught for non-new buckets as follows. if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) d.SetId("") @@ -1099,6 +1190,8 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf case tfresource.NotFound(err), tfawserr.ErrCodeEquals(err, errCodeMethodNotAllowed, errCodeNotImplemented, errCodeXNotImplemented): d.Set("object_lock_configuration", nil) d.Set("object_lock_enabled", nil) + case tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden): // RM-4400 - more descriptive 403 errors + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting object lock configuration: %s", d.Id(), err) default: if partition := meta.(*conns.AWSClient).Partition; partition == names.StandardPartitionID || partition == names.USGovCloudPartitionID { return diag.Errorf("reading S3 Bucket (%s) object lock configuration: %s", d.Id(), err) @@ -1121,6 +1214,11 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf return diags } + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while discovering region: %s", d.Id(), err) + } + if err != nil { return sdkdiag.AppendErrorf(diags, "reading S3 Bucket (%s) location: %s", d.Id(), err) } @@ -1591,15 +1689,20 @@ func resourceBucketDelete(ctx context.Context, d *schema.ResourceData, meta inte } func findBucket(ctx context.Context, conn *s3.Client, bucket string, optFns ...func(*s3.Options)) error { - input := &s3.HeadBucketInput{ + // RM-4400 - remove HeadBucket because it requires s3:ListBucket permission + input := &s3.GetBucketEncryptionInput{ Bucket: aws.String(bucket), } - _, err := conn.HeadBucket(ctx, input, optFns...) + // RM-4400 - remove HeadBucket because it requires s3:ListBucket permission + _, err := conn.GetBucketEncryption(ctx, input, optFns...) + + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("permissions error on S3 Bucket (%s) while getting encryption configuration: %s", bucket, err) + } - // For directory buckets that no longer exist it's the CreateSession call invoked by HeadBucket that returns "NoSuchBucket", - // and that error code is flattend into HeadBucket's error message -- hence the 'errs.Contains' call. - if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotFound) || tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) || errs.Contains(err, errCodeNoSuchBucket) { + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotFound) || tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) || tfawserr.ErrCodeEquals(err, errCodeServerSideEncryptionConfigurationNotFound) { return &retry.NotFoundError{ LastError: err, LastRequest: input, diff --git a/internal/service/s3/bucket_analytics_configuration.go b/internal/service/s3/bucket_analytics_configuration.go index bd871190aaa..1fcb655d2f7 100644 --- a/internal/service/s3/bucket_analytics_configuration.go +++ b/internal/service/s3/bucket_analytics_configuration.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -439,6 +440,11 @@ func findAnalyticsConfiguration(ctx context.Context, conn *s3.Client, bucket, id output, err := conn.GetBucketAnalyticsConfiguration(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting analytics configuration: %s", id, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket, errCodeNoSuchConfiguration) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/s3/bucket_inventory.go b/internal/service/s3/bucket_inventory.go index ebab723ed74..9561eac78ff 100644 --- a/internal/service/s3/bucket_inventory.go +++ b/internal/service/s3/bucket_inventory.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -454,6 +455,11 @@ func findInventoryConfiguration(ctx context.Context, conn *s3.Client, bucket, id output, err := conn.GetBucketInventoryConfiguration(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while inventory configuration: %s", id, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket, errCodeNoSuchConfiguration) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/s3/bucket_metric.go b/internal/service/s3/bucket_metric.go index 1debc04a9a1..a2bf8ee23d1 100644 --- a/internal/service/s3/bucket_metric.go +++ b/internal/service/s3/bucket_metric.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -309,6 +310,11 @@ func findMetricsConfiguration(ctx context.Context, conn *s3.Client, bucket, id s output, err := conn.GetBucketMetricsConfiguration(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting metrics configuration: %s", id, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket, errCodeNoSuchConfiguration) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/s3/bucket_notification.go b/internal/service/s3/bucket_notification.go index cf9d08773dd..c847e3f6fe2 100644 --- a/internal/service/s3/bucket_notification.go +++ b/internal/service/s3/bucket_notification.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "net/http" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -396,6 +397,11 @@ func findBucketNotificationConfiguration(ctx context.Context, conn *s3.Client, b output, err := conn.GetBucketNotificationConfiguration(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting notification configuration: %s", bucket, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/s3/bucket_policy.go b/internal/service/s3/bucket_policy.go index 61ac3031b32..1baff582ca1 100644 --- a/internal/service/s3/bucket_policy.go +++ b/internal/service/s3/bucket_policy.go @@ -6,6 +6,7 @@ package s3 import ( "context" "log" + "net/http" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -163,6 +164,11 @@ func findBucketPolicy(ctx context.Context, conn *s3.Client, bucket string) (stri output, err := conn.GetBucketPolicy(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("[WARN] permissions error on S3 Bucket (%s) while getting policy configuration: %s", bucket, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket, errCodeNoSuchBucketPolicy) { return "", &retry.NotFoundError{ LastError: err, diff --git a/internal/service/s3/bucket_public_access_block.go b/internal/service/s3/bucket_public_access_block.go index 422d0972c09..eb551295bc7 100644 --- a/internal/service/s3/bucket_public_access_block.go +++ b/internal/service/s3/bucket_public_access_block.go @@ -6,6 +6,7 @@ package s3 import ( "context" "log" + "net/http" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -194,6 +195,11 @@ func findPublicAccessBlockConfiguration(ctx context.Context, conn *s3.Client, bu output, err := conn.GetPublicAccessBlock(ctx, input) + // RM-4400 - more descriptive 403 errors + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusForbidden) { + log.Printf("permissions error on S3 Bucket (%s) while getting public access block configuration: %s", bucket, err) + } + if tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket, errCodeNoSuchPublicAccessBlockConfiguration) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/ssm/parameter.go b/internal/service/ssm/parameter.go index 4b267ceccb5..94162207809 100644 --- a/internal/service/ssm/parameter.go +++ b/internal/service/ssm/parameter.go @@ -220,7 +220,7 @@ func resourceParameterRead(ctx context.Context, d *schema.ResourceData, meta int input := &ssm.GetParameterInput{ Name: aws.String(d.Id()), - WithDecryption: aws.Bool(true), + WithDecryption: aws.Bool(false), } var resp *ssm.GetParameterOutput