Skip to content

UPSTREAM: 56408: admission: do not leak admission plugin config types outside of the plugin#18111

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sttts:sttts-admission-config-type-leak
Jan 17, 2018
Merged

UPSTREAM: 56408: admission: do not leak admission plugin config types outside of the plugin#18111
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sttts:sttts-admission-config-type-leak

Conversation

@sttts
Copy link
Contributor

@sttts sttts commented Jan 15, 2018

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 15, 2018
@openshift-merge-robot openshift-merge-robot added needs-api-review vendor-update Touching vendor dir or related files labels Jan 15, 2018
@sttts sttts added kind/bug Categorizes issue or PR as related to a bug. sig/master labels Jan 15, 2018
@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

I expected ripples into the openshift code. We don't register anything?

@sttts
Copy link
Contributor Author

sttts commented Jan 15, 2018

I expected ripples into the openshift code. We don't register anything?

Me too. Haven't looked into details, but it compiles. And after https://github.com/openshift/origin/pull/18111/files#diff-993f8edd422e7a353de4d88a20ceb669L45 the ConfigScheme is gone. So it looks like we didn't register anything. Which leads to the question: what do we test about admission configuration?

@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

Which leads to the question: what do we test about admission configuration?

Integration tests.

@deads2k
Copy link
Contributor

deads2k commented Jan 15, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2018
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 15, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, sttts

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2018
@deads2k deads2k mentioned this pull request Jan 15, 2018
Closed
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 16, 2018

/retest

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

@sttts
Copy link
Contributor Author

sttts commented Jan 16, 2018

/retest

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 16, 2018

/retest

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

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Jan 17, 2018

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit 80aa82c into openshift:master Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@enj enj Awaiting requested review from enj

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@mfojtik mfojtik

@deads2k deads2k

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. needs-api-review sig/master size/L Denotes a PR that changes 100-499 lines, ignoring generated files. vendor-update Touching vendor dir or related files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sttts @deads2k @openshift-ci-robot @openshift-bot @openshift-merge-robot @mfojtik