Check if spec.retry is nil before dereferencing it. (#2052)

google · Jan 7, 2021 · 68320b0 · 68320b0
1 parent 87b9a08
commit 68320b0
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 3 deletions.
diff --git a/pkg/reconciler/trigger/trigger.go b/pkg/reconciler/trigger/trigger.go
@@ -315,10 +315,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 {

diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go
@@ -98,6 +98,16 @@ var (
 			},
 		},
 	}
+	brokerDeliverySpecWithoutRetry = &eventingduckv1beta1.DeliverySpec{
+		BackoffDelay:  &backoffDelay,
+		BackoffPolicy: &backoffPolicy,
+		DeadLetterSink: &duckv1.Destination{
+			URI: &apis.URL{
+				Scheme: "pubsub",
+				Host:   deadLetterTopicID,
+			},
+		},
+	}
 )
 
 func init() {
@@ -477,6 +487,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,