From ab0efcf50508bbd78112155cf05725484f628412 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Apr 2023 16:40:29 -0400 Subject: [PATCH 1/3] networkmanager/core: Remove policy_document arg --- .../service/networkmanager/core_network.go | 77 +--------------- .../networkmanager/core_network_test.go | 91 ------------------- 2 files changed, 4 insertions(+), 164 deletions(-) diff --git a/internal/service/networkmanager/core_network.go b/internal/service/networkmanager/core_network.go index 5b644e36cca..5e7db2914d6 100644 --- a/internal/service/networkmanager/core_network.go +++ b/internal/service/networkmanager/core_network.go @@ -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" @@ -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), @@ -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, @@ -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, @@ -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} @@ -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 @@ -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 @@ -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), diff --git a/internal/service/networkmanager/core_network_test.go b/internal/service/networkmanager/core_network_test.go index ae72395c3ba..a6bc1387d12 100644 --- a/internal/service/networkmanager/core_network_test.go +++ b/internal/service/networkmanager/core_network_test.go @@ -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"), ), @@ -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" @@ -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", @@ -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", @@ -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()), @@ -351,7 +287,6 @@ func TestAccNetworkManagerCoreNetwork_withoutPolicyDocumentUpdateToCreateBasePol Config: testAccCoreNetworkConfig_basic(), Check: resource.ComposeTestCheckFunc( testAccCheckCoreNetworkExists(ctx, resourceName), - resource.TestCheckResourceAttr(resourceName, "policy_document", ""), ), }, { @@ -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", @@ -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" {} From 7d75213a356737e0ea4012138dae9a6e2beef9cd Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Apr 2023 16:51:31 -0400 Subject: [PATCH 2/3] Add changelog, update docs --- .changelog/30875.txt | 7 +++++++ website/docs/r/networkmanager_core_network.html.markdown | 5 +---- 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 .changelog/30875.txt diff --git a/.changelog/30875.txt b/.changelog/30875.txt new file mode 100644 index 00000000000..39e28b3957d --- /dev/null +++ b/.changelog/30875.txt @@ -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 +``` \ No newline at end of file diff --git a/website/docs/r/networkmanager_core_network.html.markdown b/website/docs/r/networkmanager_core_network.html.markdown index 0fa9ee9c95d..f176559516f 100644 --- a/website/docs/r/networkmanager_core_network.html.markdown +++ b/website/docs/r/networkmanager_core_network.html.markdown @@ -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 @@ -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 { @@ -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 From 52d30ea41189c0c826b98922b272ce5ed93f001e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 25 Apr 2023 10:48:22 -0400 Subject: [PATCH 3/3] docs: Remove unneeded note --- .../networkmanager_core_network_policy_attachment.html.markdown | 2 -- 1 file changed, 2 deletions(-) diff --git a/website/docs/r/networkmanager_core_network_policy_attachment.html.markdown b/website/docs/r/networkmanager_core_network_policy_attachment.html.markdown index 296993157ad..841aea6e0e0 100644 --- a/website/docs/r/networkmanager_core_network_policy_attachment.html.markdown +++ b/website/docs/r/networkmanager_core_network_policy_attachment.html.markdown @@ -10,8 +10,6 @@ description: |- Provides a Core Network Policy Attachment resource. This puts a Core Network Policy to an existing Core Network and executes the change set, which deploys changes globally based on the policy submitted (Sets the policy to `LIVE`). -~> **NOTE on Core Networks and Policy Attachments:** For a given policy attachment, this resource is incompatible with using the [`aws_networkmanager_core_network` resource](/docs/providers/aws/r/networkmanager_core_network.html) `policy_document` argument. When using that argument and this resource, both will attempt to manage the core network's policy document and Terraform will show a permanent difference. - ~> **NOTE:** Deleting this resource will not delete the current policy defined in this resource. Deleting this resource will also not revert the current `LIVE` policy to the previous version. ## Example Usage