cluster-only service's URL might not be right

[duglin]

In what area(s)?

/area networking

What version of Knative?

0.6.0

Issue

I created an internal only service via:

apiVersion: serving.knative.dev/v1alpha1
kind: Service
metadata:
  name: echo-internal
  labels:
    serving.knative.dev/visibility: cluster-local
spec:
  runLatest:
    configuration:
      revisionTemplate:
        spec:
          container:
            image: duglin/echo

I also modified by domainTemplate to use - instead of . between the service name and the domain name.

When I look at the status section of this service I see this (removed the noise):

status:
  address:
    hostname: echo-internal.default.svc.cluster.local
    url: http://echo-internal.default.svc.cluster.local
  domain: echo-internal-default.svc.cluster.local
  domainInternal: echo-internal.default.svc.cluster.local
  url: http://echo-internal-default.svc.cluster.local

First, some questions:

• why are there so many fields that appear to be duplicates? E.g. do we need address.hostname and domainInternal ?
• url appears to be new for v0.6 - why is it needed? It seems a bit odd to assume that http is the preferred protocol. Why not just have the hostname and let the user decide if they want to use http or https?
• can we be more consistent on the use of "domain" vs "hostname" ? Personally, I prefer "hostname" but either way we should be consistent - not just here but then in other places like our configMaps (e.g. domainTemplate).

The issue:

• notice domain: echo-internal-default.svc.cluster.local and url: http://echo-internal-default.svc.cluster.local both use - in the url so I'm assuming these are meant to be externally facing URLs, however this is an internal only service.
• should these fields be empty?

/cc @ZhiminXiang @tcnghia

[markusthoemmes]

• Both domain and domainInternal are deprecated. I think you can just disregard them at this point. We cannot simply delete them though, for backwards compatibility.
• address implements the Addressable interface to provide interop with for example Knative Eventing.
• The URL contains the scheme to signify which scheme should be used to talk to the service. Not every service might support both HTTP and HTTPS. Services which have a provisioned cert for example can be assumed to support HTTPS.

[duglin]

@markusthoemmes thanks. Why do we need to include a scheme at all since it might not even support http?

[markusthoemmes]

@duglin We require to support HTTP, don't we (today at least)? It makes it far easier for a client to decide which URL to hit.

[duglin]

Not sure - I'm assuming someone can setup their ingress to only accept https which means the http URL would be incorrect. Would be odd to mandate an insecure connection.

[markusthoemmes]

You're talking about a non-knative managed ingress? That's certainly true, yes. I'll defer to @ZhiminXiang and @tcnghia who are far more knowledgeable on the HTTPS terrain than I am.

[mattmoor]

This seems like a bug in the application of the domain template. @duglin do you want to send a PR?

[duglin]

need some guidance from @ZhiminXiang @tcnghia first on how things should look

[duglin]

@ZhiminXiang @tcnghia WDYT?

[ZhiminXiang]

The echo-internal-default.svc.cluster.local in domain and URL field is wrong as it cannot be resolved by k8s DNS server. As a result, it is not reachable within cluster.

For private Knative service, we should not apply DomainTemplate because DomainTemplate is for external host/domain.
We can either discard domain and URL fields for private Knative service or apply the default format {Name}.{Namespace}.svc.cluster.local to them.

[mattmoor]

We should just elide the DomainTemplate for cluster-local services. @duglin I'm going to assign to you in 0.8.

/assign @duglin