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 cert-manager documentation #1317

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

Add cert-manager documentation #1317

wants to merge 8 commits into from

Conversation

djwfyi
Copy link
Collaborator

@djwfyi djwfyi commented Sep 11, 2024

Adds cert-manager docs for Kubernetes outputs.

Closes #1245

Partially addresses #1273

Staged:

@djwfyi djwfyi self-assigned this Sep 11, 2024
Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

I think we need to re-structure a bit. I'll do a closer pass on content after that

source/operations/cert-manager.rst Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
Install cert-manager.
`Release 1.12.X LTS <https://cert-manager.io/docs/releases/release-notes/release-notes-1.12/>`__ is preferred, but you may install latest.

The following command installs version 1.12.13 using ``kubectl``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth doing a substitution here in case the version bumps later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps? Not sure how critical that is to stay up. Mostly we want this lineage. Which particular release within 1.12.x is less important from our perspective.

source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved

The following graphic depicts how various namespaces make use of either an ``Issuer`` or ``ClusterIssuer`` type.

- cert-manager is installed in the ``cert-manager`` namespace, which does not have either an ``issuer`` or a ``ClusterIssuer``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused by the cert-manager namespace in the diagram, is minio-operator:issuer not an issuer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can address that with other comments on the graphic.
It should not have either an Issuer or ClusterIssuer in that namespace. But it operates separately from the other namespaces.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See suggestion in other thread here
#1317 (comment)


.. important::

Replace the filler strings with values for your tenant:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a usual term for nonspecific placeholder text?

Or perhaps something like "Replace the placeholder values as described below"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Trying an alternative. I think "placeholder text" is probably the clearest expression.

Copy link
Collaborator

@feorlen feorlen left a comment

Choose a reason for hiding this comment

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

Noted a couple typos, plus one thing I didn't understand. Maybe it's me, however.

Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

Much improved - feedback given

source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager.rst Outdated Show resolved Hide resolved

The following graphic depicts how various namespaces make use of either an ``Issuer`` or ``ClusterIssuer`` type.

- cert-manager is installed in the ``cert-manager`` namespace, which does not have either an ``issuer`` or a ``ClusterIssuer``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this if we're dropping a reference to it in the diagram?

Maybe:

The following diagram provides a high level view of the relationship between Cluster Issuers and Issuers in a cert-manager context:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Holding for the new diagram. Will leave unresolved until then.


.. image:: /images/k8s/cert-manager-cluster.svg
:width: 600px
:alt: A Kubernetes cluster with five namespaces, shown as a box for each namespace in the cluster. The minio-operator namespace contains a "minio-operator: issuer" local issuer. The default namespace contains a "root: ClusterIssuer" cluster issuer. The cert-manager namespace contains a "minio-operator: issuer" local issuer. The remaining two namespaces are individual tenants, "tenant-1" and "tenant-2", each with its own local issuer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to update the alt text

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Waiting for the new diagram first. Then will update.

source/operations/cert-manager.rst Outdated Show resolved Hide resolved
source/operations/cert-manager/cert-manager-tenants.rst Outdated Show resolved Hide resolved
source/operations/cert-manager/cert-manager-tenants.rst Outdated Show resolved Hide resolved
source/operations/cert-manager/cert-manager-tenants.rst Outdated Show resolved Hide resolved
source/operations/network-encryption.rst Outdated Show resolved Hide resolved
@djwfyi djwfyi added the blocked label Sep 27, 2024
@djwfyi
Copy link
Collaborator Author

djwfyi commented Sep 27, 2024

This PR is block until we get an updated graphic from the design team.

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

Successfully merging this pull request may close these issues.

Document TLS Certificates flow with Cert Manager
3 participants