implement Vault update (public_endpoint only) #152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jfreda
reviewed
Jun 21, 2021
| DeleteContext: resourceVaultClusterDelete, | ||
| Timeouts: &schema.ResourceTimeout{ | ||
| Create: &createVaultClusterTimeout, | ||
| Create: &createUpdateVaultClusterTimeout, | ||
| Update: &createUpdateTimeoutDuration, |
There was a problem hiding this comment.
Should this be &createUpdateVaultClusterTimeout instead of using Consul's value?
Suggested change
| Update: &createUpdateTimeoutDuration, | |
| Update: &createUpdateVaultClusterTimeout, |
There was a problem hiding this comment.
Ah woops, good spot! Think I'm gonna rename the Consul values to be more specific too
| @@ -22,7 +22,7 @@ var defaultVaultClusterTimeout = time.Minute * 5 | |||
|
|
|||
| // createTimeout is the amount of time that can elapse | |||
There was a problem hiding this comment.
Nit
Suggested change
| // createTimeout is the amount of time that can elapse | |
| // createUpdateVaultClusterTimeout is the amount of time that can elapse |
| resource "hcp_vault_cluster" "test" { | ||
| cluster_id = "test-vault-cluster" | ||
| hvn_id = hcp_hvn.test.hvn_id | ||
| tier = "dev" | ||
| tier = "dev" |
There was a problem hiding this comment.
Style nit: looks like a mix of tabs and spaces here
There was a problem hiding this comment.
Gonna make note of this in our test writing readme.
| resource "hcp_vault_cluster" "test" { | ||
| cluster_id = "test-vault-cluster" | ||
| hvn_id = hcp_hvn.test.hvn_id | ||
| tier = "dev" |
There was a problem hiding this comment.
Style nit: looks like a mix of tabs and spaces here
There was a problem hiding this comment.
Oops that just needed a rebase
internal/clients/vault_cluster.go
Outdated
| @@ -85,3 +85,28 @@ func CreateVaultClusterAdminToken(ctx context.Context, client *Client, loc *shar | |||
|
|
|||
| return resp.Payload, nil | |||
| } | |||
|
|
|||
| // UpdateVaultCluster will make a call to the Vault service to enable or disable public IPs for the Vault cluster. | |||
There was a problem hiding this comment.
Nit
Suggested change
| // UpdateVaultCluster will make a call to the Vault service to enable or disable public IPs for the Vault cluster. | |
| // UpdateVaultClusterPublicIps will make a call to the Vault service to enable or disable public IPs for the Vault cluster. |
currently only 'public_endpoint' can be updated
vault cluster
to make it more clear which cluster we're talking about :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🛠️ Description
Jumping off @bplotnick's contribution #118, I've implemented the Vault cluster resource Update function to enable updating
public_endpointwithout forcing the recreation of the cluster.🏗️ Acceptance tests
Output from acceptance testing: