Skip to content

Conversation

julz
Copy link
Contributor

@julz julz commented Nov 9, 2020

Documents the DomainMapping feature which will be added in alpha in v0.19.

This probably needs lots of work @abrennan89 @mpetason, but opening as a first version, anyway.

In particular looking for guidance on how best to call out that this is an Alpha feature etc.

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 9, 2020
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2020
@csantanapr
Copy link
Member

Documents the DomainMapping feature which will be added in alpha in v0.18.

You meant to say v0.19 ?


If the Domain Mapping feature is installed, you can serve
custom domains backed by Knative Services. For example, if you
own the "MyDomain.com" domain name, you can point DNS at your
Copy link
Member

Choose a reason for hiding this comment

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

Maybe using lower case is better “mydomain.com”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d'oh, thanks, fixed

@julz
Copy link
Contributor Author

julz commented Nov 9, 2020

You meant to say v0.19 ?

I did. And as one of the release managers for 0.19: oops 😂 .

type: "docs"
---

If the Domain Mapping feature is installed, you can serve
Copy link
Member

Choose a reason for hiding this comment

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

This sentence sounds like the user needs to install an optional components when installing knative..
Is this true or you mean that user use a knative 0.19+ that contains this CRD and that the state of the feature is alpha and we want feedback early

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the user does need to install an additional component -- it's not installed by default in this release (there's an extra yaml file, much like installing the monitoring bundle or HPA autoscaler). I'll PR an update to install docs separately with that optional step.

Copy link
Member

Choose a reason for hiding this comment

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

Add a reference and link here you don’t have to put the actual steps here

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 will when I've added that step to the docs :)

Copy link
Member

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 add to this doc the limitations, that not all net ingress support this feature, you can list them here, or mention in the install doc and point there from here.

apiVersion: serving.knative.dev/v1alpha1
kind: DomainMapping
metadata:
name: mydomain.com
Copy link
Member

Choose a reason for hiding this comment

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

Is there a spec.domain field? Because I would be very disappointed if the actual domain value is the name of the CR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's intentionally the name of the DM to prevent collisions. Here's one of the conversations in the design doc where we decided this: https://docs.google.com/document/d/1yXSuKpQqZM5tczBFpPG8TGQoWSkvmoA17nPwDd1UxEI/edit?disco=AAAAG2yt488.

To create a mapping from a custom domain to a Knative Service, you need to
create a YAML file that defines a Domain Mapping.
This YAML file specifies the domain name to map and the Knative Service to use
to service requests.
Copy link
Member

Choose a reason for hiding this comment

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

Put some reference to the example “mydomain.com” explain if using this value would cover all sub domains ? *.mydomain.com or it needs to a specific domain ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added an extra para here, ptal

metadata:
name: mydomain.com
namespace: default
spec:
Copy link
Member

Choose a reason for hiding this comment

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

spec.domain missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, see above

linkTitle: "Creating Domain Mappings (Alpha)"
weight: 64
type: "docs"
---
Copy link
Member

Choose a reason for hiding this comment

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

The intro needs some use case if not would very confusing for the end user on why she would need this feature.

You can put something like
knative by default offeres a comfigmap “config-domain” in the control plane namespace (Ie knative-serving”) that allow an admin with write access to the namespace edit the comfigmap to configure domain names including a selector to be use which services corresponds to a domain name..
But in some cases the admin would not want this ConfigMap to be edited and would prefer users to define the domain name mapping to be fine by users in the namespace where the knative service is deploy.
This feature would allow to define the mapping in their namespace by creating a Kubernetes resource DomainMapping

Copy link
Contributor Author

@julz julz Nov 9, 2020

Choose a reason for hiding this comment

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

How about just "Knative Services are automatically given a default domain name based on the cluster configuration. You can also map a single custom domain name that you own to a specific Knative Service using the Domain Mapping feature."?

Copy link
Member

Choose a reason for hiding this comment

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

sure

@julz julz changed the title Add DomainMapping docs WIP: Add DomainMapping docs Nov 9, 2020
@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2020
@julz julz changed the title WIP: Add DomainMapping docs Add DomainMapping docs Nov 9, 2020
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2020
@julz
Copy link
Contributor Author

julz commented Nov 9, 2020

/hold so I can link to the install instructions in the first sentence (once I PR that step to them :)).

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 9, 2020
@abrennan89
Copy link
Contributor

@julz is this one ready for docs review / merge?

@abrennan89
Copy link
Contributor

Question: is this related to #1964? Might it close that issue?

@julz
Copy link
Contributor Author

julz commented Jan 4, 2021

/unhold should be ready for review now @abrennan89

@julz
Copy link
Contributor Author

julz commented Jan 4, 2021

Question: is this related to #1964? Might it close that issue?

I think it might @abrennan89. This documents the feature that - iiuc - @ahmetb was expecting the other doc to be about. Hopefully @ahmetb can confirm if so

@ahmetb
Copy link
Contributor

ahmetb commented Jan 4, 2021

Correct. That issue asks for this PR.

@abrennan89 abrennan89 linked an issue Jan 4, 2021 that may be closed by this pull request
## Before you begin

1. You need to enable the DomainMapping feature (and a supported Knative
Ingress implementation) to use it. See [the Install instructions](../install/).
Copy link

Choose a reason for hiding this comment

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

where on that page should people look? I didn't see anything covering this topic. Perhaps we could point to the exact page of interest?

@abrennan89 abrennan89 modified the milestones: v0.20.0, Backlog Jan 14, 2021
Copy link
Contributor

@nak3 nak3 left a comment

Choose a reason for hiding this comment

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

@julz It seems mydomain.com is already used by some organization in the real world so should we change the example domain like mydomain.example.org which is listed in RFC2606 - 3. Reserved Example Second Level Domain Names?

@julz
Copy link
Contributor Author

julz commented Jan 18, 2021

oops - thanks @nak3, will update!

@julz
Copy link
Contributor Author

julz commented Jan 21, 2021

Ready for another look @abrennan89 @nak3

@csantanapr
Copy link
Member

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2021
@nak3
Copy link
Contributor

nak3 commented Jan 21, 2021

/lgtm

@csantanapr
Copy link
Member

/approve

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csantanapr, julz

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

@nak3
Copy link
Contributor

nak3 commented Jan 21, 2021

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2021
@knative-prow-robot knative-prow-robot merged commit f449dff into knative:master Jan 21, 2021
@nak3
Copy link
Contributor

nak3 commented Jan 22, 2021

/cherrypick release-0.20
/cherrypick release-0.19

@knative-prow-robot
Copy link
Contributor

@nak3: new pull request created: #3183

In response to this:

/cherrypick release-0.20
/cherrypick release-0.19

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.

@nak3
Copy link
Contributor

nak3 commented Jan 22, 2021

/cherrypick release-0.19

@knative-prow-robot
Copy link
Contributor

@nak3: new pull request created: #3184

In response to this:

/cherrypick release-0.19

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.

RichieEscarez pushed a commit to RichieEscarez/docs that referenced this pull request Mar 6, 2021
* Add DomainMapping docs

* x

* My oh my

* Add paragraph about wildcard (lack of) behaviour

* Update description and link to install docs

* Update link, use example.org domain, and elaborate contract for Ref
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. kind/serving lgtm Indicates that a PR is ready to be merged. 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.

Custom domains doc only explains customizing default domain

7 participants