HCPF-704: Fix Missing/Incorrect Resource-level project_id Implementations

[aidan-mundy]

🛠️ Description

Fixes missing/incorrect resource-specific, resource-level project_id implementations for:

• hcp_packer_channel
  • Read (was missing)
  • Update (was missing)
  • Delete (was missing)
• hcp_azure_peering_connection
  • Create (was incorrect, assumed HVN used provider-level value)
• data.hcp_azure_peering_connection
  • Read (was incorrect, assumed HVN used provider-level value)
• hcp_hvn_route
  • Read (was missing)
• hcp_vault_cluster_admin_token
  • Read (was missing)
  • Logging (was incorrect, didn't pull from loc)
• hcp_consul_cluster_root_token
  • Read (was missing)
  • Delete (was missing)
• data.consul_agent_helm_config
  • Read (had redundant code)
• hcp_boundary_cluster
  • Update (was incorrect, implementation existed but was not used)

Deprecates some unnecessary fields on some resources and data sources:

• The project_id attribute is no longer settable for hcp_hvn_peering_connection and data.hcp_hvn_peering_connection.
  • The value of the field was required to match the project ID for hvn_1 and will now be automatically determined instead.
  • This is a breaking change that will require practitioners to remove the project_id field from the configuration for affected resources and data sources before the end of the deprecation period.
  • During the deprecation period, the field will be settable, but practitioner-defined values will be ignored.
• The project_id attribute is no longer settable for hcp_hvn_route and data.hcp_hvn_route.
  • The value of the field was required to match the project ID for hvn_link and will now be automatically determined instead.
  • This is a breaking change that will require practitioners to remove the project_id field from the configuration for affected resources and data sources before the end of the deprecation period.
  • During the deprecation period, the field will be settable, but practitioner-defined values will be ignored.
• The hvn_2 attribute is no longer settable for data.hcp_hvn_peering_connection.
  • The value of the attribute is not needed to fetch data, and it was never validated against the real value for hvn_2, which may result in unintended behavior or unexpected errors downstream. The value will now be populated automatically.
  • This is a breaking change that will require practitioners to remove the hvn_2 attribute from the configuration for affected data sources before the end of the deprecation period.
  • During the deprecation period, the field will be settable, but practitioner-defined values will be ignored.

Also, minor code cleanliness changes for accessing the OrgID in some of the above (and some other) resources.

Modified resources and data sources may experience a minor regression with the following symptoms (Note: they may already experience these symptoms):

• Resources/data sources where the resource-level project_id attribute was set at some point, but later removed from the configuration file, will continue to use the last known user-supplied value for the resource-level project_id attribute, regardless of any provider-level project_id value.
• Resources/data sources that have never had a user-supplied resource-level project_id attribute will use the provider-level project_id value that was present when the resource was created. Updates to the provider-level project_id will not affect these resources.

This behavior is already present in some forms on some resources and data sources, and will be fixed in a later update. The regression is considered acceptable in this case because the missing/incorrect implementations for resource-level project_ids have undefined behavior that is potentially dangerous/destructive.

🏗️ Acceptance tests

• Are there any feature flags that are required to use this functionality?
• Have you added an acceptance test for the functionality being added?
• Have you run the acceptance tests on this branch?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAcc.*PackerChannel.*'
==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml 
TF_ACC=1 go test ./internal/... -v -run=TestAcc.*PackerChannel.* -timeout 360m -parallel=10
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/clients    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/consul     (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      (cached) [no tests to run]
=== RUN   TestAccPackerChannel
--- PASS: TestAccPackerChannel (9.71s)
=== RUN   TestAccPackerChannel_AssignedIteration
--- PASS: TestAccPackerChannel_AssignedIteration (9.57s)
=== RUN   TestAccPackerChannel_UpdateAssignedIteration
--- PASS: TestAccPackerChannel_UpdateAssignedIteration (15.23s)
=== RUN   TestAccPackerChannel_UpdateAssignedIterationWithFingerprint
--- PASS: TestAccPackerChannel_UpdateAssignedIterationWithFingerprint (7.14s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider   42.031s

$ make testacc TESTARGS='-run=TestAcc.*HvnPeering.*'   
==> Checking that code complies with gofmt requirements...
golangci-lint run --config ./golangci-config.yml 
TF_ACC=1 go test ./internal/... -v -run=TestAcc.*HvnPeering.* -timeout 360m -parallel=10
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/clients    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/consul     (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/input      (cached) [no tests to run]
=== RUN   TestAccHvnPeeringConnection
--- PASS: TestAccHvnPeeringConnection (175.02s)
PASS
ok      github.com/hashicorp/terraform-provider-hcp/internal/provider   175.331s

I don't have an environment configured for running acceptance tests on the provider other than for HCP Packer. If someone else can run the remaining tests, that would be greatly appreciated.

[uruemu]

LGTM for Boundary.

Thanks for making the change.

[nywilken]

Overall this looks good to me. I have a few questions about the location data and a request for one or more test to HCP Packer.

[sylviamoss]

Out of curiosity, when computed can this field be set in the resource even though when overwritten?

[aidan-mundy]

Computed informs that the provider may calculate (or pull from API) a value for that attribute, usually for when it isn't provided by the user. However, even when it is provided by the user in the configuration, the provider is still able to override the value in two ways:

• In state only, by setting the value as part of the Read, Create, or Update steps.
  • This can cause TF to generate new plans every time terraform apply is called.
  • Maintains consistency between config and plans, but not state.
• In the planning process, by using CustomizeDiff
  • This can create a situation where changes to the configuration file don't result in a plan being generated.
  • Maintains consistency between plans and state, but not config.

With the changes here, if the user provides a value for provider_id, it will be ignored in the planning process using CustomizeDiff and the provider will calculate the correct value from the required hvn_link attribute, outputting it to provider_id. This does create a situation where the configuration file could contain provider_id = "1234", but the value of {resource}.provider_id is "abcd" (or similar). However, this is acceptable because previously the field could cause errors/crashes/incorrect behavior, but that is now fixed alongside a deprecation warning from the CLI any time the attribute is manually set by the user and any downstream uses of {resource}.provider_id in Terraform would generate a plan which will inform the user that the value has changed.

[sylviamoss]

The hcp packer channel resource code looks good considering the known issue that will be addressed in another PR. 👍🏼

[nywilken]

Nicely done. LGTM

[hanshasselberg]

LGTM, thank you very much for working through this.