add safeguards for nil pointer dereference in delivery spec (#2148)

pkg/reconciler/trigger/trigger.go

@@ -51,6 +51,7 @@ const (
 	triggerReconciled = "TriggerReconciled"
 	triggerFinalized  = "TriggerFinalized"
 
+	defaultMinimumBackoff = 1 * time.Second
 	// Default maximum backoff duration used in the backoff retry policy for
 	// pubsub subscriptions. 600 seconds is the longest supported time.
 	defaultMaximumBackoff = 600 * time.Second
@@ -284,6 +285,12 @@ func (r *Reconciler) reconcileRetryTopicAndSubscription(ctx context.Context, tri
 // getPubsubRetryPolicy gets the eventing retry policy from the Broker delivery
 // spec and translates it to a pubsub retry policy.
 func getPubsubRetryPolicy(spec *eventingduckv1beta1.DeliverySpec) *pubsub.RetryPolicy {
+	if spec == nil {
+		return &pubsub.RetryPolicy{
+			MinimumBackoff: defaultMinimumBackoff,
+			MaximumBackoff: defaultMaximumBackoff,
+		}
+	}
 	// The Broker delivery spec is translated to a pubsub retry policy in the
 	// manner defined in the following post:
 	// https://github.com/google/knative-gcp/issues/1392#issuecomment-655617873
@@ -305,7 +312,7 @@ func getPubsubRetryPolicy(spec *eventingduckv1beta1.DeliverySpec) *pubsub.RetryP
 // getPubsubDeadLetterPolicy gets the eventing dead letter policy from the
 // Broker delivery spec and translates it to a pubsub dead letter policy.
 func getPubsubDeadLetterPolicy(projectID string, spec *eventingduckv1beta1.DeliverySpec) *pubsub.DeadLetterPolicy {
-	if spec.DeadLetterSink == nil {
+	if spec == nil || spec.DeadLetterSink == nil {
 		return nil
 	}
 	// Translate to the pubsub dead letter policy format.

pkg/reconciler/trigger/trigger_test.go

@@ -612,6 +612,65 @@ func TestAllCasesTrigger(t *testing.T) {
 				}),
 			},
 		},
+		{
+			Name: "Check topic config and labels - broker without spec.delivery",
+			Key:  testKey,
+			Objects: []runtime.Object{
+				NewBroker(brokerName, testNS,
+					WithBrokerClass(brokerv1beta1.BrokerClass),
+					WithInitBrokerConditions,
+					WithBrokerReady("url"),
+					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{}{
+				"dataResidencyConfigMap": NewDataresidencyConfigMapFromRegions([]string{"us-east1"}),
+			},
+			PostConditions: []func(*testing.T, *TableRow){
+				OnlyTopics("cre-tgr_testnamespace_test-trigger_abc123"),
+				OnlySubscriptions("cre-tgr_testnamespace_test-trigger_abc123"),
+				SubscriptionHasRetryPolicy("cre-tgr_testnamespace_test-trigger_abc123",
+					&pubsub.RetryPolicy{
+						MaximumBackoff: 600 * time.Second,
+						MinimumBackoff: 1 * time.Second,
+					}),
+				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: "Trigger created, broker ready, subscriber is addressable, nil pubsub client",
 			Key:  testKey,