Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

feat/trigger: enable trigger to send to different namespace #2016

Merged

Conversation

zhongduo
Copy link
Contributor

After this change, when the namepsace is specified in the subscriber
the URI can be resolved correctly to the specified namespace instead
of forcing it to the be same as the trigger namespace.

Fixes #2015

Proposed Changes

  • Don't over the namespace of a trigger
  • 🎁 Allow triggers to send to a sink in different namespace

Release Note

- 🎁 Allow triggers to send to a sink in different namespace

Docs

After this change, when the namepsace is specified in the subscriber
the URI can be resolved correctly to the specified namespace instead
of forcing it to the be same as the trigger namespace.
@google-cla google-cla bot added the cla: yes (override cla status due to multiple authors bug) label Dec 22, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhongduo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zhongduo
Copy link
Contributor Author

/assign @Harwayne

@tayarani
Copy link
Member

/lgtm

@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-google-knative-gcp-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/trigger/trigger.go 67.1% 66.4% -0.7

@liu-cong
Copy link
Contributor

/lgtm

BTW, can you test this in a cluster? Just to confirm this isn't something else that blocks cross-namespace. I remember there used to be some logic in eventing webhook.

@zhongduo
Copy link
Contributor Author

/lgtm

BTW, can you test this in a cluster? Just to confirm this isn't something else that blocks cross-namespace. I remember there used to be some logic in eventing webhook.

I am actively testing it now with my experiment to see if cross namespace delivery is possible. After this change, I can now send from curl pod in default namespace to broker/trigger in cloud-run-events-example namespace, then got routed back to event-display default namespace.

@knative-prow-robot knative-prow-robot merged commit c0e6cfb into google:master Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trigger unable to deliver to different namespace
6 participants