Convert DeadLetterSinkURI references to DeliveryStatus. (#5746)

Add DeliveryStatus to Channelable.
knative · Sep 23, 2021 · e05ccfc · e05ccfc
1 parent 1afbc6a
commit e05ccfc
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 150 deletions.
diff --git a/docs/delivery/README.md b/docs/delivery/README.md
@@ -123,7 +123,7 @@ capabilities and are free to add more delivery options.
 
 ### Exposing underlying DLC
 
-Channel implementation supporting dead letter channel should advertise it in
+Channel implementation supporting dead letter channel should resolve it to a URI in
 their status.
 
 ```go
@@ -133,7 +133,7 @@ type DeliveryStatus struct {
 	// DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
 	// where failed events are sent to.
 	// +optional
-	DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
+	DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
 }
 ```
 

diff --git a/docs/eventing-api.md b/docs/eventing-api.md
@@ -263,6 +263,24 @@ SubscribableStatus
 </tr>
 <tr>
 <td>
+<code>DeliveryStatus</code><br/>
+<em>
+<a href="#duck.knative.dev/v1.DeliveryStatus">
+DeliveryStatus
+</a>
+</em>
+</td>
+<td>
+<p>
+(Members of <code>DeliveryStatus</code> are embedded into this type.)
+</p>
+<em>(Optional)</em>
+<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+resolved delivery options.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>deadLetterChannel</code><br/>
 <em>
 <a href="https://pkg.go.dev/knative.dev/pkg/apis/duck/v1#KReference">
@@ -273,7 +291,8 @@ knative.dev/pkg/apis/duck/v1.KReference
 <td>
 <em>(Optional)</em>
 <p>DeadLetterChannel is a KReference and is set by the channel when it supports native error handling via a channel
-Failed messages are delivered here.</p>
+Failed messages are delivered here.
+Deprecated in favor of DeliveryStatus, to be removed September 2022.</p>
 </td>
 </tr>
 </tbody>
@@ -375,7 +394,10 @@ For exponential policy, backoff delay is backoffDelay*2^<numberOfRetries>.</p>
 <h3 id="duck.knative.dev/v1.DeliveryStatus">DeliveryStatus
 </h3>
 <p>
-<p>DeliveryStatus contains the Status of an object supporting delivery options.</p>
+(<em>Appears on:</em><a href="#duck.knative.dev/v1.ChannelableStatus">ChannelableStatus</a>, <a href="#eventing.knative.dev/v1.BrokerStatus">BrokerStatus</a>, <a href="#eventing.knative.dev/v1.TriggerStatus">TriggerStatus</a>, <a href="#messaging.knative.dev/v1.SubscriptionStatusPhysicalSubscription">SubscriptionStatusPhysicalSubscription</a>)
+</p>
+<p>
+<p>DeliveryStatus contains the Status of an object supporting delivery options. This type is intended to be embedded into a status struct.</p>
 </p>
 <table>
 <thead>
@@ -387,16 +409,16 @@ For exponential policy, backoff delay is backoffDelay*2^<numberOfRetries>.</p>
 <tbody>
 <tr>
 <td>
-<code>deadLetterChannel</code><br/>
+<code>deadLetterSinkUri</code><br/>
 <em>
-<a href="https://pkg.go.dev/knative.dev/pkg/apis/duck/v1#KReference">
-knative.dev/pkg/apis/duck/v1.KReference
+<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
+knative.dev/pkg/apis.URL
 </a>
 </em>
 </td>
 <td>
 <em>(Optional)</em>
-<p>DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
+<p>DeadLetterSink is a KReference that is the reference to the native, platform specific channel
 where failed events are sent to.</p>
 </td>
 </tr>
@@ -1721,6 +1743,23 @@ knative.dev/pkg/apis/duck/v1.Addressable
 delivered into the Broker mesh.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>DeliveryStatus</code><br/>
+<em>
+<a href="#duck.knative.dev/v1.DeliveryStatus">
+DeliveryStatus
+</a>
+</em>
+</td>
+<td>
+<p>
+(Members of <code>DeliveryStatus</code> are embedded into this type.)
+</p>
+<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+resolved delivery options.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="eventing.knative.dev/v1.TriggerFilter">TriggerFilter
@@ -1889,16 +1928,19 @@ knative.dev/pkg/apis.URL
 </tr>
 <tr>
 <td>
-<code>deadLetterSinkUri</code><br/>
+<code>DeliveryStatus</code><br/>
 <em>
-<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
-knative.dev/pkg/apis.URL
+<a href="#duck.knative.dev/v1.DeliveryStatus">
+DeliveryStatus
 </a>
 </em>
 </td>
 <td>
-<em>(Optional)</em>
-<p>DeadLetterSinkURI is the resolved URI of the dead letter sink for this Trigger.</p>
+<p>
+(Members of <code>DeliveryStatus</code> are embedded into this type.)
+</p>
+<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+resolved delivery options.</p>
 </td>
 </tr>
 </tbody>
@@ -3828,16 +3870,19 @@ knative.dev/pkg/apis.URL
 </tr>
 <tr>
 <td>
-<code>deadLetterSinkUri</code><br/>
+<code>DeliveryStatus</code><br/>
 <em>
-<a href="https://pkg.go.dev/knative.dev/pkg/apis#URL">
-knative.dev/pkg/apis.URL
+<a href="#duck.knative.dev/v1.DeliveryStatus">
+DeliveryStatus
 </a>
 </em>
 </td>
 <td>
-<em>(Optional)</em>
-<p>ReplyURI is the fully resolved URI for the spec.delivery.deadLetterSink.</p>
+<p>
+(Members of <code>DeliveryStatus</code> are embedded into this type.)
+</p>
+<p>DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+resolved delivery options.</p>
 </td>
 </tr>
 </tbody>

diff --git a/pkg/apis/duck/v1/channelable_types.go b/pkg/apis/duck/v1/channelable_types.go
@@ -63,8 +63,13 @@ type ChannelableStatus struct {
 	duckv1.AddressStatus `json:",inline"`
 	// Subscribers is populated with the statuses of each of the Channelable's subscribers.
 	SubscribableStatus `json:",inline"`
+	// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+	// resolved delivery options.
+	// +optional
+	DeliveryStatus `json:",inline"`
 	// DeadLetterChannel is a KReference and is set by the channel when it supports native error handling via a channel
 	// Failed messages are delivered here.
+	// Deprecated in favor of DeliveryStatus, to be removed September 2022.
 	// +optional
 	DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
 }

diff --git a/pkg/apis/duck/v1/delivery_types.go b/pkg/apis/duck/v1/delivery_types.go
@@ -115,10 +115,10 @@ const (
 	BackoffPolicyExponential BackoffPolicyType = "exponential"
 )
 
-// DeliveryStatus contains the Status of an object supporting delivery options.
+// DeliveryStatus contains the Status of an object supporting delivery options. This type is intended to be embedded into a status struct.
 type DeliveryStatus struct {
-	// DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
+	// DeadLetterSink is a KReference that is the reference to the native, platform specific channel
 	// where failed events are sent to.
 	// +optional
-	DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`
+	DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
 }
diff --git a/pkg/apis/duck/v1/zz_generated.deepcopy.go b/pkg/apis/duck/v1/zz_generated.deepcopy.go
diff --git a/pkg/apis/duck/v1beta1/delivery_conversion.go b/pkg/apis/duck/v1beta1/delivery_conversion.go
@@ -76,24 +76,4 @@ func (sink *DeliverySpec) ConvertFrom(ctx context.Context, from apis.Convertible
 	}
 }
 
-// ConvertTo implements apis.Convertible
-func (source *DeliveryStatus) ConvertTo(ctx context.Context, to apis.Convertible) error {
-	switch sink := to.(type) {
-	case *eventingduckv1.DeliveryStatus:
-		sink.DeadLetterChannel = source.DeadLetterChannel
-		return nil
-	default:
-		return fmt.Errorf("unknown version, got: %T", sink)
-	}
-}
-
-// ConvertFrom implements apis.Convertible
-func (sink *DeliveryStatus) ConvertFrom(ctx context.Context, from apis.Convertible) error {
-	switch source := from.(type) {
-	case *eventingduckv1.DeliveryStatus:
-		sink.DeadLetterChannel = source.DeadLetterChannel
-		return nil
-	default:
-		return fmt.Errorf("unknown version, got: %T", source)
-	}
-}
+// DeliveryStatus v1beta1 is not convertable to v1 (Channel ref type vs URL)
diff --git a/pkg/apis/duck/v1beta1/delivery_conversion_test.go b/pkg/apis/duck/v1beta1/delivery_conversion_test.go
@@ -23,7 +23,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	v1 "knative.dev/eventing/pkg/apis/duck/v1"
 	"knative.dev/pkg/apis"
-	duckv1 "knative.dev/pkg/apis/duck/v1"
 	pkgduck "knative.dev/pkg/apis/duck/v1"
 )
 
@@ -39,18 +38,6 @@ func TestDeliverySpecConversionBadType(t *testing.T) {
 	}
 }
 
-func TestDeliveryStatusConversionBadType(t *testing.T) {
-	good, bad := &DeliveryStatus{}, &DeliveryStatus{}
-
-	if err := good.ConvertTo(context.Background(), bad); err == nil {
-		t.Errorf("ConvertTo() = %#v, wanted error", bad)
-	}
-
-	if err := good.ConvertFrom(context.Background(), bad); err == nil {
-		t.Errorf("ConvertFrom() = %#v, wanted error", good)
-	}
-}
-
 // Test v1beta1 -> v1 -> v1beta1
 func TestDeliverySpecConversion(t *testing.T) {
 	var retryCount int32 = 10
@@ -205,78 +192,5 @@ func TestDeliverySpecConversionV1(t *testing.T) {
 	}
 }
 
-// Test v1beta1 -> v1 -> v1beta1
-func TestDeliveryStatusConversion(t *testing.T) {
-	tests := []struct {
-		name string
-		in   *DeliveryStatus
-		err  *string
-	}{{
-		name: "min configuration",
-		in: &DeliveryStatus{
-			DeadLetterChannel: &duckv1.KReference{
-				Kind:       "dlKind",
-				Namespace:  "dlNamespace",
-				Name:       "dlName",
-				APIVersion: "dlAPIVersion",
-			},
-		},
-	}}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			ver := &v1.DeliveryStatus{}
-			err := test.in.ConvertTo(context.Background(), ver)
-			if err != nil {
-				if test.err == nil || *test.err != err.Error() {
-					t.Error("ConvertTo() =", err)
-				}
-				return
-			}
-			got := &DeliveryStatus{}
-			if err := got.ConvertFrom(context.Background(), ver); err != nil {
-				t.Error("ConvertFrom() =", err)
-			}
-			if diff := cmp.Diff(test.in, got); diff != "" {
-				t.Error("roundtrip (-want, +got) =", diff)
-			}
-		})
-	}
-}
-
-// Test v1 -> v1beta1 -> v1
-func TestDeliveryStatusConversionV1(t *testing.T) {
-	tests := []struct {
-		name string
-		in   *v1.DeliveryStatus
-		err  *string
-	}{{
-		name: "min configuration",
-		in: &v1.DeliveryStatus{
-			DeadLetterChannel: &duckv1.KReference{
-				Kind:       "dlKind",
-				Namespace:  "dlNamespace",
-				Name:       "dlName",
-				APIVersion: "dlAPIVersion",
-			},
-		},
-	}}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			ver := &DeliveryStatus{}
-			err := ver.ConvertFrom(context.Background(), test.in)
-			if err != nil {
-				if test.err == nil || *test.err != err.Error() {
-					t.Error("ConvertFrom() =", err)
-				}
-				return
-			}
-			got := &v1.DeliveryStatus{}
-			if err := ver.ConvertTo(context.Background(), got); err != nil {
-				t.Error("ConvertTo() =", err)
-			}
-			if diff := cmp.Diff(test.in, got); diff != "" {
-				t.Error("roundtrip (-want, +got) =", diff)
-			}
-		})
-	}
-}
+// v1beta1 and v1 DeliveryStatus are not convertable to each other.
+// (channel ref vs apis.URL)
diff --git a/pkg/apis/eventing/v1/broker_types.go b/pkg/apis/eventing/v1/broker_types.go
@@ -92,6 +92,10 @@ type BrokerStatus struct {
 	// delivered into the Broker mesh.
 	// +optional
 	Address duckv1.Addressable `json:"address,omitempty"`
+
+	// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+	// resolved delivery options.
+	eventingduckv1.DeliveryStatus `json:",inline"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

diff --git a/pkg/apis/eventing/v1/trigger_types.go b/pkg/apis/eventing/v1/trigger_types.go
@@ -120,9 +120,9 @@ type TriggerStatus struct {
 	// +optional
 	SubscriberURI *apis.URL `json:"subscriberUri,omitempty"`
 
-	// DeadLetterSinkURI is the resolved URI of the dead letter sink for this Trigger.
-	// +optional
-	DeadLetterSinkURI *apis.URL `json:"deadLetterSinkUri,omitempty"`
+	// DeliveryStatus contains a resolved URL to the dead letter sink address, and any other
+	// resolved delivery options.
+	eventingduckv1.DeliveryStatus `json:",inline"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

diff --git a/pkg/apis/eventing/v1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1/zz_generated.deepcopy.go