Skip to content

Commit

Permalink
Eventing TLS: migrate all resolver.URIResolver usages over to Address…
Browse files Browse the repository at this point in the history
…ableFromDestinationV1 (#6883)

Fixes #6878

<!-- Please include the 'why' behind your changes if no issue exists -->

## Proposed Changes

<!-- Please categorize your changes:
- 🎁 Add new feature
- 🐛 Fix bug
- 🧹 Update or clean up current behavior
- 🗑️ Remove feature or internal logic
-->

- updated dependency to
`knative.dev/pkg@v0.0.0-20230420071539-300df436f953`

### Pre-review Checklist

<!-- If these boxes are not checked, you will be asked to complete these
requirements or explain why they do not apply to your PR. -->

- [x] **At least 80% unit test coverage**
- [x] **E2E tests** for any new behavior
- [ ] **Docs PR** for any user-facing impact
- [ ] **Spec PR** for any new API feature
- [ ] **Conformance test** for any change to the spec

**Release Note**

<!--
📄 If this change has user-visible impact, write a release
note in the block
below. Include the string "action required" if additional action is
required of
users switching to the new release, for example in case of a breaking
change.

Write as if you are speaking to users, not other Knative contributors.
If this
change has no user-visible impact, no release note is needed.
-->

```release-note

```


**Docs**

<!--
📖 If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.
-->

---------

Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Knative Automation <automation@knative.team>
Co-authored-by: Rahul kumar <68837569+Rahul-Kumar-prog@users.noreply.github.com>
Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Co-authored-by: knative-automation <automation@knative.team>
  • Loading branch information
4 people authored Apr 27, 2023
1 parent 0a12a6c commit 6a5c7ee
Show file tree
Hide file tree
Showing 12 changed files with 173 additions and 77 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ require (
k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2
knative.dev/hack v0.0.0-20230417170854-f591fea109b3
knative.dev/hack/schema v0.0.0-20230417170854-f591fea109b3
knative.dev/pkg v0.0.0-20230418073056-dfad48eaa5d0
knative.dev/pkg v0.0.0-20230420071539-300df436f953
knative.dev/reconciler-test v0.0.0-20230420091239-6c21623d2555
sigs.k8s.io/yaml v1.3.0
)
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1047,8 +1047,8 @@ knative.dev/hack v0.0.0-20230417170854-f591fea109b3 h1:+W4WBOq83tfGXKhtv8OB/uJeY
knative.dev/hack v0.0.0-20230417170854-f591fea109b3/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
knative.dev/hack/schema v0.0.0-20230417170854-f591fea109b3 h1:TUHxKhNDLCX/XaqNaX9PY+247jEYD5aerg1woAhmEzw=
knative.dev/hack/schema v0.0.0-20230417170854-f591fea109b3/go.mod h1:GeIb+PLd5mllawcpHEGF5J5fYTQrvgEO5liao8lUKUs=
knative.dev/pkg v0.0.0-20230418073056-dfad48eaa5d0 h1:EFQcoUo8I4bc+U3y6tR1B3ONYZSHWUdAfI7Vh7dae8g=
knative.dev/pkg v0.0.0-20230418073056-dfad48eaa5d0/go.mod h1:2qWPP9Gjh9Q7ETti+WRHnBnGCSCq+6q7m3p/nmUQviE=
knative.dev/pkg v0.0.0-20230420071539-300df436f953 h1:GAB1JB35FWv5zqypwzmO7v4EPY8xXsomPtHEtb8xdx4=
knative.dev/pkg v0.0.0-20230420071539-300df436f953/go.mod h1:2qWPP9Gjh9Q7ETti+WRHnBnGCSCq+6q7m3p/nmUQviE=
knative.dev/reconciler-test v0.0.0-20230420091239-6c21623d2555 h1:DpYnNF7NiSKgPIh64nwSCAsr+6a48R9DhK9nE8X/+ps=
knative.dev/reconciler-test v0.0.0-20230420091239-6c21623d2555/go.mod h1:vZihHUKPh+sMzzx2lHgkM2tKr+f/lZqMJ9O7Jw0DaYs=
pgregory.net/rapid v0.3.3 h1:jCjBsY4ln4Atz78QoBWxUEvAHaFyNDQg9+WU62aCn1U=
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/apiserversource/apiserversource.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *v1.ApiServerSour
}
}

