Merge pull request #44867 from hashicorp/b-lakeformation-non-iam-principals

resource/aws_lakeformation_permissions: Allow use with IAM Identity Center Groups

Author: gdavison

.changelog/44867.txt

@@ -0,0 +1,15 @@
+```release-note:new-resource
+aws_lakeformation_identity_center_configuration
+```
+
+```release-note:bug
+resource/aws_lakeformation_permissions: Allows IAM Identity Center Groups as `principal`.
+```
+
+```release-note:bug
+data-source/aws_lakeformation_permissions: Allows IAM Identity Center Groups as `principal`.
+```
+
+```release-note:enhancement
+resource/aws_identitystore_group: Adds `arn` attribute.
+```

internal/acctest/knownvalue/global_arn_no_account_id_exact.go

@@ -0,0 +1,57 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package knownvalue
+
+import (
+	"fmt"
+
+	"github.com/aws/aws-sdk-go-v2/aws/arn"
+	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
+	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
+)
+
+var _ knownvalue.Check = globalARNNoAccountIDExact{}
+
+type globalARNNoAccountIDExact struct {
+	service  string
+	resource string
+}
+
+// CheckValue determines whether the passed value is of type string, and
+// contains a matching sequence of bytes.
+func (v globalARNNoAccountIDExact) CheckValue(other any) error {
+	otherVal, ok := other.(string)
+
+	if !ok {
+		return fmt.Errorf("expected string value for GlobalARNNoAccountIDExact check, got: %T", other)
+	}
+
+	if otherVal != v.buildARNString() {
+		return fmt.Errorf("expected value %s for GlobalARNNoAccountIDExact check, got: %s", v.buildARNString(), otherVal)
+	}
+
+	return nil
+}
+
+// String returns the string representation of the value.
+func (v globalARNNoAccountIDExact) String() string {
+	return v.buildARNString()
+}
+
+func (v globalARNNoAccountIDExact) buildARNString() string {
+	return arn.ARN{
+		AccountID: "",
+		Partition: acctest.Partition(),
+		Region:    "",
+		Service:   v.service,
+		Resource:  v.resource,
+	}.String()
+}
+
+func GlobalARNNoAccountIDExact(service, resource string) knownvalue.Check {
+	return globalARNNoAccountIDExact{
+		service:  service,
+		resource: resource,
+	}
+}

internal/acctest/statecheck/expect_global_arn_no_account_id_format.go

@@ -0,0 +1,56 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package statecheck
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/terraform-plugin-testing/statecheck"
+	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
+	tfknownvalue "github.com/hashicorp/terraform-provider-aws/internal/acctest/knownvalue"
+)
+
+var _ statecheck.StateCheck = expectGlobalARNNoAccountIDFormatCheck{}
+
+type expectGlobalARNNoAccountIDFormatCheck struct {
+	base          Base
+	attributePath tfjsonpath.Path
+	arnService    string
+	arnFormat     string
+}
+
+func (e expectGlobalARNNoAccountIDFormatCheck) CheckState(ctx context.Context, request statecheck.CheckStateRequest, response *statecheck.CheckStateResponse) {
+	resource, ok := e.base.ResourceFromState(request, response)
+	if !ok {
+		return
+	}
+
+	value, err := tfjsonpath.Traverse(resource.AttributeValues, e.attributePath)
+	if err != nil {
+		response.Error = err
+		return
+	}
+
+	arnString, err := populateFromResourceState(e.arnFormat, resource)
+	if err != nil {
+		response.Error = err
+		return
+	}
+
+	knownCheck := tfknownvalue.GlobalARNNoAccountIDExact(e.arnService, arnString)
+	if err = knownCheck.CheckValue(value); err != nil { //nolint:contextcheck // knownCheck implements an interface
+		response.Error = fmt.Errorf("checking value for attribute at path: %s.%s, err: %w", e.base.ResourceAddress(), e.attributePath, err)
+		return
+	}
+}
+
+func ExpectGlobalARNNoAccountIDFormat(resourceAddress string, attributePath tfjsonpath.Path, arnService, arnFormat string) statecheck.StateCheck {
+	return expectGlobalARNNoAccountIDFormatCheck{
+		base:          NewBase(resourceAddress),
+		attributePath: attributePath,
+		arnService:    arnService,
+		arnFormat:     arnFormat,
+	}
+}

internal/generate/identitytests/resource_test.go.gtpl

@@ -69,7 +69,7 @@ CheckDestroy: acctest.CheckDestroyNoop,
 {{ if gt (len .ImportStateIDFunc) 0 -}}
 	ImportStateIdFunc: acctest.CrossRegionImportStateIdFuncAdapter(resourceName, {{ .ImportStateIDFunc }}),
 {{ else if .HasImportStateIDAttribute -}}
