search

cert-manager / website

Source code for the cert-manager.io website, including project documentation
https://cert-manager.io
Apache License 2.0
54 stars 337 forks source link

trust-manager: improve install instructions #1439

Closed inteon closed 8 months ago

inteon commented 8 months ago

Problem solved: This PR makes the install instructions more readable ( adds newlines ). This PR adds an explicit version to the install instructions, which is our recommended way of installing cert-manager (see https://cert-manager.io/docs/installation/helm/). This gets us a step closer to consistent instructions on all pages of the website.

Testing done: Ran the install instructions.

netlify[bot] commented 8 months ago

Deploy Preview for cert-manager-website ready!

Name Link
Latest commit c5ca24f350c32e1fec3cab080b26416c5e031f7a
Latest deploy log https://app.netlify.com/sites/cert-manager-website/deploys/65f1b0c70528b6000836b125
Deploy Preview https://deploy-preview-1439--cert-manager-website.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

inteon commented 8 months ago

What is the advantage of adding an explicit version?

We want to advise people to explicitly provide the version when installing (same goes for https://cert-manager.io/docs/installation/helm/).

This will additionally notify the user in case of a out-of-date chart repo that does not have the specified version.

inteon commented 8 months ago

And what happens when we update the versioned documentation? Will those past versions of cert-manager be forever locked to an older version of trust-manager and approver-policy? By versioned documentation I mean :

We probably should not make the trust-manager and approver-policy versions dependant on the cert-manager version. Maybe we should have a set of global variables that apply to all the cert-manager versions?

inteon commented 8 months ago

We want to advise people to explicitly provide the version when installing (same goes for https://cert-manager.io/docs/installation/helm/).

Why do we want to advise people to explicitly provide the version? I regard these as quick-start installation instructions and the changes here make the instructions more complicated.

And why do we want to advise people to upgrade cert-manager to 1.14.2 before they install trust-manager. What if they're trying this out on a cluster where cert-manager 1.12 is already installed and accidentally upgrade cert-manager.

This will additionally notify the user in case of a out-of-date chart repo that does not have the specified version.

But the first line is helm repo add jetstack https://charts.jetstack.io --force-update, so that should never happen, right?

As explained in the PR description, moving away from upgrade is not one of the problems this PR is solving.

inteon commented 8 months ago

@wallrj I removed the versions, will create another PR for approver-policy too.

jetstack-bot commented 8 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wallrj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/cert-manager/website/blob/master/OWNERS)~~ [wallrj] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment