Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HCCP-91 HVN route resource and import #122

Merged
merged 28 commits into from
May 21, 2021

Conversation

nombiezinja
Copy link
Contributor

@nombiezinja nombiezinja commented May 17, 2021

Ticket: https://hashicorp.atlassian.net/browse/HCCP-91

Description

Adds the hcp_hvn_route resource and import. The implementation follows examples of the existing resources, with the exception of using self_link to refer to HVNs and Target resources, rather than the ID of the HVN or Target.

A simple example of what the resource looks like for a TGW attachment:

resource "hcp_hvn_route" "tgwroute" {
  hvn_link         = hcp_hvn.main.self_link
  destination_cidr = aws_vpc.example.cidr_block
  hvn_route_id     = "exampletgwhvnroute"
  target_link      = hcp_aws_transit_gateway_attachment.example.self_link
}

A simple example of what the resource looks like for a peering:

resource "hcp_hvn_route" "peering-route" {
  hvn_link         = hcp_hvn.test-hvn.self_link
  destination_cidr = "192.168.0.0/20"
  hvn_route_id     = "examplepeeringroute"
  target_link      = hcp_aws_network_peering.example.self_link
}

To Test

The example included here can be used to test the functionality of this resource.
Please note that

  • if creating a route for a TGW attachment, the AWS config cannot be the same account as the HVN

Additional notes

  • This PR also makes the fields peer_vpc_cidr_block and destination_cidr optional. This is an interim solution, as based on discussions with product we have reached consensus that we will make the breaking changes in the next release by simply removing these fields. However, for the purpose of this PR these fields need to be optional, otherwise submitting a value during creation of peering/tgw attachment will result in a route having been created for these resources, causing an error during later HVN route creation

  • This PR does not include acceptance tests, as they will be in a subsequent PR, along with the acceptance tests for TGW attachment resrouce

  • A follow-up PR will also remove peer_vpc_cidr_block and destination_cidr fields from the peering and TGW attachment resource, as aforementioned. It will also make the user-settable ID field required for both of these resrouces. It is not included in this PR as the provider team has agreed that it will make this review easier.

@nombiezinja nombiezinja requested a review from a team May 17, 2021 18:58
Copy link
Contributor

@bcmdarroch bcmdarroch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Left a few comments sprinkled throughout - only major one about dropping the cidr block.

I'm hoping to do another test run tomorrow - accidentally kicked it off on my env with routes feature flag turned off. 🙈 Are acceptance tests planned for a follow-up? I can help with those, since we don't have any multi-provider test examples yet.

internal/provider/resource_hvn_route.go Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_aws_network_peering.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
docs/resources/hvn_route.md Show resolved Hide resolved
docs/resources/hvn_route.md Outdated Show resolved Hide resolved
internal/clients/hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_aws_network_peering.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
@smaant smaant changed the base branch from main to feature-hvn-routes May 19, 2021 18:38
@smaant smaant force-pushed the HCCP-91-HVN-route-resource branch from 0b7d2a8 to 5d90b7c Compare May 19, 2021 20:28
@hashicorp-cla
Copy link

hashicorp-cla commented May 19, 2021

CLA assistant check
All committers have signed the CLA.

@smaant smaant force-pushed the HCCP-91-HVN-route-resource branch from de67601 to e9c4555 Compare May 20, 2021 21:32
Copy link
Contributor

@bcmdarroch bcmdarroch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I haven't gotten the chance to test this out locally but code all looks great. Just had some nitpicks on documentation.

internal/provider/data_source_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/data_source_hvn_route.go Outdated Show resolved Hide resolved
internal/provider/link.go Outdated Show resolved Hide resolved
internal/provider/resource_hvn_route.go Outdated Show resolved Hide resolved
docs/resources/hvn_route.md Show resolved Hide resolved
examples/data-sources/hcp_hvn_route/variables.tf Outdated Show resolved Hide resolved
docs/resources/hvn_route.md Outdated Show resolved Hide resolved
smaant and others added 2 commits May 21, 2021 12:20
@smaant smaant merged commit da152f2 into feature-hvn-routes May 21, 2021
@smaant smaant deleted the HCCP-91-HVN-route-resource branch May 21, 2021 21:13
bcmdarroch added a commit that referenced this pull request Jun 4, 2021
* added create, scaffolded read and delete for hvn route resource

* Adds delete for HVN route resource

* Adds hvn route import function

* Handle both peering and tgw attachment resource types in HVN route resources

* Regenerate docs, add example s for hvn route resource

* Re-run go generate after adding  example

* ACreate hvn route function checks for target existence before proceeding

* Add peering to example for hvn route, regenerate docs

* Resolves comments - better logging and commenting for HVN route resource

* removed unnecessary validation

* removed todos

* removed tgw attachment from hvn route example

* added examples of the hvn route target

* moved hvn route creation into clients

* simplified parsing target_link for the hvn route resource

* dropped checking for hvn route existance

* fixed hvn routes import

* gofmt hvn_route.go

* redo hvn route import to use route ID

* go mod tidy

* redo hvn route datasource to use route ID

* renamed hvn -> hvn_link

* refactored WaitForHVNRouteToBeActive

* unified hvn route errors/logs

* small refactoring

* improved logs

* messages improvements

Co-authored-by: Brenna Hewer-Darroch <[email protected]>

* regenarated docs

Co-authored-by: Anton Panferov <[email protected]>
Co-authored-by: Brenna Hewer-Darroch <[email protected]>
bcmdarroch added a commit that referenced this pull request Jun 4, 2021
* added create, scaffolded read and delete for hvn route resource

* Adds delete for HVN route resource

* Adds hvn route import function

* Handle both peering and tgw attachment resource types in HVN route resources

* Regenerate docs, add example s for hvn route resource

* Re-run go generate after adding  example

* ACreate hvn route function checks for target existence before proceeding

* Add peering to example for hvn route, regenerate docs

* Resolves comments - better logging and commenting for HVN route resource

* removed unnecessary validation

* removed todos

* removed tgw attachment from hvn route example

* added examples of the hvn route target

* moved hvn route creation into clients

* simplified parsing target_link for the hvn route resource

* dropped checking for hvn route existance

* fixed hvn routes import

* gofmt hvn_route.go

* redo hvn route import to use route ID

* go mod tidy

* redo hvn route datasource to use route ID

* renamed hvn -> hvn_link

* refactored WaitForHVNRouteToBeActive

* unified hvn route errors/logs

* small refactoring

* improved logs

* messages improvements

Co-authored-by: Brenna Hewer-Darroch <[email protected]>

* regenarated docs

Co-authored-by: Anton Panferov <[email protected]>
Co-authored-by: Brenna Hewer-Darroch <[email protected]>
bcmdarroch added a commit that referenced this pull request Jun 4, 2021
* added create, scaffolded read and delete for hvn route resource

* Adds delete for HVN route resource

* Adds hvn route import function

* Handle both peering and tgw attachment resource types in HVN route resources

* Regenerate docs, add example s for hvn route resource

* Re-run go generate after adding  example

* ACreate hvn route function checks for target existence before proceeding

* Add peering to example for hvn route, regenerate docs

* Resolves comments - better logging and commenting for HVN route resource

* removed unnecessary validation

* removed todos

* removed tgw attachment from hvn route example

* added examples of the hvn route target

* moved hvn route creation into clients

* simplified parsing target_link for the hvn route resource

* dropped checking for hvn route existance

* fixed hvn routes import

* gofmt hvn_route.go

* redo hvn route import to use route ID

* go mod tidy

* redo hvn route datasource to use route ID

* renamed hvn -> hvn_link

* refactored WaitForHVNRouteToBeActive

* unified hvn route errors/logs

* small refactoring

* improved logs

* messages improvements

Co-authored-by: Brenna Hewer-Darroch <[email protected]>

* regenarated docs

Co-authored-by: Anton Panferov <[email protected]>
Co-authored-by: Brenna Hewer-Darroch <[email protected]>
bcmdarroch added a commit that referenced this pull request Jun 7, 2021
* HCCP-91 HVN route resource (#122)

* added create, scaffolded read and delete for hvn route resource

* Adds delete for HVN route resource

* Adds hvn route import function

* Handle both peering and tgw attachment resource types in HVN route resources

* Regenerate docs, add example s for hvn route resource

* Re-run go generate after adding  example

* ACreate hvn route function checks for target existence before proceeding

* Add peering to example for hvn route, regenerate docs

* Resolves comments - better logging and commenting for HVN route resource

* removed unnecessary validation

* removed todos

* removed tgw attachment from hvn route example

* added examples of the hvn route target

* moved hvn route creation into clients

* simplified parsing target_link for the hvn route resource

* dropped checking for hvn route existance

* fixed hvn routes import

* gofmt hvn_route.go

* redo hvn route import to use route ID

* go mod tidy

* redo hvn route datasource to use route ID

* renamed hvn -> hvn_link

* refactored WaitForHVNRouteToBeActive

* unified hvn route errors/logs

* small refactoring

* improved logs

* messages improvements

* regenarated docs

* HCCP-138 breaking changes for Peering and TGW attachment (#128)

* HCCP-138 required id and removed cidr from peering

* HCCP-138 fixed tgw-attachment resource import

* HCCP-138 removed cidrs from tgw-attachment

* bonus: drop deleted guide example

* update peering examples in guides

* HVN route migration guide (#129)

* add note to changelog

* update readme

* add hvn route migration guide

* id -> ID

* bonus: fix error typo

* go gen

* add context to HVN route intro

* update link in banner to registry migration guide

* update changelog

* update version in examples

* add warning banner to hvn route doc

* added handling 404 when deleting hvn route (#137)

* HCCP-184 acceptance tests for HVN route, TGW attachment and network peering (#130)

* HCCP-184 added acceptance tests for HVN route resource

* HCCP-184 added acceptance tests for TGW attachment resource

* HCCP-184 added acceptance tests for network peering resource

* HCCP-138 added clarification about AWS credentials

* HCCP-184 renamed tgw attachment acceptance test resource

* HCCP-184 improved tests doc

* HCCP-184 fixed test after rebasing

* added dedicated timeout for hvn route delete (#138)

Co-authored-by: Ti Zhang <[email protected]>
Co-authored-by: Anton Panferov <[email protected]>
Co-authored-by: Brenna Hewer-Darroch <[email protected]>
aidan-mundy added a commit that referenced this pull request Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants