Skip to content

Commit

Permalink
[release-1.9] fixing bug in name of Solender to avoid CLA troubles, i…
Browse files Browse the repository at this point in the history
…s fixing … (#1181)

* fixing bug in name of Eric Solander to avoid CLA troubles, is fixing the triger subscriber namespace bug in case the subscriber has a different namespace from the trigger

* set defaults is taking care so the namespace does not get empty, so this should be good enough to pass

---------

Co-authored-by: Gabriel Freites <[email protected]>
  • Loading branch information
knative-prow-robot and gabo1208 authored Jul 26, 2023
1 parent 8590f9b commit 51f3e0b
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 6 deletions.
6 changes: 0 additions & 6 deletions pkg/reconciler/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,6 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, t *eventingv1.Trigger) p
logging.FromContext(ctx).Info("Reconciled rabbitmq binding", zap.Any("binding", binding))
t.Status.MarkDependencySucceeded()

if t.Spec.Subscriber.Ref != nil {
// To call URIFromDestination(dest apisv1alpha1.Destination, parent interface{}), dest.Ref must have a Namespace
// We will use the Namespace of Trigger as the Namespace of dest.Ref
t.Spec.Subscriber.Ref.Namespace = t.GetNamespace()
}

subscriberURI, err := r.uriResolver.URIFromDestinationV1(ctx, t.Spec.Subscriber, t)
if err != nil {
logging.FromContext(ctx).Error("Unable to get the Subscriber's URI", zap.Error(err))
Expand Down
77 changes: 77 additions & 0 deletions pkg/reconciler/trigger/trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import (
const (
systemNS = "knative-testing"
testNS = "test-namespace"
otherNS = "other-namespace"
brokerClass = "RabbitMQBroker"
brokerName = "test-broker"
brokerUID = "broker-test-uid"
Expand Down Expand Up @@ -449,6 +450,64 @@ func TestReconcile(t *testing.T) {
WithTriggerSubscriberResolvedSucceeded(),
WithTriggerStatusSubscriberURI(subscriberURI)),
}},
}, {
Name: fmt.Sprintf("%s: Creates everything with ref subscriber different ns", name),
Key: testKey,
Objects: []runtime.Object{
ReadyBroker(config),
makeSubscriberAddressableAsUnstructuredWithNamespace(otherNS),
markReady(createQueue(config, false, "")),
markReady(createBinding(false, false, "")),
NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(triggerUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, otherNS)),
createSecret(rabbitURL),
createRabbitMQBrokerConfig(""),
createRabbitMQCluster(""),
},
WantCreates: []runtime.Object{
createDispatcherDeployment(false, ""),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(triggerUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, otherNS),
WithTriggerBrokerReady(),
WithTriggerDeadLetterSinkNotConfigured(),
WithTriggerDependencyReady(),
WithTriggerSubscribed(),
WithTriggerSubscriberResolvedSucceeded(),
WithTriggerStatusSubscriberURI(subscriberURI)),
}},
}, {
Name: fmt.Sprintf("%s: Creates everything with ref subscriber no ns provided", name),
Key: testKey,
Objects: []runtime.Object{
ReadyBroker(config),
makeSubscriberAddressableAsUnstructured(),
markReady(createQueue(config, false, "")),
markReady(createBinding(false, false, "")),
NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(triggerUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, "")),
createSecret(rabbitURL),
createRabbitMQBrokerConfig(""),
createRabbitMQCluster(""),
},
WantCreates: []runtime.Object{
createDispatcherDeployment(false, ""),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(triggerUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, testNS),
WithTriggerBrokerReady(),
WithTriggerDeadLetterSinkNotConfigured(),
WithTriggerDependencyReady(),
WithTriggerSubscribed(),
WithTriggerSubscriberResolvedSucceeded(),
WithTriggerStatusSubscriberURI(subscriberURI)),
}},
}, {
Name: fmt.Sprintf("%s: Fails to resolve ref", name),
Key: testKey,
Expand Down Expand Up @@ -1397,6 +1456,24 @@ func makeSubscriberAddressableAsUnstructured() *unstructured.Unstructured {
}
}

func makeSubscriberAddressableAsUnstructuredWithNamespace(ns string) *unstructured.Unstructured {
return &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": subscriberAPIVersion,
"kind": subscriberKind,
"metadata": map[string]interface{}{
"namespace": ns,
"name": subscriberName,
},
"status": map[string]interface{}{
"address": map[string]interface{}{
"url": subscriberURI,
},
},
},
}
}

func makeSubscriberNotAddressableAsUnstructured() *unstructured.Unstructured {
return &unstructured.Unstructured{
Object: map[string]interface{}{
Expand Down

0 comments on commit 51f3e0b

Please sign in to comment.