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

raft autopilot updates #1705

Merged
merged 15 commits into from
Jan 3, 2023
Merged

raft autopilot updates #1705

merged 15 commits into from
Jan 3, 2023

Conversation

fairclothjm
Copy link
Contributor

@fairclothjm fairclothjm commented Dec 19, 2022

This PR adds a new field disable_upgrade_migration to the Raft autopilot resource. Additionally, we create a new data source for Raft autopilot state.

Output from acceptance testing:

$ TESTARGS="--run TestAccDataSourceRaftAutoPilotState" make testacc-ent
make testacc TF_ACC_ENTERPRISE=1
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test --run TestAccDataSourceRaftAutoPilotState -timeout 30m ./...
?   	github.com/hashicorp/terraform-provider-vault	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/coverage	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/generate	[no test files]
ok  	github.com/hashicorp/terraform-provider-vault/codegen	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/generated	[no test files]
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode	1.305s [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode	0.991s [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet	1.644s [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role	2.333s [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template	2.061s [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation	0.715s [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/helper	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/internal/consts	[no test files]
ok  	github.com/hashicorp/terraform-provider-vault/internal/identity/entity	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/internal/identity/group	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/internal/identity/mfa	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/internal/pki	[no test files]
ok  	github.com/hashicorp/terraform-provider-vault/internal/provider	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/schema	[no test files]
ok  	github.com/hashicorp/terraform-provider-vault/testutil	(cached) [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/util	(cached) [no tests to run]
ok  	github.com/hashicorp/terraform-provider-vault/vault	15.791s

FieldGroupName = "group_name"
FieldExternal = "external"
FieldInternal = "internal"
FieldAPIHostname = "api_hostname"
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 have been sorted. I thought it might be ok do so since I was adding fields that were changing the indent level for all entries. However, I am beginning to doubt the value in doing this as it could make rebasing on top of this quite annoying.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that is definitely a valid point! I'm now wondering if it's better to leave these unsorted and as they were for now, and we can decide on the benefits of sorting once we have finished updating all resources/datasources to use field constants — something we are tracking for long term sustainability of this project which will add to this list a lot. Might be worth adding all the constants and then sorting them in one go. What do you think?

Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

PR's looking great! Thanks for adding these 🙏🏼

website/docs/d/raft_autopilot_state.html.md Outdated Show resolved Hide resolved
website/docs/d/raft_autopilot_state.html.md Outdated Show resolved Hide resolved
FieldGroupName = "group_name"
FieldExternal = "external"
FieldInternal = "internal"
FieldAPIHostname = "api_hostname"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that is definitely a valid point! I'm now wondering if it's better to leave these unsorted and as they were for now, and we can decide on the benefits of sorting once we have finished updating all resources/datasources to use field constants — something we are tracking for long term sustainability of this project which will add to this list a lot. Might be worth adding all the constants and then sorting them in one go. What do you think?


// serializeFields is a map of fields that have complex structures that we will
// serialize for convenience instead of defining the schema explicitly
var serializeFields = map[string]string{
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM!

@vinay-gopalan vinay-gopalan added this to the 3.12.0 milestone Dec 23, 2022
@fairclothjm fairclothjm merged commit d0d7987 into main Jan 3, 2023
@fairclothjm fairclothjm deleted the vault-6022-raft-autopilot branch January 3, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants