feat: Filter on istio gateways based on ingress annotations

[pastequo]

Changes

Add capability to restrict the list of istio gateways that will expose a knative service

• Introduce new config to attach constant, named exposition, to istio gateways
• The controller looks for a dedicated annotation istio.knative.dev/exposition on knative Ingress
• If the annotation is absent, there is no change of behavior
• If the annotation is present, only the gateways that have the annotation value (aka exposition value) are used
• exposition filters gateways that will be used for the given visibility

History

• Change annotation networking.knative.dev/istio-exposition -> istio.knative.dev/exposition
• Add a check to validate config consistency. If an exposition references an unknown gateway, an error is returned
• Clarify that this notion doesn't allow an user to expand the gateway selection outside of the visibility scope
• Add clarification about a default behavior in notes

Example

New configuration example

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  gateway.ns1.gtw1: "ing1.istio-system.svc.cluster.local"
  gateway.ns2.gtw2: "ing2.istio-system.svc.cluster.local"
  local-gateway.ns3.gtw3: "ing3.istio-system.svc.cluster.local"
  exposition.ns1.gtw1: "expo1"
  exposition.ns2.gtw2: "expo2,expo3"
  exposition.ns3.gtw3: "expo3"

Which leads to the following according to the annotation value

Knative annotation	visibility	gateways used	local gateways used
istio.knative.dev/exposition=expo1	external	ns1.gtw1	(empty)
istio.knative.dev/exposition=expo2	external	ns2.gtw2	(empty)
istio.knative.dev/exposition=expo3	external	ns3.gtw3	ns3.gtw3
istio.knative.dev/exposition=expo1,expo2	external	ns1.gtw1, ns2.gtw2	(empty)
istio.knative.dev/exposition=expo-unknown	external	(empty)	(empty)
(empty)	external	ns1.gtw1,ns2.gtw2	ns3.gtw3
istio.knative.dev/exposition=expo1	cluster-local	(empty*)	(empty)
istio.knative.dev/exposition=expo2	cluster-local	(empty*)	(empty)
istio.knative.dev/exposition=expo3	cluster-local	(empty*)	ns3.gtw3
istio.knative.dev/exposition=expo1,expo2	cluster-local	(empty*)	(empty)
istio.knative.dev/exposition=expo-unknown	cluster-local	(empty*)	(empty)
(empty)	cluster-local	(empty*)	ns3.gtw3

(empty*) in the code, for backward compatibility, the list could not be empty -> https://github.com/knative-extensions/net-istio/blob/release-1.12/pkg/reconciler/ingress/ingress.go#L180-L185

Notes

• The code checks that the config is consistent.
  If an exposition references an unknown gateway, an error is returned. In the example below, exposition.ns-unknown.gtw-unknown is problematic

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  gateway.ns1.gtw1: "ing1.istio-system.svc.cluster.local"
  local-gateway.ns2.gtw2: "ing2.istio-system.svc.cluster.local"
  exposition.ns-unknown.gtw-unknown: "expo1"

• The default behavior describes here https://github.com/knative-extensions/net-istio/blob/release-1.12/pkg/reconciler/ingress/ingress.go#L180-L185 also applies visibility filter

/kind enhancement

Fixes #1124

Release Note

Docs

---

Initial Version

Changes

• Introduce 2 new annotations on Ingress: serving.knative.dev/istio-gateways & serving.knative.dev/istio-local-gateways
• If an annotation is set, net-istio won't use the full list of gateway configured in the config. An intersection of the 2 lists will be used.
• If TLS is specified, the custom gateway that are created specifically for wildcard also refer to istio ingress gateway filtered by the annotation

Example

2 ingress gateway have been created while installing istio, in istio-system namespace

• istio-private-ingressgateway: This ingress is exposed on a private network
• istio-public-ingressgateway: This ingress is exposed on internet

The split public/private is purely informative, any gateway topology is supported

Both gateways are defined in net-istio config-istio ConfigMap

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  gateway.knative-serving.knative-private-ingress-gateway: "istio-private-ingressgateway.istio-system.svc.cluster.local"
  gateway.knative-serving.knative-public-ingress-gateway: "istio-public-ingressgateway.istio-system.svc.cluster.local"
  local-gateway....

