HCPF-704: Fix Missing/Incorrect Resource-level project_id Implement…

…ations (#522) * Fix `hcp_packer_channel` * Fix `hcp_azure_peering_connection` * Fix `hcp_hvn_route` * Fix `hcp_vault_cluster_admin_token` * Fix `hcp_consul_cluster_root_token` * Fix `hcp_boundary_cluster` * Fix `data.hcp_azure_peering_connection` * Fix `data.consul_agent_helm_config` * Align resources with code style for grabbing orgID * Add changelog * Elaborate in changelog * Remove unnecessary fields from hvn_peering_connection Squashed commit of the following: commit ba0cd45a6e7b2b51f439848c415890cfdabc823a Author: Aidan Mundy <[email protected]> Date: Mon Jun 5 20:50:23 2023 -0400 Final changes commit 18f6991 Author: Aidan Mundy <[email protected]> Date: Mon Jun 5 20:32:11 2023 -0400 Readd forcenew for resource commit fae8926 Author: Aidan Mundy <[email protected]> Date: Mon Jun 5 20:29:27 2023 -0400 Mark old fields as deprecated commit 14018b3 Author: Aidan Mundy <[email protected]> Date: Mon Jun 5 20:14:58 2023 -0400 Regen docs commit ca737a0 Author: Aidan Mundy <[email protected]> Date: Mon Jun 5 20:13:48 2023 -0400 Remove unnecessary HVN Peering fields * Deprecate `project_id` on `data.hvn_route` * Fix hvn_peering_connection data source test * Deprecate `project_id` for `hcp_hvn_route` resource * Update deprecation notice to include `hcp_hvn_route` * Fix typo * Fix docs for `data.hcp_hvn_peering_connection` Reflects deprecation of unused `hvn_2` attribute.
hashicorp · Jun 7, 2023 · 82fe9a7 · 82fe9a7
1 parent f42ada2
commit 82fe9a7
Show file tree

Hide file tree

Showing 20 changed files with 249 additions and 209 deletions.
diff --git a/.changelog/522.txt b/.changelog/522.txt
@@ -0,0 +1,32 @@
+```release-note:bug
+Fixed several missing/incorrect implementations for the resource-level 
+`project_id` attribute that could lead to undefined or undesirable behavior on 
+some resources and data sources when the `project_id` attribute had been used 
+and its most recent value was different from the provider-level `project_id`,
+whether or not the attribute was still present in the configuration file.
+
+NOTE: See associated PR for caveats on temporary regressions.
+```
+
+```release-note:deprecation
+Setting the `project_id` attribute on `hcp_hvn_peering_connection` and 
+`data.hcp_hvn_peering_connection` is now deprecated. The value of the field was 
+required to match the project ID for `hvn_1` and will now be determined 
+automatically. Remove the `project_id` field from the configuration for 
+affected resources and data sources.
+```
+
+```release-note:deprecation
+Setting the `hvn_2` attribute of `data.hcp_hvn_peering_connection` is now 
+deprecated. The value of the attribute is not needed to fetch data, and it was 
+never validated against the real value for `hvn_2`. The value will now be 
+populated automatically. Remove the `hvn_2` attribute from the configuration 
+for affected data sources.
+```
+
+```release-note:deprecation
+Setting the `project_id` attribute on `hcp_hvn_route` and `data.hcp_hvn_route`
+is now deprecated. The value of the field was required to match the project ID 
+in `hvn_link` and will now be determined automatically. Remove the `project_id` 
+field from the configuration for affected resources and data sources.
+```
diff --git a/docs/data-sources/hvn_peering_connection.md b/docs/data-sources/hvn_peering_connection.md
@@ -16,7 +16,6 @@ The HVN peering connection data source provides information about an existing pe
 data "hcp_hvn_peering_connection" "test" {
   peering_id = var.peering_id
   hvn_1      = var.hvn_1
-  hvn_2      = var.hvn_2
 }
 ```
 
@@ -26,20 +25,20 @@ data "hcp_hvn_peering_connection" "test" {
 ### Required
 
 - `hvn_1` (String) The unique URL of one of the HVNs being peered.
-- `hvn_2` (String) The unique URL of one of the HVNs being peered.
 - `peering_id` (String) The ID of the peering connection.
 
 ### Optional
 
-- `project_id` (String) The ID of the HCP project where the HVN peering connection is located.
+- `hvn_2` (String, Deprecated) The unique URL of one of the HVNs being peered. Setting this attribute is deprecated, but it will remain usable in read-only form.
+- `project_id` (String, Deprecated) The ID of the HCP project where the HVN peering connection is located. Always matches hvn_1's project ID. Setting this attribute is deprecated, but it will remain usable in read-only form.
 - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts))
 
 ### Read-Only
 
 - `created_at` (String) The time that the peering connection was created.
 - `expires_at` (String) The time after which the peering connection will be considered expired if it hasn't transitioned into `ACCEPTED` or `ACTIVE` state.
 - `id` (String) The ID of this resource.
-- `organization_id` (String) The ID of the HCP organization where the peering connection is located. Always matches the HVNs' organization.
+- `organization_id` (String) The ID of the HCP organization where the peering connection is located. Always matches both HVNs' organization ID
 - `self_link` (String) A unique URL identifying the peering connection
 - `state` (String) The state of the HVN peering connection.
 

diff --git a/docs/data-sources/hvn_route.md b/docs/data-sources/hvn_route.md
@@ -29,7 +29,7 @@ data "hcp_hvn_route" "example" {
 
 ### Optional
 
-- `project_id` (String) The ID of the HCP project where the HVN route is located.
+- `project_id` (String, Deprecated) The ID of the HCP project where the HVN route is located. Always matches the project ID in `hvn_link`. Setting this attribute is deprecated, but it will remain usable in read-only form.
 - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts))
 
 ### Read-Only

diff --git a/docs/resources/hvn_peering_connection.md b/docs/resources/hvn_peering_connection.md
@@ -43,15 +43,15 @@ resource "hcp_hvn_peering_connection" "peer_1" {
 
 ### Optional
 
-- `project_id` (String) The ID of the HCP project where the HVN peering connection is located.
+- `project_id` (String, Deprecated) The ID of the HCP project where HVN peering connection is located. Always matches hvn_1's project ID. Setting this attribute is deprecated, but it will remain usable in read-only form.
 - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts))
 
 ### Read-Only
 
 - `created_at` (String) The time that the peering connection was created.
 - `expires_at` (String) The time after which the peering connection will be considered expired if it hasn't transitioned into `ACCEPTED` or `ACTIVE` state.
 - `id` (String) The ID of this resource.
-- `organization_id` (String) The ID of the HCP organization where the peering connection is located. Always matches the HVNs' organization.
+- `organization_id` (String) The ID of the HCP organization where the peering connection is located. Always matches both HVNs' organization ID.
 - `peering_id` (String) The ID of the peering connection.
 - `self_link` (String) A unique URL identifying the peering connection
 - `state` (String) The state of the HVN peering connection.

diff --git a/docs/resources/hvn_route.md b/docs/resources/hvn_route.md
@@ -67,7 +67,7 @@ resource "hcp_hvn_route" "example-peering-route" {
 
 ### Optional
 
-- `project_id` (String) The ID of the HCP project where the HVN route is located.
+- `project_id` (String, Deprecated) The ID of the HCP project where the HVN route is located. Always matches the project ID in `hvn_link`. Setting this attribute is deprecated, but it will remain usable in read-only form.
 - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts))
 
 ### Read-Only

diff --git a/examples/data-sources/hcp_hvn_peering_connection/data-source.tf b/examples/data-sources/hcp_hvn_peering_connection/data-source.tf
@@ -1,5 +1,4 @@
 data "hcp_hvn_peering_connection" "test" {
   peering_id = var.peering_id
   hvn_1      = var.hvn_1
-  hvn_2      = var.hvn_2
 }
diff --git a/examples/data-sources/hcp_hvn_peering_connection/variables.tf b/examples/data-sources/hcp_hvn_peering_connection/variables.tf
@@ -7,8 +7,3 @@ variable "hvn_1" {
   description = "The unique URL of one of the HVNs being peered."
   type        = string
 }
-
-variable "hvn_2" {
-  description = "The unique URL of one of the HVNs being peered."
-  type        = string
-}
diff --git a/internal/provider/data_source_azure_peering_connection.go b/internal/provider/data_source_azure_peering_connection.go
@@ -113,19 +113,19 @@ func dataSourceAzurePeeringConnection() *schema.Resource {
 
 func dataSourceAzurePeeringConnectionRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client)
-	orgID := client.Config.OrganizationID
-
 	peeringID := d.Get("peering_id").(string)
-	loc := &sharedmodels.HashicorpCloudLocationLocation{
-		OrganizationID: client.Config.OrganizationID,
-		ProjectID:      client.Config.ProjectID,
-	}
-	hvnLink, err := buildLinkFromURL(d.Get("hvn_link").(string), HvnResourceType, orgID)
+
+	hvnLink, err := buildLinkFromURL(d.Get("hvn_link").(string), HvnResourceType, client.Config.OrganizationID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 	waitForActive := d.Get("wait_for_active_state").(bool)
 
+	loc := &sharedmodels.HashicorpCloudLocationLocation{
+		OrganizationID: hvnLink.Location.OrganizationID,
+		ProjectID:      hvnLink.Location.ProjectID,
+	}
+
 	// Query for the peering.
 	log.Printf("[INFO] Reading peering connection (%s)", peeringID)
 	peering, err := clients.GetPeeringByID(ctx, client, peeringID, hvnLink.ID, loc)

diff --git a/internal/provider/data_source_consul_agent_helm_config.go b/internal/provider/data_source_consul_agent_helm_config.go
@@ -122,15 +122,8 @@ func dataSourceConsulAgentHelmConfigRead(ctx context.Context, d *schema.Resource
 		return diag.Errorf("unable to retrieve project ID: %v", err)
 	}
 
-	organizationID := client.Config.OrganizationID
-
-	v, ok := d.GetOk("project_id")
-	if ok {
-		projectID = v.(string)
-	}
-
 	loc := &models.HashicorpCloudLocationLocation{
-		OrganizationID: organizationID,
+		OrganizationID: client.Config.OrganizationID,
 		ProjectID:      projectID,
 	}
 

diff --git a/internal/provider/data_source_consul_agent_kubernetes_secret.go b/internal/provider/data_source_consul_agent_kubernetes_secret.go
@@ -76,12 +76,10 @@ func dataSourceConsulAgentKubernetesSecretRead(ctx context.Context, d *schema.Re
 		return diag.Errorf("unable to retrieve project ID: %v", err)
 	}
 
-	organizationID := client.Config.OrganizationID
-
 	clusterID := d.Get("cluster_id").(string)
 
 	loc := &models.HashicorpCloudLocationLocation{
-		OrganizationID: organizationID,
+		OrganizationID: client.Config.OrganizationID,
 		ProjectID:      projectID,
 	}
 

diff --git a/internal/provider/data_source_hvn_peering_connection.go b/internal/provider/data_source_hvn_peering_connection.go
@@ -7,10 +7,8 @@ import (
 	"context"
 	"log"
 
-	sharedmodels "github.com/hashicorp/hcp-sdk-go/clients/cloud-shared/v1/models"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/hashicorp/terraform-provider-hcp/internal/clients"
 )
 
@@ -33,22 +31,31 @@ func dataSourceHvnPeeringConnection() *schema.Resource {
 				Type:        schema.TypeString,
 				Required:    true,
 			},
+			// Computed outputs
 			"hvn_2": {
-				Description: "The unique URL of one of the HVNs being peered.",
+				Description: "The unique URL of one of the HVNs being peered. Setting this attribute is deprecated, but it will remain usable in read-only form.",
 				Type:        schema.TypeString,
-				Required:    true,
+				Computed:    true,
+				Optional:    true,
+				Deprecated: `
+Setting the 'hvn_2' attribute is deprecated, but it will remain usable in read-only form.
+Previously, the value for this attribute was not used to fetch data, and it was not validated against the actual value of 'hvn_2'. Now, the value will be populated automatically.
+Remove this attribute from the configuration for any affected resources.
+`,
 			},
-			// Optional inputs
 			"project_id": {
-				Description:  "The ID of the HCP project where the HVN peering connection is located.",
-				Type:         schema.TypeString,
-				Computed:     true,
-				Optional:     true,
-				ValidateFunc: validation.IsUUID,
+				Description: "The ID of the HCP project where the HVN peering connection is located. Always matches hvn_1's project ID. Setting this attribute is deprecated, but it will remain usable in read-only form.",
+				Type:        schema.TypeString,
+				Computed:    true,
+				Optional:    true,
+				Deprecated: `
+Setting the 'project_id' attribute is deprecated, but it will remain usable in read-only form.
+Previously, the value for this attribute was required to match the project ID contained in 'hvn_1'. Now, the value will be calculated automatically.
+Remove this attribute from the configuration for any affected resources.
+`,
 			},
-			// Computed outputs
 			"organization_id": {
-				Description: "The ID of the HCP organization where the peering connection is located. Always matches the HVNs' organization.",
+				Description: "The ID of the HCP organization where the peering connection is located. Always matches both HVNs' organization ID",
 				Type:        schema.TypeString,
 				Computed:    true,
 			},
@@ -78,31 +85,29 @@ func dataSourceHvnPeeringConnection() *schema.Resource {
 
 func dataSourceHvnPeeringConnectionRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client)
-	orgID := client.Config.OrganizationID
-
-	peeringID := d.Get("peering_id").(string)
-
-	projectID, err := GetProjectID(d.Get("project_id").(string), client.Config.ProjectID)
-	if err != nil {
-		return diag.Errorf("unable to retrieve project ID: %v", err)
-	}
 
-	loc := &sharedmodels.HashicorpCloudLocationLocation{
-		OrganizationID: client.Config.OrganizationID,
-		ProjectID:      projectID,
-	}
-	hvnLink1, err := buildLinkFromURL(d.Get("hvn_1").(string), HvnResourceType, orgID)
+	hvn1Link, err := buildLinkFromURL(d.Get("hvn_1").(string), HvnResourceType, client.Config.OrganizationID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	peeringID := d.Get("peering_id").(string)
 	log.Printf("[INFO] Reading peering connection (%s)", peeringID)
-	peering, err := clients.GetPeeringByID(ctx, client, peeringID, hvnLink1.ID, loc)
+	peering, err := clients.GetPeeringByID(ctx, client, peeringID, hvn1Link.ID, hvn1Link.Location)
 	if err != nil {
 		return diag.Errorf("unable to retrieve peering connection (%s): %v", peeringID, err)
 	}
 
 	// Peering connection found, update resource data.
+	hvn2Link := newLink(peering.Target.HvnTarget.Hvn.Location, HvnResourceType, peering.Target.HvnTarget.Hvn.ID)
+	hvn2URL, err := linkURL(hvn2Link)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	if err := d.Set("hvn_2", hvn2URL); err != nil {
+		return diag.FromErr(err)
+	}
+
 	if err := setHvnPeeringResourceData(d, peering); err != nil {
 		return diag.FromErr(err)
 	}

diff --git a/internal/provider/data_source_hvn_route.go b/internal/provider/data_source_hvn_route.go
@@ -7,10 +7,8 @@ import (
 	"context"
 	"log"
 
-	sharedmodels "github.com/hashicorp/hcp-sdk-go/clients/cloud-shared/v1/models"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 
 	"github.com/hashicorp/terraform-provider-hcp/internal/clients"
 )
@@ -34,15 +32,18 @@ func dataSourceHVNRoute() *schema.Resource {
 				Type:        schema.TypeString,
 				Required:    true,
 			},
-			// Optional inputs
+			// Computed outputs
 			"project_id": {
-				Description:  "The ID of the HCP project where the HVN route is located.",
-				Type:         schema.TypeString,
-				Computed:     true,
-				Optional:     true,
-				ValidateFunc: validation.IsUUID,
+				Description: "The ID of the HCP project where the HVN route is located. Always matches the project ID in `hvn_link`. Setting this attribute is deprecated, but it will remain usable in read-only form.",
+				Type:        schema.TypeString,
+				Computed:    true,
+				Optional:    true,
+				Deprecated: `
+Setting the 'project_id' attribute is deprecated, but it will remain usable in read-only form.
+Previously, the value for this attribute was required to match the project ID contained in 'hvn_link'. Now, the value will be calculated automatically.
+Remove this attribute from the configuration for any affected resources.
+`,
 			},
-			// Computed outputs
 			"self_link": {
 				Description: "A unique URL identifying the HVN route.",
 				Type:        schema.TypeString,
@@ -76,37 +77,27 @@ func dataSourceHVNRouteRead(ctx context.Context, d *schema.ResourceData, meta in
 	client := meta.(*clients.Client)
 
 	hvn := d.Get("hvn_link").(string)
-	hvnLink, err := parseLinkURL(hvn, HvnResourceType)
+	hvnLink, err := buildLinkFromURL(hvn, HvnResourceType, client.Config.OrganizationID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	projectID, err := GetProjectID(d.Get("project_id").(string), client.Config.ProjectID)
-	if err != nil {
-		return diag.Errorf("unable to retrieve project ID: %v", err)
-	}
-
-	loc := &sharedmodels.HashicorpCloudLocationLocation{
-		OrganizationID: client.Config.OrganizationID,
-		ProjectID:      projectID,
-	}
-
 	routeID := d.Get("hvn_route_id").(string)
-	routeLink := newLink(loc, HVNRouteResourceType, routeID)
+	routeLink := newLink(hvnLink.Location, HVNRouteResourceType, routeID)
 	routeURL, err := linkURL(routeLink)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 	d.SetId(routeURL)
 
 	log.Printf("[INFO] Reading HVN route (%s)", routeID)
-	route, err := clients.GetHVNRoute(ctx, client, hvnLink.ID, routeID, loc)
+	route, err := clients.GetHVNRoute(ctx, client, hvnLink.ID, routeID, hvnLink.Location)
 	if err != nil {
 		return diag.Errorf("unable to retrieve HVN route (%s): %v", routeID, err)
 	}
 
 	// HVN route found, update resource data.
-	if err := setHVNRouteResourceData(d, route, loc); err != nil {
+	if err := setHVNRouteResourceData(d, route, hvnLink.Location); err != nil {
 		return diag.FromErr(err)
 	}
 

diff --git a/internal/provider/resource_azure_peering_connection.go b/internal/provider/resource_azure_peering_connection.go
@@ -143,13 +143,7 @@ func resourceAzurePeeringConnectionCreate(ctx context.Context, d *schema.Resourc
 	peerTenantID := d.Get("peer_tenant_id").(string)
 	peerResourceGroupName := d.Get("peer_resource_group_name").(string)
 
-	orgID := client.Config.OrganizationID
-	loc := &sharedmodels.HashicorpCloudLocationLocation{
-		OrganizationID: orgID,
-		ProjectID:      client.Config.ProjectID,
-	}
-
-	hvnLink, err := buildLinkFromURL(d.Get("hvn_link").(string), HvnResourceType, orgID)
+	hvnLink, err := buildLinkFromURL(d.Get("hvn_link").(string), HvnResourceType, client.Config.OrganizationID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -165,6 +159,11 @@ func resourceAzurePeeringConnectionCreate(ctx context.Context, d *schema.Resourc
 	}
 	log.Printf("[INFO] HVN (%s) found, proceeding with peering connection create", hvnLink.ID)
 
+	loc := &sharedmodels.HashicorpCloudLocationLocation{
+		OrganizationID: hvnLink.Location.OrganizationID,
+		ProjectID:      hvnLink.Location.ProjectID,
+	}
+
 	// Check if peering already exists
 	if peeringID != "" {
 		_, err = clients.GetPeeringByID(ctx, client, peeringID, hvnLink.ID, loc)

diff --git a/internal/provider/resource_boundary_cluster.go b/internal/provider/resource_boundary_cluster.go
@@ -257,17 +257,15 @@ func resourceBoundaryClusterCreate(ctx context.Context, d *schema.ResourceData,
 
 func resourceBoundaryClusterUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client)
-	loc := &sharedmodels.HashicorpCloudLocationLocation{
-		OrganizationID: client.Config.OrganizationID,
-		ProjectID:      client.Config.ProjectID,
-	}
 
 	link, err := buildLinkFromURL(d.Id(), BoundaryClusterResourceType, client.Config.OrganizationID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
 	clusterID := link.ID
+	loc := link.Location
+
 	upgradeType, maintenanceWindow, diagErr := getBoundaryClusterMaintainanceWindowConfig(d)
 	if diagErr != nil {
 		return diagErr