Skip to content
This repository was archived by the owner on Jun 4, 2021. It is now read-only.

Adding Broker read permissions#252

Merged
knative-prow-robot merged 8 commits into
knative:masterfrom
nachocano:broker-perm
Mar 18, 2019
Merged

Adding Broker read permissions#252
knative-prow-robot merged 8 commits into
knative:masterfrom
nachocano:broker-perm

Conversation

@nachocano
Copy link
Copy Markdown
Contributor

Proposed Changes

Release Note

NONE

@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 15, 2019
@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 15, 2019
Comment thread config/201-clusterrole.yaml Outdated
Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nachocano
Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

I think we might only need

resources:
- channels/status
- brokers/status

But it seems fine to start with this and test the other later.

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, nachocano

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

The pull request process is described here

Details 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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2019
verbs: *everything

# Channels read
# Channels and Brokers read
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Harwayne
Copy link
Copy Markdown
Contributor

/lgtm

@Harwayne
Copy link
Copy Markdown
Contributor

This might be a good case for Aggregated ClusterRole.

Essentially, we label ClusterRoles that are needed to for resolving Addressables, then all ServiceAccounts that need to resolve Addressables simply get the aggregated role. Then any updates just need to be made in one place and suddenly all ServiceAccounts have the correct permissions.

@Harwayne
Copy link
Copy Markdown
Contributor

This might be a good case for Aggregated ClusterRole.

Essentially, we label ClusterRoles that are needed to for resolving Addressables, then all ServiceAccounts that need to resolve Addressables simply get the aggregated role. Then any updates just need to be made in one place and suddenly all ServiceAccounts have the correct permissions.

Created knative/eventing#916 to track this idea.

@evankanderson
Copy link
Copy Markdown
Member

You need to remove the "[WIP]" from the title when you want Prow to merge this.

@nachocano nachocano changed the title [WIP] Adding Broker read permissions Adding Broker read permissions Mar 18, 2019
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 18, 2019
@nachocano
Copy link
Copy Markdown
Contributor Author

You need to remove the "[WIP]" from the title when you want Prow to merge this.

Done! Thanks @evankanderson, my bad.

@knative-prow-robot knative-prow-robot merged commit 551dc5e into knative:master Mar 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants