switch our last two special cased admission plugins to config #20699
Conversation
openshift-ci-robot
added
the
size/XL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k 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 |
openshift-ci-robot
added
the
approved
deads2k
force-pushed
the
admission-03-config
branch
from
65e960b to
3eb1736
Compare
| @@ -0,0 +1,14 @@ | |||
| package install | |||
pkg/cmd/server/start/start_master.go
Outdated
| @@ -448,6 +449,11 @@ func (m *Master) Start() error { | |||
| etcdserver.RunEtcd(m.config.EtcdConfig) | |||
| } | |||
|
|
|||
| // convert the networkconfig to admissionconfig | |||
There was a problem hiding this comment.
nit: comment doesn't say more than the code line
| @@ -0,0 +1,4 @@ | |||
| // +k8s:deepcopy-gen=package,register | |||
|
|
|||
| // Package api is the internal version of the API. | |||
| ) | ||
|
|
||
| // SchemeGroupVersion is group version used to register these objects | ||
| var SchemeGroupVersion = schema.GroupVersion{Group: "", Version: "v1"} |
There was a problem hiding this comment.
legacy group name for new apis?
There was a problem hiding this comment.
legacy group name for new apis?
it's consistent with the rest of the config.
| "strings" | ||
|
|
||
| apierrs "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
| admission "k8s.io/apiserver/pkg/admission" | ||
| kapi "k8s.io/kubernetes/pkg/apis/core" | ||
|
|
||
| "github.com/golang/glog" |
| }) | ||
| } | ||
|
|
||
| func readConfig(reader io.Reader) (*externalipranger.ExternalIPRangerAdmissionConfig, error) { | ||
| if reader == nil || reflect.ValueOf(reader).IsNil() { |
There was a problem hiding this comment.
oh, wow. we really still need the second case?
There was a problem hiding this comment.
oh, wow. we really still need the second case?
Not all nils are created equal. As I recall, yes.
There was a problem hiding this comment.
such a nil is a bug somewhere else.
There was a problem hiding this comment.
such a nil is a bug somewhere else.
I won't argue it, but this is consistent with the rest of our admission plugins and I'm disinclined to change them at this juncture.
|
Looks good overall. Missing some test. |
I think the previous unit and integration tests cover it. |
deads2k
force-pushed
the
admission-03-config
branch
2 times, most recently
from
d590d2b to
f156c28
Compare
|
comments addressed. |
|
New changes are detected. LGTM label has been removed. |
deads2k
force-pushed
the
admission-03-config
branch
from
f156c28 to
b6707b5
Compare
We had two admission plugins that had special wiring to plumb configuration without using the "normal" admission config mechanism. This removes those last two by creating the admission config types and then filling them in by synthetically injecting the values in.
@smarterclayton I think these were yours
@openshift/sig-master this is on the path to separate kubeapiserver launch in #20578
/assign @mfojtik @sttts