Skip to content

Commit

Permalink
Merge pull request #30875 from hashicorp/t-nm-core-remove-policy-doc
Browse files Browse the repository at this point in the history 
networkmanager/core: Remove `policy_document` arg
  • Loading branch information
@YakDriver
YakDriver authored Apr 25, 2023
2 parents 16b72ad + 52d30ea commit d5e9e20
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 170 deletions.
7 changes: 7 additions & 0 deletions .changelog/30875.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:breaking-change
resource/aws_networkmanager_core_network: Removed `policy_document` argument -- use `aws_networkmanager_core_network_policy_attachment` resource instead
```

```release-note:note
resource/aws_networkmanager_core_network: Update configurations to use the `aws_networkmanager_core_network_policy_attachment` resource instead of the `policy_document` argument
```
77 changes: 4 additions & 73 deletions internal/service/networkmanager/core_network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ import (
"github.com/aws/aws-sdk-go/service/networkmanager"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"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"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
Expand All @@ -43,10 +41,7 @@ func ResourceCoreNetwork() *schema.Resource {
StateContext: schema.ImportStatePassthroughContext,
},

CustomizeDiff: customdiff.Sequence(
resourceCoreNetworkCustomizeDiff,
verify.SetTagsDiff,
),
CustomizeDiff: verify.SetTagsDiff,

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(30 * time.Minute),
Expand Down Expand Up @@ -77,10 +72,9 @@ func ResourceCoreNetwork() *schema.Resource {
ConflictsWith: []string{"base_policy_region"},
},
"create_base_policy": {
Type: schema.TypeBool,
Optional: true,
Default: false,
ConflictsWith: []string{"policy_document"},
Type: schema.TypeBool,
Optional: true,
Default: false,
},
"created_at": {
Type: schema.TypeString,
Expand Down Expand Up @@ -118,23 +112,6 @@ func ResourceCoreNetwork() *schema.Resource {
ForceNew: true,
ValidateFunc: validation.StringLenBetween(0, 50),
},
"policy_document": {
Deprecated: "Use the aws_networkmanager_core_network_policy_attachment resource instead. " +
"This attribute will be removed in the next major version of the provider.",
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.All(
validation.StringLenBetween(0, 10000000),
validation.StringIsJSON,
),
DiffSuppressFunc: verify.SuppressEquivalentJSONDiffs,
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v)
return json
},
ConflictsWith: []string{"create_base_policy"},
},
"segments": {
Type: schema.TypeList,
Computed: true,
Expand Down Expand Up @@ -181,14 +158,9 @@ func resourceCoreNetworkCreate(ctx context.Context, d *schema.ResourceData, meta
input.Description = aws.String(v.(string))
}

if v, ok := d.GetOk("policy_document"); ok {
input.PolicyDocument = aws.String(v.(string))
}

// check if the user wants to create a base policy document
// this creates the core network with a starting policy document set to LIVE
// this is required for the first terraform apply if there attachments to the core network
// and the core network is created without the policy_document argument set
if _, ok := d.GetOk("create_base_policy"); ok {
// if user supplies a region or multiple regions use it in the base policy, otherwise use current region
regions := []interface{}{meta.(*conns.AWSClient).Region}
Expand Down Expand Up @@ -251,24 +223,6 @@ func resourceCoreNetworkRead(ctx context.Context, d *schema.ResourceData, meta i
}
d.Set("state", coreNetwork.State)

// getting the policy document uses a different API call
// policy document is also optional
coreNetworkPolicy, err := FindCoreNetworkPolicyByID(ctx, conn, d.Id())

if tfresource.NotFound(err) {
d.Set("policy_document", nil)
} else if err != nil {
return diag.Errorf("reading Network Manager Core Network (%s) policy: %s", d.Id(), err)
} else {
encodedPolicyDocument, err := protocol.EncodeJSONValue(coreNetworkPolicy.PolicyDocument, protocol.NoEscape)

if err != nil {
return diag.Errorf("encoding Network Manager Core Network (%s) policy document: %s", d.Id(), err)
}

d.Set("policy_document", encodedPolicyDocument)
}

SetTagsOut(ctx, coreNetwork.Tags)

return nil
Expand All @@ -292,18 +246,6 @@ func resourceCoreNetworkUpdate(ctx context.Context, d *schema.ResourceData, meta
}
}

if d.HasChange("policy_document") {
err := PutAndExecuteCoreNetworkPolicy(ctx, conn, d.Id(), d.Get("policy_document").(string))

if err != nil {
return diag.FromErr(err)
}

if _, err := waitCoreNetworkUpdated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutUpdate)); err != nil {
return diag.Errorf("waiting for Network Manager Core Network (%s) update: %s", d.Id(), err)
}
}

if d.HasChange("create_base_policy") {
if _, ok := d.GetOk("create_base_policy"); ok {
// if user supplies a region or multiple regions use it in the base policy, otherwise use current region
Expand Down Expand Up @@ -358,17 +300,6 @@ func resourceCoreNetworkDelete(ctx context.Context, d *schema.ResourceData, meta
return nil
}

func resourceCoreNetworkCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
if d.HasChange("policy_document") {
if o, n := d.GetChange("policy_document"); !verify.JSONStringsEqual(o.(string), n.(string)) {
d.SetNewComputed("edges")
d.SetNewComputed("segments")
}
}

return nil
}

func FindCoreNetworkByID(ctx context.Context, conn *networkmanager.NetworkManager, id string) (*networkmanager.CoreNetwork, error) {
input := &networkmanager.GetCoreNetworkInput{
CoreNetworkId: aws.String(id),
Expand Down
91 changes: 0 additions & 91 deletions internal/service/networkmanager/core_network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestAccNetworkManagerCoreNetwork_basic(t *testing.T) {
resource.TestCheckResourceAttrSet(resourceName, "created_at"),
resource.TestCheckResourceAttr(resourceName, "description", ""),
resource.TestMatchResourceAttr(resourceName, "id", regexp.MustCompile(`core-network-.+`)),
resource.TestCheckResourceAttr(resourceName, "policy_document", ""),
resource.TestCheckResourceAttr(resourceName, "state", networkmanager.CoreNetworkStateAvailable),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
),
Expand Down Expand Up @@ -151,64 +150,6 @@ func TestAccNetworkManagerCoreNetwork_description(t *testing.T) {
})
}

func TestAccNetworkManagerCoreNetwork_policyDocument(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_networkmanager_core_network.test"
originalSegmentValue := "segmentValue1"
updatedSegmentValue := "segmentValue2"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckCoreNetworkDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccCoreNetworkConfig_policyDocument(originalSegmentValue),
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"65022-65534\"],\"edge-locations\":[{\"location\":\"%s\"}],\"vpn-ecmp-support\":true},\"segments\":[{\"isolate-attachments\":false,\"name\":\"%s\",\"require-attachment-acceptance\":true}],\"version\":\"2021.12\"}", acctest.Region(), originalSegmentValue)),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "edges.*", map[string]string{
"asn": "65022",
"edge_location": acctest.Region(),
"inside_cidr_blocks.#": "0",
}),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "segments.*", map[string]string{
"edge_locations.#": "1",
"edge_locations.0": acctest.Region(),
"name": originalSegmentValue,
"shared_segments.#": "0",
}),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"create_base_policy"},
},
{
Config: testAccCoreNetworkConfig_policyDocument(updatedSegmentValue),
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"65022-65534\"],\"edge-locations\":[{\"location\":\"%s\"}],\"vpn-ecmp-support\":true},\"segments\":[{\"isolate-attachments\":false,\"name\":\"%s\",\"require-attachment-acceptance\":true}],\"version\":\"2021.12\"}", acctest.Region(), updatedSegmentValue)),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "edges.*", map[string]string{
"asn": "65022",
"edge_location": acctest.Region(),
"inside_cidr_blocks.#": "0",
}),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "segments.*", map[string]string{
"edge_locations.#": "1",
"edge_locations.0": acctest.Region(),
"name": updatedSegmentValue,
"shared_segments.#": "0",
}),
),
},
},
})
}

func TestAccNetworkManagerCoreNetwork_createBasePolicyDocumentWithoutRegion(t *testing.T) {
ctx := acctest.Context(t)
resourceName := "aws_networkmanager_core_network.test"
Expand All @@ -224,7 +165,6 @@ func TestAccNetworkManagerCoreNetwork_createBasePolicyDocumentWithoutRegion(t *t
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "create_base_policy", "true"),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"64512-65534\"],\"edge-locations\":[{\"location\":\"%s\"}],\"vpn-ecmp-support\":false},\"segments\":[{\"description\":\"base-policy\",\"isolate-attachments\":false,\"name\":\"segment\",\"require-attachment-acceptance\":false}],\"version\":\"2021.12\"}", acctest.Region())),
resource.TestCheckNoResourceAttr(resourceName, "base_policy_region"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "edges.*", map[string]string{
"asn": "64512",
Expand Down Expand Up @@ -264,7 +204,6 @@ func TestAccNetworkManagerCoreNetwork_createBasePolicyDocumentWithRegion(t *test
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "create_base_policy", "true"),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"64512-65534\"],\"edge-locations\":[{\"location\":\"%s\"}],\"vpn-ecmp-support\":false},\"segments\":[{\"description\":\"base-policy\",\"isolate-attachments\":false,\"name\":\"segment\",\"require-attachment-acceptance\":false}],\"version\":\"2021.12\"}", acctest.AlternateRegion())),
resource.TestCheckResourceAttr(resourceName, "base_policy_region", acctest.AlternateRegion()),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "edges.*", map[string]string{
"asn": "64512",
Expand Down Expand Up @@ -304,9 +243,6 @@ func TestAccNetworkManagerCoreNetwork_createBasePolicyDocumentWithMultiRegion(t
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "create_base_policy", "true"),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"64512-65534\"],\"edge-locations\":[{\"location\":\"%s\"},{\"location\":\"%s\"}],\"vpn-ecmp-support\":false},\"segments\":[{\"description\":\"base-policy\",\"isolate-attachments\":false,\"name\":\"segment\",\"require-attachment-acceptance\":false}],\"version\":\"2021.12\"}", acctest.AlternateRegion(), acctest.Region())),
// use test below if locations are unordered
// resource.TestMatchResourceAttr(resourceName, "policy_document", regexp.MustCompile(`{"core-network-configuration":{"asn-ranges":\["64512-65534"\],"edge-locations":\[{"location":".+"},{"location":".+"}\],"vpn-ecmp-support":false},"segments":\[{"description":"base-policy","isolate-attachments":false,"name":"segment","require-attachment-acceptance":false}\],"version":"2021.12"}`)),
resource.TestCheckResourceAttr(resourceName, "base_policy_regions.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "base_policy_regions.*", acctest.AlternateRegion()),
resource.TestCheckTypeSetElemAttr(resourceName, "base_policy_regions.*", acctest.Region()),
Expand Down Expand Up @@ -351,7 +287,6 @@ func TestAccNetworkManagerCoreNetwork_withoutPolicyDocumentUpdateToCreateBasePol
Config: testAccCoreNetworkConfig_basic(),
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "policy_document", ""),
),
},
{
Expand All @@ -365,7 +300,6 @@ func TestAccNetworkManagerCoreNetwork_withoutPolicyDocumentUpdateToCreateBasePol
Check: resource.ComposeTestCheckFunc(
testAccCheckCoreNetworkExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, "create_base_policy", "true"),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"core-network-configuration\":{\"asn-ranges\":[\"64512-65534\"],\"edge-locations\":[{\"location\":\"%s\"}],\"vpn-ecmp-support\":false},\"segments\":[{\"description\":\"base-policy\",\"isolate-attachments\":false,\"name\":\"segment\",\"require-attachment-acceptance\":false}],\"version\":\"2021.12\"}", acctest.Region())),
resource.TestCheckNoResourceAttr(resourceName, "base_policy_region"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "edges.*", map[string]string{
"asn": "64512",
Expand Down Expand Up @@ -478,31 +412,6 @@ resource "aws_networkmanager_core_network" "test" {
`, description)
}

