Add support for transport TLS certificate other/common name suffix #5189
Conversation
| // CommonNameSuffix when defined will be prefixed with the Pod name and used as the common name, | ||
| // and the first DNSName as well an OtherName required by Elasticsearch in the Subject Alternative Name extension of | ||
| // each Elasticsearch node's transport TLS certificate. | ||
| CommonNameSuffix string |
There was a problem hiding this comment.
Naming: should this be called OtherNameSuffix because this is actually the relevant one (CommonName is only populated for legacy reasons)
There was a problem hiding this comment.
Yeah I think that makes sense to go with otherName 👍
| if userConfiguredSuffix == "" { | ||
| return fmt.Sprintf("%s.node.%s.%s.es.local", pod.Name, es.Name, es.Namespace) | ||
| } | ||
| return fmt.Sprintf("%s.%s", pod.Name, userConfiguredSuffix) |
There was a problem hiding this comment.
Do we want to handle user error here: e.g. .my.suffix vs my.suffix and try to do "the right thing"?
There was a problem hiding this comment.
Hmm good question. I think it's best to not do any magic here (if you manipulate this field you probably know what you're doing).
There was a problem hiding this comment.
I initially thought we would not even set the . in between by default.
But I don't know if there'd be any case where we wouldn't want a . there.
| if userConfiguredSuffix == "" { | ||
| return fmt.Sprintf("%s.node.%s.%s.es.local", pod.Name, es.Name, es.Namespace) | ||
| } | ||
| return fmt.Sprintf("%s.%s", pod.Name, userConfiguredSuffix) |
There was a problem hiding this comment.
Hmm good question. I think it's best to not do any magic here (if you manipulate this field you probably know what you're doing).
| // CommonNameSuffix when defined will be prefixed with the Pod name and used as the common name, | ||
| // and the first DNSName as well an OtherName required by Elasticsearch in the Subject Alternative Name extension of | ||
| // each Elasticsearch node's transport TLS certificate. | ||
| CommonNameSuffix string |
There was a problem hiding this comment.
Yeah I think that makes sense to go with otherName 👍
Co-authored-by: Sebastien Guilloux <contact.sebgl@gmail.com>
…lastic#5189) Add a new option called otherNameSuffix to allow users to influence the name construction in the OtherName SAN extension of Elasticsearch node certificates for the transport layer. Co-authored-by: Sebastien Guilloux <contact.sebgl@gmail.com>
…lastic#5189) Add a new option called otherNameSuffix to allow users to influence the name construction in the OtherName SAN extension of Elasticsearch node certificates for the transport layer. Co-authored-by: Sebastien Guilloux <contact.sebgl@gmail.com>
Fixes #5148