Defaulting for Channel should resolve the namespace from context, not the resource

[aliok]

Describe the bug
When I use Kube REST api in the way kn uses it, I can see behavior differences when I post these 2:

• {"kind":"Channel","apiVersion":"messaging.knative.dev/v1beta1","metadata":{"name":"my-channel"}}
• {"kind":"Channel","apiVersion":"messaging.knative.dev/v1beta1","metadata":{"name":"my-channel", "namespace":"knativetutorial"}}

URL posted is same: apis/messaging.knative.dev/v1beta1/namespaces/knativetutorial/channels

This problem makes eventing webhook pick the wrong default channel for namespace.

See following.

Make IMC cluster default and KafkaChannel knativetutorial namespace default:

$ cat <<EOF | kubectl apply -f -
apiVersion: v1
kind: ConfigMap
metadata:
  name: default-ch-webhook
  namespace: knative-eventing
data:
  default-ch-config: |
    clusterDefault:
      apiVersion: messaging.knative.dev/v1beta1
      kind: InMemoryChannel
    namespaceDefaults:
      knativetutorial:
        apiVersion: messaging.knative.dev/v1alpha1
        kind: KafkaChannel
        spec:
          numPartitions: 2
          replicationFactor: 1
EOF

Start proxy:

$ kubectl proxy --port=8080 &

Post 2 channel requests to the same namespace URL; 1 with namespace in metadata, other not:

$ curl -XPOST  \
    -d '{"kind":"Channel","apiVersion":"messaging.knative.dev/v1beta1","metadata":{"name":"my-channel-1", "namespace":"knativetutorial"}}' \
    -H "Content-Type: application/json" \
    'http://localhost:8080/apis/messaging.knative.dev/v1beta1/namespaces/knativetutorial/channels'

$ curl -XPOST  \
    -d '{"kind":"Channel","apiVersion":"messaging.knative.dev/v1beta1","metadata":{"name":"my-channel-2"}}' \
    -H "Content-Type: application/json" \
    'http://localhost:8080/apis/messaging.knative.dev/v1beta1/namespaces/knativetutorial/channels'

First channel is created with KafkaChannel, the 2nd created with IMC.

$ k get channels -A
NAMESPACE         NAME           URL                                                                AGE   READY   REASON
knativetutorial   my-channel-1   http://my-channel-1-kn-channel.knativetutorial.svc.cluster.local   6s    True    
knativetutorial   my-channel-2   http://my-channel-2-kn-channel.knativetutorial.svc.cluster.local   5s    True    

$ k get kafkachannels -A
NAMESPACE         NAME           READY   REASON   URL                                                                AGE
knativetutorial   my-channel-1   True             http://my-channel-1-kn-channel.knativetutorial.svc.cluster.local   15s

$ k get imc -A
NAMESPACE         NAME           URL                                                                AGE   READY   REASON
knativetutorial   my-channel-2   http://my-channel-2-kn-channel.knativetutorial.svc.cluster.local   11s   True    

Expected behavior
Defaulting to use the context namespace.

To Reproduce
Steps are above.

Knative release version
0.17, 0.18, 0.19

Additional context
Add any other context about the problem here such as proposed priority

[aliok]

This doesn't happen when you apply YAML with kubectl since kubectl embeds the namespace in the request body.

[navidshaikh]

This doesn't happen when you apply YAML with kubectl since kubectl embeds the namespace in the request body.

kn could carry this as workaround for now, it'll embed the namespace in the request body.

[aliok]

sounds good!

[aliok]

so,

• this problem doesn't happen with kubectl
• kn is adding a workaround, that should also be fine
• all the other usecases where people are creating Channels programmatically with namespace defaults: just embed the namespace in the request as a workaround

[grantr]

This might also affect broker class defaulting by namespace (if we even have that feature, I can't remember)

[slinkydeveloper]

Since the kn workaround seems like was merged, can we close this one @aliok ?

[aliok]

Let's keep this open since this is a good-first-issue? At least until the bot marks it stale?

[github-actions]

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.

[aliok]

/remove-lifecycle stale

[rhuss]

Since the kn workaround seems like was merged, can we close this one @aliok ?

I still think that the issue described here is more than just helping kn to run the e2e tests and is a general issue with other clients as well that do not add the namespace to the request (which IMO is totally legit)

[github-actions]

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

[gab-satchi]

/assign

[vishal-chdhry]

/assign