While creating a knative service, the user can add an annotation to specify on which gateway the service should be exposed. Local gateways can also be filtered (not illustrated on this example)

• Only on private network:

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: my-private-service
  annotations:
    serving.knative.dev/istio-gateways: knative-serving/knative-private-ingress-gateway

will result for the creation of the virtual service my-private-service-ingress:

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: my-private-service-ingress
spec:
  gateways:
  - knative-serving/knative-private-ingress-gateway
  - knative-serving/knative-local-gateway # whatever the name it has
...

• Only on internet:

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: my-public-service
  annotations:
    serving.knative.dev/istio-gateways: knative-serving/knative-public-ingress-gateway

will result for the creation of the virtual service my-public-service-ingress:

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: my-public-service-ingress
spec:
  gateways:
  - knative-serving/knative-public-ingress-gateway
  - knative-serving/knative-local-gateway # whatever the name it has
...

• On both:

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: my-service
  annotations:
    serving.knative.dev/istio-gateways: knative-serving/knative-public-ingress-gateway,knative-serving/knative-private-ingress-gateway

or

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: my-service

will result for the creation of the virtual service my-service-ingress:

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: my-service-ingress
spec:
  gateways:
  - knative-serving/knative-public-ingress-gateway
  - knative-serving/knative-private-ingress-gateway
  - knative-serving/knative-local-gateway # whatever the name it has
...

Notes

• Intersection versus Directly use the list from the user annotation. In term of negative impact:
  • intersection: if the user sets an undefined gateway, the value will disappear silently
  • non-intersection: the annotation values still have to be define in the configuration to create the gateways. The user could have pre-created the gateways but net-istio is "dupplicating" the gateways for TLS. That would fail, the gateway wouldn't have any selector
• If the user doesn't set any valid value, the code would try to create a virtual service without any gateway. The virtual service is not created at all. That could be mitigated if local-gateway can't be filtered
• This design relies on the fact that annotation on knative service cr will be copy to knative ingress cr
• Should the code log some message if a gateway from an annotation had been ignored ? Should it raise an error instead ?

/kind enhancement

Fixes #1124

Release Note

Docs

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: pastequo (1abf1c2, b1a4572, dec0fe2, 0860276, 8d4cbaa, 47a656a, 5cca43e, 6784431, 967a506, 9d29f44, 4c52811)

[knative-prow]

Welcome @pastequo! It looks like this is your first PR to knative-sandbox/net-istio 🎉

[knative-prow]

Hi @pastequo. Thanks for your PR.

I'm waiting for a knative-sandbox 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.

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.

[KauzClay]

hey @pastequo , thanks for this contribution!

• Intersection versus Directly use the list from the user annotation. In term of negative impact:
  • intersection: if the user sets an undefined gateway, the value will disappear silently
  • non-intersection: the annotation values still have to be define in the configuration to create the gateways. The user could have pre-created the gateways but net-istio is "dupplicating" the gateways for TLS. That would fail, the gateway wouldn't have any selector

I'm not sure I'm following exactly, could you elaborate a bit more on the difference between intersection and non-intersection?

In both cases, it seems like it could only use whatever was defined in the configmap. You wouldn't want to have it reconciled by a gateway that knative didn't know about, right? Sorry if I'm misinterpreting things.

• If the user doesn't set any valid value, the code would try to create a virtual service without any gateway. The virtual service is not created at all. That could be mitigated if local-gateway can't be filtered

I may be misunderstanding, but my instinct would be to fall back to the current default behavior of applying every gateway that is listed in the configmap. That way it wouldn't really change anything for people not using this annotation/use case.

Should the code log some message if a gateway from an annotation had been ignored ? Should it raise an error instead ?

Maybe we could update the status on the Kingress with a condition explaining this? Though idk if this is too ingress-specific to make its way up into Knative core stuff?

---

Also, one of things I was thinking about with this feature is that, if you use different gateways, I think you also need to consider what domains are being used. I think in @rblaine95's case, they wanted to use the same domain for both gateways, so it is less of a problem.

But could there be a scenario where DNS is set up to map one domain to gateway A and another to gateway B? Then you'd need to make sure the Ksvc with the gateway A annotation has the right labels so that the domain it is given is correct.

In this case, I'm thinking this might be a hard error to discover. Idk if there would be a way to make net-istio warn you that the domain on a ksvc isn't served by the gateway it is tied to, since that info kinda lies outside of Knative.

[pastequo]

Thanks for your feedbacks !

Check user annotation

non-intersection vs intersection was not very clear, let me rephrase it:
Should the code check that the gateways set in annotations are defined in config-istio configmap ? If yes, what to do in case of an invalid value ? (see next section)

The TLS part of the code required gateways to be defined in the config-istio configmap, the non-TLS part doesn't

Invalid value management

Let's enumerate the different possibilities and merge into it the "logging question".

A value in the annotations can be invalid for 2 reasons:

• it's not well formatted (the code is expected a qualified name in the form of $namespace/$gateway-name)
• config-istio configmap doesn't reference this gateway

I'm assuming the code should respond the same way if a value is invalid, whatever the reason.

From my understanding, when reconciling an ingress, the code is merging the "gateways" with the "local-gateways" and put the result in the virtual service named $serviceName-ingress. For the TLS specific path, only the "gateways" are considered.

All values are valid (well-formatted & present in config-istio configmap)

Happy path, just apply the config

There is no annotation

Still happy path, default to the list in the config-istio configmap

All values are invalid (either not well-formatted or not present in config-istio)

It either means:

• both "local" and "non-local" annotations only contain invalid values
• "non-local" annotation only contain invalid values & TLS is activated

What is the expected behavior:

• Should the reconciliation fail ?
• Should it default to the list in the config-istio configmap ? (As a user, I would find this behavior very surprising)
• Should the virtual services be still created with invalid values ?
• How to surface the error(s) ?

Some values are invalid, some are valid

• Should the reconciliation fail ?
• Should the configuration be partly applied (with only the right value) ?
• Should it default to the list in the config-istio configmap ? (As a user, I would find this behavior very surprising)
• • Should the virtual services be still created with invalid values ?
• How to surface the error(s) ?

Local gateways

I definitely lack knowledge around the concept of "local-gateways", I don't see what is their purpose. So I don't know if they should be "select-able" as well with annotations. Strictly speaking about the original issue(s), I don't think it's required

DNS consideration

Regarding the DNS part, my first instinct would be to say it's out of scope.

In the config-domain configmap, something like that could be defined

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-domain
  namespace: knative-serving
data:
  my-public-domain.com: |
    selector:
      traffic: public
  my-private-domain.com: |
    selector:
     traffic: private

Then I think it's up to the end users to ensure they are creating something consistent, not like below

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: my-badly-configured-service
  annotations:
    serving.knative.dev/istio-gateways: knative-serving/knative-public-ingress-gateway # public here
  labels:
    traffic: private # private here
spec:

But it could be a nice information to surface. What could be done is to get the "hosts" value in the gateways spec and compared it (assuming the hosts of the gateway can contain wildcard) to the "hosts" field in the ingress spec (for rules with externalIP visibility only ?)

Edit: but if the gateway host is simply a wildcard, the check wouldn't do much

[pastequo]

I pushed a new commit that handles error differently, in a more protective way. Regarding my previous post:

• All values are invalid (either not well-formatted or not present in config-istio): -> reconciliation fails
• Some values are invalid, some are valid: -> reconciliation fails

So the actual error is surfaced by the logs (the status of the ingress resource being generic)

[dprotaso]

/ok-to-test

[codecov]

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (7d929a6) 81.66% compared to head (4c52811) 81.79%.
Report is 18 commits behind head on main.

Files	Patch %	Lines
pkg/reconciler/ingress/resources/gateway.go	80.89%	15 Missing and 2 partials ⚠️
pkg/reconciler/ingress/config/istio.go	96.29%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1137      +/-   ##
==========================================
+ Coverage   81.66%   81.79%   +0.13%     
==========================================
  Files          19       19              
  Lines        1680     1780     +100     
==========================================
+ Hits         1372     1456      +84     
- Misses        220      234      +14     
- Partials       88       90       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dprotaso]

Thanks for the PR - I appreciate you tackling this use case.

