Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixing bug in name of Solender to avoid CLA troubles, is fixing … #1147

Merged
merged 2 commits into from
Jun 27, 2023
Merged

fixing bug in name of Solender to avoid CLA troubles, is fixing … #1147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7cf823
fixing bug in name of Eric Solander to avoid CLA troubles, is fixing …
gabo1208 Jun 27, 2023
8af3a2b
set defaults is taking care so the namespace does not get empty, so t…
gabo1208 Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions pkg/reconciler/trigger/trigger.go
View file
Open in desktop
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
View file
Open in desktop
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(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function https://github.com/knative-sandbox/eventing-rabbitmq/pull/1147/files#diff-c5ee2c23a8a6dd4c7aa3e6620d57f2bfd00a6b37dd072ade8bfd386dd4952d21L1406 populates a namespace. Therefor the codecoverage is not being covered in the line specified by codecov.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function sets the name space to https://github.com/knative-sandbox/eventing-rabbitmq/pull/1147/files#diff-c5ee2c23a8a6dd4c7aa3e6620d57f2bfd00a6b37dd072ade8bfd386dd4952d21R78

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