Add channelname as a target#111
Merged
Merged
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vaikas-google The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-prow-robot
added
approved
scothis
reviewed
Jun 25, 2018
| namespace: default | ||
| roleRef: | ||
| kind: ClusterRole | ||
| name: cluster-admin |
Contributor
There was a problem hiding this comment.
This is a lot of privilege for a binding, but I don't see it used anywhere.
Contributor
Author
There was a problem hiding this comment.
I tried to address that in the README.md under the creation of the Service Account. Basically because the Bind creates a deployment, it would need access to that. I was just trying to keep the example small and address that with comments.
"This example creates a Service Account and grants
it cluster admin access, and you probably wouldn't want to do that in production
settings, but for this example it will suffice just fine."
Contributor
There was a problem hiding this comment.
I guess this is effectively the same level of privilege that was used before this change, the difference is that it's explicitly set in sample code that will be deployed by users.
From past experience, users will copy/paste sample code and deploy it to prod without really thinking about what I means (or reading the readme 😃). It's fine to clean this up in a followup PR, but it's something we shouldn't leave hanging around.
| Trigger EventTrigger `json:"trigger"` | ||
|
|
||
| // Service Account to run binding container. If left out, uses "default" | ||
| ServiceAccountName string `json:"serviceAccountName,omitempty"` |
Contributor
There was a problem hiding this comment.
Do the binder jobs and receive adapter use the same service account?
Contributor
Author
There was a problem hiding this comment.
That's the idea yes.
Contributor
There was a problem hiding this comment.
I'm struggling to think of a good case where we'd want them to differ, but it feels like there should be a situation. Until then, simpler is better.
google-prow-robot
added
size/L
Contributor
Author
|
/retest
…On Mon, Jun 25, 2018 at 11:08 AM Google Prow Robot ***@***.***> wrote:
@vaikas-google <https://github.com/vaikas-google>: The following test
*failed*, say /retest to rerun them all:
Test name Commit Details Rerun command
pull-knative-eventing-integration-tests 7cb402f
<7cb402f>
link
<https://gubernator-internal.googleplex.com/build/ela-prow/pr-logs/pull/knative_eventing/111/pull-knative-eventing-integration-tests/73/> /test
pull-knative-eventing-integration-tests
Full PR test history
<https://gubernator-internal.googleplex.com/pr/knative_eventing/111>. Your
PR dashboard <https://gubernator-internal.googleplex.com/pr/vaikas-google>
.
Instructions for interacting with me using PR comments are available here
<https://git.k8s.io/community/contributors/guide/pull-requests.md>. If
you have questions or suggestions related to my behavior, please file an
issue against the kubernetes/test-infra
<https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:>
repository. I understand the commands that are listed here
<https://go.k8s.io/bot-commands>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#111 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKwedNgYOX_3rkp9mQ2UmlXiDS0ZQWrjks5uASc4gaJpZM4U2cST>
.
|
Contributor
|
With #112 merged we can enforce either /lgtm |
scothis
added a commit
to scothis/eventing
that referenced
this pull request
Sep 11, 2018
From [ROLES.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/ROLES.md#approver): > Reviewer of the codebase for at least 3 months or 50% of project lifetime, whichever is shorter - [First Issue](knative#80). Opened 6/11 - [First PR](knative#66). Opened 5/31 - [First Review](knative#79 (review)) 6/11 > Primary reviewer for at least 10 substantial PRs to the codebase - knative#422 (review) - knative#414 (review) - knative#325 (review) - knative#225 (review) - knative#189 (review) - knative#168 (review) - knative#165 (review) - knative#99 (review) - knative#79 (review) - knative#111 (review) > Reviewed or merged at least 30 PRs to the codebase - [Reviewed 23 PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+reviewed-by%3Ascothis) - [Authored 34 merged PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+author%3Ascothis+is%3Amerged) - [Authored 5 open PRs](https://github.com/knative/eventing/pulls/scothis) > Nominated by an area lead From [WORKING_GROUPS.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/WORKING-GROUPS.md#events) /assign @vaikas-google > With no objections from other leads 🤞 /cc @evankanderson @grantr @inlined @mattmoor
knative-prow-robot
pushed a commit
that referenced
this pull request
Sep 12, 2018
From [ROLES.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/ROLES.md#approver): > Reviewer of the codebase for at least 3 months or 50% of project lifetime, whichever is shorter - [First Issue](#80). Opened 6/11 - [First PR](#66). Opened 5/31 - [First Review](#79 (review)) 6/11 > Primary reviewer for at least 10 substantial PRs to the codebase - #422 (review) - #414 (review) - #325 (review) - #225 (review) - #189 (review) - #168 (review) - #165 (review) - #99 (review) - #79 (review) - #111 (review) > Reviewed or merged at least 30 PRs to the codebase - [Reviewed 23 PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+reviewed-by%3Ascothis) - [Authored 34 merged PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+author%3Ascothis+is%3Amerged) - [Authored 5 open PRs](https://github.com/knative/eventing/pulls/scothis) > Nominated by an area lead From [WORKING_GROUPS.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/WORKING-GROUPS.md#events) /assign @vaikas-google > With no objections from other leads 🤞 /cc @evankanderson @grantr @inlined @mattmoor
matzew
pushed a commit
to matzew/eventing
that referenced
this pull request
Apr 11, 2019
From [ROLES.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/ROLES.md#approver): > Reviewer of the codebase for at least 3 months or 50% of project lifetime, whichever is shorter - [First Issue](knative#80). Opened 6/11 - [First PR](knative#66). Opened 5/31 - [First Review](knative#79 (review)) 6/11 > Primary reviewer for at least 10 substantial PRs to the codebase - knative#422 (review) - knative#414 (review) - knative#325 (review) - knative#225 (review) - knative#189 (review) - knative#168 (review) - knative#165 (review) - knative#99 (review) - knative#79 (review) - knative#111 (review) > Reviewed or merged at least 30 PRs to the codebase - [Reviewed 23 PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+reviewed-by%3Ascothis) - [Authored 34 merged PRs](https://github.com/knative/eventing/pulls?utf8=✓&q=is%3Apr+author%3Ascothis+is%3Amerged) - [Authored 5 open PRs](https://github.com/knative/eventing/pulls/scothis) > Nominated by an area lead From [WORKING_GROUPS.MD](https://github.com/knative/docs/blob/dfc53c67c8e80d30b8863353c9e9b4ad00c41fa0/community/WORKING-GROUPS.md#events) /assign @vaikas-google > With no objections from other leads 🤞 /cc @evankanderson @grantr @inlined @mattmoor
matzew
pushed a commit
to matzew/eventing
that referenced
this pull request
May 28, 2019
Install Knative Serving via Operator and minimal Istio
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Note: This is branched from: #109 so for diffing purposes you probably have to do some git stuff to see only the changes from this PR.
Note: Still need to figure out the best way to take external events (say github example) and wire it to channel. But wanted to get this out so we can start running e2e tests without requiring an external DNS name.