Use svc.cluster.local as default domain #13259

psschwei · 2022-08-29T20:36:05Z

Signed-off-by: Paul S. Schweigert paul@paulschweigert.com

Release Note

Uses the cluster domain suffix `svc.cluster.local` as the default domain. As routes using the cluster domain suffix are not exposed through Ingress, users will need to [configure DNS](https://knative.dev/docs/install/yaml-install/serving/install-serving-with-yaml/#configure-dns) in order to expose their services (most users probably already are).

/hold

codecov · 2022-09-06T16:37:22Z

Codecov Report

Base: 86.52% // Head: 86.49% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (13bed11) compared to base (19cd295).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13259      +/-   ##
==========================================
- Coverage   86.52%   86.49%   -0.04%     
==========================================
  Files         196      196              
  Lines       14526    14526              
==========================================
- Hits        12569    12564       -5     
- Misses       1659     1663       +4     
- Partials      298      299       +1

Impacted Files	Coverage Δ
pkg/reconciler/route/config/domain.go	`100.00% <ø> (ø)`
pkg/reconciler/revision/reconcile_resources.go	`66.88% <0.00%> (-1.99%)`	⬇️
pkg/autoscaler/statserver/server.go	`77.77% <0.00%> (-1.49%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

psschwei · 2022-09-06T19:09:05Z

Need to wait on #13283 to run the kind e2e tests

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

nak3 · 2022-09-07T05:15:08Z

We probably want to update the comment in config-domain.yaml - https://github.com/knative/serving/blob/release-1.7/config/core/configmaps/domain.yaml#L43-L46

nak3 · 2022-09-07T05:21:14Z

pkg/reconciler/nscert/nscert_test.go

@@ -623,7 +623,7 @@ func networkConfig() *netcfg.Config {
 func domainConfig() *routecfg.Domain {
 	domainConfig := &routecfg.Domain{
 		Domains: map[string]*routecfg.LabelSelector{
-			"example.com": {},
+			"svc.cluster.local": {},


I think we should keep example.com for this nscert test.
net-certmanager does not create certificates for cluster local (svc.cluster.local) so we should assume that users set the domain as example.com as it is.

sounds good... in that vein, I added an overlay to set the domain for some of the e2e tests, let me know if you think we should remove that

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

knative-prow · 2022-09-07T17:52:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: psschwei

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:

~~config/OWNERS~~ [psschwei]
~~pkg/reconciler/nscert/OWNERS~~ [psschwei]
~~pkg/reconciler/route/OWNERS~~ [psschwei]
~~test/OWNERS~~ [psschwei]

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

nak3 · 2022-09-08T00:05:15Z

/lgtm
/hold

Thank you! LGTM.

nader-ziada

there are a couple of other spots that I think need updating as well

in the test readme, its says If you set up your cluster using the getting started docs, Routes created in the test will use the domain example.com
-

serving/test/config/tls/generate.sh

Line 22 in 6d3d676

openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -subj '/O=Knative Community/CN=example.com' -keyout rootCAKey.pem -out rootCACert.pem
serving/test/config/autotls/certmanager/caissuer/generate.sh

Line 20 in d5d489c

openssl req -x509 -sha256 -new -nodes -key rootCAKey.pem -days 36500 -out rootCACert.pem -subj '/CN=example.com/O=Knative Community/C=US'

there might be something else I missed

nader-ziada · 2022-09-08T14:54:26Z

config/core/configmaps/domain.yaml

+    # These are example settings of domain.
+    # example.com will be used for all routes, but it is the least-specific rule so it
+    # will only be used if no other domain matches.
+    example.com: |


should we change this in the example for clarity?

nak3 · 2022-09-11T11:17:46Z

For the example.com in the following scripts:

serving/test/config/autotls/certmanager/caissuer/generate.sh
serving/test/config/tls/generate.sh

We would like to test the TLS with the external domain so it is OK to keep example.com.

nader-ziada · 2022-09-11T20:36:05Z

For the example.com in the following scripts:

serving/test/config/autotls/certmanager/caissuer/generate.sh

serving/test/config/tls/generate.sh

We would like to test the TLS with the external domain so it is OK to keep example.com.

Sgtm

nader-ziada · 2022-09-14T20:59:35Z

Need to wait on #13283 to run the kind e2e tests

this is merged, is this PR here ready to remove the hold?

psschwei · 2022-09-14T21:04:27Z

Not yet, we gave until Sept 28 23 for anyone to comment on the mailing lists to changing the default domain

psschwei · 2022-10-12T19:12:13Z

Had no objections on the mailing list, so going to remove the hold on this tomorrow

psschwei · 2022-10-14T19:42:29Z

/hold cancel

psschwei mentioned this pull request Aug 29, 2022

Default domain in config-domain should be svc.cluster.local instead of example.com? #13182

Closed

knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 6, 2022

psschwei changed the title ~~[WIP] testing cluster local as default domain~~ [WIP] Use svc.cluster.local as default domain Sep 6, 2022

knative-prow bot added the area/test-and-release It flags unit/e2e/conformance/perf test issues for product features label Sep 6, 2022

psschwei added 4 commits September 6, 2022 16:17

testing cluster local as default domain

d72ec64

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

update default domain on tests

43dcb87

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

missed a few

5e96ae5

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

add domain for e2e tests

ff3ffe8

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

psschwei force-pushed the default-domain-test branch from ddacd8b to ff3ffe8 Compare September 6, 2022 20:17

psschwei changed the title ~~[WIP] Use svc.cluster.local as default domain~~ Use svc.cluster.local as default domain Sep 6, 2022

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2022

nak3 reviewed Sep 7, 2022

View reviewed changes

review fixes

13bed11

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 7, 2022

knative-prow bot assigned nak3 Sep 8, 2022

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2022

nader-ziada reviewed Sep 8, 2022

View reviewed changes

knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2022

knative-prow bot merged commit f9a75d7 into knative:main Oct 14, 2022

psschwei mentioned this pull request Oct 17, 2022

Fix default ksvc url knative-extensions/container-freezer#188

Merged

psschwei mentioned this pull request Jan 30, 2023

domain name "cluster.local" doesn't work with "kourier" but works with "kourier-internal" knative-extensions/net-kourier#984

Closed

yuzisun mentioned this pull request Feb 4, 2023

404 Not Found doing First InferenceService example with Knative 1.8 kserve/kserve#2596

Closed

psschwei mentioned this pull request Mar 24, 2023

Knative Services are private by default since v1.8 knative/docs#5509

Closed

dprotaso mentioned this pull request May 6, 2023

Route's k8s Service External Name is self-referential #13954

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use svc.cluster.local as default domain #13259

Use svc.cluster.local as default domain #13259

psschwei commented Aug 29, 2022 •

edited

codecov bot commented Sep 6, 2022 •

edited

psschwei commented Sep 6, 2022

nak3 commented Sep 7, 2022

nak3 Sep 7, 2022

psschwei Sep 7, 2022

knative-prow bot commented Sep 7, 2022

nak3 commented Sep 8, 2022

nader-ziada left a comment

nader-ziada Sep 8, 2022

nak3 commented Sep 11, 2022

nader-ziada commented Sep 11, 2022

nader-ziada commented Sep 14, 2022 •

edited

psschwei commented Sep 14, 2022 •

edited

psschwei commented Oct 12, 2022

psschwei commented Oct 14, 2022

Use svc.cluster.local as default domain #13259

Use svc.cluster.local as default domain #13259

Conversation

psschwei commented Aug 29, 2022 • edited

codecov bot commented Sep 6, 2022 • edited

Codecov Report

psschwei commented Sep 6, 2022

nak3 commented Sep 7, 2022

nak3 Sep 7, 2022

Choose a reason for hiding this comment

psschwei Sep 7, 2022

Choose a reason for hiding this comment

knative-prow bot commented Sep 7, 2022

nak3 commented Sep 8, 2022

nader-ziada left a comment

Choose a reason for hiding this comment

nader-ziada Sep 8, 2022

Choose a reason for hiding this comment

nak3 commented Sep 11, 2022

nader-ziada commented Sep 11, 2022

nader-ziada commented Sep 14, 2022 • edited

psschwei commented Sep 14, 2022 • edited

psschwei commented Oct 12, 2022

psschwei commented Oct 14, 2022

psschwei commented Aug 29, 2022 •

edited

codecov bot commented Sep 6, 2022 •

edited

nader-ziada commented Sep 14, 2022 •

edited

psschwei commented Sep 14, 2022 •

edited