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

feat(pki): Add resource for cluster configuration #1949

Merged
merged 6 commits into from
Feb 23, 2024

Conversation

Viper61
Copy link
Contributor

@Viper61 Viper61 commented Jul 22, 2023

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates #1947

Release note for CHANGELOG:

* Add support to set cluster configuration for PKI Secrets Engine

Output from acceptance testing:

$ TESTARGS="--run TestPkiSecretBackendConfigCluster_basic" make testacc'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test --run TestPkiSecretBackendConfigCluster_basic -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]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.036s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    0.071s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    0.055s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    0.075s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        0.079s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    0.020s [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]
?       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/generated/resources/transform/transformation      0.056s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  0.062s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider 0.047s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/testutil  0.011s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      0.005s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/vault     4.934s

@igor-nikiforov
Copy link

@fairclothjm could you please review this PR? Thanks!

@melonger
Copy link

@fairclothjm in reference to the comment: #1947 (comment)

This is announced as full feature and is being advocated as a working feature from your services teams. Could you please review the pull request so we can get the feature brought in?

It's clearly part of the API: https://developer.hashicorp.com/vault/api-docs/secret/pki#acme-certificate-issuance

Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @Viper61! I did an initial pass and it looks good so far. If you are interested in carrying this forward could you please make the requested changes and add a changelog entry? Thanks!

vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
@Viper61
Copy link
Contributor Author

Viper61 commented Feb 16, 2024

Thanks for the review @fairclothjm!
I've rewrote a large part of the code, taking in account items you mentionned.
Golang is still very new to me, please let of know if you see further improvments that need to be made.

@github-actions github-actions bot added size/XL and removed size/L labels Feb 17, 2024
Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

Great work so far @Viper61! A couple of things in addition to my comments:

  • Could you please rebase on main as you have merge conflicts and the CHANGELOG is outdated
  • After the requested changes are done, if you are ok with me pushing to your branch I can add some regex parsing for import correctness.

Thanks!

vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_config_cluster.go Outdated Show resolved Hide resolved
@Viper61
Copy link
Contributor Author

Viper61 commented Feb 17, 2024

  • Could you please rebase on main as you have merge conflicts and the CHANGELOG is outdated

New rebase done and I updated the CHANGELOG. No more conflict for now.

  • After the requested changes are done, if you are ok with me pushing to your branch I can add some regex parsing for import correctness.

Sure, be my guest @fairclothjm .

@github-actions github-actions bot added size/XL and removed size/L labels Feb 23, 2024
Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @Viper61 !

@fairclothjm fairclothjm added this to the 3.26.0 milestone Feb 23, 2024
@fairclothjm fairclothjm merged commit 2649a73 into hashicorp:main Feb 23, 2024
2 checks passed
@Viper61 Viper61 deleted the feat/pki-config-cluster branch February 23, 2024 18:02
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.

4 participants