Improve --sink by adding support for svc kind and other addresables

[csantanapr]

Feature request

Ability to use any addressable resource for the --sink flag (ie Broker, Service, etc..)

Use case

While fixing an issue in the sample pingsource docs https://github.com/knative/docs/pull/3217

I cant use a short version of --sink to specify a normal K8s service, not a ksvc

In this case event-display is not a knative service, just a normal Kind: Service

A Service is addresable,

Using --sink service:event-display

❯ kn source ping create test-ping-source \
  --namespace pingsource-example \
  --schedule "*/1 * * * *" \
  --data '{"message": "Hello world!"}' \
  --sink service:event-display
Error: unsupported sink prefix: 'service', please use prefix 'ksvc' for knative service
Run 'kn --help' for usage

Using --sink svc:event-display

❯ kn source ping create test-ping-source \
  --namespace pingsource-example \
  --schedule "*/1 * * * *" \
  --data '{"message": "Hello world!"}' \
  --sink svc:event-display

[rhuss]

A bit of history context:

In previous versions of kn we mapped a svc: prefix also to a Knative service, but realised that this can be confused with a regular K8s service. Therefor we freed up that slot for svc: for a K8s service, but decided to not include the mapping by default into kn as a K8s service is not part of the Knative API contract. Instead we suggest to add such a mapping to .config/kn/config.yaml where you can configure arbitrary mappings for prefixes to GKV coordinates of addressables.

If the community and users feel that we can add references to k8s services as part of the Knative client, then we are happy to do so. We should not be dogmatic, but still always remind ourselves about the Knative API boundaries, which are indeed a bit fuzzy (see the discussion around whether ConfigMaps/Secrets or even Namespaces are part of the Knative API as these entities are referenced from Knative CO).

For now and as a workaround you can always use the URL of the Addressable that you want to use as a sink (but that you know already).

Apologies that the appropriate documentation is still missing.

[vaikas]

This is neat:

Instead we suggest to add such a mapping to .config/kn/config.yaml where you can configure arbitrary mappings for prefixes to GKV coordinates of addressables.

Is there a default map that we ship with? I took a quick look at this repo but didn't find one. Perhaps we could create a default and if it doesn't fit the folks desires, they can mod as they see fit, but wouldn't have to add them from scratch?

[rhuss]

No, we don't initialize the configuration file initially, but this is a good idea anyway (also as a starting point to find out what can be customized). One caveat though is that using a configuration file that would need to be customised doesn't help much in a CI/CD context when kn is served from within a container.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[rhuss]

/remove-lifecycle stale

[rhuss]

This is related to #1232 and let's track it over there. I can still see a value for having a plain k8s service to be used, and if I look in the Knative code, there is also special handling for k8s service to make it look like an Addressable (which it is not).

So, should we add a svc: or service: prefix for a Kubernetes service ? although its not strictly part of the Knative API ? // cc @knative/serving-writers , see https://github.com/knative/client/issues/1222#issuecomment-779091461 for more context.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[dprotaso]

This is related to #1232 and let's track it over there. I can still see a value for having a plain k8s service to be used, and if I look in the Knative code, there is also special handling for k8s service to make it look like an Addressable (which it is not).

So, should we add a svc: or service: prefix for a Kubernetes service ? although its not strictly part of the Knative API ? // cc @knative/serving-writers , see #1222 (comment) for more context.

Both are supported by kubectl

[rhuss]

Both are supported by kubectl

Not sure I got your comment @dprotaso , wdym with supported by kubectl ? Sure, but the question was whether we want to add support for a K8s specific resource to kn or not ? E.g would this work with Google Cloud Run, too ? I still feel that the boundary, what constitutes a pure Knative API (and can be used by kn directly) and what is Kubernetes land (for which we have to introduce a plugin, like kn-service-log) quite blurry.

Tbh, I would be pragmatic here and directly introduce service: and svc: to let point to a K8s service (i.e. use this GAV for the sink reference)

[dprotaso]

sorry I misread - my initial comment was referring to

So, should we add a svc: or service: prefix for a Kubernetes service ?

These are both supported by kubectl as svc is the short name for the full service

I realize you were cc'ing me for

although its not strictly part of the Knative API ? // cc @knative/serving-writers

Technically CloudRun doesn't support eventing - so 🤷

[csantanapr]

My two cents from UX point of view. just support "svc" and "ksvc" this way is easy to remember there are 2 different ones.

Adding "service" bad idea as I will always question which one is referring since the kn CLI a "kn service" is ksvc

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[cardil]

/remove-lifecycle stale /reopen /assign /triage accepted

[knative-prow[bot]]

@cardil: Reopened this issue.

In response to this:

/remove-lifecycle stale /reopen /assign /triage accepted

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-sigs/prow repository.