Skip to content

Commit

Permalink
Merge pull request #22166 from hashicorp/b-glacier-policy-diffs
Browse files Browse the repository at this point in the history 
glacier/vault: Fix spurious policy diffs
  • Loading branch information
@YakDriver
YakDriver authored Dec 13, 2021
2 parents a5897a0 + 5d3da99 commit c9d2668
Show file tree
Hide file tree
Showing 5 changed files with 286 additions and 59 deletions.
7 changes: 7 additions & 0 deletions .changelog/22166.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_glacier_vault: Fix erroneous diffs in `access_policy` when no changes made or policies are equivalent
```

```release-note:bug
resource/aws_glacier_vault_lock: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
```
16 changes: 13 additions & 3 deletions internal/service/glacier/vault.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ func ResourceVault() *schema.Resource {
Optional: true,
ValidateFunc: validation.StringIsJSON,
DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v)
return json
},
},

"notification": {
Expand Down Expand Up @@ -212,10 +216,12 @@ func resourceVaultRead(d *schema.ResourceData, meta interface{}) error {
} else if err != nil {
return fmt.Errorf("error getting access policy for Glacier Vault (%s): %w", d.Id(), err)
} else if pol != nil && pol.Policy != nil {
policy, err := structure.NormalizeJsonString(aws.StringValue(pol.Policy.Policy))
policy, err := verify.PolicyToSet(d.Get("access_policy").(string), aws.StringValue(pol.Policy.Policy))

if err != nil {
return fmt.Errorf("access policy contains an invalid JSON: %w", err)
return err
}

d.Set("access_policy", policy)
}

Expand Down Expand Up @@ -278,7 +284,11 @@ func resourceVaultNotificationUpdate(conn *glacier.Glacier, d *schema.ResourceDa

func resourceVaultPolicyUpdate(conn *glacier.Glacier, d *schema.ResourceData) error {
vaultName := d.Id()
policyContents := d.Get("access_policy").(string)
policyContents, err := structure.NormalizeJsonString(d.Get("access_policy").(string))

if err != nil {
return fmt.Errorf("policy (%s) is invalid JSON: %w", policyContents, err)
}

policy := &glacier.VaultAccessPolicy{
Policy: aws.String(policyContents),
Expand Down
30 changes: 24 additions & 6 deletions internal/service/glacier/vault_lock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
Expand Down Expand Up @@ -43,6 +44,10 @@ func ResourceVaultLock() *schema.Resource {
ForceNew: true,
DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
ValidateFunc: verify.ValidIAMPolicyJSON,
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v)
return json
},
},
"vault_name": {
Type: schema.TypeString,
Expand All @@ -58,10 +63,16 @@ func resourceVaultLockCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).GlacierConn
vaultName := d.Get("vault_name").(string)

policy, err := structure.NormalizeJsonString(d.Get("policy").(string))

if err != nil {
return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
}

input := &glacier.InitiateVaultLockInput{
AccountId: aws.String("-"),
Policy: &glacier.VaultLockPolicy{
Policy: aws.String(d.Get("policy").(string)),
Policy: aws.String(policy),
},
VaultName: aws.String(vaultName),
}
Expand All @@ -88,7 +99,7 @@ func resourceVaultLockCreate(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error completing Glacier Vault (%s) Lock: %s", vaultName, err)
}

if err := waitForGlacierVaultLockCompletion(conn, vaultName); err != nil {
if err := waitVaultLockCompletion(conn, vaultName); err != nil {
return fmt.Errorf("error waiting for Glacier Vault Lock (%s) completion: %s", d.Id(), err)
}

Expand Down Expand Up @@ -123,9 +134,16 @@ func resourceVaultLockRead(d *schema.ResourceData, meta interface{}) error {
}

d.Set("complete_lock", aws.StringValue(output.State) == "Locked")
d.Set("policy", output.Policy)
d.Set("vault_name", d.Id())

policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(output.Policy))

if err != nil {
return err
}

d.Set("policy", policyToSet)

return nil
}

Expand All @@ -150,7 +168,7 @@ func resourceVaultLockDelete(d *schema.ResourceData, meta interface{}) error {
return nil
}

func glacierVaultLockRefreshFunc(conn *glacier.Glacier, vaultName string) resource.StateRefreshFunc {
func vaultLockRefreshFunc(conn *glacier.Glacier, vaultName string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
input := &glacier.GetVaultLockInput{
AccountId: aws.String("-"),
Expand All @@ -176,11 +194,11 @@ func glacierVaultLockRefreshFunc(conn *glacier.Glacier, vaultName string) resour
}
}

func waitForGlacierVaultLockCompletion(conn *glacier.Glacier, vaultName string) error {
func waitVaultLockCompletion(conn *glacier.Glacier, vaultName string) error {
stateConf := &resource.StateChangeConf{
Pending: []string{"InProgress"},
Target: []string{"Locked"},
Refresh: glacierVaultLockRefreshFunc(conn, vaultName),
Refresh: vaultLockRefreshFunc(conn, vaultName),
Timeout: 5 * time.Minute,
}

Expand Down
124 changes: 111 additions & 13 deletions internal/service/glacier/vault_lock_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,23 @@ import (
func TestAccGlacierVaultLock_basic(t *testing.T) {
var vaultLock1 glacier.GetVaultLockOutput
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
glacierVaultResourceName := "aws_glacier_vault.test"
vaultResourceName := "aws_glacier_vault.test"
resourceName := "aws_glacier_vault_lock.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, glacier.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckGlacierVaultLockDestroy,
CheckDestroy: testAccCheckVaultLockDestroy,
Steps: []resource.TestStep{
{
Config: testAccGlacierVaultLockConfigCompleteLock(rName, false),
Config: testAccVaultLockConfigCompleteLock(rName, false),
Check: resource.ComposeTestCheckFunc(
testAccCheckGlacierVaultLockExists(resourceName, &vaultLock1),
testAccCheckVaultLockExists(resourceName, &vaultLock1),
resource.TestCheckResourceAttr(resourceName, "complete_lock", "false"),
resource.TestCheckResourceAttr(resourceName, "ignore_deletion_error", "false"),
resource.TestCheckResourceAttrSet(resourceName, "policy"),
resource.TestCheckResourceAttrPair(resourceName, "vault_name", glacierVaultResourceName, "name"),
resource.TestCheckResourceAttrPair(resourceName, "vault_name", vaultResourceName, "name"),
),
},
{
Expand All @@ -49,23 +49,23 @@ func TestAccGlacierVaultLock_basic(t *testing.T) {
func TestAccGlacierVaultLock_completeLock(t *testing.T) {
var vaultLock1 glacier.GetVaultLockOutput
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
glacierVaultResourceName := "aws_glacier_vault.test"
vaultResourceName := "aws_glacier_vault.test"
resourceName := "aws_glacier_vault_lock.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, glacier.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckGlacierVaultLockDestroy,
CheckDestroy: testAccCheckVaultLockDestroy,
Steps: []resource.TestStep{
{
Config: testAccGlacierVaultLockConfigCompleteLock(rName, true),
Config: testAccVaultLockConfigCompleteLock(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckGlacierVaultLockExists(resourceName, &vaultLock1),
testAccCheckVaultLockExists(resourceName, &vaultLock1),
resource.TestCheckResourceAttr(resourceName, "complete_lock", "true"),
resource.TestCheckResourceAttr(resourceName, "ignore_deletion_error", "true"),
resource.TestCheckResourceAttrSet(resourceName, "policy"),
resource.TestCheckResourceAttrPair(resourceName, "vault_name", glacierVaultResourceName, "name"),
resource.TestCheckResourceAttrPair(resourceName, "vault_name", vaultResourceName, "name"),
),
},
{
Expand All @@ -78,7 +78,37 @@ func TestAccGlacierVaultLock_completeLock(t *testing.T) {
})
}

func testAccCheckGlacierVaultLockExists(resourceName string, getVaultLockOutput *glacier.GetVaultLockOutput) resource.TestCheckFunc {
func TestAccGlacierVaultLock_ignoreEquivalentPolicy(t *testing.T) {
var vaultLock1 glacier.GetVaultLockOutput
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
vaultResourceName := "aws_glacier_vault.test"
resourceName := "aws_glacier_vault_lock.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, glacier.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckVaultLockDestroy,
Steps: []resource.TestStep{
{
Config: testAccVaultLockPolicyOrderConfig(rName, false),
Check: resource.ComposeTestCheckFunc(
testAccCheckVaultLockExists(resourceName, &vaultLock1),
resource.TestCheckResourceAttr(resourceName, "complete_lock", "false"),
resource.TestCheckResourceAttr(resourceName, "ignore_deletion_error", "false"),
resource.TestCheckResourceAttrSet(resourceName, "policy"),
resource.TestCheckResourceAttrPair(resourceName, "vault_name", vaultResourceName, "name"),
),
},
{
Config: testAccVaultLockPolicyNewOrderConfig(rName, false),
PlanOnly: true,
},
},
})
}

func testAccCheckVaultLockExists(resourceName string, getVaultLockOutput *glacier.GetVaultLockOutput) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resourceName]
if !ok {
Expand Down Expand Up @@ -110,7 +140,7 @@ func testAccCheckGlacierVaultLockExists(resourceName string, getVaultLockOutput
}
}

func testAccCheckGlacierVaultLockDestroy(s *terraform.State) error {
func testAccCheckVaultLockDestroy(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).GlacierConn

for _, rs := range s.RootModule().Resources {
Expand Down Expand Up @@ -139,7 +169,7 @@ func testAccCheckGlacierVaultLockDestroy(s *terraform.State) error {
return nil
}

func testAccGlacierVaultLockConfigCompleteLock(rName string, completeLock bool) string {
func testAccVaultLockConfigCompleteLock(rName string, completeLock bool) string {
return fmt.Sprintf(`
resource "aws_glacier_vault" "test" {
name = %q
Expand Down Expand Up @@ -176,3 +206,71 @@ resource "aws_glacier_vault_lock" "test" {
}
`, rName, completeLock, completeLock)
}

func testAccVaultLockPolicyOrderConfig(rName string, completeLock bool) string {
return fmt.Sprintf(`
resource "aws_glacier_vault" "test" {
name = %[1]q
}
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
resource "aws_glacier_vault_lock" "test" {
complete_lock = %[2]t
ignore_deletion_error = %[2]t
vault_name = aws_glacier_vault.test.name
policy = jsonencode({
Version = "2012-10-17"
Statement = [{
Principal = {
AWS = "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:root"
}
Effect = "Allow"
Action = [
"glacier:InitiateMultipartUpload",
"glacier:AbortMultipartUpload",
"glacier:CompleteMultipartUpload",
"glacier:DeleteArchive",
]
Resource = aws_glacier_vault.test.arn
}]
})
}
`, rName, completeLock)
}

func testAccVaultLockPolicyNewOrderConfig(rName string, completeLock bool) string {
return fmt.Sprintf(`
resource "aws_glacier_vault" "test" {
name = %[1]q
}
data "aws_caller_identity" "current" {}
data "aws_partition" "current" {}
resource "aws_glacier_vault_lock" "test" {
complete_lock = %[2]t
ignore_deletion_error = %[2]t
vault_name = aws_glacier_vault.test.name
policy = jsonencode({
Version = "2012-10-17"
Statement = [{
Principal = {
AWS = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:root"]
}
Effect = "Allow"
Action = [
"glacier:InitiateMultipartUpload",
"glacier:DeleteArchive",
"glacier:CompleteMultipartUpload",
"glacier:AbortMultipartUpload",
]
Resource = [aws_glacier_vault.test.arn]
}]
})
}
`, rName, completeLock)
}
Loading

0 comments on commit c9d2668

Please sign in to comment.