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

docs make kubernetes_ca_cert optional on kubernetes auth #25963

Merged

Conversation

thyton
Copy link
Contributor

@thyton thyton commented Mar 15, 2024

update kubernetes auth docs and api-docs to indicate

  • kubernetes_ca_cert field optional
  • the host's root CA set is used for TLS when kubernetes_ca_cert is not provided and disable_local_ca_jwt is true if running Vault in a Kubernetes pod.

@thyton thyton requested a review from a team as a code owner March 15, 2024 00:30
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Mar 15, 2024
@thyton thyton added docs pr/no-changelog pr/no-milestone and removed hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed labels Mar 15, 2024
Copy link

CI Results:
All Go tests succeeded! ✅

Copy link

Build Results:
All builds succeeded! ✅

@peteski22 peteski22 added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Mar 15, 2024
@thyton thyton marked this pull request as draft March 17, 2024 21:50
@thyton thyton marked this pull request as ready for review March 25, 2024 17:43
@thyton thyton requested review from benashz and a team March 26, 2024 17:34
website/content/docs/auth/kubernetes.mdx Outdated Show resolved Hide resolved

When Vault is running in a non-Kubernetes environment, either
`kubernetes_ca_cert` or `pem_keys` must be set by the user.
If this behavior is disabled by setting `disable_local_ca_jwt` to `true`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Which behavior are you referring to?

Copy link
Contributor Author

@thyton thyton Mar 26, 2024

Choose a reason for hiding this comment

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

I'm referring to the behavior of kubernetes_ca_cert defaulting to the local CA cert if Vault is running a Kubernetes pod from the previous paragraph.

If Vault is running in a Kubernetes Pod, the `kubernetes_ca_cert` and
`token_reviewer_jwt` parameters will automatically default to the local CA cert
(`/var/run/secrets/kubernetes.io/serviceaccount/ca.crt`) and local service
account JWT (`/var/run/secrets/kubernetes.io/serviceaccount/token`).

I can reiterate to make it clearer here. Let me know your thought.

Suggested change
If this behavior is disabled by setting `disable_local_ca_jwt` to `true`,
If `kubernetes_ca_cert` defaulting to the local CA cert is disabled by setting `disable_local_ca_jwt` to `true`,

website/content/api-docs/auth/kubernetes.mdx Outdated Show resolved Hide resolved
thyton and others added 3 commits March 26, 2024 11:17
Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>
Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>
Comment on lines 33 to 34
- `kubernetes_ca_cert` `(string: "")` - Optional PEM encoded CA cert for use by the TLS client used to talk with the Kubernetes API.
NOTE: Every line must end with a newline: `\n`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `kubernetes_ca_cert` `(string: "")` - Optional PEM encoded CA cert for use by the TLS client used to talk with the Kubernetes API.
NOTE: Every line must end with a newline: `\n`
- `kubernetes_ca_cert` `(string: "")` - Optional PEM encoded CA cert. The TLS
client can use the cert to talk with the Kubernetes API. **Every line in the
cert file must end with a newline character (`\n`) rather than a carriage
return (`\r`)**.

Added an assumed clarification re: newline vs carriage return, but it prompted a question: do we read CRLF as ending in a newline (\r\n)? Or must it be explicitly LF (\n)?
Style correction: active voice

Copy link
Contributor Author

@thyton thyton Mar 26, 2024

Choose a reason for hiding this comment

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

I've tested locally. Ending each line in kubernetes_ca_cert with \r\n fails vault-plugin-auth-kubernetes unit tests. Therefore, it must be explicitly LF \n.

Wondering if we should preserve the old NOTE's content and drop "rather than a carriage return (\r)" for simplicity.

If not set, the local CA cert will be used if running in a Kubernetes pod.
If not set and `disable_local_ca_jwt` set to true, the system's trust store will be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If not set and `disable_local_ca_jwt` set to true, the system's trust store will be used.
If `kubernetes_ca_cert` is unset and `disable_local_ca_jwt` is `true`, Vault uses the Kubernetes trust store.

What is "the system" in "the system's trust store"? Is it Vault or Kubernetes? (I made an entirely uninformed guess for the sake of editing to provide an example of what I would suggest)

Copy link
Contributor Author

@thyton thyton Mar 26, 2024

Choose a reason for hiding this comment

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

"the system" here is:

  • the operating system if Vault is running on a machine
  • the container operating system if Vault is running in a container

Copy link
Contributor Author

@thyton thyton Mar 26, 2024

Choose a reason for hiding this comment

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

Is it Vault or Kubernetes?

Vault (specifically, the TLS client of the Vault plugin) will use the system's trust store for certificate verification.

website/content/api-docs/auth/kubernetes.mdx Outdated Show resolved Hide resolved
website/content/api-docs/auth/kubernetes.mdx Outdated Show resolved Hide resolved
website/content/docs/auth/kubernetes.mdx Outdated Show resolved Hide resolved
Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>
thyton added 2 commits March 26, 2024 13:45
…-optional' of github.com:hashicorp/vault into VAULT-1729-docs-auth-kubernetes-make-kubernetes-ca-cert-optional
@thyton
Copy link
Contributor Author

thyton commented Mar 26, 2024

Thank you both for the suggestions!

@thyton thyton requested review from benashz and schavis March 26, 2024 21:11
thyton added 2 commits March 26, 2024 14:20
…-optional' of github.com:hashicorp/vault into VAULT-1729-docs-auth-kubernetes-make-kubernetes-ca-cert-optional
@thyton thyton merged commit df477f6 into main Mar 27, 2024
31 of 32 checks passed
@thyton thyton deleted the VAULT-1729-docs-auth-kubernetes-make-kubernetes-ca-cert-optional branch March 27, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog pr/no-milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants