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

Hvt 4182 http proxy for hcpv in hcp tfp #577

Merged
merged 11 commits into from
Aug 14, 2023

Conversation

helenfufu
Copy link
Contributor

@helenfufu helenfufu commented Aug 10, 2023

🛠️ Description

Updating the hcp_vault_cluster resource to have proxy_endpoint (writeable) and vault_proxy_endpoint_url (read-only) fields. The proxy_endpoint field allows users to toggle the proxy on/off for their cluster. This feature was released on Mon Aug 7 for HCP Vault Dedicated through the UI.

As part of this change, we are also refactoring public_endpoint updates to be grouped into UpdateVaultClusterConfig call and moving off of UpdateVaultClusterPublicIps. Please refer to the ADR for more details.

🏗️ 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:

AWS
$ make testacc TESTARGS='-run=TestAccVaultClusterAWS'
TF_ACC=1 go test ./internal/... -v -run=TestAccVaultClusterAWS -timeout 360m -parallel=10
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/clients	0.617s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/consul	0.771s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/input	0.577s [no tests to run]
=== RUN   TestAccVaultClusterAWS
=== PAUSE TestAccVaultClusterAWS
=== CONT  TestAccVaultClusterAWS
--- PASS: TestAccVaultClusterAWS (3218.37s)
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider	3219.240s

Checking the three updates that are run on acceptance tests:

First update is tier only:
Screen Shot 2023-08-10 at 10 26 39 AM

Second update now updates public and proxy in addition to observability on update call (mvu update is through different endpoint)
Screen Shot 2023-08-10 at 10 27 08 AM

Third update now updates public and proxy in addition to observability and tier on update call

Azure (slightly affected by update refactoring, but Azure currently only tests tier update)
$ make testacc TESTARGS='-run=TestAccVaultClusterAzure'
TF_ACC=1 go test ./internal/... -v -run=TestAccVaultClusterAzure -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   TestAccVaultClusterAzure
=== PAUSE TestAccVaultClusterAzure
=== CONT  TestAccVaultClusterAzure
--- PASS: TestAccVaultClusterAzure (3453.93s)
PASS
ok  	github.com/hashicorp/terraform-provider-hcp/internal/provider	3454.288s
Validation with local binary

First, created clusters on latest 0.68.0 (when HCP TFP has no knowledge of proxy fields, so clusters are created with zero enum i.e. HTTP_PROXY_OPTION_INVALID) using test.tf:

terraform {
  required_version = ">=1.0"
  required_providers {
    hcp = {
      source  = "hashicorp/hcp"
      version = "0.68.0"
    }

    # hcp = {
    #   source  = "localhost/providers/hcp"
    #   version = "0.0.1"
    # }
  }
}

// Organization principal
provider "hcp" {
  client_id     = "Aok5s8hobLi3aTM9bds56ZQFuIdZQWMO"
  client_secret = "xYUYZy_VHLGEl2p9VfR6D_a3LA4F2lKNZC06THf_E9HcIaQs0uI6EP-uvQz9-3UN"
}

// Create a HashiCorp Virtual Network (HVN):
resource "hcp_hvn" "terraform-hvn" {
  hvn_id         = "terraform-hvn"
  cloud_provider = "aws"
  region         = "us-west-2"
  cidr_block     = "172.25.16.0/20"
}

resource "hcp_hvn" "terraform-hvn-az" {
  hvn_id         = "terraform-hvn-az"
  cloud_provider = "azure"
  region         = "eastus"
  cidr_block     = "172.26.16.0/20"
}

// Create HCPV cluster on AWS:
resource "hcp_vault_cluster" "tf-vault-cluster" {
  depends_on      = [hcp_hvn.terraform-hvn]
  hvn_id          = hcp_hvn.terraform-hvn.hvn_id
  cluster_id      = "tf-vault-cluster"
  tier            = "starter_small"
  public_endpoint = true
  lifecycle {
    prevent_destroy = true
  }
}

// Create HCPV cluster on Azure:
resource "hcp_vault_cluster" "tf-vault-cluster-az" {
  depends_on      = [hcp_hvn.terraform-hvn-az]
  hvn_id          = hcp_hvn.terraform-hvn-az.hvn_id
  cluster_id      = "tf-vault-cluster-az"
  tier            = "starter_small"
  public_endpoint = true
  lifecycle {
    prevent_destroy = true
  }
}

Followed guide here to test using local binary and point my test.tf file to

    hcp = {
      source  = "localhost/providers/hcp"
      version = "0.0.1"
    }

After I created the clusters, I had to run terraform refresh for the new field to be picked up (otherwise it would try to populate the field to DISABLED default). After refreshing then running terraform plan, no changes show up as expected. Even though the cluster was created with INVALID enum, on terraform refresh, the cloud-vault-service API masks invalid values as DISABLED here, which is why we are able to get "no change."

[~/test/hvt-4182/test-prod]$ terraform refresh
hcp_hvn.terraform-hvn-az: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.network.hvn/terraform-hvn-az]
hcp_hvn.terraform-hvn: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.network.hvn/terraform-hvn]
hcp_vault_cluster.tf-vault-cluster-az: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.vault.cluster/tf-vault-cluster-az]
hcp_vault_cluster.tf-vault-cluster: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.vault.cluster/tf-vault-cluster]
[~/test/hvt-4182/test-prod]$ terraform plan
hcp_hvn.terraform-hvn: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.network.hvn/terraform-hvn]
hcp_hvn.terraform-hvn-az: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.network.hvn/terraform-hvn-az]
hcp_vault_cluster.tf-vault-cluster-az: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.vault.cluster/tf-vault-cluster-az]
hcp_vault_cluster.tf-vault-cluster: Refreshing state... [id=/project/4e61ebde-1d37-4c03-aeed-c3622d0d1e8d/hashicorp.vault.cluster/tf-vault-cluster]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Also validated the following with the local binary:

  • AWS create with DISABLED
  • Azure create with DISABLED
  • AWS create with ENABLED
  • AWS update to ENABLED
  • AWS update to DISABLED
  • Azure create with ENABLED returns service error
  • Azure update to ENABLED returns service error
  • Azure update for both public IP and scaling no longer takes 2 updates (40 minutes); it only takes one update now.

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 10, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines +351 to +359
// Remove invalid option from allowed list in error message
expectedEnumList = strings.ReplaceAll(
expectedEnumList,
strings.ToLower(string(vaultmodels.HashicorpCloudVault20201125HTTPProxyOptionHTTPPROXYOPTIONINVALID)),
"",
)

// Format as comma-separated list
expectedEnumList = strings.Join(strings.Fields(expectedEnumList), ", ")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These steps are extra compared to our other enum validations (for example on the MVU enums). I added this so users don't receive a confusing error message that the invalid/zero enum is actually allowed.

Talked about this a little more in-depth on Michael's PR: #575 (comment)

Choose a reason for hiding this comment

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

thanks for finding this

@helenfufu helenfufu marked this pull request as ready for review August 10, 2023 19:34
Copy link

@himran92 himran92 left a comment

Choose a reason for hiding this comment

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

looks good to me

@helenfufu helenfufu merged commit e5e64d2 into main Aug 14, 2023
6 checks passed
@helenfufu helenfufu deleted the hvt-4182-http-proxy-for-hcpv-in-hcp-tfp branch August 14, 2023 18:07
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.

None yet

4 participants