-
Notifications
You must be signed in to change notification settings - Fork 47
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-4407 implement ip allowlist in terraform provider #625
HVT-4407 implement ip allowlist in terraform provider #625
Conversation
…urce cluster create function
…in-terraform-provider
…in-terraform-provider
…in-terraform-provider
Still in the process of running acceptance tests, but wanted to do a draft PR for now to get early feedback on the progress so far:) |
…in-terraform-provider
…m-provider' of github.com:hashicorp/terraform-provider-hcp into aslamovamir-HVT-4407-implement-ip-allowlist-in-terraform-provider
This reverts commit bca48e4.
29e79cf
to
340e013
Compare
…in-terraform-provider
@aoripov I have been able to manually test this PR and updated the PR description accordingly. Is this kind of testing comprehensive enough? Also, updating the ip allowlist seems to work (referenced in PR description), even though when the |
…in-terraform-provider
Actually, it is triggering the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great to me. Awesome testing in the PR description!
For the final testing (with updates from master
branch and updated terraform CLI version), I think it's enough to check the AWS and Azure acceptance tests pass. Less priority on the manual end-to-end tests - if it gives peace of mind, one manual end-to-end test should be sufficient and no need to repeat all the cases from the PR description.
Plan to stamp after the final testing
Sounds good, Helen, thank you! I am working on rerunning acceptance tests once again with the upgraded Terraform CLI. |
…in-terraform-provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stamping for same reason as Slack comment:
My concerns are addressed (re-running the acceptance tests as they currently stand without encountering the incompatibility errors you were facing earlier).
Will defer to Gaurav for final review of validation suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. I realized validation on 50 doesn't make sense as MaxItem construct should correctly enforce it :)
@@ -295,17 +298,27 @@ func updateClusterTier(t *testing.T, in *inputT) resource.TestStep { | |||
} | |||
} | |||
|
|||
// This step verifies the successful update of "public_endpoint", "proxy_endpoint", "audit_log", "metrics" and MVU config. | |||
// This step verifies the successful update of "public_endpoint", "proxy_endpoint", "ip_allowlist", "audit_log", "metrics" and MVU config. | |||
func updatePublicProxyObservabilityAndMVU(t *testing.T, in *inputT) resource.TestStep { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: func name is growing or should grow with ipAllowList
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have already renamed the function to updateNetworkObservabilityAndMVU
:)
🛠️ Description
Adding a new field
ip_allowlist
to thehcp_vault_cluster
resource to support IP Allowlisting - a Block List (list of objects).Jira ticket: https://hashicorp.atlassian.net/browse/HVT-4407
🏗️ Acceptance tests
Output from acceptance testing:
AWS
Azure
Consul cluster test
End to End Tests (PROD)
Test AWS Cluster with IP Allowlist
Resource CREATE
make dev
in root of repotest/
main.tf
with following :terraform init
terraform plan
terraform plan
:terraform apply
terraform apply
:Datasource (READ ONLY)
main.tf
with the following:terraform apply
Resource UPDATE
hcp_vault_cluster
resource:terraform apply
Test AWS and Azure clusters together with IP allowlist
Resource CREATE
make dev
in root of repotest2/
main.tf
with following :terraform init
terraform plan
terraform plan
:terraform apply
terraform apply
:Resource UPDATE
terraform apply
:For AWS:https://cadence.us-east-1.aws.hashicorp.cloud/domains/hcp/workflows/vault%2Fproject%2Faa02bed1-7174-44ae-8dee-8fc94243534b%2Fhashicorp.vault.cluster%2Ftf-vault-cluster%2Fupdate-networkconfig/53446f76-4c3d-4816-8e39-ad36698dd4bc/summary
For Azure:https://cadence.us-east-1.aws.hashicorp.cloud/domains/hcp/workflows/vault%2Fproject%2Faa02bed1-7174-44ae-8dee-8fc94243534b%2Fhashicorp.vault.cluster%2Ftf-vault-cluster-az%2Fcde217db-0bb5-4d07-879f-9962d5811206%2Fupdate/fe981d2f-8473-4272-9473-bbf67d027c9e/summary
Test a cluster by adding IP allowlist after cluster creation
make dev
in root of repotest3/
main.tf
with following :terraform init
.terraform apply
:terraform apply
:https://cadence.us-east-1.aws.hashicorp.cloud/domains/hcp/workflows/vault%2Fproject%2Faa02bed1-7174-44ae-8dee-8fc94243534b%2Fhashicorp.vault.cluster%2Fip-allowlist-cluster-2%2Fupdate-networkconfig/d6d872e0-114f-4b85-addc-36bb404a2a28/summary
Validation
No more than 50 CIDRs
Description Length
Invalid CIDR