split serving options into substructs #36507
Conversation
| @@ -80,7 +80,9 @@ cluster's shared state through which all other components interact.`, | |||
|
|
|||
| // Run runs the specified APIServer. This should never exit. | |||
| func Run(s *options.ServerRunOptions) error { | |||
| genericvalidation.VerifyEtcdServersList(s.GenericServerRunOptions) | |||
| if errs := s.GenericServerRunOptions.EtcdOptions.Validate(); len(errs) > 0 { | |||
| glog.Fatal(errs) | |||
There was a problem hiding this comment.
Run is of type error. Why not return?
| s.ServiceAccountKeyFiles = []string{s.GenericServerRunOptions.TLSPrivateKeyFile} | ||
| if len(s.ServiceAccountKeyFiles) == 0 && s.GenericServerRunOptions.SecureServingOptions.ServerCert.CertKey.KeyFile != "" { | ||
| if authenticator.IsValidServiceAccountKeyFile(s.GenericServerRunOptions.SecureServingOptions.ServerCert.CertKey.KeyFile) { | ||
| s.ServiceAccountKeyFiles = []string{s.GenericServerRunOptions.SecureServingOptions.ServerCert.CertKey.KeyFile} |
There was a problem hiding this comment.
do we need the ...Options postfix?
There was a problem hiding this comment.
do we need the ...Options postfix?
Removing from instance. I think I want it on the type.
| @@ -55,18 +55,20 @@ func newStorageFactory() genericapiserver.StorageFactory { | |||
| } | |||
|
|
|||
| func NewServerRunOptions() *genericoptions.ServerRunOptions { | |||
| serverOptions := genericoptions.NewServerRunOptions().WithEtcdOptions() | |||
| serverOptions.InsecurePort = InsecurePort | |||
| serverOptions := genericoptions.NewServerRunOptions().WithEtcdOptions().WithSecureServingOptions().WithInsecureServingOptions() | |||
There was a problem hiding this comment.
With...().With...
Do we really want this pattern?
There was a problem hiding this comment.
Do we really want this pattern?
We might. It could actually work for some of our downstream projects. Like I said in the etcd pull. I see that as getting all the pieces ready and then making a choice.
| genericvalidation.ValidateRunOptions(serverOptions) | ||
| genericvalidation.VerifyEtcdServersList(serverOptions) | ||
| if errs := serverOptions.EtcdOptions.Validate(); len(errs) > 0 { | ||
| panic(errs) |
k8s-github-robot
added
size/XL
| @@ -0,0 +1,231 @@ | |||
| /* | |||
There was a problem hiding this comment.
git mv serving_options.go serving.go
|
overall direction 👍 👍 |
deads2k
force-pushed
the
api-39-split-secure-serving
branch
from
7837996
to
3c810c5
Compare
deads2k
force-pushed
the
api-39-split-secure-serving
branch
from
3c810c5
to
84bf5cf
Compare
|
Jenkins GCE Node e2e failed for commit 84bf5cf. Full PR test history. The magic incantation to run this job again is |
|
When you're right, you're right. I'm splitting them out of the main struct to be able manage them separately for the next set of pulls in authentication and authorization. |
| @@ -157,6 +148,10 @@ func (o *ServerRunOptions) WithEtcdOptions() *ServerRunOptions { | |||
| o.Etcd = NewDefaultEtcdOptions() | |||
| return o | |||
| } | |||
| func (o *ServerRunOptions) WithSecureServingOptions() *ServerRunOptions { | |||
There was a problem hiding this comment.
I expected a call to this in NewServerRunOptions. Alternatively, rename this to WithSecureServing**Defaults**.
There was a problem hiding this comment.
I expected a call to this in NewServerRunOptions. Alternatively, rename this to WithSecureServingDefaults.
Gets removed later in this pull.
deads2k
force-pushed
the
api-39-split-secure-serving
branch
from
1b99ffd
to
b283c56
Compare
|
@k8s-bot unit test this |
deads2k
force-pushed
the
api-39-split-secure-serving
branch
from
b283c56
to
b231cdc
Compare
|
Jenkins unit/integration failed for commit b231cdc. Full PR test history. The magic incantation to run this job again is |
|
closing in favor of #36604 |
This continues the work of splitting the options struct into workable pieces by splitting out the secure and insecure serving options.
@sttts @ncdc @kubernetes/sig-api-machinery
This change is