Add support for multiple alias and remove duplication of SSL certificates (#4472)

This commit is contained in:
Manuel Alejandro de Brito Fontes 2019-08-26 10:58:44 -04:00 committed by GitHub
parent 4847bb02f0
commit 8def5ef7ca
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 190 additions and 101 deletions

View file

@ -27,6 +27,7 @@ resty \
-I ./rootfs/etc/nginx/lua \
--shdict "configuration_data 5M" \
--shdict "certificate_data 16M" \
--shdict "certificate_servers 1M" \
--shdict "balancer_ewma 1M" \
--shdict "balancer_ewma_last_touched_at 1M" \
--shdict "balancer_ewma_locks 512k" \

View file

@ -331,13 +331,13 @@ Enables automatic conversion of preload links specified in the “Link” respon
### Server Alias
To add Server Aliases to an Ingress rule add the annotation `nginx.ingress.kubernetes.io/server-alias: "<alias>"`.
This will create a server with the same configuration, but a different `server_name` as the provided host.
Allows the definition of one or more aliases in the server definition of the NGINX configuration using the annotation `nginx.ingress.kubernetes.io/server-alias: "<alias 1>,<alias 2>"`.
This will create a server with the same configuration, but adding new values to the `server_name` directive.
!!! Note
A server-alias name cannot conflict with the hostname of an existing server. If it does the server-alias annotation will be ignored.
If a server-alias is created and later a new server with the same hostname is created,
the new server configuration will take place over the alias configuration.
A server-alias name cannot conflict with the hostname of an existing server. If it does, the server-alias annotation will be ignored.
If a server-alias is created and later a new server with the same hostname is created, the new server configuration will take
place over the alias configuration.
For more information please see [the `server_name` documentation](http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name).

View file

@ -17,7 +17,11 @@ limitations under the License.
package alias
import (
"sort"
"strings"
networking "k8s.io/api/networking/v1beta1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"
@ -35,5 +39,25 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
// Parse parses the annotations contained in the ingress rule
// used to add an alias to the provided hosts
func (a alias) Parse(ing *networking.Ingress) (interface{}, error) {
return parser.GetStringAnnotation("server-alias", ing)
val, err := parser.GetStringAnnotation("server-alias", ing)
if err != nil {
return []string{}, err
}
aliases := sets.NewString()
for _, alias := range strings.Split(val, ",") {
alias = strings.TrimSpace(alias)
if len(alias) == 0 {
continue
}
if !aliases.Has(alias) {
aliases.Insert(alias)
}
}
l := aliases.List()
sort.Strings(l)
return l, nil
}

View file

@ -17,6 +17,7 @@ limitations under the License.
package alias
import (
"reflect"
"testing"
api "k8s.io/api/core/v1"
@ -36,14 +37,15 @@ func TestParse(t *testing.T) {
testCases := []struct {
annotations map[string]string
expected string
expected []string
}{
{map[string]string{annotation: "www.example.com"}, "www.example.com"},
{map[string]string{annotation: "*.example.com www.example.*"}, "*.example.com www.example.*"},
{map[string]string{annotation: `~^www\d+\.example\.com$`}, `~^www\d+\.example\.com$`},
{map[string]string{annotation: ""}, ""},
{map[string]string{}, ""},
{nil, ""},
{map[string]string{annotation: "a.com, b.com, , c.com"}, []string{"a.com", "b.com", "c.com"}},
{map[string]string{annotation: "www.example.com"}, []string{"www.example.com"}},
{map[string]string{annotation: "*.example.com,www.example.*"}, []string{"*.example.com", "www.example.*"}},
{map[string]string{annotation: `~^www\d+\.example\.com$`}, []string{`~^www\d+\.example\.com$`}},
{map[string]string{annotation: ""}, []string{}},
{map[string]string{}, []string{}},
{nil, []string{}},
}
ing := &networking.Ingress{
@ -57,7 +59,7 @@ func TestParse(t *testing.T) {
for _, testCase := range testCases {
ing.SetAnnotations(testCase.annotations)
result, _ := ap.Parse(ing)
if result != testCase.expected {
if !reflect.DeepEqual(result, testCase.expected) {
t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations)
}
}

View file

@ -74,7 +74,7 @@ const DeniedKeyName = "Denied"
type Ingress struct {
metav1.ObjectMeta
BackendProtocol string
Alias string
Aliases []string
BasicDigestAuth auth.Config
Canary canary.Config
CertificateAuth authtls.Config
@ -124,7 +124,7 @@ type Extractor struct {
func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
return Extractor{
map[string]parser.IngressAnnotation{
"Alias": alias.NewParser(cfg),
"Aliases": alias.NewParser(cfg),
"BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg),
"Canary": canary.NewParser(cfg),
"CertificateAuth": authtls.NewParser(cfg),

View file

@ -401,8 +401,11 @@ func (n *NGINXController) getConfiguration(ingresses []*ingress.Ingress) (sets.S
if !hosts.Has(server.Hostname) {
hosts.Insert(server.Hostname)
}
if server.Alias != "" && !hosts.Has(server.Alias) {
hosts.Insert(server.Alias)
for _, alias := range server.Aliases {
if !hosts.Has(alias) {
hosts.Insert(alias)
}
}
if !server.SSLPassthrough {
@ -931,7 +934,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
du *ingress.Backend) map[string]*ingress.Server {
servers := make(map[string]*ingress.Server, len(data))
aliases := make(map[string]string, len(data))
allAliases := make(map[string][]string, len(data))
bdef := n.store.GetDefaultBackend()
ngxProxy := proxy.Config{
@ -1061,16 +1064,13 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
host = defServerName
}
if anns.Alias != "" {
if servers[host].Alias == "" {
servers[host].Alias = anns.Alias
if _, ok := aliases["Alias"]; !ok {
aliases["Alias"] = host
}
} else {
klog.Warningf("Aliases already configured for server %q, skipping (Ingress %q)",
host, ingKey)
if len(servers[host].Aliases) == 0 {
servers[host].Aliases = anns.Aliases
if _, ok := allAliases[host]; !ok {
allAliases[host] = anns.Aliases
}
} else {
klog.Warningf("Aliases already configured for server %q, skipping (Ingress %q)", host, ingKey)
}
if anns.ServerSnippet != "" {
@ -1133,10 +1133,12 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
}
}
for alias, host := range aliases {
if _, ok := servers[alias]; ok {
klog.Warningf("Conflicting hostname (%v) and alias (%v). Removing alias to avoid conflicts.", host, alias)
servers[host].Alias = ""
for host, hostAliases := range allAliases {
for index, alias := range hostAliases {
if _, ok := servers[alias]; ok {
klog.Warningf("Conflicting hostname (%v) and alias (%v). Removing alias to avoid conflicts.", host, alias)
servers[host].Aliases = append(servers[host].Aliases[:index], servers[host].Aliases[index+1:]...)
}
}
}

View file

@ -991,30 +991,38 @@ func configureBackends(rawBackends []*ingress.Backend) error {
return nil
}
type sslConfiguration struct {
Certificates map[string]string `json:"certificates"`
Servers map[string]string `json:"servers"`
}
// configureCertificates JSON encodes certificates and POSTs it to an internal HTTP endpoint
// that is handled by Lua
func configureCertificates(rawServers []*ingress.Server) error {
servers := make([]*ingress.Server, 0)
configuration := &sslConfiguration{
Certificates: map[string]string{},
Servers: map[string]string{},
}
for _, server := range rawServers {
if server.SSLCert == nil {
for _, rawServer := range rawServers {
if rawServer.SSLCert == nil {
continue
}
servers = append(servers, &ingress.Server{
Hostname: server.Hostname,
SSLCert: &ingress.SSLCert{
PemCertKey: server.SSLCert.PemCertKey,
},
})
uid := rawServer.SSLCert.UID
if server.Alias != "" && ssl.IsValidHostname(server.Alias, server.SSLCert.CN) {
servers = append(servers, &ingress.Server{
Hostname: server.Alias,
SSLCert: &ingress.SSLCert{
PemCertKey: server.SSLCert.PemCertKey,
},
})
if _, ok := configuration.Certificates[uid]; !ok {
configuration.Certificates[uid] = rawServer.SSLCert.PemCertKey
}
configuration.Servers[rawServer.Hostname] = uid
for _, alias := range rawServer.Aliases {
if !ssl.IsValidHostname(alias, rawServer.SSLCert.CN) {
continue
}
configuration.Servers[alias] = uid
}
}
@ -1024,15 +1032,14 @@ func configureCertificates(rawServers []*ingress.Server) error {
continue
}
servers = append(servers, &ingress.Server{
Hostname: redirect.From,
SSLCert: &ingress.SSLCert{
PemCertKey: redirect.SSLCert.PemCertKey,
},
})
configuration.Servers[redirect.From] = redirect.SSLCert.UID
if _, ok := configuration.Certificates[redirect.SSLCert.UID]; !ok {
configuration.Certificates[redirect.SSLCert.UID] = redirect.SSLCert.PemCertKey
}
}
statusCode, _, err := nginx.NewPostStatusRequest("/configuration/servers", "application/json", servers)
statusCode, _, err := nginx.NewPostStatusRequest("/configuration/servers", "application/json", configuration)
if err != nil {
return err
}

View file

@ -185,7 +185,7 @@ func TestConfigureDynamically(t *testing.T) {
}
body := string(b)
endpointStats[r.URL.Path] += 1
endpointStats[r.URL.Path]++
switch r.URL.Path {
case "/configuration/backends":
@ -206,7 +206,7 @@ func TestConfigureDynamically(t *testing.T) {
}
case "/configuration/servers":
{
if !strings.Contains(body, "[]") {
if !strings.Contains(body, `{"certificates":{},"servers":{}}`) {
t.Errorf("controllerPodsCount should be present in JSON content: %v", body)
}
}
@ -337,6 +337,7 @@ func TestConfigureCertificates(t *testing.T) {
Hostname: "myapp.fake",
SSLCert: &ingress.SSLCert{
PemCertKey: "fake-cert",
UID: "c89a5111-b2e9-4af8-be19-c2a4a924c256",
},
}}
@ -354,18 +355,18 @@ func TestConfigureCertificates(t *testing.T) {
if err != nil && err != io.EOF {
t.Fatal(err)
}
var postedServers []ingress.Server
err = jsoniter.ConfigCompatibleWithStandardLibrary.Unmarshal(b, &postedServers)
var conf sslConfiguration
err = jsoniter.ConfigCompatibleWithStandardLibrary.Unmarshal(b, &conf)
if err != nil {
t.Fatal(err)
}
if len(servers) != len(postedServers) {
if len(servers) != len(conf.Servers) {
t.Errorf("Expected servers to be the same length as the posted servers")
}
for i, server := range servers {
if !server.Equal(&postedServers[i]) {
for _, server := range servers {
if server.SSLCert.UID != conf.Servers[server.Hostname] {
t.Errorf("Expected servers and posted servers to be equal")
}
}

View file

@ -99,7 +99,7 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error
return nil, fmt.Errorf("key 'tls.key' missing from Secret %q", secretName)
}
sslCert, err = ssl.CreateSSLCert(cert, key)
sslCert, err = ssl.CreateSSLCert(cert, key, string(secret.UID))
if err != nil {
return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err)
}

View file

@ -70,6 +70,7 @@ var (
"balancer_ewma": 10,
"balancer_ewma_last_touched_at": 10,
"balancer_ewma_locks": 1,
"certificate_servers": 5,
}
)

View file

@ -52,6 +52,9 @@ type SSLCert struct {
// Pem encoded certificate and key concatenated
PemCertKey string `json:"pemCertKey,omitempty"`
// UID unique identifier of the Kubernetes Secret
UID string `json:"uid"`
}
// GetObjectKind implements the ObjectKind interface as a noop

View file

@ -184,8 +184,8 @@ type Server struct {
SSLCert *SSLCert `json:"sslCert"`
// Locations list of URIs configured in the server.
Locations []*Location `json:"locations,omitempty"`
// Alias return the alias of the server name
Alias string `json:"alias,omitempty"`
// Aliases return the alias of the server name
Aliases []string `json:"aliases,omitempty"`
// RedirectFromToWWW returns if a redirect to/from prefix www is required
RedirectFromToWWW bool `json:"redirectFromToWWW,omitempty"`
// CertificateAuth indicates the this server requires mutual authentication

View file

@ -269,9 +269,24 @@ func (s1 *Server) Equal(s2 *Server) bool {
if !(s1.SSLCert).Equal(s2.SSLCert) {
return false
}
if s1.Alias != s2.Alias {
if len(s1.Aliases) != len(s2.Aliases) {
return false
}
for _, a1 := range s1.Aliases {
found := false
for _, a2 := range s2.Aliases {
if a1 == a2 {
found = true
break
}
}
if !found {
return false
}
}
if s1.RedirectFromToWWW != s2.RedirectFromToWWW {
return false
}
@ -528,6 +543,9 @@ func (s1 *SSLCert) Equal(s2 *SSLCert) bool {
if s1.PemCertKey != s2.PemCertKey {
return false
}
if s1.UID != s2.UID {
return false
}
return sets.StringElementsMatch(s1.CN, s2.CN)
}

View file

@ -45,6 +45,10 @@ import (
"k8s.io/klog"
)
// FakeSSLCertificateUID defines the default UID to use for the fake SSL
// certificate generated by the ingress controller
var FakeSSLCertificateUID = "00000000-0000-0000-0000-000000000000"
var (
oidExtensionSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17}
)
@ -75,7 +79,7 @@ func verifyPemCertAgainstRootCA(pemCert *x509.Certificate, ca []byte) error {
}
// CreateSSLCert validates cert and key, extracts common names and returns corresponding SSLCert object
func CreateSSLCert(cert, key []byte) (*ingress.SSLCert, error) {
func CreateSSLCert(cert, key []byte, uid string) (*ingress.SSLCert, error) {
var pemCertBuffer bytes.Buffer
pemCertBuffer.Write(cert)
@ -139,6 +143,7 @@ func CreateSSLCert(cert, key []byte) (*ingress.SSLCert, error) {
CN: cn.List(),
ExpireTime: pemCert.NotAfter,
PemCertKey: pemCertBuffer.String(),
UID: uid,
}, nil
}
@ -341,7 +346,7 @@ func AddOrUpdateDHParam(name string, dh []byte) (string, error) {
func GetFakeSSLCert() *ingress.SSLCert {
cert, key := getFakeHostSSLCert("ingress.local")
sslCert, err := CreateSSLCert(cert, key)
sslCert, err := CreateSSLCert(cert, key, FakeSSLCertificateUID)
if err != nil {
klog.Fatalf("unexpected error creating fake SSL Cert: %v", err)
}

View file

@ -79,7 +79,7 @@ func TestStoreSSLCertOnDisk(t *testing.T) {
c := encodeCertPEM(cert.Cert)
k := encodePrivateKeyPEM(cert.Key)
sslCert, err := CreateSSLCert(c, k)
sslCert, err := CreateSSLCert(c, k, FakeSSLCertificateUID)
if err != nil {
t.Fatalf("unexpected error creating SSL certificate: %v", err)
}
@ -114,7 +114,7 @@ func TestCACert(t *testing.T) {
k := encodePrivateKeyPEM(cert.Key)
ca := encodeCertPEM(CA.Cert)
sslCert, err := CreateSSLCert(c, k)
sslCert, err := CreateSSLCert(c, k, FakeSSLCertificateUID)
if err != nil {
t.Fatalf("unexpected error creating SSL certificate: %v", err)
}
@ -197,7 +197,7 @@ func TestCreateSSLCert(t *testing.T) {
c := encodeCertPEM(cert.Cert)
k := encodePrivateKeyPEM(cert.Key)
sslCert, err := CreateSSLCert(c, k)
sslCert, err := CreateSSLCert(c, k, FakeSSLCertificateUID)
if err != nil {
t.Fatalf("unexpected error checking SSL certificate: %v", err)
}

View file

@ -3,6 +3,7 @@ local cjson = require("cjson.safe")
-- this is the Lua representation of Configuration struct in internal/ingress/types.go
local configuration_data = ngx.shared.configuration_data
local certificate_data = ngx.shared.certificate_data
local certificate_servers = ngx.shared.certificate_servers
local _M = {}
@ -35,7 +36,12 @@ local function fetch_request_body()
end
function _M.get_pem_cert_key(hostname)
return certificate_data:get(hostname)
local uid = certificate_servers:get(hostname)
if not uid then
return nil
end
return certificate_data:get(uid)
end
local function handle_servers()
@ -45,30 +51,39 @@ local function handle_servers()
return
end
local raw_servers = fetch_request_body()
local raw_configuration = fetch_request_body()
local servers, err = cjson.decode(raw_servers)
if not servers then
ngx.log(ngx.ERR, "could not parse servers: ", err)
local configuration, err = cjson.decode(raw_configuration)
if not configuration then
ngx.log(ngx.ERR, "could not parse configuration: ", err)
ngx.status = ngx.HTTP_BAD_REQUEST
return
end
local err_buf = {}
for _, server in ipairs(servers) do
if server.hostname and server.sslCert.pemCertKey then
local success, set_err, forcible = certificate_data:set(server.hostname, server.sslCert.pemCertKey)
if not success then
local err_msg = string.format("error setting certificate for %s: %s\n", server.hostname, tostring(set_err))
table.insert(err_buf, err_msg)
end
if forcible then
local msg = string.format("certificate_data dictionary is full, LRU entry has been removed to store %s",
server.hostname)
ngx.log(ngx.WARN, msg)
end
else
ngx.log(ngx.WARN, "hostname or pemCertKey are not present")
for server, uid in pairs(configuration.servers) do
local success, set_err, forcible = certificate_servers:set(server, uid)
if not success then
local err_msg = string.format("error setting certificate for %s: %s\n", server, tostring(set_err))
table.insert(err_buf, err_msg)
end
if forcible then
local msg = string.format("certificate_servers dictionary is full, LRU entry has been removed to store %s",
server)
ngx.log(ngx.WARN, msg)
end
end
for uid, cert in pairs(configuration.certificates) do
local success, set_err, forcible = certificate_data:set(uid, cert)
if not success then
local err_msg = string.format("error setting certificate for %s: %s\n", uid, tostring(set_err))
table.insert(err_buf, err_msg)
end
if forcible then
local msg = string.format("certificate_data dictionary is full, LRU entry has been removed to store %s", uid)
ngx.log(ngx.WARN, msg)
end
end

View file

@ -11,6 +11,8 @@ end
local EXAMPLE_CERT = read_file("rootfs/etc/nginx/lua/test/fixtures/example-com-cert.pem")
local DEFAULT_CERT = read_file("rootfs/etc/nginx/lua/test/fixtures/default-cert.pem")
local DEFAULT_CERT_HOSTNAME = "_"
local UUID = "2ea8adb5-8ebb-4b14-a79b-0cdcd892e884"
local DEFAULT_UUID = "00000000-0000-0000-0000-000000000000"
local function assert_certificate_is_set(cert)
spy.on(ngx, "log")
@ -45,50 +47,57 @@ describe("Certificate", function()
ngx.exit = function(status) end
ngx.shared.certificate_data:set(DEFAULT_CERT_HOSTNAME, DEFAULT_CERT)
ngx.shared.certificate_servers:set(DEFAULT_CERT_HOSTNAME, DEFAULT_UUID)
ngx.shared.certificate_data:set(DEFAULT_UUID, DEFAULT_CERT)
end)
after_each(function()
ngx = unmocked_ngx
ngx.shared.certificate_data:flush_all()
ngx.shared.certificate_servers:flush_all()
end)
it("sets certificate and key when hostname is found in dictionary", function()
ngx.shared.certificate_data:set("hostname", EXAMPLE_CERT)
ngx.shared.certificate_servers:set("hostname", UUID)
ngx.shared.certificate_data:set(UUID, EXAMPLE_CERT)
assert_certificate_is_set(EXAMPLE_CERT)
end)
it("sets certificate and key for wildcard cert", function()
ssl.server_name = function() return "sub.hostname", nil end
ngx.shared.certificate_data:set("*.hostname", EXAMPLE_CERT)
ngx.shared.certificate_servers:set("*.hostname", UUID)
ngx.shared.certificate_data:set(UUID, EXAMPLE_CERT)
assert_certificate_is_set(EXAMPLE_CERT)
end)
it("sets certificate and key for domain with trailing dot", function()
ssl.server_name = function() return "hostname.", nil end
ngx.shared.certificate_data:set("hostname", EXAMPLE_CERT)
ngx.shared.certificate_servers:set("hostname", UUID)
ngx.shared.certificate_data:set(UUID, EXAMPLE_CERT)
assert_certificate_is_set(EXAMPLE_CERT)
end)
it("fallbacks to default certificate and key for domain with many trailing dots", function()
ssl.server_name = function() return "hostname..", nil end
ngx.shared.certificate_data:set("hostname", EXAMPLE_CERT)
ngx.shared.certificate_servers:set("hostname", UUID)
ngx.shared.certificate_data:set(UUID, EXAMPLE_CERT)
assert_certificate_is_set(DEFAULT_CERT)
end)
it("sets certificate and key for nested wildcard cert", function()
ssl.server_name = function() return "sub.nested.hostname", nil end
ngx.shared.certificate_data:set("*.nested.hostname", EXAMPLE_CERT)
ngx.shared.certificate_servers:set("*.nested.hostname", UUID)
ngx.shared.certificate_data:set(UUID, EXAMPLE_CERT)
assert_certificate_is_set(EXAMPLE_CERT)
end)
it("logs error message when certificate in dictionary is invalid", function()
ngx.shared.certificate_data:set("hostname", "something invalid")
ngx.shared.certificate_servers:set("hostname", "something invalid")
spy.on(ngx, "log")
@ -108,7 +117,8 @@ describe("Certificate", function()
end)
it("fails when hostname does not have certificate and default cert is invalid", function()
ngx.shared.certificate_data:set(DEFAULT_CERT_HOSTNAME, "invalid")
ngx.shared.certificate_servers:set(DEFAULT_CERT_HOSTNAME, UID)
ngx.shared.certificate_data:set(UID, "invalid")
spy.on(ngx, "log")

View file

@ -506,7 +506,7 @@ http {
## start server {{ $server.Hostname }}
server {
server_name {{ $server.Hostname }} {{ $server.Alias }};
server_name {{ $server.Hostname }} {{range $server.Aliases }}{{ . }} {{ end }};
{{ if gt (len $cfg.BlockUserAgents) 0 }}
if ($block_ua) {