Skip to content

Commit

Permalink
Merge pull request #36268 from lawliet89/b-seclake-subscriber-source-…
Browse files Browse the repository at this point in the history 
…count

Fix `aws_securitylake_subscriber` to accept more than one source blocks
  • Loading branch information
@gdavison
gdavison authored May 3, 2024
2 parents 4f828c0 + d08b70d commit 3a7abcd
Show file tree
Hide file tree
Showing 17 changed files with 1,409 additions and 447 deletions.
31 changes: 31 additions & 0 deletions .changelog/36268.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
```release-note:bug
resource/aws_securitylake_subscriber: Allow more than one log source
```

```release-note:bug
resource/aws_securitylake_aws_log_source: Correctly handles unspecified `source_version`
```

```release-note:bug
resource/aws_securitylake_aws_log_source: Prevents errors when creating multiple log sources concurrently
```

```release-note:bug
resource/aws_securitylake_custom_log_source: Validates length of `source_name` parameter
```

```release-note:bug
resource/aws_securitylake_custom_log_source: Prevents errors when creating multiple log sources concurrently
```

```release-note:bug
resource/aws_securitylake_subscriber: Correctly handles unspecified `access_type`
```

```release-note:bug
resource/aws_securitylake_subscriber: Correctly requires `source_name` parameter for `aws_log_source_resource` and `custom_log_source_resource`
```

```release-note:bug
resource/aws_securitylake_subscriber: Correctly handles unspecified `source_version` parameter for `aws_log_source_resource` and `custom_log_source_resource`
```
9 changes: 7 additions & 2 deletions internal/service/securitylake/aws_log_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ func (r *awsLogSourceResource) Create(ctx context.Context, request resource.Crea
return
}

_, err := conn.CreateAwsLogSource(ctx, input)
_, err := retryDataLakeConflictWithMutex(ctx, func() (*securitylake.CreateAwsLogSourceOutput, error) {
return conn.CreateAwsLogSource(ctx, input)
})

