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

Rearrange documentation and add additional content #1279

Merged
merged 27 commits into from Jan 31, 2019

Conversation

munnerz
Copy link
Member

@munnerz munnerz commented Jan 29, 2019

What this PR does / why we need it:

This PR does a few things:

  • rearranges our documentation to collate information and have a more formal structure for where info should go
  • add additional content (e.g. webhook info, troubleshooting guide)
  • add placeholders for more additional content still to come
  • adds additional cross-links in helpful places (e.g. linking to issuer setup docs)
  • collates the issuer setup guides into a single tasks/ section
  • improves the installation guide with more helpful information & links out
  • sets up redirects for the pages that have moved

Which issue this PR fixes fixes #1151, fixes #1269

Special notes for your reviewer:

I've setup readthedocs on my fork of cert-manager, and you can view a sample of these new docs here: https://cert-manager-munnerz.readthedocs.io/en/revamp-docs/index.html

Release note:

Overhaul documentation and add additional content

/cc @heckj @DanielMorsing

Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. kind/documentation Categorizes issue or PR as related to documentation. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 29, 2019
@munnerz munnerz closed this Jan 29, 2019
@munnerz munnerz deleted the revamp-docs branch January 29, 2019 23:46
@munnerz munnerz restored the revamp-docs branch January 30, 2019 00:03
@munnerz munnerz reopened this Jan 30, 2019
@munnerz munnerz added this to In progress in v0.7 via automation Jan 30, 2019
@munnerz munnerz added this to the v0.7 milestone Jan 30, 2019
@munnerz munnerz moved this from In progress to Needs review in v0.7 Jan 30, 2019
Copy link
Contributor

@rimusz rimusz left a comment

Choose a reason for hiding this comment

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

docs/getting-started/webhook.rst Show resolved Hide resolved
docs/getting-started/install.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@rimusz rimusz left a comment

Choose a reason for hiding this comment

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

docs/getting-started/webhook.rst Show resolved Hide resolved
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Copy link
Contributor

@rimusz rimusz left a comment

Choose a reason for hiding this comment

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

LGTM

@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: munnerz, rimusz

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

docs/getting-started/install.rst Outdated Show resolved Hide resolved
docs/getting-started/install.rst Outdated Show resolved Hide resolved
docs/getting-started/install.rst Outdated Show resolved Hide resolved
docs/getting-started/install.rst Outdated Show resolved Hide resolved
docs/getting-started/webhook.rst Show resolved Hide resolved
docs/getting-started/webhook.rst Show resolved Hide resolved
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
@wwwil
Copy link
Contributor

wwwil commented Jan 30, 2019

Not in the PR but theres a typo in this section: https://cert-manager-munnerz.readthedocs.io/en/revamp-docs/tasks/issuers/setup-vault.html#vault-authentication-with-a-approle

The sentence "... parameter as no effect ..." should be "... parameter has no effect ...".

Also here: https://cert-manager-munnerz.readthedocs.io/en/revamp-docs/tasks/issuers/setup-vault.html#vault-authentication-with-a-token

"... many authentication backend supported by Vault." should be "backends".
"You need to be aware that cert-manager do not refresh these tokens" should use "does not".

This sections has some issues "For testing purpose a root token which do not expire is generated at Vault installation time. WARNING: a root token should only be used for testing purpose only." I'd advise something like: "For testing purposes a root token is generated at Vault installation time. WARNING: Root tokens do not expire, so should only be used for testing purposes."

@wwwil
Copy link
Contributor

wwwil commented Jan 30, 2019

It might be worth adding a note to the upgrading page about backing up first with a link to the relevant page.

Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
@munnerz
Copy link
Member Author

munnerz commented Jan 31, 2019

After discussion on the community call yesterday, we're going to cherry pick these docs changes into the v0.6 branch as they contain a fair bit of extra useful information 😄

@munnerz
Copy link
Member Author

munnerz commented Jan 31, 2019

/milestone v0.6

@jetstack-bot jetstack-bot modified the milestones: v0.7, v0.6 Jan 31, 2019
@munnerz munnerz added this to In progress in v0.6 via automation Jan 31, 2019
@munnerz munnerz removed this from Needs review in v0.7 Jan 31, 2019
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
Signed-off-by: James Munnelly <james@munnelly.eu>
@munnerz
Copy link
Member Author

munnerz commented Jan 31, 2019

Going to merge this now as it is a net-improvement to the docs in its current state.

I'll follow up with additional PRs if anyone has any other comments, and to fill in/rewrite some areas that need it 😄

@munnerz munnerz added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2019
@jetstack-bot jetstack-bot merged commit 9e61b31 into cert-manager:master Jan 31, 2019
v0.6 automation moved this from In progress to Done Jan 31, 2019
jetstack-bot added a commit that referenced this pull request Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
No open projects
v0.6
  
Done
Development

Successfully merging this pull request may close these issues.

A invalid certificate spec (such as unknown unit) blocks other certificate signing. Documentation overhaul
4 participants