Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f/aws_fsx_lustre_file_system specify custom KMS Key #15057

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1cf3995
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
1138cb1
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
9f6f591
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
62af651
Merge branch 'master' into aws_fsx_lustre_file_system-custom-KMS-Key
nikhil-goenka Sep 20, 2020
bc0219f
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
4875d2f
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
55a1184
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
d5889b1
Merge remote-tracking branch 'origin/aws_fsx_lustre_file_system-custo…
nikhil-goenka Sep 20, 2020
8b564f7
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
919a1d5
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
1a0f5dd
aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 8, 2020
9305b8b
Merge remote-tracking branch 'origin/aws_fsx_lustre_file_system-custo…
nikhil-goenka Sep 21, 2020
36c9331
Merge branch 'master' into aws_fsx_lustre_file_system-custom-KMS-Key
nikhil-goenka Sep 21, 2020
9eab2ac
tech-debt/aws_rds_cluster:TestAccAWSRDSCluster_SnapshotIdentifier_Pre…
nikhil-goenka Sep 21, 2020
e372705
tech-debt/aws_rds_cluster:TestAccAWSRDSCluster_SnapshotIdentifier_Pre…
nikhil-goenka Sep 21, 2020
2301bd0
tech-debt/aws_rds_cluster:TestAccAWSRDSCluster_SnapshotIdentifier_Pre…
nikhil-goenka Sep 21, 2020
ca7d23a
f/aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 22, 2020
07ac294
f/aws_fsx_lustre_file_system specify custom KMS Key
nikhil-goenka Sep 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions aws/resource_aws_fsx_lustre_file_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ func resourceAwsFsxLustreFileSystem() *schema.Resource {
fsx.LustreDeploymentTypePersistent1,
}, false),
},
"kms_key_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validateArn,
},
"per_unit_storage_throughput": {
Type: schema.TypeInt,
Optional: true,
Expand Down Expand Up @@ -154,6 +161,11 @@ func resourceAwsFsxLustreFileSystemCreate(d *schema.ResourceData, meta interface
},
}

//Applicable only for TypePersistent1
if v, ok := d.GetOk("kms_key_id"); ok {
input.KmsKeyId = aws.String(v.(string))
}

if v, ok := d.GetOk("automatic_backup_retention_days"); ok {
input.LustreConfiguration.AutomaticBackupRetentionDays = aws.Int64(int64(v.(int)))
}
Expand Down Expand Up @@ -293,6 +305,10 @@ func resourceAwsFsxLustreFileSystemRead(d *schema.ResourceData, meta interface{}
d.Set("per_unit_storage_throughput", lustreConfig.PerUnitStorageThroughput)
}

if filesystem.KmsKeyId != nil {
d.Set("kms_key_id", filesystem.KmsKeyId)
}

if err := d.Set("network_interface_ids", aws.StringValueSlice(filesystem.NetworkInterfaceIds)); err != nil {
return fmt.Errorf("error setting network_interface_ids: %w", err)
}
Expand Down
78 changes: 78 additions & 0 deletions aws/resource_aws_fsx_lustre_file_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ func TestAccAWSFsxLustreFileSystem_automaticBackupRetentionDays(t *testing.T) {
func TestAccAWSFsxLustreFileSystem_DeploymentTypePersistent1(t *testing.T) {
var filesystem fsx.FileSystem
resourceName := "aws_fsx_lustre_file_system.test"
datakmsKeyArn := "data.aws_kms_alias.fsx"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to below

Suggested change
datakmsKeyArn := "data.aws_kms_alias.fsx"


resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -446,6 +447,7 @@ func TestAccAWSFsxLustreFileSystem_DeploymentTypePersistent1(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "per_unit_storage_throughput", "50"),
resource.TestCheckResourceAttr(resourceName, "deployment_type", fsx.LustreDeploymentTypePersistent1),
resource.TestCheckResourceAttr(resourceName, "automatic_backup_retention_days", "0"),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", datakmsKeyArn, "target_key_arn"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally TestCheckResourceAttrPair() is a really good check, however since the data source is not guaranteed to exist the first FSx provisioning in a new account, we should opt for the less fun regular expression:

Suggested change
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", datakmsKeyArn, "target_key_arn"),
testAccMatchResourceAttrRegionalARN(resourceName, "kms_key_id", "kms", regexp.MustCompile(`key/.+`)),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified

),
},
{
Expand All @@ -458,6 +460,44 @@ func TestAccAWSFsxLustreFileSystem_DeploymentTypePersistent1(t *testing.T) {
})
}

