Skip to content

feat: update secretTemplate documentation#634

Merged
jetstack-bot merged 5 commits intocert-manager:release-nextfrom
jonathansp:master
Aug 4, 2021
Merged

feat: update secretTemplate documentation#634
jetstack-bot merged 5 commits intocert-manager:release-nextfrom
jonathansp:master

Conversation

@jonathansp
Copy link
Contributor

Signed-off-by: jonathansp jonathansimonprates@gmail.com

Update docs to include cert-manager/cert-manager#3828 feature.

cc @maelvls

@jetstack-bot jetstack-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Jun 30, 2021
@jetstack-bot
Copy link
Contributor

Hi @jonathansp. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jetstack-bot jetstack-bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 30, 2021
@jonathansp
Copy link
Contributor Author

/assign @jakexks

@wallrj
Copy link
Member

wallrj commented Jun 30, 2021

/ok-to-test

Thanks for all your work on this @jonathansp

@jetstack-bot jetstack-bot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 30, 2021
Comment on lines +88 to +94
If `secretTemplate` is present, annotations and labels set in this property
will be copied over to `example-com-tls` secret. Both properties are optional.
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, how do we let people know that this is a feature that only exists since 1.5? The user would have to look for differences between 1.4 and 1.5 to find that out.

I'd go with smth like:

Suggested change
If `secretTemplate` is present, annotations and labels set in this property
will be copied over to `example-com-tls` secret. Both properties are optional.
If `secretTemplate` _(since cert-manager 1.5)_ is present, annotations and labels set in this property
will be copied over to `example-com-tls` secret. Both properties are optional.

@irbekrm what do you think? 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Our docs are versioned so I don't think this is too much of a concern.

Copy link
Member

@maelvls maelvls Jul 29, 2021

Choose a reason for hiding this comment

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

@JoshVanL I find it very frustrating to have to select one by one the documentation versions to find when a feature was introduced and know if your version of cert-manager works with that feature, since Google always returns the latest version of the pages.

I am often in the situation where I start using a feature and realize later on that the feature was introduced in a later version. This happened 3 times in the last month while using Traefik. I just wish they had a tiny note on every Ingress annotation that reminds me "when" this annotation was added.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we may want to merge this into release-next branch, not into master though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated and rebased @irbekrm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just added a release note based on the PR description and code comments, let me know if it needs to be rewritten. @benlangfeld I took the liberty to add your name to the contributors' list above.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like our stupid spellchecker doesn't like the github usernames. You'll have to add them to the .spelling file. See https://github.com/cert-manager/website/blame/master/.spelling#L265

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usernames added to .spelling.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jonathansp I'm flattered, but I didn't do anything nearly worth being included in a list of contributors :)

@netlify
Copy link

netlify bot commented Aug 3, 2021

✔️ Deploy Preview for cert-manager-website ready!

🔨 Explore the source changes: 66a75bd

🔍 Inspect the deploy log: https://app.netlify.com/sites/cert-manager-website/deploys/610a813736af840008b14b1b

😎 Browse the preview: https://deploy-preview-634--cert-manager-website.netlify.app

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Aug 3, 2021
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

@jonathansp Please update the PR to be merged in to the release-next branch (as @irbekrm mentioned above)

That's the branch containing the documentation and release notes for the upcoming 1.5 release, which will be the first release containing your secret template work.

Thanks.

@wallrj
Copy link
Member

wallrj commented Aug 3, 2021

@jonathansp Also please update the release-notes page with a paragraph about the new secret template feature. Thanks.

@maelvls
Copy link
Member

maelvls commented Aug 4, 2021

@jonathansp Do you think you could click "Edit" on the PR and select "release-next" instead of "master" as the base branch? That would probably also require a rebase onto release-next. Please let me know if you need help with that.

@jonathansp
Copy link
Contributor Author

@jonathansp Do you think you could click "Edit" on the PR and select "release-next" instead of "master" as the base branch? That would probably also require a rebase onto release-next. Please let me know if you need help with that.

Sorry @maelvls, couple of meetings this morning. Will do.

@jetstack-bot jetstack-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2021
@jonathansp jonathansp requested a review from wallrj August 4, 2021 11:09
Signed-off-by: jonathansp <jonathansimonprates@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Aug 4, 2021
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

Thanks again. This looks great.

/lgtm
/approve

Special thanks to the external contributors who contributed to this release:
- TODO

* [@jonathansp](https://github.com/jonathansp)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2021
@wallrj wallrj added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager and removed do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels Aug 4, 2021
@jonathansp
Copy link
Contributor Author

/retest

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed lgtm Indicates that a PR is ready to be merged. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Aug 4, 2021
Signed-off-by: jonathansp <jonathansimonprates@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Aug 4, 2021
Copy link
Member

@wallrj wallrj 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 jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2021
@wallrj wallrj added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2021
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: jonathansp, wallrj

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

The pull request process is described here

Details 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

@jetstack-bot jetstack-bot merged commit 4b5a826 into cert-manager:release-next Aug 4, 2021
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. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. ok-to-test size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants