implement Vault update (public_endpoint only) (#152)

* adds updatePublicIPs to vault client helper

* adds update to vault resource
currently only 'public_endpoint' can be updated

* add update test step

* go gen docs

* refactor vault cluster test config to use variable
vault cluster

* update timeout variable names
to make it more clear which cluster we're talking about :)

* tabs to spaces in vault test + bonus test readme update

* fix comment

contributing/writing-tests.md

@@ -20,7 +20,11 @@ contributions.
 
 ## Acceptance Tests Take a While to Run
 
-These acceptance tests create real resources, some of which can take up to 15-20 minutes each to spin up and tear down. For this reason, we urge contributors to consolidate acceptance tests on related resources in one test file. For example, the `resource_vault_cluster_test.go` reuses one test config to test the Vault cluster resource, the Vault cluster datasource, and the dependent Vault cluster admin token resource. This helps speed up the acceptance test runtime by creating a Vault cluster, the most time-intensive resource, only once.
+These acceptance tests create real resources, some of which can take up to 15-20 minutes each to spin up and tear down. For this reason, we urge contributors to consolidate acceptance tests on related resources in one test file to avoid creating multiples of the same resource over the course of the tests. Data sources and dependent resources can be consolidated into their corresponding resource test.
+
+For example, the `resource_vault_cluster_test.go` reuses one test config to test the Vault cluster resource, the Vault cluster datasource, and the dependent Vault cluster admin token resource. This helps speed up the acceptance test runtime by creating a Vault cluster, the most time-intensive resource, only once.
+
+Exceptions may be made when the HCL required to test a single resource is particularly complex, as in `resource_aws_transit_gateway_attachment_test.go`. In such cases, test readability should be preferred over consolidation.
 
 ## Running an Acceptance Test
 
@@ -128,6 +132,8 @@ When executing the test, the following steps are taken for each `TestStep`:
    }
    ```
 
+   **Note:** Use spaces instead of tabs in test HCL.
+
 1. Assertions are run using the provider API. These use the provider API
    directly rather than asserting against the resource state. For example, to
    verify that the `hcp_consul_cluster` described above was created
@@ -215,10 +221,3 @@ When executing the test, the following steps are taken for each `TestStep`:
    ```
 
    These functions usually test only for the resource directly under test.
-
-## Test Time and Consolidation
-
-Because of the increased length of time it takes to run acceptance tests, efforts
-should be made to not create multiples of the same resource for testing purposes. For
-example, datasource tests have been consolidated into their corresponding resource
-tests so that resources may be reused.

docs/resources/vault_cluster.md

@@ -62,6 +62,7 @@ Optional:
 - **create** (String)
 - **default** (String)
 - **delete** (String)
+- **update** (String)
 
 ## Import
 

internal/clients/vault_cluster.go

@@ -85,3 +85,28 @@ func CreateVaultClusterAdminToken(ctx context.Context, client *Client, loc *shar
 
 	return resp.Payload, nil
 }
+
+// UpdateVaultClusterPublicIps will make a call to the Vault service to enable or disable public IPs for the Vault cluster.
+func UpdateVaultClusterPublicIps(ctx context.Context, client *Client, loc *sharedmodels.HashicorpCloudLocationLocation,
+	clusterID string, enablePublicIps bool) (*vaultmodels.HashicorpCloudVault20201125UpdatePublicIpsResponse, error) {
+
+	updateParams := vault_service.NewUpdatePublicIpsParams()
+	updateParams.Context = ctx
+	updateParams.ClusterID = clusterID
+	updateParams.LocationProjectID = loc.ProjectID
+	updateParams.LocationOrganizationID = loc.OrganizationID
+	updateParams.Body = &vaultmodels.HashicorpCloudVault20201125UpdatePublicIpsRequest{
+		// ClusterID and Location are repeated because the values above are required to populate the URL,
+		// and the values below are required in the API request body
+		ClusterID:       clusterID,
+		Location:        loc,
+		EnablePublicIps: enablePublicIps,
+	}
+
+	updateResp, err := client.Vault.UpdatePublicIps(updateParams, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	return updateResp.Payload, nil
+}

internal/provider/data_source_consul_cluster.go

@@ -16,7 +16,7 @@ func dataSourceConsulCluster() *schema.Resource {
 		Description: "The cluster data source provides information about an existing HCP Consul cluster.",
 		ReadContext: dataSourceConsulClusterRead,
 		Timeouts: &schema.ResourceTimeout{
-			Default: &defaultClusterTimeoutDuration,
+			Default: &defaultConsulClusterTimeout,
 		},
 		Schema: map[string]*schema.Schema{
 			// Required inputs

internal/provider/data_source_vault_cluster.go

@@ -16,7 +16,7 @@ func dataSourceVaultCluster() *schema.Resource {
 		Description: "The cluster data source provides information about an existing HCP Vault cluster.",
 		ReadContext: dataSourceVaultClusterRead,
 		Timeouts: &schema.ResourceTimeout{
-			Default: &defaultClusterTimeoutDuration,
+			Default: &defaultVaultClusterTimeout,
 		},
 		Schema: map[string]*schema.Schema{
 			// Required inputs

internal/provider/resource_consul_cluster.go

@@ -17,17 +17,17 @@ import (
 	"github.com/hashicorp/terraform-provider-hcp/internal/input"
 )
 
-// defaultClusterTimeoutDuration is the amount of time that can elapse
+// defaultClusterTimeout is the amount of time that can elapse
 // before a cluster read operation should timeout.
-var defaultClusterTimeoutDuration = time.Minute * 5
+var defaultConsulClusterTimeout = time.Minute * 5
 
-// createUpdateTimeoutDuration is the amount of time that can elapse
+// createUpdateTimeout is the amount of time that can elapse
 // before a cluster create or update operation should timeout.
-var createUpdateTimeoutDuration = time.Minute * 35
+var createUpdateConsulClusterTimeout = time.Minute * 35
 
-// deleteTimeoutDuration is the amount of time that can elapse
+// deleteTimeout is the amount of time that can elapse
 // before a cluster delete operation should timeout.
-var deleteTimeoutDuration = time.Minute * 25
+var deleteConsulClusterTimeout = time.Minute * 25
 
 // consulCusterResourceCloudProviders is the list of cloud providers
 // where a HCP Consul cluster can be provisioned.
@@ -44,10 +44,10 @@ func resourceConsulCluster() *schema.Resource {
 		UpdateContext: resourceConsulClusterUpdate,
 		DeleteContext: resourceConsulClusterDelete,
 		Timeouts: &schema.ResourceTimeout{
-			Default: &defaultClusterTimeoutDuration,
-			Create:  &createUpdateTimeoutDuration,
-			Update:  &createUpdateTimeoutDuration,
-			Delete:  &deleteTimeoutDuration,
+			Default: &defaultConsulClusterTimeout,
+			Create:  &createUpdateConsulClusterTimeout,
+			Update:  &createUpdateConsulClusterTimeout,
+			Delete:  &deleteConsulClusterTimeout,
 		},
 		Importer: &schema.ResourceImporter{
 			StateContext: resourceConsulClusterImport,

internal/provider/resource_vault_cluster.go

@@ -20,11 +20,11 @@ import (
 // before a cluster read operation should timeout.
 var defaultVaultClusterTimeout = time.Minute * 5
 
-// createTimeout is the amount of time that can elapse
+// createUpdateVaultClusterTimeout is the amount of time that can elapse
 // before a cluster create operation should timeout.
-var createVaultClusterTimeout = time.Minute * 35
+var createUpdateVaultClusterTimeout = time.Minute * 35
 
-// deleteTimeout is the amount of time that can elapse
+// deleteVaultClusterTimeout is the amount of time that can elapse
 // before a cluster delete operation should timeout.
 var deleteVaultClusterTimeout = time.Minute * 25
 
@@ -33,9 +33,11 @@ func resourceVaultCluster() *schema.Resource {
 		Description:   "The Vault cluster resource allows you to manage an HCP Vault cluster.",
 		CreateContext: resourceVaultClusterCreate,
 		ReadContext:   resourceVaultClusterRead,
+		UpdateContext: resourceVaultClusterUpdate,
 		DeleteContext: resourceVaultClusterDelete,
 		Timeouts: &schema.ResourceTimeout{
-			Create:  &createVaultClusterTimeout,
+			Create:  &createUpdateVaultClusterTimeout,
+			Update:  &createUpdateVaultClusterTimeout,
 			Delete:  &deleteVaultClusterTimeout,
 			Default: &defaultVaultClusterTimeout,
 		},
@@ -58,6 +60,7 @@ func resourceVaultCluster() *schema.Resource {
 				ForceNew:         true,
 				ValidateDiagFunc: validateSlugID,
 			},
+			// Optional fields
 			"tier": {
 				Description:      "Tier of the HCP Vault cluster. Valid options for tiers - `dev`, `standard_small`, `standard_medium`, `standard_large`. See [pricing information](https://cloud.hashicorp.com/pricing/vault).",
 				Type:             schema.TypeString,
@@ -69,13 +72,11 @@ func resourceVaultCluster() *schema.Resource {
 					return strings.ToLower(old) == strings.ToLower(new)
 				},
 			},
-			// optional fields
 			"public_endpoint": {
 				Description: "Denotes that the cluster has a public endpoint. Defaults to false.",
 				Type:        schema.TypeBool,
 				Default:     false,
 				Optional:    true,
-				ForceNew:    true,
 			},
 			"min_vault_version": {
 				Description:      "The minimum Vault version to use when creating the cluster. If not specified, it is defaulted to the version that is currently recommended by HCP.",
@@ -84,7 +85,7 @@ func resourceVaultCluster() *schema.Resource {
 				ValidateDiagFunc: validateSemVer,
 				ForceNew:         true,
 			},
-			// computed outputs
+			// Computed outputs
 			"organization_id": {
 				Description: "The ID of the organization this HCP Vault cluster is located in.",
 				Type:        schema.TypeString,
@@ -266,6 +267,61 @@ func resourceVaultClusterRead(ctx context.Context, d *schema.ResourceData, meta
 	return nil
 }
 
+func resourceVaultClusterUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	client := meta.(*clients.Client)
+
+	link, err := buildLinkFromURL(d.Id(), VaultClusterResourceType, client.Config.OrganizationID)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	clusterID := link.ID
+	loc := link.Location
+
+	log.Printf("[INFO] Reading Vault cluster (%s) [project_id=%s, organization_id=%s]", clusterID, loc.ProjectID, loc.OrganizationID)
+
+	cluster, err := clients.GetVaultClusterByID(ctx, client, loc, clusterID)
+	if err != nil {
+		if clients.IsResponseCodeNotFound(err) {
+			log.Printf("[WARN] Vault cluster (%s) not found, removing from state", clusterID)
+			d.SetId("")
+			return nil
+		}
+
+		return diag.Errorf("unable to fetch Vault cluster (%s): %v", clusterID, err)
+	}
+
+	// Confirm public_endpoint has changed. This is currently the only field that can be updated.
+	changed := d.HasChange("public_endpoint")
+	if !changed {
+		return nil
+	}
+
+	// Invoke update cluster endpoint.
+	updateResp, err := clients.UpdateVaultClusterPublicIps(ctx, client, cluster.Location, clusterID, d.Get("public_endpoint").(bool))
+	if err != nil {
+		return diag.Errorf("error updating Vault cluster (%s): %v", clusterID, err)
+	}
+
+	// Wait for the update cluster operation.
+	if err := clients.WaitForOperation(ctx, client, "update Vault cluster", cluster.Location, updateResp.Operation.ID); err != nil {
+		return diag.Errorf("unable to update Vault cluster (%s): %v", clusterID, err)
+	}
+
+	// Get the updated Vault cluster.
+	cluster, err = clients.GetVaultClusterByID(ctx, client, loc, clusterID)
+
+	if err != nil {
+		return diag.Errorf("unable to retrieve Vault cluster (%s): %v", clusterID, err)
+	}
+
+	if err := setVaultClusterResourceData(d, cluster); err != nil {
+		return diag.FromErr(err)
+	}
+
+	return nil
+}
+
 func resourceVaultClusterDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client)
 

internal/provider/resource_vault_cluster_test.go

@@ -10,18 +10,33 @@ import (
 	"github.com/hashicorp/terraform-provider-hcp/internal/clients"
 )
 
-var testAccVaultClusterConfig = `
-resource "hcp_hvn" "test" {
-	hvn_id           = "test-hvn"
-	cloud_provider   = "aws"
-	region           = "us-west-2"
+var vaultCluster = `
+resource "hcp_vault_cluster" "test" {
+	cluster_id         = "test-vault-cluster"
+	hvn_id             = hcp_hvn.test.hvn_id
+	tier               = "dev"
 }
+`
 
+// sets public_endpoint to true
+var updatedVaultCluster = `
 resource "hcp_vault_cluster" "test" {
-	cluster_id       = "test-vault-cluster"
-	hvn_id           = hcp_hvn.test.hvn_id
-	tier             = "dev"
+	cluster_id         = "test-vault-cluster"
+	hvn_id             = hcp_hvn.test.hvn_id
+	tier               = "dev"
+	public_endpoint    = true
 }
+`
+
+func setTestAccVaultClusterConfig(vaultCluster string) string {
+	return fmt.Sprintf(`
+resource "hcp_hvn" "test" {
+	hvn_id            = "test-hvn"
+	cloud_provider    = "aws"
+	region            = "us-west-2"
+}
+
+%s
 
 data "hcp_vault_cluster" "test" {
 	cluster_id       = hcp_vault_cluster.test.cluster_id
@@ -30,7 +45,8 @@ data "hcp_vault_cluster" "test" {
 resource "hcp_vault_cluster_admin_token" "test" {
 	cluster_id       = hcp_vault_cluster.test.cluster_id
 }
-`
+`, vaultCluster)
+}
 
 // This includes tests against both the resource, the corresponding datasource, and the dependent admin token resource
 // to shorten testing time.
@@ -46,7 +62,7 @@ func TestAccVaultCluster(t *testing.T) {
 		Steps: []resource.TestStep{
 			// This step tests Vault cluster and admin token resource creation.
 			{
-				Config: testConfig(testAccVaultClusterConfig),
+				Config: testConfig(setTestAccVaultClusterConfig(vaultCluster)),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckVaultClusterExists(vaultClusterResourceName),
 					resource.TestCheckResourceAttr(vaultClusterResourceName, "cluster_id", "test-vault-cluster"),
@@ -86,7 +102,7 @@ func TestAccVaultCluster(t *testing.T) {
 			},
 			// This step is a subsequent terraform apply that verifies that no state is modified.
 			{
-				Config: testConfig(testAccVaultClusterConfig),
+				Config: testConfig(setTestAccVaultClusterConfig(vaultCluster)),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckVaultClusterExists(vaultClusterResourceName),
 					resource.TestCheckResourceAttr(vaultClusterResourceName, "cluster_id", "test-vault-cluster"),
@@ -107,7 +123,7 @@ func TestAccVaultCluster(t *testing.T) {
 			},
 			// Tests datasource
 			{
-				Config: testConfig(testAccVaultClusterConfig),
+				Config: testConfig(setTestAccVaultClusterConfig(vaultCluster)),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttrPair(vaultClusterResourceName, "cluster_id", vaultClusterDataSourceName, "cluster_id"),
 					resource.TestCheckResourceAttrPair(vaultClusterResourceName, "hvn_id", vaultClusterDataSourceName, "hvn_id"),
@@ -126,6 +142,18 @@ func TestAccVaultCluster(t *testing.T) {
 					resource.TestCheckResourceAttrPair(vaultClusterResourceName, "created_at", vaultClusterDataSourceName, "created_at"),
 				),
 			},
+			// This step verifies the successful update of updatable fields.
+			{
+				Config: testConfig(setTestAccVaultClusterConfig(updatedVaultCluster)),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckVaultClusterExists(vaultClusterResourceName),
+					resource.TestCheckResourceAttr(vaultClusterResourceName, "public_endpoint", "true"),
+					resource.TestCheckResourceAttrSet(vaultClusterResourceName, "vault_public_endpoint_url"),
+					testAccCheckFullURL(vaultClusterResourceName, "vault_public_endpoint_url", "8200"),
+					resource.TestCheckResourceAttrSet(vaultClusterResourceName, "vault_private_endpoint_url"),
+					testAccCheckFullURL(vaultClusterResourceName, "vault_private_endpoint_url", "8200"),
+				),
+			},
 		},
 	})
 }