Skip to content

Add ConsoleLogLinks to exceptions list for Console CRDs #23382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Jul 20, 2019
Merged

Add ConsoleLogLinks to exceptions list for Console CRDs #23382

openshift-merge-robot merged 1 commit into from
Jul 20, 2019

Conversation

@benjaminapetersen
Copy link
Contributor

@benjaminapetersen benjaminapetersen commented Jul 15, 2019

RE openshift/console-operator#246
Extending initial exceptions in #23231

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 15, 2019
@benjaminapetersen benjaminapetersen changed the title Add ConsoleLogLinks to exceptions list for Console CRDs [WIP] Add ConsoleLogLinks to exceptions list for Console CRDs Jul 15, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2019
@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jul 15, 2019

@rhamilto @spadgett fyi but held.

enj
enj suggested changes Jul 16, 2019
View reviewed changes
@benjaminapetersen benjaminapetersen force-pushed the console/rbac/group-rules/crd-exception-for-log-links branch from 3903fa2 to bc90d7b Compare July 17, 2019 17:30
@benjaminapetersen benjaminapetersen force-pushed the console/rbac/group-rules/crd-exception-for-log-links branch from bc90d7b to 98d6efd Compare July 17, 2019 19:09
benjaminapetersen
benjaminapetersen commented Jul 17, 2019
View reviewed changes
test/extended/authorization/rbac/groups_default_rules.go
// These custom resources are used to extend console functionality
// The console team is working on eliminating this exception in the near future
rbacv1helpers.NewRule(read...).Groups(consoleGroup).Resources("consoleclidownloads", "consolelinks", "consolenotifications").RuleOrDie(),
rbacv1helpers.NewRule(read...).Groups(consoleGroup).Resources("consoleclidownloads", "consolelinks", "consoleexternalloglinks", "consolenotifications").RuleOrDie(),
Copy link
Contributor Author

@benjaminapetersen benjaminapetersen Jul 17, 2019

Choose a reason for hiding this comment

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

@spadgett @rhamilto @jhadvig currently on consoleexternalloglinks. I believe we are settled on this.

@enj
Copy link
Contributor

enj commented Jul 18, 2019

image

/retest

This is fine. @bparees gets to merge.

@bparees
Copy link
Contributor

bparees commented Jul 18, 2019

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 18, 2019
spadgett
spadgett approved these changes Jul 18, 2019
View reviewed changes
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2019
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 18, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, bparees, spadgett

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

@benjaminapetersen benjaminapetersen changed the title [WIP] Add ConsoleLogLinks to exceptions list for Console CRDs Add ConsoleLogLinks to exceptions list for Console CRDs Jul 18, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 18, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 18, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett spadgett mentioned this pull request Jul 18, 2019
Merged
@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 18, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member

spadgett commented Jul 18, 2019

could not print execution order: could not sort nodes: steps are missing dependencies

/retest

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jul 18, 2019 •
edited
Loading

/retest

pretty sure this is a flake:

could not print execution order: could not sort nodes: steps are missing dependencies
--

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 19, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 19, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 19, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 19, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jul 19, 2019

/retest

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jul 19, 2019

Hmm. origin queue looks stuck yet, prob should have waited.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 19, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member

spadgett commented Jul 19, 2019

/retest

2 similar comments
@spadgett
Copy link
Member

spadgett commented Jul 19, 2019

/retest

@spadgett
Copy link
Member

spadgett commented Jul 20, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit 763e079 into openshift:master Jul 20, 2019
@benjaminapetersen benjaminapetersen mentioned this pull request Oct 15, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spadgett spadgett spadgett approved these changes

@mrogers950 mrogers950 Awaiting requested review from mrogers950

+1 more reviewer

@enj enj enj requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@spadgett spadgett

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@benjaminapetersen @enj @bparees @openshift-ci-robot @openshift-bot @spadgett @openshift-merge-robot