knative / eventing

Event-driven application platform for Kubernetes
https://knative.dev/docs/eventing
Apache License 2.0
1.41k stars 590 forks source link

Can't apply broker yaml twice without change when kind is lowercase #5887

Closed csantanapr closed 1 year ago

csantanapr commented 2 years ago

Describe the bug Can't apply broker yaml twice without change

Expected behavior That in a CICD and the YAML get's re-apply I don't get errors

To Reproduce Install Eventing, in-memory, and mt-channel-broker I use this script (https://github.com/csantanapr/knative-kind/blob/master/04-eventing.sh#L49) Run the script twice or apply the broker yaml twice

kubectl apply -f - <<EOF
apiVersion: eventing.knative.dev/v1
kind: broker
metadata:
 name: example-broker
 namespace: default
EOF

On the first create works, on the second one without changing anything it doesn't work

Knative release version

to: Resource: "eventing.knative.dev/v1, Resource=brokers", GroupVersionKind: "eventing.knative.dev/v1, Kind=broker" Name: "example-broker", Namespace: "default" for: "STDIN": error when creating patch with: original: {"apiVersion":"eventing.knative.dev/v1","kind":"broker","metadata":{"annotations":{},"name":"example-broker","namespace":"default"}}

modified: {"apiVersion":"eventing.knative.dev/v1","kind":"broker","metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"eventing.knative.dev/v1\",\"kind\":\"broker\",\"metadata\":{\"annotations\":{},\"name\":\"example-broker\",\"namespace\":\"default\"}}\n"},"name":"example-broker","namespace":"default"}}

current: {"apiVersion":"eventing.knative.dev/v1","kind":"Broker","metadata":{"annotations":{"eventing.knative.dev/broker.class":"MTChannelBasedBroker","eventing.knative.dev/creator":"docker-for-desktop","eventing.knative.dev/lastModifier":"docker-for-desktop","kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"eventing.knative.dev/v1\",\"kind\":\"broker\",\"metadata\":{\"annotations\":{},\"name\":\"example-broker\",\"namespace\":\"default\"}}\n"},"creationTimestamp":"2021-11-10T14:27:54Z","generation":1,"name":"example-broker","namespace":"default","resourceVersion":"16079","uid":"c6150837-53d4-41ad-835f-05ce9c14f3c4"},"spec":{"config":{"apiVersion":"v1","kind":"ConfigMap","name":"config-br-default-channel","namespace":"knative-eventing"}},"status":{"address":{"url":"http://broker-ingress.knative-eventing.svc.cluster.local/default/example-broker"},"annotations":{"knative.dev/channelAPIVersion":"messaging.knative.dev/v1","knative.dev/channelAddress":"http://example-broker-kne-trigger-kn-channel.default.svc.cluster.local","knative.dev/channelKind":"InMemoryChannel","knative.dev/channelName":"example-broker-kne-trigger"},"conditions":[{"lastTransitionTime":"2021-11-10T14:27:54Z","status":"True","type":"Addressable"},{"lastTransitionTime":"2021-11-10T14:27:54Z","message":"No dead letter sink is configured.","reason":"DeadLetterSinkNotConfigured","severity":"Info","status":"True","type":"DeadLetterSinkResolved"},{"lastTransitionTime":"2021-11-10T14:27:54Z","status":"True","type":"FilterReady"},{"lastTransitionTime":"2021-11-10T14:27:54Z","status":"True","type":"IngressReady"},{"lastTransitionTime":"2021-11-10T14:27:54Z","status":"True","type":"Ready"},{"lastTransitionTime":"2021-11-10T14:27:54Z","status":"True","type":"TriggerChannelReady"}],"observedGeneration":1}}

for: "STDIN": precondition failed for: map[kind:broker]

odacremolbap commented 2 years ago

/assign

benmoss commented 2 years ago

A weird thing about this is that it only seems to happen with kind: broker, not kind: Broker. Might reduce the urgency of the fix a little 😄

csantanapr commented 2 years ago

Thank you @odacremolbap for picking up this issue ❤️

lionelvillard commented 2 years ago

Good catch @benmoss !

@csantanapr can you reproduce using kind: Broker?

odacremolbap commented 2 years ago

@benmoss that seems to be it!

And furthermore, core objects are slightly affected by it (but it doesn't allow to create a resource with a downcased kind):

kubectl apply -f - <<EOF
kind: Service
apiVersion: v1
metadata:
  name: downcase-my-kind
spec:
  selector:
    app: nada
  ports:
    - port: 9876
EOF

kubectl apply -f - <<EOF
kind: service
apiVersion: v1
metadata:
  name: downcase-my-kind
spec:
  selector:
    app: nada
  ports:
    - port: 9876
EOF

And any CRD seems to also be affected, not just brokers (and it allows downcased kinds)

kubectl apply -f - <<EOF
apiVersion: eventing.knative.dev/v1
kind: trigger
metadata:
 name: example-trigger
spec:
 broker: example-broker
 subscriber:
  uri: http://noop
EOF

kubectl apply -f - <<EOF
apiVersion: eventing.knative.dev/v1
kind: trigger
metadata:
 name: example-trigger
spec:
 broker: example-broker
 subscriber:
  uri: http://noop
EOF

I think that core objects are being validated at kubectl, since the schema is well known and imported, and creating a resource with casing issues will be rejected, meanwhile CRDs most probably will get their kind converted to resource and any casing issue will be gone in the process at creation time.

Then when updating, the patching library must be case sensitive, hence the rejection.

I would close this issue, this is not knative/eventing related.