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 how to create custom domains with kn domain #4209

Merged
merged 3 commits into from
Sep 10, 2021

Conversation

rhuss
Copy link
Contributor

@rhuss rhuss commented Sep 9, 2021

Proposed Changes

  • Add a description of how to leverage kn domain for managing custom domains
  • Extended YAML description for CustomDomain include the tls: section for enabling HTTPS

Note, that I'm not sure about the formatting as I add the kn usage in the middle of a numbered list and am not sure how it will be rendered. Any help for fixing the formatting is appreciated.

Fixes #3496

@netlify
Copy link

netlify bot commented Sep 9, 2021

✔️ Deploy Preview for knative ready!

🔨 Explore the source changes: 9b70620

🔍 Inspect the deploy log: https://app.netlify.com/sites/knative/deploys/613a32af9434b40008275719

😎 Browse the preview: https://deploy-preview-4209--knative.netlify.app

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 9, 2021
@knative-prow-robot knative-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2021
@snneji
Copy link
Contributor

snneji commented Sep 9, 2021

Hi @rhuss, some of the formatting fixes will need to span lines that you've not edited in the PR, so it will be difficult for me to use the suggestion feature to suggest the correct formatting.

Would it be OK to merge the PR and then open a cleanup issue for us to fix the formatting and style for you? Or would you prefer me to leave comments on the PR for you to fix the formatting yourself?

@rhuss
Copy link
Contributor Author

rhuss commented Sep 9, 2021

@snneji happy to merge it now and hand it over to you to fix the formatting. So maybe give it a quick look about whether the content is fine so far ?

Copy link
Contributor

@snneji snneji left a comment

Choose a reason for hiding this comment

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

Looks OK to merge to me. Just a small fix here and the rest of the fixes can be dealt with in the cleanup issue.

One other thing - does this PR need to be cherry picked? It seems to be based on an issue that was first created for v0.22.

docs/developer/serving/services/custom-domains.md Outdated Show resolved Hide resolved
@rhuss
Copy link
Contributor Author

rhuss commented Sep 9, 2021

kn supports the domain command since 0.22, not sure about the tls: option though. @julz do you know when tls: was introduced to DomainMapping ? Was it there from the beginning ?

@abrennan89 abrennan89 requested a review from julz September 9, 2021 18:43
@julz
Copy link
Member

julz commented Sep 10, 2021

wasn't there in the beginning, looks like it landed in 0.24 (same time DM went beta)

@snneji
Copy link
Contributor

snneji commented Sep 10, 2021

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2021
@snneji snneji mentioned this pull request Sep 10, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhuss, snneji

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

@snneji
Copy link
Contributor

snneji commented Sep 10, 2021

Issue to fix formatting: #4225

@knative-prow-robot knative-prow-robot merged commit 861298b into knative:mkdocs Sep 10, 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. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

document Client CLI kn domain
4 participants