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

Add support for SSH sign endpoint #2409

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rculpepper
Copy link
Contributor

Description

Relates OR Closes #0000

Checklist

  • Added CHANGELOG entry (only for user-facing changes)
  • Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

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

@rculpepper rculpepper requested a review from a team February 24, 2025 15:52
@rculpepper rculpepper marked this pull request as ready for review February 24, 2025 15:52
@rculpepper rculpepper requested a review from a team as a code owner February 24, 2025 15:52
@rculpepper rculpepper requested a review from beagins February 24, 2025 15:52
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

Looks great overall, two small suggestions and the conflicts will need to be resolved before approval.

consts.FieldCertType: {
Type: schema.TypeString,
Optional: true,
Default: "user",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the preference these days is not to specify Default on the TFVP side and just allow the backend to specify them.


payload := map[string]interface{}{
"public_key": publicKey,
"ttl": ttl,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for all optional fields we should only add them to the payload if it was provided within the tf configuration. So we probably need another loop iterating over all the optional fields and do a GetOk and adding only if the boolean was true.

}
`

func testDataSourceSSHSecretBackendSign_check(s *terraform.State) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonderful test!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants