diff --git a/contributing/writing-tests.md b/contributing/writing-tests.md index c545913fd..5fbc335cd 100644 --- a/contributing/writing-tests.md +++ b/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. diff --git a/docs/resources/vault_cluster.md b/docs/resources/vault_cluster.md index acb8ff428..63414d3cd 100644 --- a/docs/resources/vault_cluster.md +++ b/docs/resources/vault_cluster.md @@ -62,6 +62,7 @@ Optional: - **create** (String) - **default** (String) - **delete** (String) +- **update** (String) ## Import diff --git a/internal/clients/vault_cluster.go b/internal/clients/vault_cluster.go index 630855157..b3d8ea745 100644 --- a/internal/clients/vault_cluster.go +++ b/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 +} diff --git a/internal/provider/data_source_consul_cluster.go b/internal/provider/data_source_consul_cluster.go index 12c11ef72..5bea33da4 100644 --- a/internal/provider/data_source_consul_cluster.go +++ b/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 diff --git a/internal/provider/data_source_vault_cluster.go b/internal/provider/data_source_vault_cluster.go index bb3ed1e13..251918cfb 100644 --- a/internal/provider/data_source_vault_cluster.go +++ b/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 diff --git a/internal/provider/resource_consul_cluster.go b/internal/provider/resource_consul_cluster.go index 639a5c305..8c905ad30 100644 --- a/internal/provider/resource_consul_cluster.go +++ b/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, diff --git a/internal/provider/resource_vault_cluster.go b/internal/provider/resource_vault_cluster.go index 871993e68..335a7ec80 100644 --- a/internal/provider/resource_vault_cluster.go +++ b/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) diff --git a/internal/provider/resource_vault_cluster_test.go b/internal/provider/resource_vault_cluster_test.go index 0b1725c8f..eca87b205 100644 --- a/internal/provider/resource_vault_cluster_test.go +++ b/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"), + ), + }, }, }) }