Skip to content
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.

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

Bug 1759182: Add a RBAC checker for external IP ranger #23826

Merged
merged 1 commit into from Dec 13, 2019
Merged

Bug 1759182: Add a RBAC checker for external IP ranger #23826

merged 1 commit into from Dec 13, 2019

Conversation

abhat
Copy link
Contributor

@abhat abhat commented Sep 18, 2019

In case the supplied external IPs in the service spec are valid,
allow the RBAC check to override the specified ranges in the
external IP range checker.

@openshift-ci-robot
Copy link

@abhat: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Add a RBAC checker for external IP ranger

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2019
@abhat
Copy link
Contributor Author

abhat commented Sep 18, 2019

/assign @danwinship
/cc @squeed

@abhat
Copy link
Contributor Author

abhat commented Sep 18, 2019

Backport for 4.1.

@abhat abhat changed the title Add a RBAC checker for external IP ranger Bug 1752045: Add a RBAC checker for external IP ranger Sep 18, 2019
@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1752045, which is invalid:

  • expected the bug to target the "4.1.z" release, but it targets "4.3.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1752045: Add a RBAC checker for external IP ranger

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 18, 2019
@abhat abhat force-pushed the backport_41_rbac_external_ipranger branch 3 times, most recently from 5dc2716 to 015eb75 Compare September 26, 2019 16:35
@abhat abhat force-pushed the backport_41_rbac_external_ipranger branch from 015eb75 to b94ebcf Compare October 1, 2019 19:26
@abhat abhat changed the title Bug 1752045: Add a RBAC checker for external IP ranger Bug 1759182: Add a RBAC checker for external IP ranger Oct 7, 2019
@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1759182, which is invalid:

  • expected dependent Bugzilla bug 1752045 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1759182: Add a RBAC checker for external IP ranger

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@abhat abhat marked this pull request as ready for review October 7, 2019 20:43
@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 Oct 7, 2019
@abhat
Copy link
Contributor Author

abhat commented Oct 7, 2019

/bugzilla refresh

@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1759182, which is invalid:

  • expected dependent Bugzilla bug 1752045 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@danwinship
Copy link
Contributor

/lgtm
but as the bot says, this needs to wait until the 4.3 bug has been QA'ed before it can merge.
Also, you need to do a 4.2 backport as well.

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 7, 2019
@abhat
Copy link
Contributor Author

abhat commented Oct 10, 2019

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Oct 10, 2019
@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1759182, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 10, 2019
@danwinship
Copy link
Contributor

@abhat you need a 4.2 backport. We can't merge in an older release without merging in all newer releases first, to ensure that fixes don't regress when people upgrade

@openshift-bot
Copy link
Contributor

/retest

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@abhat
Copy link
Contributor Author

abhat commented Nov 12, 2019

/retest

@openshift-bot
Copy link
Contributor

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@abhat abhat force-pushed the backport_41_rbac_external_ipranger branch from b94ebcf to 31e5b67 Compare November 14, 2019 02:45
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2019
@abhat
Copy link
Contributor Author

abhat commented Nov 14, 2019

/test unit

@abhat
Copy link
Contributor Author

abhat commented Nov 14, 2019

/test cmd

@stlaz
Copy link
Member

stlaz commented Nov 15, 2019

/assign @stlaz

stlaz
stlaz reviewed Nov 18, 2019
View reviewed changes
Copy link
Member

@stlaz stlaz left a comment

Choose a reason for hiding this comment

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

minor code style nits

Do you have e2e test for this?

pkg/network/admission/externalipranger/externalip_admission.go Outdated Show resolved Hide resolved
pkg/network/admission/externalipranger/externalip_admission_test.go Outdated Show resolved Hide resolved
@abhat
Copy link
Contributor Author

abhat commented Nov 19, 2019

minor code style nits

Do you have e2e test for this?

No, right now we only have unit-tests. I will add a e2e later in a separate PR.

@abhat
Add a RBAC checker for external IP ranger
239eef3 
In case the supplied external IPs in the service spec are valid,
allow the RBAC check to override the specified ranges in the
external IP range checker.
@abhat abhat force-pushed the backport_41_rbac_external_ipranger branch from 31e5b67 to 239eef3 Compare November 19, 2019 16:24
@ashcrow
Copy link
Member

ashcrow commented Nov 19, 2019

Removing cherry-pick-approved for now. The next patch manager should re-review.

@ashcrow ashcrow removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Nov 19, 2019
@eparis
Copy link
Member

eparis commented Nov 20, 2019

/retest

@abhat
Copy link
Contributor Author

abhat commented Dec 3, 2019

/test cmd

@abhat
Copy link
Contributor Author

abhat commented Dec 3, 2019

/test unit

@squeed
Copy link
Contributor

squeed commented Dec 9, 2019

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhat, danwinship, knobunc, squeed

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

@bparees bparees added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Dec 12, 2019
@openshift-merge-robot openshift-merge-robot merged commit 606920e into openshift:release-4.1 Dec 13, 2019
@openshift-ci-robot
Copy link

@abhat: All pull requests linked via external trackers have merged. Bugzilla bug 1759182 has been moved to the MODIFIED state.

In response to this:

Bug 1759182: Add a RBAC checker for external IP ranger

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stlaz stlaz stlaz left review comments

@danwinship danwinship Awaiting requested review from danwinship

@knobunc knobunc Awaiting requested review from knobunc

@squeed squeed Awaiting requested review from squeed

Assignees

@danwinship danwinship

@squeed squeed

@stlaz stlaz

@knobunc knobunc

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet