Add Deprecation Warnings to Topic and PullSubscription

pkg/apis/pubsub/internal/deprecated_condition.go

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2020 The Knative Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package internal
+
+import (
+	"reflect"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/apis"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+const deprecationMessage = "This Kind is deprecated and the CRD has been made 'internal'. This " +
+	"means, end users should not create objects of this CRD directly. If you need a non-internal " +
+	"variant, then please let us know by commenting on " +
+	"https://github.com/google/knative-gcp/issues/905. Moreover, the object must be deleted " +
+	"before upgrading to 0.16 to avoid orphaning the Google Cloud Platform resources that were " +
+	"created on behalf of this object, such as Pub/Sub Topics and Subscriptions."
+
+// deprecatedCondition is the condition to add to types that will be removed in 0.16.
+// See https://github.com/google/knative-gcp/issues/905 for more context.
+var deprecatedCondition = apis.Condition{
+	Type:     "Deprecated",
+	Reason:   "WillBeRemoved",
+	Status:   corev1.ConditionTrue,
+	Severity: apis.ConditionSeverityWarning,
+	Message:  deprecationMessage,
+}
+
+// MarkDeprecated adds the DeprecatedCondition to the supplied conditions and returns the new
+// conditions.
+func MarkDeprecated(conditions duckv1.Conditions) duckv1.Conditions {
+	dc := deprecatedCondition.DeepCopy()
+	for i, c := range conditions {
+		if c.Type == dc.Type {
+			// If we'd only update the LastTransitionTime, then return.
+			dc.LastTransitionTime = c.LastTransitionTime
+			if !reflect.DeepEqual(dc, &c) {
+				dc.LastTransitionTime = deprecatedCondition.LastTransitionTime
+				conditions[i] = *dc
+			}
+			return conditions
+		}
+	}
+	dc.LastTransitionTime = apis.VolatileTime{
+		Inner: metav1.NewTime(time.Now()),
+	}
+	conditions = append(conditions, *dc)
+	return conditions
+}

pkg/apis/pubsub/internal/deprecated_condition_test.go

@@ -0,0 +1,114 @@
+/*
+ * Copyright 2020 The Knative Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package internal
+
+import (
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/apis"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+var (
+	conditions duckv1.Conditions = []apis.Condition{
+		{
+			Type:               "PublisherReady",
+			Status:             "True",
+			Severity:           "Warning",
+			LastTransitionTime: apis.VolatileTime{Inner: metav1.NewTime(time.Now())},
+			Reason:             "publisherReady",
+			Message:            "The Publisher is ready",
+		},
+	}
+
+	conditionsWithDeprecated = append(conditions.DeepCopy(), deprecatedCondition)
+)
+
+func TestMarkDeprecated_NotPresent(t *testing.T) {
+	want := conditionsWithDeprecated.DeepCopy()
+	got := MarkDeprecated(conditions.DeepCopy())
+
+	// The lastTransitionTime is set to the current time when this is called, so assert that it is
+	// within some reasonable bound of now.
+	dc := getDeprecatedCondition(t, got)
+	if now, ct := time.Now(), dc.LastTransitionTime.Inner; !ct.Add(time.Minute).After(now) {
+		t.Errorf("DeprecatedCondition's lastTransitionTime is too far in the past: %v (now: %v)", ct, now)
+	}
+
+	if diff := cmp.Diff(want, got, cmpopts.IgnoreTypes(apis.VolatileTime{})); diff != "" {
+		t.Errorf("Unexpected status (-want +got): %v", diff)
+	}
+}
+
+func TestMarkDeprecated_OnlyTimeChanges(t *testing.T) {
+	// If only time changes, then we don't want to make the change. So in this case the initial
+	// conditions are the same as the wanted conditions.
+	_, want := addDeprecatedCondition(conditions, func(c *apis.Condition) {
+		c.LastTransitionTime = apis.VolatileTime{
+			Inner: metav1.NewTime(c.LastTransitionTime.Inner.Add(-1 * time.Minute)),
+		}
+	})
+	got := MarkDeprecated(want)
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("Unexpected status (-want +got): %v", diff)
+	}
+}
+
+func TestMarkDeprecated_ReasonChanges(t *testing.T) {
+	init, want := addDeprecatedCondition(conditions, func(c *apis.Condition) {
+		c.Reason = "someOtherReason"
+	})
+	got := MarkDeprecated(init)
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("Unexpected status (-want +got): %v", diff)
+	}
+}
+
+func TestMarkDeprecated_NoChange(t *testing.T) {
+	init := conditionsWithDeprecated.DeepCopy()
+	want := conditionsWithDeprecated.DeepCopy()
+	got := MarkDeprecated(init)
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("Unexpected status (-want +got): %v", diff)
+	}
+}
+
+// addDeprecatedCondition adds the deprecated condition to the passed in initial conditions. It also
+// applies the function f to the the 'altered' conditions. This is intended to be the input to
+// MarkDeprecated, whereas 'unaltered' conditions are meant to the output.
+func addDeprecatedCondition(init duckv1.Conditions, f func(*apis.Condition)) (altered, unaltered duckv1.Conditions) {
+	dc := deprecatedCondition.DeepCopy()
+	f(dc)
+	altered = append(init, *dc)
+	unaltered = append(init, *deprecatedCondition.DeepCopy())
+	return altered, unaltered
+}
+
+func getDeprecatedCondition(t *testing.T, conds duckv1.Conditions) apis.Condition {
+	t.Helper()
+	for i := range conds {
+		if conds[i].Type == deprecatedCondition.Type {
+			return conds[i]
+		}
+	}
+	t.Fatalf("Unable to find the deprecated conditions %v", conds)
+	return apis.Condition{}
+}

pkg/apis/pubsub/v1alpha1/deprecated_condition.go

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2019 The Knative Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package v1alpha1
+
+import (
+	"github.com/google/knative-gcp/pkg/apis/pubsub/internal"
+)
+
+// MarkDeprecated adds a warning condition that this object is deprecated and will be dropped in a
+// future release. Note that this does not affect the Ready condition.
+func (s *PullSubscriptionStatus) MarkDeprecated() {
+	s.Conditions = internal.MarkDeprecated(s.Conditions)
+}
+
+// MarkDeprecated adds a warning condition that this object is deprecated and will be dropped in a
+// future release. Note that this does not affect the Ready condition.
+func (ts *TopicStatus) MarkDeprecated() {
+	ts.Conditions = internal.MarkDeprecated(ts.Conditions)
+}

pkg/apis/pubsub/v1beta1/deprecated_condition.go

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2019 The Knative Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package v1beta1
+
+import (
+	"github.com/google/knative-gcp/pkg/apis/pubsub/internal"
+)
+
+// MarkDeprecated adds a warning condition that this object is deprecated and will be dropped in a
+// future release. Note that this does not affect the Ready condition.
+func (s *PullSubscriptionStatus) MarkDeprecated() {
+	s.Conditions = internal.MarkDeprecated(s.Conditions)
+}
+
+// MarkDeprecated adds a warning condition that this object is deprecated and will be dropped in a
+// future release. Note that this does not affect the Ready condition.
+func (ts *TopicStatus) MarkDeprecated() {
+	ts.Conditions = internal.MarkDeprecated(ts.Conditions)
+}

pkg/reconciler/pubsub/pullsubscription/keda/pullsubscription_test.go

@@ -139,6 +139,7 @@ func newPullSubscription(subscriptionId string) *pubsubv1alpha1.PullSubscription
 		}),
 		WithPubSubPullSubscriptionSubscriptionID(subscriptionId),
 		WithPubSubInitPullSubscriptionConditions,
+		WithPubSubPullSubscriptionDeprecated(),
 		WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 		WithPubSubPullSubscriptionMarkSink(sinkURI),
 	)
@@ -241,6 +242,7 @@ func TestAllCases(t *testing.T) {
 				// Updates
 				WithPubSubPullSubscriptionStatusObservedGeneration(generation),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSinkNotFound(),
 			),
 		}},
@@ -259,6 +261,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
 			),
@@ -289,6 +292,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -314,6 +318,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
 			),
@@ -346,6 +351,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -371,6 +377,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
 			),
@@ -403,6 +410,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -428,6 +436,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
 			),
@@ -460,6 +469,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -485,6 +495,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
 			),
@@ -518,6 +529,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -564,6 +576,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSink(sinkURI),
@@ -626,6 +639,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionMarkSubscribed(testSubscriptionID),
@@ -697,6 +711,7 @@ func TestAllCases(t *testing.T) {
 					Topic: testTopicID,
 				}),
 				WithPubSubInitPullSubscriptionConditions,
+				WithPubSubPullSubscriptionDeprecated(),
 				WithPubSubPullSubscriptionProjectID(testProject),
 				WithPubSubPullSubscriptionSink(sinkGVK, sinkName),
 				WithPubSubPullSubscriptionTransformer(transformerGVK, transformerName),

pkg/reconciler/pubsub/pullsubscription/reconciler.go

@@ -101,6 +101,8 @@ func (r *Base) ReconcileKind(ctx context.Context, ps *v1alpha1.PullSubscription)
 	ps.Status.InitializeConditions()
 	ps.Status.ObservedGeneration = ps.Generation
 
+	ps.Status.MarkDeprecated()
+
 	// If pullsubscription doesn't have ownerReference and GCP ServiceAccount is provided, reconcile workload identity.
 	// Otherwise, its owner will reconcile workload identity.
 	if (ps.OwnerReferences == nil || len(ps.OwnerReferences) == 0) && ps.Spec.GoogleServiceAccount != "" {