diff --git a/internal/service/securitylake/securitylake_test.go b/internal/service/securitylake/securitylake_test.go index d81f93772f8c..3c6502fb28f4 100644 --- a/internal/service/securitylake/securitylake_test.go +++ b/internal/service/securitylake/securitylake_test.go @@ -50,12 +50,14 @@ func TestAccSecurityLake_serial(t *testing.T) { "replication": testAccDataLake_replication, }, "Subscriber": { - "accessType": testAccSubscriber_accessType, - "basic": testAccSubscriber_basic, - "customLogs": testAccSubscriber_customLogSource, - "disappears": testAccSubscriber_disappears, - "tags": testAccSubscriber_tags, - "updated": testAccSubscriber_update, + "accessType": testAccSubscriber_accessType, + "basic": testAccSubscriber_basic, + "customLogs": testAccSubscriber_customLogSource, + "disappears": testAccSubscriber_disappears, + "multipleSources": testAccSubscriber_multipleSources, + "tags": testAccSubscriber_tags, + "updated": testAccSubscriber_update, + "migrateSource": testAccSubscriber_migrate_source, }, } diff --git a/internal/service/securitylake/subscriber.go b/internal/service/securitylake/subscriber.go index 3f5a04310a56..f80790aa1162 100644 --- a/internal/service/securitylake/subscriber.go +++ b/internal/service/securitylake/subscriber.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" + "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" @@ -32,6 +33,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/framework" fwflex "github.com/hashicorp/terraform-provider-aws/internal/framework/flex" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" + "github.com/hashicorp/terraform-provider-aws/internal/slices" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" @@ -100,10 +102,10 @@ func (r *subscriberResource) Schema(ctx context.Context, request resource.Schema names.AttrTagsAll: tftags.TagsAttributeComputedOnly(), }, Blocks: map[string]schema.Block{ - "source": schema.ListNestedBlock{ - Validators: []validator.List{ - listvalidator.IsRequired(), - listvalidator.SizeAtLeast(1), + "source": schema.SetNestedBlock{ + Validators: []validator.Set{ + setvalidator.IsRequired(), + setvalidator.SizeAtLeast(1), }, NestedObject: schema.NestedBlockObject{ Blocks: map[string]schema.Block{ @@ -242,7 +244,7 @@ func (r *subscriberResource) Create(ctx context.Context, request resource.Create subscriber, err := waitSubscriberCreated(ctx, conn, data.ID.ValueString(), r.CreateTimeout(ctx, data.Timeouts)) if err != nil { - response.Diagnostics.AddError(fmt.Sprintf("waiting for Security Lake Subscriber (%s) create", data.ID.ValueString()), err.Error()) + response.Diagnostics.AddError(fmt.Sprintf("waiting for Security Lake Subscriber (%s) to be created", data.ID.ValueString()), err.Error()) return } @@ -347,16 +349,20 @@ func (r *subscriberResource) Update(ctx context.Context, request resource.Update return } - _, err = waitSubscriberUpdated(ctx, conn, new.ID.ValueString(), r.CreateTimeout(ctx, new.Timeouts)) + subscriber, err := waitSubscriberUpdated(ctx, conn, new.ID.ValueString(), r.CreateTimeout(ctx, new.Timeouts)) if err != nil { - response.Diagnostics.AddError(fmt.Sprintf("waiting for Security Lake Subscriber (%s) create", new.ID.ValueString()), err.Error()) + response.Diagnostics.AddError(fmt.Sprintf("waiting for Security Lake Subscriber (%s) to be updated", new.ID.ValueString()), err.Error()) + + return + } + var subscriberIdentity subscriberIdentityModel + response.Diagnostics.Append(fwflex.Flatten(ctx, subscriber.SubscriberIdentity, &subscriberIdentity)...) + if response.Diagnostics.HasError() { return } - new.ResourceShareArn = old.ResourceShareArn - new.ResourceShareName = old.ResourceShareName - new.SubscriberEndpoint = old.SubscriberEndpoint + response.Diagnostics.Append(new.refreshFromOutput(ctx, subscriberIdentity, subscriber)...) } response.Diagnostics.Append(response.State.Set(ctx, &new)...) @@ -510,7 +516,7 @@ func expandSubscriptionValueSources(ctx context.Context, subscriberSourcesModels if !item.AwsLogSourceResource.IsNull() && (len(item.AwsLogSourceResource.Elements()) > 0) { var awsLogSources []awsLogSubscriberSourceModel diags.Append(item.AwsLogSourceResource.ElementsAs(ctx, &awsLogSources, false)...) - subscriberLogSource := expandSubscriberLogSourceSource(ctx, awsLogSources) + subscriberLogSource := expandSubscriberAwsLogSourceSource(ctx, awsLogSources) sources = append(sources, subscriberLogSource) } if (!item.CustomLogSourceResource.IsNull()) && (len(item.CustomLogSourceResource.Elements()) > 0) { @@ -524,7 +530,7 @@ func expandSubscriptionValueSources(ctx context.Context, subscriberSourcesModels return sources, diags } -func expandSubscriberLogSourceSource(ctx context.Context, awsLogSources []awsLogSubscriberSourceModel) *awstypes.LogSourceResourceMemberAwsLogSource { +func expandSubscriberAwsLogSourceSource(ctx context.Context, awsLogSources []awsLogSubscriberSourceModel) *awstypes.LogSourceResourceMemberAwsLogSource { if len(awsLogSources) == 0 { return nil } @@ -553,38 +559,63 @@ func expandSubscriberCustomLogSourceSource(ctx context.Context, customLogSources return customLogSourceResource } -func flattenSubscriberSourcesModel(ctx context.Context, apiObject []awstypes.LogSourceResource) (types.List, diag.Diagnostics) { +func flattenSubscriberSources(ctx context.Context, apiObject []awstypes.LogSourceResource) (types.Set, diag.Diagnostics) { var diags diag.Diagnostics elemType := types.ObjectType{AttrTypes: subscriberSourcesModelAttrTypes} + result := types.SetNull(elemType) - obj := map[string]attr.Value{} + var elems []types.Object for _, item := range apiObject { - switch v := item.(type) { - case *awstypes.LogSourceResourceMemberAwsLogSource: - subscriberLogSource, d := flattenSubscriberLogSourceResourceModel(ctx, &v.Value, nil, "aws") - diags.Append(d...) - obj = map[string]attr.Value{ - "aws_log_source_resource": subscriberLogSource, - "custom_log_source_resource": types.ListNull(customLogSubscriberSourceModelAttrTypes), - } - case *awstypes.LogSourceResourceMemberCustomLogSource: - subscriberLogSource, d := flattenSubscriberLogSourceResourceModel(ctx, nil, &v.Value, "custom") - diags.Append(d...) - obj = map[string]attr.Value{ - "aws_log_source_resource": types.ListNull(logSubscriberSourcesModelAttrTypes), - "custom_log_source_resource": subscriberLogSource, - } + elem, d := flattenSubscriberSourcesModel(ctx, item) + diags.Append(d...) + if d.HasError() { + return result, diags } + elems = append(elems, elem) } - objVal, d := types.ObjectValue(subscriberSourcesModelAttrTypes, obj) + setVal, d := types.SetValue(elemType, slices.ApplyToAll(elems, func(o types.Object) attr.Value { + return o + })) diags.Append(d...) - listVal, d := types.ListValue(elemType, []attr.Value{objVal}) + return setVal, diags +} + +func flattenSubscriberSourcesModel(ctx context.Context, apiObject awstypes.LogSourceResource) (types.Object, diag.Diagnostics) { + var diags diag.Diagnostics + result := types.ObjectUnknown(subscriberSourcesModelAttrTypes) + + obj := map[string]attr.Value{} + + switch v := apiObject.(type) { + case *awstypes.LogSourceResourceMemberAwsLogSource: + subscriberLogSource, d := flattenSubscriberLogSourceResourceModel(ctx, &v.Value, nil, "aws") + diags.Append(d...) + if d.HasError() { + return result, diags + } + obj = map[string]attr.Value{ + "aws_log_source_resource": subscriberLogSource, + "custom_log_source_resource": types.ListNull(customLogSubscriberSourceModelAttrTypes), + } + case *awstypes.LogSourceResourceMemberCustomLogSource: + subscriberLogSource, d := flattenSubscriberLogSourceResourceModel(ctx, nil, &v.Value, "custom") + diags.Append(d...) + if d.HasError() { + return result, diags + } + obj = map[string]attr.Value{ + "aws_log_source_resource": types.ListNull(logSubscriberSourcesModelAttrTypes), + "custom_log_source_resource": subscriberLogSource, + } + } + + result, d := types.ObjectValue(subscriberSourcesModelAttrTypes, obj) diags.Append(d...) - return listVal, diags + return result, diags } func flattenSubscriberLogSourceResourceModel(ctx context.Context, awsLogApiObject *awstypes.AwsLogSourceResource, customLogApiObject *awstypes.CustomLogSourceResource, logSourceType string) (types.List, diag.Diagnostics) { @@ -706,7 +737,7 @@ type subscriberResourceModel struct { AccessTypes types.String `tfsdk:"access_type"` SubscriberArn types.String `tfsdk:"arn"` ID types.String `tfsdk:"id"` - Sources types.List `tfsdk:"source"` + Sources types.Set `tfsdk:"source"` SubscriberDescription types.String `tfsdk:"subscriber_description"` SubscriberIdentity fwtypes.ListNestedObjectValueOf[subscriberIdentityModel] `tfsdk:"subscriber_identity"` SubscriberName types.String `tfsdk:"subscriber_name"` @@ -764,14 +795,14 @@ func (rd *subscriberResourceModel) refreshFromOutput(ctx context.Context, subscr rd.AccessTypes = fwflex.StringValueToFramework(ctx, subscriber.AccessTypes[0]) rd.SubscriberIdentity = fwtypes.NewListNestedObjectValueOfPtrMust(ctx, &subscriberIdentity) - sourcesOutput, d := flattenSubscriberSourcesModel(ctx, subscriber.Sources) - diags.Append(d...) rd.ResourceShareArn = fwflex.StringToFrameworkLegacy(ctx, subscriber.ResourceShareArn) rd.ResourceShareName = fwflex.StringToFramework(ctx, subscriber.ResourceShareName) rd.S3BucketArn = fwflex.StringToFramework(ctx, subscriber.S3BucketArn) rd.SubscriberEndpoint = fwflex.StringToFramework(ctx, subscriber.SubscriberEndpoint) rd.SubscriberStatus = fwflex.StringValueToFramework(ctx, subscriber.SubscriberStatus) rd.RoleArn = fwflex.StringToFramework(ctx, subscriber.RoleArn) + sourcesOutput, d := flattenSubscriberSources(ctx, subscriber.Sources) + diags.Append(d...) rd.Sources = sourcesOutput rd.SubscriberName = fwflex.StringToFramework(ctx, subscriber.SubscriberName) rd.SubscriberDescription = fwflex.StringToFramework(ctx, subscriber.SubscriberDescription) diff --git a/internal/service/securitylake/subscriber_test.go b/internal/service/securitylake/subscriber_test.go index 75a9691c8da7..d8a637260e1c 100644 --- a/internal/service/securitylake/subscriber_test.go +++ b/internal/service/securitylake/subscriber_test.go @@ -13,7 +13,11 @@ import ( "github.com/aws/aws-sdk-go-v2/service/securitylake/types" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfsecuritylake "github.com/hashicorp/terraform-provider-aws/internal/service/securitylake" @@ -112,6 +116,7 @@ func testAccSubscriber_customLogSource(t *testing.T) { resourceName := "aws_securitylake_subscriber.test" var subscriber types.SubscriberResource rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceName := randomCustomLogSourceName() resource.Test(t, resource.TestCase{ PreCheck: func() { @@ -124,7 +129,7 @@ func testAccSubscriber_customLogSource(t *testing.T) { CheckDestroy: testAccCheckSubscriberDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccSubscriberConfig_customLog(rName), + Config: testAccSubscriberConfig_customLog(rName, sourceName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckSubscriberExists(ctx, resourceName, &subscriber), resource.TestCheckResourceAttr(resourceName, "subscriber_name", rName), @@ -251,7 +256,6 @@ func testAccSubscriber_update(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "source.#", "1"), resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.#", "1"), resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.0.source_name", "ROUTE53"), - resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.0.source_version", "1.0"), resource.TestCheckResourceAttr(resourceName, "subscriber_identity.#", "1"), resource.TestCheckResourceAttr(resourceName, "subscriber_identity.0.external_id", "example"), ), @@ -270,7 +274,6 @@ func testAccSubscriber_update(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "source.#", "1"), resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.#", "1"), resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.0.source_name", "VPC_FLOW"), - resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.0.source_version", "1.0"), resource.TestCheckResourceAttr(resourceName, "subscriber_identity.#", "1"), resource.TestCheckResourceAttr(resourceName, "subscriber_identity.0.external_id", "updated"), ), @@ -284,6 +287,188 @@ func testAccSubscriber_update(t *testing.T) { }) } +func testAccSubscriber_multipleSources(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_securitylake_subscriber.test" + var subscriber types.SubscriberResource + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.SecurityLake) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.SecurityLakeServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSubscriberDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccSubscriberConfig_sources2(rName, "VPC_FLOW", "ROUTE53"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSubscriberExists(ctx, resourceName, &subscriber), + resource.TestCheckResourceAttr(resourceName, "subscriber_name", rName), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, + tfjsonpath.New("source"), + knownvalue.SetExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectPartial(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("VPC_FLOW"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectPartial(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("ROUTE53"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + }), + ), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccSubscriberConfig_sources2(rName, "ROUTE53", "VPC_FLOW"), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + ExpectNonEmptyPlan: false, + }, + { + Config: testAccSubscriberConfig_sources1(rName, "ROUTE53"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSubscriberExists(ctx, resourceName, &subscriber), + resource.TestCheckResourceAttr(resourceName, "subscriber_name", rName), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, + tfjsonpath.New("source"), + knownvalue.SetExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectPartial(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("ROUTE53"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + }), + ), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccSubscriberConfig_sources2(rName, "VPC_FLOW", "S3_DATA"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSubscriberExists(ctx, resourceName, &subscriber), + resource.TestCheckResourceAttr(resourceName, "subscriber_name", rName), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, + tfjsonpath.New("source"), + knownvalue.SetExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectPartial(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("VPC_FLOW"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectPartial(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("S3_DATA"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + }), + ), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccSubscriber_migrate_source(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_securitylake_subscriber.test" + var subscriber types.SubscriberResource + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.SecurityLake) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.SecurityLakeServiceID), + CheckDestroy: testAccCheckSubscriberDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.47.0", + }, + }, + Config: testAccSubscriberConfig_migrate_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSubscriberExists(ctx, resourceName, &subscriber), + resource.TestCheckResourceAttr(resourceName, "subscriber_name", rName), + resource.TestCheckResourceAttr(resourceName, "source.#", "1"), + resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.#", "1"), + resource.TestCheckResourceAttr(resourceName, "source.0.aws_log_source_resource.0.source_name", "ROUTE53"), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, + tfjsonpath.New("source"), + knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "aws_log_source_resource": knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "source_name": knownvalue.StringExact("ROUTE53"), + "source_version": knownvalue.StringExact("2.0"), + }), + }), + "custom_log_source_resource": knownvalue.ListSizeExact(0), + }), + }), + ), + }, + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccSubscriberConfig_basic(rName), + PlanOnly: true, + }, + }, + }) +} + func testAccCheckSubscriberDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).SecurityLakeClient(ctx) @@ -338,7 +523,7 @@ resource "aws_securitylake_subscriber" "test" { subscriber_name = %[1]q source { aws_log_source_resource { - source_name = "ROUTE53" + source_name = "ROUTE53" } } subscriber_identity { @@ -351,9 +536,9 @@ resource "aws_securitylake_subscriber" "test" { resource "aws_securitylake_aws_log_source" "test" { source { - accounts = [data.aws_caller_identity.current.account_id] - regions = [data.aws_region.current.name] - source_name = "ROUTE53" + accounts = [data.aws_caller_identity.current.account_id] + regions = [data.aws_region.current.name] + source_name = "ROUTE53" } depends_on = [aws_securitylake_data_lake.test] } @@ -362,8 +547,9 @@ data "aws_region" "current" {} `, rName)) } -func testAccSubscriberConfig_customLog(rName string) string { - return acctest.ConfigCompose(testAccDataLakeConfig_basic(), fmt.Sprintf(` +func testAccSubscriberConfig_customLog(rName, sourceName string) string { + return acctest.ConfigCompose( + testAccDataLakeConfig_basic(), fmt.Sprintf(` resource "aws_securitylake_subscriber" "test" { subscriber_name = %[1]q subscriber_description = "Example" @@ -382,7 +568,8 @@ resource "aws_securitylake_subscriber" "test" { } resource "aws_securitylake_custom_log_source" "test" { - source_name = %[1]q + source_name = %[2]q + source_version = "1.5" configuration { crawler_configuration { @@ -390,7 +577,7 @@ resource "aws_securitylake_custom_log_source" "test" { } provider_identity { - external_id = "%[1]s-test" + external_id = "%[2]s-test" principal = data.aws_caller_identity.current.account_id } } @@ -399,7 +586,7 @@ resource "aws_securitylake_custom_log_source" "test" { } resource "aws_iam_role" "test" { - name = %[1]q + name = %[2]q path = "/service-role/" assume_role_policy = <