Allow optional SSL client authentication setting #6440
Conversation
rhr323
force-pushed
the
allow-optional-client-authentication-setting
branch
from
e6c30de to
740e013
Compare
|
|
||
| func validateClientAuthentication(config *common.CanonicalConfig, index int) field.ErrorList { | ||
| type ClientAuthSetting struct { | ||
| Value string `config:"xpack.security.http.ssl.client_authentication"` |
There was a problem hiding this comment.
Let me know if there is an easier / generic way to access a setting given the key as string, so we don't need to define a struct for it.
There was a problem hiding this comment.
I think we could expose func (c *Config) String(name string, idx int, opts ...Option) (string, error) in the CanonicalConfig wrapper:
func (c *CanonicalConfig) String(name string) (string, error) {
return c.asUCfg().String(name, -1, Options...)
}
There was a problem hiding this comment.
Does this resolve path separators correctly ? I.e. xpack.security.http... I don't remember.
There was a problem hiding this comment.
Looks like it works as expected! I added the wrapper and a few basic test cases.
rhr323
force-pushed
the
allow-optional-client-authentication-setting
branch
from
740e013 to
e1a26c4
Compare
| @@ -22,7 +22,6 @@ The following Elasticsearch settings are managed by ECK: | |||
| * `xpack.security.authc.reserved_realm.enabled` | |||
| * `xpack.security.enabled` | |||
| * `xpack.security.http.ssl.certificate` | |||
| * `xpack.security.http.ssl.client_authentication` | |||
There was a problem hiding this comment.
I wonder if it's best to leave this our from the list of ECK managed settings, or if we should explain what values are supported / not supported.
There was a problem hiding this comment.
I would probably add new section "Partially restricted settings" and say xpack.security.http.ssl.client_authentication (supported values none, optional)
|
|
||
| func validateClientAuthentication(config *common.CanonicalConfig, index int) field.ErrorList { | ||
| type ClientAuthSetting struct { | ||
| Value string `config:"xpack.security.http.ssl.client_authentication"` |
There was a problem hiding this comment.
I think we could expose func (c *Config) String(name string, idx int, opts ...Option) (string, error) in the CanonicalConfig wrapper:
func (c *CanonicalConfig) String(name string) (string, error) {
return c.asUCfg().String(name, -1, Options...)
}
rhr323
force-pushed
the
allow-optional-client-authentication-setting
branch
from
e1a17a6 to
ff32292
Compare
| * `xpack.security.http.ssl.enabled` | ||
| * `xpack.security.http.ssl.key` | ||
| * `xpack.security.transport.ssl.certificate` | ||
| * `xpack.security.transport.ssl.enabled` | ||
| * `xpack.security.transport.ssl.key` | ||
| * `xpack.security.transport.ssl.verification_mode` | ||
|
|
||
| The following Elasticsearch settings are not supported by ECK: | ||
|
|
||
| * `xpack.security.http.ssl.client_authentication`: `required` |
Elasticsearch supports a few different settings for client authentication settings for https. This PR allows
xpack.security.http.ssl.client_authenticationto be set to eitheroptionalornone, but it will keep emitting an unsupported warning in case it is set torequired.Fixes: #6369