if err != nil {
response.Diagnostics.AddError("creating Security Lake AWS Log Source", err.Error())
Expand All @@ -144,6 +146,7 @@ func (r *awsLogSourceResource) Create(ctx context.Context, request resource.Crea

sourceData.Accounts.SetValue = fwflex.FlattenFrameworkStringValueSet(ctx, logSource.Accounts)
sourceData.SourceVersion = fwflex.StringToFramework(ctx, logSource.SourceVersion)
data.Source = fwtypes.NewListNestedObjectValueOfPtrMust(ctx, sourceData)

response.Diagnostics.Append(response.State.Set(ctx, data)...)
}
Expand Down Expand Up @@ -212,7 +215,9 @@ func (r *awsLogSourceResource) Delete(ctx context.Context, request resource.Dele
input.Sources = []awstypes.AwsLogSourceConfiguration{*logSource}
}

_, err := conn.DeleteAwsLogSource(ctx, input)
_, err := retryDataLakeConflictWithMutex(ctx, func() (*securitylake.DeleteAwsLogSourceOutput, error) {
return conn.DeleteAwsLogSource(ctx, input)
})

if errs.IsA[*awstypes.ResourceNotFoundException](err) {
return
Expand Down
190 changes: 172 additions & 18 deletions internal/service/securitylake/aws_log_source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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/plancheck"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand All @@ -28,13 +29,62 @@ func testAccAWSLogSource_basic(t *testing.T) {
PreCheck: func() {
acctest.PreCheck(ctx, t)
acctest.PreCheckPartitionHasService(t, names.SecurityLake)
testAccPreCheck(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.SecurityLakeServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckAWSLogSourceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccAWSLogSourceConfig_basic(),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSLogSourceExists(ctx, resourceName, &logSource),
resource.TestCheckResourceAttr(resourceName, "source.#", "1"),
resource.TestCheckResourceAttrSet(resourceName, "source.0.accounts.#"),
acctest.CheckResourceAttrAccountID(resourceName, "source.0.accounts.0"),
func(s *terraform.State) error {
return resource.TestCheckTypeSetElemAttr(resourceName, "source.0.accounts.*", acctest.AccountID())(s)
},
resource.TestCheckResourceAttr(resourceName, "source.0.regions.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "source.0.regions.*", acctest.Region()),
resource.TestCheckResourceAttr(resourceName, "source.0.source_name", "ROUTE53"),
resource.TestCheckResourceAttr(resourceName, "source.0.source_version", "2.0"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAWSLogSourceConfig_sourceVersion("2.0"),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
},
},
})
}

func testAccAWSLogSource_sourceVersion(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_securitylake_aws_log_source.test"
var logSource types.AwsLogSourceConfiguration

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: testAccCheckAWSLogSourceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccAWSLogSourceConfig_sourceVersion("1.0"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSLogSourceExists(ctx, resourceName, &logSource),
resource.TestCheckResourceAttr(resourceName, "source.#", "1"),
Expand All @@ -50,6 +100,23 @@ func testAccAWSLogSource_basic(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAWSLogSourceConfig_sourceVersion("2.0"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSLogSourceExists(ctx, resourceName, &logSource),
resource.TestCheckResourceAttr(resourceName, "source.#", "1"),
resource.TestCheckResourceAttr(resourceName, "source.0.accounts.#", "1"),
resource.TestCheckResourceAttr(resourceName, "source.0.regions.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "source.0.regions.*", acctest.Region()),
resource.TestCheckResourceAttr(resourceName, "source.0.source_name", "ROUTE53"),
resource.TestCheckResourceAttr(resourceName, "source.0.source_version", "2.0"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand All @@ -64,9 +131,11 @@ func testAccAWSLogSource_multiRegion(t *testing.T) {
PreCheck: func() {
acctest.PreCheck(ctx, t)
acctest.PreCheckPartitionHasService(t, names.SecurityLake)
acctest.PreCheckMultipleRegion(t, 2)
testAccPreCheck(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.SecurityLakeServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
ProtoV5ProviderFactories: acctest.ProtoV5FactoriesAlternate(ctx, t),
CheckDestroy: testAccCheckAWSLogSourceDestroy(ctx),
Steps: []resource.TestStep{
{
Expand All @@ -78,8 +147,6 @@ func testAccAWSLogSource_multiRegion(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "source.0.regions.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "source.0.regions.*", acctest.Region()),
resource.TestCheckTypeSetElemAttr(resourceName, "source.0.regions.*", acctest.AlternateRegion()),
resource.TestCheckResourceAttr(resourceName, "source.0.source_name", "ROUTE53"),
resource.TestCheckResourceAttr(resourceName, "source.0.source_version", "1.0"),
),
},
{
Expand All @@ -100,7 +167,7 @@ func testAccAWSLogSource_disappears(t *testing.T) {
PreCheck: func() {
acctest.PreCheck(ctx, t)
acctest.PreCheckPartitionHasService(t, names.SecurityLake)
acctest.PreCheckOrganizationsAccount(ctx, t)
testAccPreCheck(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.SecurityLakeServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Expand All @@ -118,6 +185,46 @@ func testAccAWSLogSource_disappears(t *testing.T) {
})
}

func testAccAWSLogSource_multiple(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_securitylake_aws_log_source.test"
resourceName2 := "aws_securitylake_aws_log_source.test2"
var logSource, logSource2 types.AwsLogSourceConfiguration

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: testAccCheckAWSLogSourceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccAWSLogSourceConfig_multiple(),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSLogSourceExists(ctx, resourceName, &logSource),
testAccCheckAWSLogSourceExists(ctx, resourceName2, &logSource2),

resource.TestCheckResourceAttr(resourceName, "source.#", "1"),
resource.TestCheckResourceAttr(resourceName, "source.0.source_name", "ROUTE53"),
resource.TestCheckResourceAttr(resourceName, "source.0.source_version", "2.0"),

resource.TestCheckResourceAttr(resourceName2, "source.#", "1"),
resource.TestCheckResourceAttr(resourceName2, "source.0.source_name", "S3_DATA"),
resource.TestCheckResourceAttr(resourceName2, "source.0.source_version", "2.0"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccCheckAWSLogSourceDestroy(ctx context.Context) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).SecurityLakeClient(ctx)
Expand Down Expand Up @@ -166,34 +273,81 @@ func testAccCheckAWSLogSourceExists(ctx context.Context, n string, v *types.AwsL
}

func testAccAWSLogSourceConfig_basic() string {
return acctest.ConfigCompose(testAccDataLakeConfig_basic(), fmt.Sprintf(`
data "aws_caller_identity" "test" {}
return acctest.ConfigCompose(
testAccDataLakeConfig_basic(), `
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"
}
depends_on = [aws_securitylake_data_lake.test]
}
data "aws_region" "current" {}
`)
}

func testAccAWSLogSourceConfig_sourceVersion(version string) string {
return acctest.ConfigCompose(
testAccDataLakeConfig_basic(), fmt.Sprintf(`
resource "aws_securitylake_aws_log_source" "test" {
source {
accounts = [data.aws_caller_identity.test.account_id]
regions = [%[1]q]
accounts = [data.aws_caller_identity.current.account_id]
regions = [data.aws_region.current.name]
source_name = "ROUTE53"
source_version = "1.0"
source_version = %[1]q
}
depends_on = [aws_securitylake_data_lake.test]
}
`, acctest.Region()))
data "aws_region" "current" {}
`, version))
}

func testAccAWSLogSourceConfig_multiRegion(rName string) string {
return acctest.ConfigCompose(testAccDataLakeConfig_replication(rName), fmt.Sprintf(`
data "aws_caller_identity" "test" {}
return acctest.ConfigCompose(
acctest.ConfigMultipleRegionProvider(2),
testAccDataLakeConfig_replication(rName), `
resource "aws_securitylake_aws_log_source" "test" {
source {
accounts = [data.aws_caller_identity.test.account_id]
regions = [%[1]q, %[2]q]
source_name = "ROUTE53"
source_version = "1.0"
accounts = [data.aws_caller_identity.current.account_id]
regions = [data.aws_region.current.name, data.aws_region.alternate.name]
source_name = "ROUTE53"
}
depends_on = [aws_securitylake_data_lake.test, aws_securitylake_data_lake.region_2]
}
`, acctest.Region(), acctest.AlternateRegion()))
data "aws_region" "current" {}
data "aws_region" "alternate" {
provider = awsalternate
}
`)
}

func testAccAWSLogSourceConfig_multiple() string {
return acctest.ConfigCompose(
testAccDataLakeConfig_basic(), `
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"
}
depends_on = [aws_securitylake_data_lake.test]
}
resource "aws_securitylake_aws_log_source" "test2" {
source {
accounts = [data.aws_caller_identity.current.account_id]
regions = [data.aws_region.current.name]
source_name = "S3_DATA"
}
depends_on = [aws_securitylake_data_lake.test]
}
data "aws_region" "current" {}
`)
}
12 changes: 10 additions & 2 deletions internal/service/securitylake/custom_log_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/securitylake"
awstypes "github.com/aws/aws-sdk-go-v2/service/securitylake/types"
"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
Expand Down Expand Up @@ -88,6 +89,9 @@ func (r *customLogSourceResource) Schema(ctx context.Context, request resource.S
},
"source_name": schema.StringAttribute{
Required: true,
Validators: []validator.String{
stringvalidator.LengthAtMost(20),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
},
Expand Down Expand Up @@ -184,7 +188,9 @@ func (r *customLogSourceResource) Create(ctx context.Context, request resource.C
return
}

output, err := conn.CreateCustomLogSource(ctx, input)
output, err := retryDataLakeConflictWithMutex(ctx, func() (*securitylake.CreateCustomLogSourceOutput, error) {
return conn.CreateCustomLogSource(ctx, input)
})

if err != nil {
response.Diagnostics.AddError("creating Security Lake Custom Log Source", err.Error())
Expand Down Expand Up @@ -266,7 +272,9 @@ func (r *customLogSourceResource) Delete(ctx context.Context, request resource.D
return
}

_, err := conn.DeleteCustomLogSource(ctx, input)
_, err := retryDataLakeConflictWithMutex(ctx, func() (*securitylake.DeleteCustomLogSourceOutput, error) {
return conn.DeleteCustomLogSource(ctx, input)
})

if errs.IsA[*awstypes.ResourceNotFoundException](err) {
return
Expand Down
Loading

0 comments on commit 3a7abcd

Please sign in to comment.