Skip to content

Commit

Permalink
Check if spec.retry is nil before dereferencing it. (google#2052)
Browse files Browse the repository at this point in the history
  • Loading branch information
Harwayne authored and tommyreddad committed Feb 12, 2021
1 parent eae120d commit 3e547b0
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 3 deletions.
9 changes: 6 additions & 3 deletions pkg/reconciler/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,13 @@ func getPubsubDeadLetterPolicy(projectID string, spec *eventingduckv1beta1.Deliv
return nil
}
// Translate to the pubsub dead letter policy format.
return &pubsub.DeadLetterPolicy{
MaxDeliveryAttempts: int(*spec.Retry),
DeadLetterTopic: fmt.Sprintf("projects/%s/topics/%s", projectID, spec.DeadLetterSink.URI.Host),
dlp := &pubsub.DeadLetterPolicy{
DeadLetterTopic: fmt.Sprintf("projects/%s/topics/%s", projectID, spec.DeadLetterSink.URI.Host),
}
if spec.Retry != nil {
dlp.MaxDeliveryAttempts = int(*spec.Retry)
}
return dlp
}

func (r *Reconciler) deleteRetryTopicAndSubscription(ctx context.Context, trig *brokerv1beta1.Trigger) error {
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 @@ -98,6 +98,16 @@ var (
},
},
}
brokerDeliverySpecWithoutRetry = &eventingduckv1beta1.DeliverySpec{
BackoffDelay: &backoffDelay,
BackoffPolicy: &backoffPolicy,
DeadLetterSink: &duckv1.Destination{
URI: &apis.URL{
Scheme: "pubsub",
Host: deadLetterTopicID,
},
},
}
)

func init() {
Expand Down Expand Up @@ -467,6 +477,73 @@ func TestAllCasesTrigger(t *testing.T) {
}),
},
},
{
Name: "Check topic config and labels - broker without spec.delivery.retry",
Key: testKey,
Objects: []runtime.Object{
NewBroker(brokerName, testNS,
WithBrokerClass(brokerv1beta1.BrokerClass),
WithInitBrokerConditions,
WithBrokerReady("url"),
WithBrokerDeliverySpec(brokerDeliverySpecWithoutRetry),
WithBrokerSetDefaults,
),
makeSubscriberAddressableAsUnstructured(),
NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(testUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, testNS),
WithTriggerSetDefaults),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: NewTrigger(triggerName, testNS, brokerName,
WithTriggerUID(testUID),
WithTriggerSubscriberRef(subscriberGVK, subscriberName, testNS),
WithTriggerBrokerReady,
WithTriggerSubscriptionReady,
WithTriggerTopicReady,
WithTriggerDependencyReady,
WithTriggerSubscriberResolvedSucceeded,
WithTriggerStatusSubscriberURI(subscriberURI),
WithTriggerSetDefaults,
),
}},
WantEvents: []string{
triggerFinalizerUpdatedEvent,
topicCreatedEvent,
subscriptionCreatedEvent,
triggerReconciledEvent,
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(testNS, triggerName, finalizerName),
},
OtherTestData: map[string]interface{}{
"pre": []PubsubAction{
Topic("test-dead-letter-topic-id"),
},
"dataResidencyConfigMap": NewDataresidencyConfigMapFromRegions([]string{"us-east1"}),
},
PostConditions: []func(*testing.T, *TableRow){
OnlyTopics("cre-tgr_testnamespace_test-trigger_abc123", "test-dead-letter-topic-id"),
OnlySubscriptions("cre-tgr_testnamespace_test-trigger_abc123"),
SubscriptionHasRetryPolicy("cre-tgr_testnamespace_test-trigger_abc123",
&pubsub.RetryPolicy{
MaximumBackoff: 5 * time.Second,
MinimumBackoff: 5 * time.Second,
}),
SubscriptionHasDeadLetterPolicy("cre-tgr_testnamespace_test-trigger_abc123",
&pubsub.DeadLetterPolicy{
DeadLetterTopic: "projects/test-project-id/topics/test-dead-letter-topic-id",
}),
TopicExistsWithConfig("cre-tgr_testnamespace_test-trigger_abc123", &pubsub.TopicConfig{
MessageStoragePolicy: pubsub.MessageStoragePolicy{
AllowedPersistenceRegions: []string{"us-east1"},
},
Labels: map[string]string{
"name": "test-trigger", "namespace": "testnamespace", "resource": "triggers",
},
}),
},
},
{
Name: "Check topic config and labels",
Key: testKey,
Expand Down

0 comments on commit 3e547b0

Please sign in to comment.