func testAccCoreNetworkConfig_policyDocument(segmentValue string) string {
return fmt.Sprintf(`
resource "aws_networkmanager_global_network" "test" {}
data "aws_networkmanager_core_network_policy_document" "test" {
core_network_configuration {
asn_ranges = ["65022-65534"]
edge_locations {
location = %[2]q
}
}
segments {
name = %[1]q
}
}
resource "aws_networkmanager_core_network" "test" {
global_network_id = aws_networkmanager_global_network.test.id
policy_document = data.aws_networkmanager_core_network_policy_document.test.json
}
`, segmentValue, acctest.Region())
}

func testAccCoreNetworkConfig_basePolicyDocumentWithoutRegion() string {
return `
resource "aws_networkmanager_global_network" "test" {}
Expand Down
5 changes: 1 addition & 4 deletions website/docs/r/networkmanager_core_network.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ description: |-

Provides a core network resource.

~> **NOTE on Core Networks and Policy Attachments:** For a given core network, this resource's `policy_document` argument is incompatible with using the [`aws_networkmanager_core_network_policy_attachment` resource](/docs/providers/aws/r/networkmanager_core_network_policy_attachment.html). When using this resource's `policy_document` argument and the `aws_networkmanager_core_network_policy_attachment` resource, both will attempt to manage the core network's policy document and Terraform will show a permanent difference.

## Example Usage

### Basic
Expand Down Expand Up @@ -176,7 +174,7 @@ The following arguments are supported:
* `description` - (Optional) Description of the Core Network.
* `base_policy_region` - (Optional, **Deprecated** use the `base_policy_regions` argument instead) The base policy created by setting the `create_base_policy` argument to `true` requires a region to be set in the `edge-locations`, `location` key. If `base_policy_region` is not specified, the region used in the base policy defaults to the region specified in the `provider` block.
* `base_policy_regions` - (Optional) A list of regions to add to the base policy. The base policy created by setting the `create_base_policy` argument to `true` requires one or more regions to be set in the `edge-locations`, `location` key. If `base_policy_regions` is not specified, the region used in the base policy defaults to the region specified in the `provider` block.
* `create_base_policy` - (Optional) Specifies whether to create a base policy when a core network is created or updated. A base policy is created and set to `LIVE` to allow attachments to the core network (e.g. VPC Attachments) before applying a policy document provided using the [`aws_networkmanager_core_network_policy_attachment` resource](/docs/providers/aws/r/networkmanager_core_network_policy_attachment.html). This base policy is needed if your core network does not have any `LIVE` policies (e.g. a core network resource created without the `policy_document` argument) and your policy document has static routes pointing to VPC attachments and you want to attach your VPCs to the core network before applying the desired policy document. Valid values are `true` or `false`. Conflicts with `policy_document`. An example of this Terraform snippet can be found above [for VPC Attachment in a single region](#with-vpc-attachment-single-region) and [for VPC Attachment multi-region](#with-vpc-attachment-multi-region). An example base policy is shown below. This base policy is overridden with the policy that you specify in the [`aws_networkmanager_core_network_policy_attachment` resource](/docs/providers/aws/r/networkmanager_core_network_policy_attachment.html).
* `create_base_policy` - (Optional) Specifies whether to create a base policy when a core network is created or updated. A base policy is created and set to `LIVE` to allow attachments to the core network (e.g. VPC Attachments) before applying a policy document provided using the [`aws_networkmanager_core_network_policy_attachment` resource](/docs/providers/aws/r/networkmanager_core_network_policy_attachment.html). This base policy is needed if your core network does not have any `LIVE` policies and your policy document has static routes pointing to VPC attachments and you want to attach your VPCs to the core network before applying the desired policy document. Valid values are `true` or `false`. An example of this Terraform snippet can be found above [for VPC Attachment in a single region](#with-vpc-attachment-single-region) and [for VPC Attachment multi-region](#with-vpc-attachment-multi-region). An example base policy is shown below. This base policy is overridden with the policy that you specify in the [`aws_networkmanager_core_network_policy_attachment` resource](/docs/providers/aws/r/networkmanager_core_network_policy_attachment.html).

```json
{
Expand Down Expand Up @@ -204,7 +202,6 @@ The following arguments are supported:
```

* `global_network_id` - (Required) The ID of the global network that a core network will be a part of.
* `policy_document` - (Optional, **Deprecated** use the [`aws_networkmanager_core_network_policy_attachment`](networkmanager_core_network_policy_attachment.html) resource instead) Policy document for creating a core network. Note that updating this argument will result in the new policy document version being set as the `LATEST` and `LIVE` policy document. Refer to the [Core network policies documentation](https://docs.aws.amazon.com/network-manager/latest/cloudwan/cloudwan-policy-change-sets.html) for more information. Conflicts with `create_base_policy`.
* `tags` - (Optional) Key-value tags for the Core Network. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level.

## Timeouts
Expand Down
Loading

0 comments on commit d5e9e20

Please sign in to comment.