I don't think annotations is the right mechanic since it gives too much control to users and I wonder if we should be cautious here. I don't know whether letting users run services that are 'unexpectedly' accessible in intranet networks is a risk. cc @nak3 @skonto for perspective.

Thus I think what I would consider a 'safer' mechanic is for the operator to perform this setup/configuration - potentially in our net-istio config map.

Though I'm mixed on what the operator mechanic could be - using a label selector would still allow users to control what gateways knative services attach too.

Maybe it's better for operators to map certain namespaces to gateways.

[nak3]

/cc @ReToCode 🙏

[pastequo]

I would argue that as-of-today the default behavior is to expose the service to all gateways defined in the configmap. Therefore this PR can be viewed as a way to define where you don't want to expose the service, meaning it filtered what the admin defined, not overwrite.

I understand your point about

I don't know whether letting users run services that are 'unexpectedly' accessible in intranet networks is a risk

But letting users run services that are 'unexpectedly' accessible from internet (and more generally from any unexpected networks) is a bigger risk imho. I think the right approach here is to not split by "public/private" but generalize to "different networks" and let the admin, thru the ingress gateway definition, put any meaning on them. Then I think it's up to the service owner to know & specify from which network their service should be accessible

Regarding the namespaces approach, I, as a service owner, am personally used to have the flexibility to define how to split my namespace. which is convenient for RBAC, networking policies, etc. If I was asked to use a static list of namespace, that would probably imply more problem.

Your proposal didn't state it has to be static but if the list is dynamic (for example deploy on namespace network1-* to expose to network1) than the proposal is equivalent to the label/annotation one, with an additional layer of complexity. Instead of choosing a annotation/label, the user is choosing a namespace

Just my 2 cents.

[dprotaso]

I think the gateway api sets a good precendent here (long term we'll want to consolidate around it)

They make this an operator concern and the gateway resource defines which routes can attach to itself

https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.AllowedRoutes

Thus the operator specifies which namespaces are allowed to use certain gateways and then the user can specify the gateways that it wants

[pastequo]

Ok, if I understand you correctly, there are 2 different functionalities to consider:

• User should be able to specify which gateway they want to use (as long as they are defined in the config)
• Operator should be able to restrict gateway access (probably to certain namespace)

The current status is that all gateways are always used. This PR allows to achieve the 1st functionality without, I think, putting constraint on the 2nd functionality.

Is there a reason to achieve the 2 functionalities simultaneously? And even in the same PR?

[dprotaso]

Following up from our discussion in the Serving WG group.

We identified this problem sorta fits into the visibility feature we have for our services. This is where a user can specify using an annotation a visibility for their Knative Service - eg. networking.knative.dev/visibility=cluster-local

Currently we associate cluster-local visibility to gateways with the local-gateway. prefix in the config-istio configmap. Similarly, external Gateways with the gateway. prefix.

User should be able to specify which gateway they want to use (as long as they are defined in the config)

I think we shouldn't expose more to the user than what's necessary - it's the wrong level of abstraction for the user.

Hence I think a mechanic for the operator to define a new visibility in config-istio and then users can use this higher level constant.

A rough idea could be

gateway.knative-serving.knative-ingress-gateway: "istio-ingressgateway.istio-system.svc.cluster.local"
gateway-visibility.vpc.knative-serving.knative-ingress-gateway: "istio-ingressgateway.istio-system.svc.cluster.local"
gateway-visibility.vpc.knative-serving.knative-vpc-gateway: "istio-ingressgateway.istio-system.svc.cluster.local"

Thus if the user specifies the annotationnetworking.knative.dev/visibility=vpc - it will look up and attach the virtual service to Istio Gateways knative-ingress-gateway & knative-vpc-gateway

Thus the template is

gateway-visibility.{visibility}.{gateway-ns}.{gateway-name}: "{target-service-for-probing}"

Operator should be able to restrict gateway access (probably to certain namespace)

I think you're right about me introducing an additional constraint. We can always add this later.

cc @nak3 @ReToCode @skonto for thoughts/ideas on alternative approaches?

[nak3]

/test latest-mesh

[dprotaso]

/hold

release is tomorrow - will unhold afterwards

[nak3]

Hi @pastequo I just tried this to understand the implementation but the status becomes meshOnly: true. I think my configuration somehow is wrong but is it possible to add an e2e test under test/e2e/? Then, I think I understand what was wrong.

$ kubectl get king  -o yaml hello-example
  ...
  status:
    conditions:
   ...
    privateLoadBalancer:
      ingress:
      - meshOnly: true
    publicLoadBalancer:
      ingress:
      - meshOnly: true

[pastequo]

Hi @pastequo I just tried this to understand the implementation but the status becomes meshOnly: true. I think my configuration somehow is wrong but is it possible to add an e2e test under test/e2e/? Then, I think I understand what was wrong.

$ kubectl get king  -o yaml hello-example
  ...
  status:
    conditions:
   ...
    privateLoadBalancer:
      ingress:
      - meshOnly: true
    publicLoadBalancer:
      ingress:
      - meshOnly: true

I don't reproduce locally what you observed

• I changed the annotation name
• I made NewIstioFromConfigMap return an error if the configuration is not consistent
• I updated the PR description accordingly
• I added a e2e test. Let me know if it's done the right way. We will see how the pipeline behaves...

(On a side note, it wasn't easy to setup a local environment to replay e2e... It might be worth documenting how you proceed. I saw some weird things too that might require clean up. For example github workflows references CONFIG_ISTIO="./third_party/istio-${{ matrix.istio-version }}/istio-kind-${{ matrix.istio-profile }}/config-istio.yaml" but the files are not named config-istio.yaml but istio.yaml. If I'm correct there isn't any config-istio.yaml )

[nak3]

Thank you. I understand that the MeshOnly: true happens when no LB matched. Is it possible to set MarkLoadBalancerNotReady instead of MeshOnly: true when there is no matched LB?

[pastequo]

Wouldn't it be more confusing ? By reading MarkLoadBalancerNotReady I expect to have a LB that will eventually become ready

[pastequo]

/retest

[nak3]

Wouldn't it be more confusing ? By reading MarkLoadBalancerNotReady I expect to have a LB that will eventually become ready

Yes, and users will take a look at their LB (=Istio Gateway) and realize that the LB does not exist due to their wrong configuration, won't they?

By the way, I realized that the designe has some problems:

(copied from #1137 (comment))

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  gateway.ns1.gtw1: "ing1.istio-system.svc.cluster.local"
  gateway.ns2.gtw2: "ing2.istio-system.svc.cluster.local"
  ...

You assume that ns1.gtw1 & ns2.gtw2 are used when annotation is empty. But serving's prober does not check if both are available. So, multiple gateways for one ksvc should have a problem.
So I think we still need a template gateway-visibility.{visibility}.{gateway-ns}.{gateway-name}: "{target-service-for-probing}" to support.

[pastequo]

• Regarding the MarkLoadBalancerNotReady status, I think the first question to answer is to agree if it is valid or not if there is no gateway (local or not) for a given visibility. If not then we need a mechanism to surface the error. If it is valid, then I think there is no need to mark the status as MarkLoadBalancerNotReady since all the expected LBs are ready (in this case 0, only the mesh is configured)

BTW, there is a subtlety between the istio ingress gateways and the istio gateways. The LBs (exposing Istio ingress gateway, aka envoy) are ready and healthy. But none of them are used because there is no istio gateways (a gathering of configuration) for this ksvc. So it could be confusing for the user, because he shouldn't look at the LB nor the istio ingress gateways

• Regarding the probes, after a quick look at the code, I think all the endpoints are checked, but only the first one is reported. I think this issue is independent from my PR. As of today - without this PR - I can configure multiple gateways in my configuration. The capability to filter on those gateways shouldn't influence the probes. It might be better to handle the 2 topics separately

[ReToCode]

I still think it is not the best idea to mix the current visibility with the new gateway selection (#1137 (comment)). Similarly, this goes the same direction.

We do have to types of gateways (external and cluster-local). We can have multiple gateways for each type and with this new feature we want to limit the selection to a subset of those. But it should not be possible to expand the gateway selection (a cluster-local domain should not be allowed to target a external visible gateway). With the current design, that is (or seams) possible.

[pastequo]

Thanks for the reply @ReToCode.

I can confirm you that I took into account your remarks: the 2 notions (visibility & exposition) are not mixed.
What I designed and hopefully I implemented is to limit the list of gateway per visibility without changing the logic on top of that that choses which list of gateway to use (based on visibility)

I will edit the design to state it clearly (done)

[dprotaso]

Funny enough @nak3's issue (#718) made me realize there's a lot more work to support this feature properly - so that's definitely something we'd want to address.

Secondly, I wonder if it would make sense to take inspiration from config-domain changes that are likely to land - knative/serving#14543 Since it seems there's still a bit of contention on how to structure config-istio.

What I like about that Serving PR is that it supports a new config format and the old one. Given that it might be best rethink what we want config-istio to look like instead of trying to shoe horn it into the existing format.

eg. we could have

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  internal-gateways: 
    - service-namespace: istio-system
      service-name: knative-cluster-local
  external-gateways: 
    - service-namespace: istio-system
      service-name:  ing2
    - service-namespace: istio-system
      service-name:  ing1      

and then introduce the concept of selectors for services and even namespace selectors

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-istio
  namespace: knative-serving
data:
  internal-gateways: |
    - namespace: istio-system
      name: knative-cluster-local
  external-gateways: |
    - namespace: istio-system
      name:  ing2
      namespaceSelector: |
        kubernetes.io/metadata.name: blah
    - service-namespace: istio-system
      service-name:  ing1      
      selector: |
         app: foo      

We could even extend this to support matching gateways depending on domain so you don't have to repeat the namespace selector in two configs

Thoughts on this approach @ReToCode & @nak3 ?

@pastequo would the above give you the flexibility you require?

It might make sense to move this proposal to a shared google document rather than a PR - @pastequo can you open an issue regarding this incase you haven't done so?

[ReToCode]

@dprotaso I like that approach, it is closer to what we actually want to configure and does not mix with the visibility (as per #1137 (comment)).

So you'd add this as an additional config to keep being reverse compatible with the new structure to have precedence?

Side-note: probably we also want to rename the internal-gateways to local-gateways.

[pastequo]

Funny enough @nak3's issue (#718) made me realize there's a lot more work to support this feature properly - so that's definitely something we'd want to address.

I don't have the history you have on this project, but it seemed to me that there wasn't a lot of work for that. All the code is plugged to work with a list of gateway except for the LB Status methods that already accept an array.

Secondly, I wonder if it would make sense to take inspiration from config-domain changes that are likely to land - knative/serving#14543 Since it seems there's still a bit of contention on how to structure config-istio.

Should it block this PR ? Reworking the config structure seems independent to me. Tho I understand that might be a good opportunity.

@pastequo would the above give you the flexibility you require?

It seems so but some remarks:

• Even if the "admin" should have capabilities to restrict which services can be exposed, I believe the "user" should be the one who in the first place explicit their requirement about how to expose their service. To achieve that, I think annotation would be more appropriate than labels, as the user wants to trigger a special behavior. So the configuration should expose a way to associate a special annotation value to a internal/external-gateway.
• selector sounds as a "labels-based" approach to me, it can be kept if we want to offer to the "admin" the capability to filter by labels
• Similarly to what I did in my description, the important part is to define what is the expected behavior when no labels/annotation matches, when the user specifies nothing, etc

It might make sense to move this proposal to a shared google document rather than a PR - @pastequo can you open an issue regarding this incase you haven't done so?

I was referencing another user issue: #1124
Would it be ok for you or should I create something else specific ?

[dprotaso]

Based on the Serving WG discussion (Nov 29th) I believe we're all in agreement that we'll close out this PR and turn this into a feature track doc

The template is here
https://docs.google.com/document/d/1FvezfvBghevCRoZUmN3SoTVtm6f_u_r5f94MEa4jNIA/edit#heading=h.n8a530nnrb

We should place the document here
https://drive.google.com/drive/u/0/folders/1_pgo1bEXv-TB42qG2zCbUK-pw_720qmA

You'll get access by joining the google group - https://groups.google.com/g/knative-dev

If you have issues ping me on slack

[knative-prow-robot]

PR needs rebase.

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.

[pastequo]

Google document added

https://docs.google.com/document/d/12X1-9nhjAhpf-Wlt3RbdoMbvx31zFBva/edit#heading=h.gjdgxs