sinkURI, err := r.sinkResolver.URIFromDestinationV1(ctx, *dest, source)
sinkAddr, err := r.sinkResolver.AddressableFromDestinationV1(ctx, *dest, source)
if err != nil {
source.Status.MarkNoSink("NotFound", "")
return newWarningSinkNotFound(dest)
}
sinkURI := sinkAddr.URL
source.Status.MarkSink(sinkURI)

// resolve namespaces to watch
Expand Down
6 changes: 4 additions & 2 deletions pkg/reconciler/broker/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, t *eventingv1.Trigger) p
t.Spec.Subscriber.Ref.Namespace = t.GetNamespace()
}

subscriberURI, err := r.uriResolver.URIFromDestinationV1(ctx, t.Spec.Subscriber, b)
subscriberAddr, err := r.uriResolver.AddressableFromDestinationV1(ctx, t.Spec.Subscriber, b)
if err != nil {
logging.FromContext(ctx).Errorw("Unable to get the Subscriber's URI", zap.Error(err))
t.Status.MarkSubscriberResolvedFailed("Unable to get the Subscriber's URI", "%v", err)
t.Status.SubscriberURI = nil
return err
}
subscriberURI := subscriberAddr.URL
t.Status.SubscriberURI = subscriberURI
t.Status.MarkSubscriberResolvedSucceeded()

Expand All @@ -147,13 +148,14 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, t *eventingv1.Trigger) p
func (r *Reconciler) resolveDeadLetterSink(ctx context.Context, b *eventingv1.Broker, t *eventingv1.Trigger) error {
// resolve the trigger's dls first, fall back to the broker's
if t.Spec.Delivery != nil && t.Spec.Delivery.DeadLetterSink != nil {
deadLetterSinkURI, err := r.uriResolver.URIFromDestinationV1(ctx, *t.Spec.Delivery.DeadLetterSink, t)
deadLetterSinkAddr, err := r.uriResolver.AddressableFromDestinationV1(ctx, *t.Spec.Delivery.DeadLetterSink, t)
if err != nil {
t.Status.DeadLetterSinkURI = nil
logging.FromContext(ctx).Errorw("Unable to get the dead letter sink's URI", zap.Error(err))
t.Status.MarkDeadLetterSinkResolvedFailed("Unable to get the dead letter sink's URI", "%v", err)
return err
}
deadLetterSinkURI := deadLetterSinkAddr.URL

t.Status.DeadLetterSinkURI = deadLetterSinkURI
t.Status.MarkDeadLetterSinkResolvedSucceeded()
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/inmemorychannel/controller/inmemorychannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,13 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, imc *v1.InMemoryChannel)

// If a DeadLetterSink is defined in Spec.Delivery then whe resolve its URI and update the stauts
if imc.Spec.Delivery != nil && imc.Spec.Delivery.DeadLetterSink != nil {
deadLetterSinkUri, err := r.uriResolver.URIFromDestinationV1(ctx, *imc.Spec.Delivery.DeadLetterSink, imc)
deadLetterSinkAddr, err := r.uriResolver.AddressableFromDestinationV1(ctx, *imc.Spec.Delivery.DeadLetterSink, imc)
if err != nil {
logging.FromContext(ctx).Errorw("Unable to get the DeadLetterSink's URI", zap.Error(err))
imc.Status.MarkDeadLetterSinkResolvedFailed("Unable to get the DeadLetterSink's URI", "%v", err)
return fmt.Errorf("Failed to resolve Dead Letter Sink URI: %v", err)
}
deadLetterSinkUri := deadLetterSinkAddr.URL
imc.Status.MarkDeadLetterSinkResolvedSucceeded(deadLetterSinkUri)
} else {
imc.Status.MarkDeadLetterSinkNotConfigured()
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/pingsource/pingsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *sourcesv1.PingSo
}
}

sinkURI, err := r.sinkResolver.URIFromDestinationV1(ctx, *dest, source)
sinkAddr, err := r.sinkResolver.AddressableFromDestinationV1(ctx, *dest, source)
if err != nil {
source.Status.MarkNoSink("NotFound", "")
return newWarningSinkNotFound(dest)
}
sinkURI := sinkAddr.URL
source.Status.MarkSink(sinkURI)