func TestAccAWSFsxLustreFileSystem_KmsKeyId(t *testing.T) {
var filesystem1, filesystem2 fsx.FileSystem
resourceName := "aws_fsx_lustre_file_system.test"
kmsKeyResourceName1 := "aws_kms_key.test1"
kmsKeyResourceName2 := "aws_kms_key.test2"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckFsxLustreFileSystemDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsFsxLustreFileSystemConfigKmsKeyId1(),
Check: resource.ComposeTestCheckFunc(
testAccCheckFsxLustreFileSystemExists(resourceName, &filesystem1),
resource.TestCheckResourceAttr(resourceName, "deployment_type", fsx.LustreDeploymentTypePersistent1),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName1, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"security_group_ids"},
},
{
Config: testAccAwsFsxLustreFileSystemConfigKmsKeyId2(),
Check: resource.ComposeTestCheckFunc(
testAccCheckFsxLustreFileSystemExists(resourceName, &filesystem2),
resource.TestCheckResourceAttr(resourceName, "deployment_type", fsx.LustreDeploymentTypePersistent1),
testAccCheckFsxWindowsFileSystemRecreated(&filesystem1, &filesystem2),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName2, "arn"),
),
},
},
})
}

func TestAccAWSFsxLustreFileSystem_DeploymentTypeScratch2(t *testing.T) {
var filesystem fsx.FileSystem
resourceName := "aws_fsx_lustre_file_system.test"
Expand Down Expand Up @@ -786,5 +826,43 @@ resource "aws_fsx_lustre_file_system" "test" {
deployment_type = "PERSISTENT_1"
per_unit_storage_throughput = %[1]d
}

data "aws_kms_alias" "fsx" {
name = "alias/aws/fsx"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this KMS Key guaranteed to exist in AWS accounts that have not provisioned FSx before? Generally, AWS-managed keys are not created until then. It might be best to remove this here and update the test step to just check for a regular expression so there is not a test dependency the first run in a new account.

Suggested change
data "aws_kms_alias" "fsx" {
name = "alias/aws/fsx"
}

Copy link
Contributor Author

@nikhil-goenka nikhil-goenka Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is a valid reason, either we can add regex match or just remove the check, since it has to be kms by default and we don't need to verify it.(how it was before)
let me know your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to confirm in a separate AWS account that the FSx KMS Key and its associated Alias is not automatically available. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

`, perUnitStorageThroughput)
}

func testAccAwsFsxLustreFileSystemConfigKmsKeyId1() string {
return testAccAwsFsxLustreFileSystemConfigBase() + `
resource "aws_kms_key" "test1" {
description = "FSx KMS Testing key"
deletion_window_in_days = 7
}

resource "aws_fsx_lustre_file_system" "test" {
storage_capacity = 1200
subnet_ids = [aws_subnet.test1.id]
deployment_type = "PERSISTENT_1"
per_unit_storage_throughput = 50
kms_key_id = aws_kms_key.test1.arn
}
`
}

func testAccAwsFsxLustreFileSystemConfigKmsKeyId2() string {
return testAccAwsFsxLustreFileSystemConfigBase() + `
resource "aws_kms_key" "test2" {
description = "FSx KMS Testing key"
deletion_window_in_days = 7
}

resource "aws_fsx_lustre_file_system" "test" {
storage_capacity = 1200
subnet_ids = [aws_subnet.test1.id]
deployment_type = "PERSISTENT_1"
per_unit_storage_throughput = 50
kms_key_id = aws_kms_key.test2.arn
}
`
}
5 changes: 3 additions & 2 deletions website/docs/r/fsx_lustre_file_system.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ The following arguments are supported:
* `security_group_ids` - (Optional) A list of IDs for the security groups that apply to the specified network interfaces created for file system access. These security groups will apply to all network interfaces.
* `tags` - (Optional) A map of tags to assign to the file system.
* `weekly_maintenance_start_time` - (Optional) The preferred start time (in `d:HH:MM` format) to perform weekly maintenance, in the UTC time zone.
* `deployment_type` - (Optional) The filesystem deployment type. One of: `SCRATCH_1`, `SCRATCH_2`, `PERSISTENT_1`.
* `per_unit_storage_throughput` - (Optional) Describes the amount of read and write throughput for each 1 tebibyte of storage, in MB/s/TiB, required for the `PERSISTENT_1` deployment_type. For valid values, see the [AWS documentation](https://docs.aws.amazon.com/fsx/latest/APIReference/API_CreateFileSystemLustreConfiguration.html).
* `deployment_type` - (Optional) - The filesystem deployment type. One of: `SCRATCH_1`, `SCRATCH_2`, `PERSISTENT_1`.
* `kms_key_id` - (Optional) ARN for the KMS Key to encrypt the file system at rest, applicable for `PERSISTENT_1`. Defaults to an AWS managed KMS Key.
* `per_unit_storage_throughput` - (Optional) - Describes the amount of read and write throughput for each 1 tebibyte of storage, in MB/s/TiB, required for the `PERSISTENT_1` deployment_type. For valid values, see the [AWS documentation](https://docs.aws.amazon.com/fsx/latest/APIReference/API_CreateFileSystemLustreConfiguration.html).
* `automatic_backup_retention_days` - (Optional) The number of days to retain automatic backups. Setting this to 0 disables automatic backups. You can retain automatic backups for a maximum of 35 days. only valid for `PERSISTENT_1` deployment_type.

## Attributes Reference
Expand Down