-	// TODO
+	ImportStateIdFunc: acctest.CrossRegionAttrImportStateIdFunc(resourceName, {{ .ImportStateIDAttribute }}),
 {{ else -}}
 	ImportStateIdFunc: acctest.CrossRegionImportStateIdFunc(resourceName),
 {{ end -}}
@@ -99,7 +99,7 @@ CheckDestroy: acctest.CheckDestroyNoop,
 {{ if gt (len .ImportStateIDFunc) 0 -}}
 	ImportStateIdFunc: acctest.CrossRegionImportStateIdFuncAdapter(resourceName, {{ .ImportStateIDFunc }}),
 {{ else if .HasImportStateIDAttribute -}}
-	// TODO
+	ImportStateIdFunc: acctest.CrossRegionAttrImportStateIdFunc(resourceName, {{ .ImportStateIDAttribute }}),
 {{ else -}}
 	ImportStateIdFunc: acctest.CrossRegionImportStateIdFunc(resourceName),
 {{ end -}}
@@ -238,9 +238,11 @@ func {{ template "testname" . }}_IdentitySerial(t *testing.T) {
 	{{- end }}
 
 	testCases := map[string]func(t *testing.T){
-		acctest.CtBasic:             {{ template "testname" . }}_Identity_Basic,
-		"ExistingResource":          {{ template "testname" . }}_Identity_ExistingResource,
-		"ExistingResourceNoRefresh": {{ template "testname" . }}_Identity_ExistingResource_NoRefresh_NoChange,
+		acctest.CtBasic: {{ template "testname" . }}_Identity_Basic,
+		{{ if .PreIdentityVersion -}}
+			"ExistingResource":          {{ template "testname" . }}_Identity_ExistingResource,
+			"ExistingResourceNoRefresh": {{ template "testname" . }}_Identity_ExistingResource_NoRefresh_NoChange,
+		{{ end -}}
 		{{ if .GenerateRegionOverrideTest -}}
 			"RegionOverride": {{ template "testname" . }}_Identity_RegionOverride,
 		{{ end -}}

internal/service/identitystore/group.go

@@ -37,6 +37,10 @@ func resourceGroup() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
+			names.AttrARN: {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
 			names.AttrDescription: {
 				Type:         schema.TypeString,
 				Optional:     true,
@@ -123,6 +127,8 @@ func resourceGroupRead(ctx context.Context, d *schema.ResourceData, meta any) di
 		return sdkdiag.AppendErrorf(diags, "reading IdentityStore Group (%s): %s", d.Id(), err)
 	}
 
+	groupARN := meta.(*conns.AWSClient).GlobalARNNoAccount(ctx, "identitystore", "group/"+groupID)
+	d.Set(names.AttrARN, groupARN)
 	d.Set(names.AttrDescription, out.Description)
 	d.Set(names.AttrDisplayName, out.DisplayName)
 	if err := d.Set("external_ids", flattenExternalIDs(out.ExternalIds)); err != nil {

internal/service/identitystore/group_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/service/identitystore"
+	"github.com/hashicorp/terraform-plugin-testing/compare"
 	sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
@@ -17,6 +18,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
 	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
+	tfstatecheck "github.com/hashicorp/terraform-provider-aws/internal/acctest/statecheck"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	tfidentitystore "github.com/hashicorp/terraform-provider-aws/internal/service/identitystore"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
@@ -42,9 +44,11 @@ func TestAccIdentityStoreGroup_basic(t *testing.T) {
 			{
 				Config: testAccGroupConfig_basic(displayName),
 				ConfigStateChecks: []statecheck.StateCheck{
+					tfstatecheck.ExpectGlobalARNNoAccountIDFormat(resourceName, tfjsonpath.New(names.AttrARN), "identitystore", "group/{group_id}"),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrDescription), knownvalue.StringExact("")),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrDisplayName), knownvalue.StringExact(displayName)),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("group_id"), knownvalue.NotNull()),
-					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("identity_store_id"), knownvalue.NotNull()),
+					statecheck.CompareValuePairs(resourceName, tfjsonpath.New("identity_store_id"), "data.aws_ssoadmin_instances.test", tfjsonpath.New("identity_store_ids").AtSliceIndex(0), compare.ValuesSame()),
 				},
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckGroupExists(ctx, resourceName, &group),
@@ -234,33 +238,35 @@ func testAccCheckGroupExists(ctx context.Context, n string, v *identitystore.Des
 
 func testAccGroupConfig_basic(displayName string) string {
 	return fmt.Sprintf(`
-data "aws_ssoadmin_instances" "test" {}
 resource "aws_identitystore_group" "test" {
-  identity_store_id = tolist(data.aws_ssoadmin_instances.test.identity_store_ids)[0]
+  identity_store_id = data.aws_ssoadmin_instances.test.identity_store_ids[0]
   display_name      = %[1]q
-  description       = "Example description"
 }
+
+data "aws_ssoadmin_instances" "test" {}
 `, displayName)
 }
 
 func testAccGroupConfig_description(description string) string {
 	return fmt.Sprintf(`
-data "aws_ssoadmin_instances" "test" {}
 resource "aws_identitystore_group" "test" {
   identity_store_id = tolist(data.aws_ssoadmin_instances.test.identity_store_ids)[0]
   display_name      = "Test display name"
   description       = %[1]q
 }
+
+data "aws_ssoadmin_instances" "test" {}
 `, description)
 }
 
 func testAccGroupConfig_displayName(displayName string) string {
 	return fmt.Sprintf(`
-data "aws_ssoadmin_instances" "test" {}
 resource "aws_identitystore_group" "test" {
   identity_store_id = tolist(data.aws_ssoadmin_instances.test.identity_store_ids)[0]
   display_name      = %[1]q
   description       = "Test description"
 }
+
+data "aws_ssoadmin_instances" "test" {}
 `, displayName)
 }

internal/service/lakeformation/consts.go

@@ -7,11 +7,16 @@ import (
 	"time"
 )
 
+type TableType string
+
+const (
+	TableTypeTable            TableType = "Table"
+	TableTypeTableWithColumns TableType = "TableWithColumns"
+)
+
 const (
-	TableNameAllTables        = "ALL_TABLES"
-	TableTypeTable            = "Table"
-	TableTypeTableWithColumns = "TableWithColumns"
-	IAMAllowedPrincipals      = "IAM_ALLOWED_PRINCIPALS"
+	TableNameAllTables   = "ALL_TABLES"
+	IAMAllowedPrincipals = "IAM_ALLOWED_PRINCIPALS"
 )
 
 const (

internal/service/lakeformation/data_cells_filter_test.go

@@ -32,7 +32,7 @@ func testAccDataCellsFilter_basic(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
-			acctest.PreCheckPartitionHasService(t, names.LakeFormation)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
 			testAccDataCellsFilterPreCheck(ctx, t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
@@ -69,7 +69,7 @@ func testAccDataCellsFilter_columnWildcard(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
-			acctest.PreCheckPartitionHasService(t, names.LakeFormation)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
 			testAccDataCellsFilterPreCheck(ctx, t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
@@ -102,7 +102,7 @@ func testAccDataCellsFilter_disappears(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
-			acctest.PreCheckPartitionHasService(t, names.LakeFormation)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
 			testAccDataCellsFilterPreCheck(ctx, t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
@@ -137,7 +137,7 @@ func testAccDataCellsFilter_rowFilter(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
-			acctest.PreCheckPartitionHasService(t, names.LakeFormation)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
 			testAccDataCellsFilterPreCheck(ctx, t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),

internal/service/lakeformation/data_lake_settings_data_source_test.go

@@ -16,7 +16,10 @@ func testAccDataLakeSettingsDataSource_basic(t *testing.T) {
 	resourceName := "data.aws_lakeformation_data_lake_settings.test"
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.LakeFormation) },
+		PreCheck: func() {
+			acctest.PreCheck(ctx, t)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
+		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
 		CheckDestroy:             testAccCheckDataLakeSettingsDestroy(ctx),
@@ -42,7 +45,10 @@ func testAccDataLakeSettingsDataSource_readOnlyAdmins(t *testing.T) {
 	resourceName := "data.aws_lakeformation_data_lake_settings.test"
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.LakeFormation) },
+		PreCheck: func() {
+			acctest.PreCheck(ctx, t)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
+		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
 		CheckDestroy:             testAccCheckDataLakeSettingsDestroy(ctx),

internal/service/lakeformation/data_lake_settings_test.go

@@ -25,7 +25,10 @@ func testAccDataLakeSettings_basic(t *testing.T) {
 	resourceName := "aws_lakeformation_data_lake_settings.test"
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.LakeFormation) },
+		PreCheck: func() {
+			acctest.PreCheck(ctx, t)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
+		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
 		CheckDestroy:             testAccCheckDataLakeSettingsDestroy(ctx),
@@ -62,7 +65,10 @@ func testAccDataLakeSettings_disappears(t *testing.T) {
 	resourceName := "aws_lakeformation_data_lake_settings.test"
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.LakeFormation) },
+		PreCheck: func() {
+			acctest.PreCheck(ctx, t)
+			acctest.PreCheckPartitionHasService(t, names.LakeFormationEndpointID)
+		},
 		ErrorCheck:               acctest.ErrorCheck(t, names.LakeFormationServiceID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
 		CheckDestroy:             testAccCheckDataLakeSettingsDestroy(ctx),