// Make sure the global mt receive adapter is running
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/sinkbinding/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,13 @@ func (s *SinkBindingSubResourcesReconciler) Reconcile(ctx context.Context, b psb
Name: sb.Spec.Sink.Ref.Name,
}, b)
}
uri, err := s.res.URIFromDestinationV1(ctx, sb.Spec.Sink, sb)
addr, err := s.res.AddressableFromDestinationV1(ctx, sb.Spec.Sink, sb)
if err != nil {
logging.FromContext(ctx).Errorf("Failed to get URI from Destination: %w", err)
sb.Status.MarkBindingUnavailable("NoURI", "URI could not be extracted from destination")
return err
}
uri := addr.URL
sb.Status.MarkSink(uri)
return nil
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/reconciler/subscription/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,15 @@ func (r *Reconciler) resolveSubscriber(ctx context.Context, subscription *v1.Sub
logging.FromContext(ctx).Debugw("Group resolved", zap.Any("spec.subscriber.ref", subscriber.Ref))
}

subscriberURI, err := r.destinationResolver.URIFromDestinationV1(ctx, *subscriber, subscription)
subscriberAddr, err := r.destinationResolver.AddressableFromDestinationV1(ctx, *subscriber, subscription)
if err != nil {
logging.FromContext(ctx).Warnw("Failed to resolve Subscriber",
zap.Error(err),
zap.Any("subscriber", subscriber))
subscription.Status.MarkReferencesNotResolved(subscriberResolveFailed, "Failed to resolve spec.subscriber: %v", err)
return pkgreconciler.NewEvent(corev1.EventTypeWarning, subscriberResolveFailed, "Failed to resolve spec.subscriber: %w", err)
}
subscriberURI := subscriberAddr.URL
// If there is a change in resolved URI, log it.
if subscription.Status.PhysicalSubscription.SubscriberURI == nil || subscription.Status.PhysicalSubscription.SubscriberURI.String() != subscriberURI.String() {
logging.FromContext(ctx).Debugw("Resolved Subscriber", zap.String("subscriberURI", subscriberURI.String()))
Expand All @@ -247,14 +248,15 @@ func (r *Reconciler) resolveReply(ctx context.Context, subscription *v1.Subscrip
// compatibility for subscriptions with reply.ref.namespace = "".
reply.SetDefaults(ctx)

replyURI, err := r.destinationResolver.URIFromDestinationV1(ctx, *reply, subscription)
replyAddr, err := r.destinationResolver.AddressableFromDestinationV1(ctx, *reply, subscription)
if err != nil {
logging.FromContext(ctx).Warnw("Failed to resolve reply",
zap.Error(err),
zap.Any("reply", reply))
subscription.Status.MarkReferencesNotResolved(replyResolveFailed, "Failed to resolve spec.reply: %v", err)
return pkgreconciler.NewEvent(corev1.EventTypeWarning, replyResolveFailed, "Failed to resolve spec.reply: %w", err)
}
replyURI := replyAddr.URL
// If there is a change in resolved URI, log it.
if subscription.Status.PhysicalSubscription.ReplyURI == nil || subscription.Status.PhysicalSubscription.ReplyURI.String() != replyURI.String() {
logging.FromContext(ctx).Debugw("Resolved reply", zap.String("replyURI", replyURI.String()))
Expand All @@ -269,7 +271,7 @@ func (r *Reconciler) resolveReply(ctx context.Context, subscription *v1.Subscrip
func (r *Reconciler) resolveDeadLetterSink(ctx context.Context, subscription *v1.Subscription, channel *eventingduckv1.Channelable) pkgreconciler.Event {
// resolve the Subscription's dls first, fall back to the Channels's
if subscription.Spec.Delivery != nil && subscription.Spec.Delivery.DeadLetterSink != nil {
deadLetterSinkURI, err := r.destinationResolver.URIFromDestinationV1(ctx, *subscription.Spec.Delivery.DeadLetterSink, subscription)
deadLetterSinkAddr, err := r.destinationResolver.AddressableFromDestinationV1(ctx, *subscription.Spec.Delivery.DeadLetterSink, subscription)
if err != nil {
subscription.Status.PhysicalSubscription.DeadLetterSinkURI = nil
logging.FromContext(ctx).Warnw("Failed to resolve spec.delivery.deadLetterSink",
Expand All @@ -278,6 +280,7 @@ func (r *Reconciler) resolveDeadLetterSink(ctx context.Context, subscription *v1
subscription.Status.MarkReferencesNotResolved(deadLetterSinkResolveFailed, "Failed to resolve spec.delivery.deadLetterSink: %v", err)
return pkgreconciler.NewEvent(corev1.EventTypeWarning, deadLetterSinkResolveFailed, "Failed to resolve spec.delivery.deadLetterSink: %w", err)
}
deadLetterSinkURI := deadLetterSinkAddr.URL

logging.FromContext(ctx).Debugw("Resolved deadLetterSink", zap.String("deadLetterSinkURI", deadLetterSinkURI.String()))
subscription.Status.PhysicalSubscription.DeadLetterSinkURI = deadLetterSinkURI
Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/subscription/subscription_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ func TestAllCases(t *testing.T) {
Key: testNS + "/" + subscriptionName,
WantEvents: []string{
Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", subscriptionName),
Eventf(corev1.EventTypeWarning, "SubscriberResolveFailed", "Failed to resolve spec.subscriber: address not set for &ObjectReference{Kind:Subscriber,Namespace:testnamespace,Name:subscriber,UID:,APIVersion:messaging.knative.dev/v1,ResourceVersion:,FieldPath:,}"),
Eventf(corev1.EventTypeWarning, "SubscriberResolveFailed", "Failed to resolve spec.subscriber: address not set for Kind = Subscriber, Namespace = testnamespace, Name = subscriber, APIVersion = messaging.knative.dev/v1, Group = , Address = "),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: NewSubscription(subscriptionName, testNS,
Expand All @@ -760,7 +760,7 @@ func TestAllCases(t *testing.T) {
WithSubscriptionSubscriberRef(subscriberGVK, subscriberName, testNS),
// The first reconciliation will initialize the status conditions.
WithInitSubscriptionConditions,
WithSubscriptionReferencesNotResolved(subscriberResolveFailed, "Failed to resolve spec.subscriber: address not set for &ObjectReference{Kind:Subscriber,Namespace:testnamespace,Name:subscriber,UID:,APIVersion:messaging.knative.dev/v1,ResourceVersion:,FieldPath:,}"),
WithSubscriptionReferencesNotResolved(subscriberResolveFailed, "Failed to resolve spec.subscriber: address not set for Kind = Subscriber, Namespace = testnamespace, Name = subscriber, APIVersion = messaging.knative.dev/v1, Group = , Address = "),
),
}},
WantPatches: []clientgotesting.PatchActionImpl{
Expand Down Expand Up @@ -854,7 +854,7 @@ func TestAllCases(t *testing.T) {
Key: testNS + "/" + subscriptionName,
WantEvents: []string{
Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", subscriptionName),
Eventf(corev1.EventTypeWarning, replyResolveFailed, "Failed to resolve spec.reply: address not set for &ObjectReference{Kind:Trigger,Namespace:testnamespace,Name:reply,UID:,APIVersion:eventing.knative.dev/v1,ResourceVersion:,FieldPath:,}"),
Eventf(corev1.EventTypeWarning, replyResolveFailed, "Failed to resolve spec.reply: address not set for Kind = Trigger, Namespace = testnamespace, Name = reply, APIVersion = eventing.knative.dev/v1, Group = , Address = "),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
Object: NewSubscription(subscriptionName, testNS,
Expand All @@ -865,7 +865,7 @@ func TestAllCases(t *testing.T) {
WithSubscriptionReply(nonAddressableGVK, replyName, testNS),
// The first reconciliation will initialize the status conditions.
WithInitSubscriptionConditions,
WithSubscriptionReferencesNotResolved(replyResolveFailed, "Failed to resolve spec.reply: address not set for &ObjectReference{Kind:Trigger,Namespace:testnamespace,Name:reply,UID:,APIVersion:eventing.knative.dev/v1,ResourceVersion:,FieldPath:,}"),
WithSubscriptionReferencesNotResolved(replyResolveFailed, "Failed to resolve spec.reply: address not set for Kind = Trigger, Namespace = testnamespace, Name = reply, APIVersion = eventing.knative.dev/v1, Group = , Address = "),
),
}},
WantPatches: []clientgotesting.PatchActionImpl{
Expand Down
9 changes: 9 additions & 0 deletions vendor/knative.dev/pkg/apis/duck/v1/knative_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,12 @@ func isKReferenceGroupAllowed(ctx context.Context) bool {
func KReferenceGroupAllowed(ctx context.Context) context.Context {
return context.WithValue(ctx, isGroupAllowed{}, struct{}{})
}

func (kr *KReference) String() string {
address := ""
if kr.Address != nil {
address = *kr.Address
}
return fmt.Sprintf("Kind = %s, Namespace = %s, Name = %s, APIVersion = %s, Group = %s, Address = %s",
kr.Kind, kr.Namespace, kr.Name, kr.APIVersion, kr.Group, address)
}
Loading

0 comments on commit 6a5c7ee

Please sign in to comment.