Skip to content

Commit

Permalink
Merge pull request #716 from jcmoraisjr/jm-secure-ca
Browse files Browse the repository at this point in the history
Add secure-verify-ca-secret annotation
  • Loading branch information
aledbf authored May 17, 2017
2 parents b4032f0 + 8b5a6e7 commit 3dc7717
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 17 deletions.
44 changes: 40 additions & 4 deletions core/pkg/ingress/annotations/secureupstream/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,61 @@ limitations under the License.
package secureupstream

import (
"fmt"
"github.com/pkg/errors"
extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"

"k8s.io/ingress/core/pkg/ingress/annotations/parser"
"k8s.io/ingress/core/pkg/ingress/resolver"
)

const (
secureUpstream = "ingress.kubernetes.io/secure-backends"
secureUpstream = "ingress.kubernetes.io/secure-backends"
secureVerifyCASecret = "ingress.kubernetes.io/secure-verify-ca-secret"
)

// Secure describes SSL backend configuration
type Secure struct {
Secure bool
CACert resolver.AuthSSLCert
}

type su struct {
certResolver resolver.AuthCertificate
}

// NewParser creates a new secure upstream annotation parser
func NewParser() parser.IngressAnnotation {
return su{}
func NewParser(resolver resolver.AuthCertificate) parser.IngressAnnotation {
return su{
certResolver: resolver,
}
}

// Parse parses the annotations contained in the ingress
// rule used to indicate if the upstream servers should use SSL
func (a su) Parse(ing *extensions.Ingress) (interface{}, error) {
return parser.GetBoolAnnotation(secureUpstream, ing)
s, _ := parser.GetBoolAnnotation(secureUpstream, ing)
ca, _ := parser.GetStringAnnotation(secureVerifyCASecret, ing)
secure := &Secure{
Secure: s,
CACert: resolver.AuthSSLCert{},
}
if !s && ca != "" {
return secure,
errors.Errorf("trying to use CA from secret %v/%v on a non secure backend", ing.Namespace, ca)
}
if ca == "" {
return secure, nil
}
caCert, err := a.certResolver.GetAuthCertificate(fmt.Sprintf("%v/%v", ing.Namespace, ca))
if err != nil {
return secure, errors.Wrap(err, "error obtaining certificate")
}
if caCert == nil {
return secure, nil
}
return &Secure{
Secure: s,
CACert: *caCert,
}, nil
}
48 changes: 43 additions & 5 deletions core/pkg/ingress/annotations/secureupstream/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import (
api "k8s.io/client-go/pkg/api/v1"
extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"

"fmt"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/ingress/core/pkg/ingress/resolver"
)

func buildIngress() *extensions.Ingress {
Expand Down Expand Up @@ -61,22 +63,58 @@ func buildIngress() *extensions.Ingress {
}
}

type mockCfg struct {
certs map[string]resolver.AuthSSLCert
}

func (cfg mockCfg) GetAuthCertificate(secret string) (*resolver.AuthSSLCert, error) {
if cert, ok := cfg.certs[secret]; ok {
return &cert, nil
}
return nil, fmt.Errorf("secret not found: %v", secret)
}

func TestAnnotations(t *testing.T) {
ing := buildIngress()
data := map[string]string{}
data[secureUpstream] = "true"
data[secureVerifyCASecret] = "secure-verify-ca"
ing.SetAnnotations(data)

_, err := NewParser().Parse(ing)
_, err := NewParser(mockCfg{
certs: map[string]resolver.AuthSSLCert{
"default/secure-verify-ca": resolver.AuthSSLCert{},
},
}).Parse(ing)
if err != nil {
t.Error("Expected error with ingress without annotations")
t.Errorf("Unexpected error on ingress: %v", err)
}
}

func TestWithoutAnnotations(t *testing.T) {
func TestSecretNotFound(t *testing.T) {
ing := buildIngress()
_, err := NewParser().Parse(ing)
data := map[string]string{}
data[secureUpstream] = "true"
data[secureVerifyCASecret] = "secure-verify-ca"
ing.SetAnnotations(data)
_, err := NewParser(mockCfg{}).Parse(ing)
if err == nil {
t.Error("Expected secret not found error on ingress")
}
}

func TestSecretOnNonSecure(t *testing.T) {
ing := buildIngress()
data := map[string]string{}
data[secureUpstream] = "false"
data[secureVerifyCASecret] = "secure-verify-ca"
ing.SetAnnotations(data)
_, err := NewParser(mockCfg{
certs: map[string]resolver.AuthSSLCert{
"default/secure-verify-ca": resolver.AuthSSLCert{},
},
}).Parse(ing)
if err == nil {
t.Error("Expected error with ingress without annotations")
t.Error("Expected CA secret on non secure backend error on ingress")
}
}
12 changes: 8 additions & 4 deletions core/pkg/ingress/controller/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func newAnnotationExtractor(cfg extractorConfig) annotationExtractor {
"Proxy": proxy.NewParser(cfg),
"RateLimit": ratelimit.NewParser(),
"Redirect": rewrite.NewParser(cfg),
"SecureUpstream": secureupstream.NewParser(),
"SecureUpstream": secureupstream.NewParser(cfg),
"SessionAffinity": sessionaffinity.NewParser(),
"SSLPassthrough": sslpassthrough.NewParser(),
"ConfigurationSnippet": snippet.NewParser(),
Expand Down Expand Up @@ -111,9 +111,13 @@ const (
sessionAffinity = "SessionAffinity"
)

func (e *annotationExtractor) SecureUpstream(ing *extensions.Ingress) bool {
val, _ := e.annotations[secureUpstream].Parse(ing)
return val.(bool)
func (e *annotationExtractor) SecureUpstream(ing *extensions.Ingress) *secureupstream.Secure {
val, err := e.annotations[secureUpstream].Parse(ing)
if err != nil {
glog.Errorf("error parsing secure upstream: %v", err)
}
secure := val.(*secureupstream.Secure)
return secure
}

func (e *annotationExtractor) HealthCheck(ing *extensions.Ingress) *healthcheck.Upstream {
Expand Down
47 changes: 45 additions & 2 deletions core/pkg/ingress/controller/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

const (
annotationSecureUpstream = "ingress.kubernetes.io/secure-backends"
annotationSecureVerifyCACert = "ingress.kubernetes.io/secure-verify-ca-secret"
annotationUpsMaxFails = "ingress.kubernetes.io/upstream-max-fails"
annotationUpsFailTimeout = "ingress.kubernetes.io/upstream-fail-timeout"
annotationPassthrough = "ingress.kubernetes.io/ssl-passthrough"
Expand All @@ -51,7 +52,14 @@ func (m mockCfg) GetSecret(name string) (*api.Secret, error) {
return m.MockSecrets[name], nil
}

func (m mockCfg) GetAuthCertificate(string) (*resolver.AuthSSLCert, error) {
func (m mockCfg) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) {
if secret, _ := m.GetSecret(name); secret != nil {
return &resolver.AuthSSLCert{
Secret: name,
CAFileName: "/opt/ca.pem",
PemSHA: "123",
}, nil
}
return nil, nil
}

Expand Down Expand Up @@ -122,12 +130,47 @@ func TestSecureUpstream(t *testing.T) {
for _, foo := range fooAnns {
ing.SetAnnotations(foo.annotations)
r := ec.SecureUpstream(ing)
if r != foo.er {
if r.Secure != foo.er {
t.Errorf("Returned %v but expected %v", r, foo.er)
}
}
}

func TestSecureVerifyCACert(t *testing.T) {
ec := newAnnotationExtractor(mockCfg{
MockSecrets: map[string]*api.Secret{
"default/secure-verify-ca": {
ObjectMeta: meta_v1.ObjectMeta{
Name: "secure-verify-ca",
},
},
},
})

anns := []struct {
it int
annotations map[string]string
exists bool
}{
{1, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert: "not"}, false},
{2, map[string]string{annotationSecureUpstream: "false", annotationSecureVerifyCACert: "secure-verify-ca"}, false},
{3, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert: "secure-verify-ca"}, true},
{4, map[string]string{annotationSecureUpstream: "true", annotationSecureVerifyCACert + "_not": "secure-verify-ca"}, false},
{5, map[string]string{annotationSecureUpstream: "true"}, false},
{6, map[string]string{}, false},
{7, nil, false},
}

for _, ann := range anns {
ing := buildIngress()
ing.SetAnnotations(ann.annotations)
res := ec.SecureUpstream(ing)
if (res.CACert.CAFileName != "") != ann.exists {
t.Errorf("Expected exists was %v on iteration %v", ann.exists, ann.it)
}
}
}

func TestHealthCheck(t *testing.T) {
ec := newAnnotationExtractor(mockCfg{})
ing := buildIngress()
Expand Down
5 changes: 4 additions & 1 deletion core/pkg/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,10 @@ func (ic *GenericController) createUpstreams(data []interface{}) map[string]*ing
glog.V(3).Infof("creating upstream %v", name)
upstreams[name] = newUpstream(name)
if !upstreams[name].Secure {
upstreams[name].Secure = secUpstream
upstreams[name].Secure = secUpstream.Secure
}
if upstreams[name].SecureCACert.Secret == "" {
upstreams[name].SecureCACert = secUpstream.CACert
}
if upstreams[name].SessionAffinity.AffinityType == "" {
upstreams[name].SessionAffinity.AffinityType = affinity.AffinityType
Expand Down
5 changes: 4 additions & 1 deletion core/pkg/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/ingress/core/pkg/ingress/annotations/ratelimit"
"k8s.io/ingress/core/pkg/ingress/annotations/rewrite"
"k8s.io/ingress/core/pkg/ingress/defaults"
"k8s.io/ingress/core/pkg/ingress/resolver"
"k8s.io/ingress/core/pkg/ingress/store"
)

Expand Down Expand Up @@ -154,8 +155,10 @@ type Backend struct {
// Allowing the use of HTTPS
// The endpoint/s must provide a TLS connection.
// The certificate used in the endpoint cannot be a self signed certificate
// TODO: add annotation to allow the load of ca certificate
Secure bool `json:"secure"`
// SecureCACert has the filename and SHA1 of the certificate authorities used to validate
// a secured connection to the backend
SecureCACert resolver.AuthSSLCert `json:"secureCert"`
// SSLPassthrough indicates that Ingress controller will delegate TLS termination to the endpoints.
SSLPassthrough bool `json:"sslPassthrough"`
// Endpoints contains the list of endpoints currently running
Expand Down

0 comments on commit 3dc7717

Please sign in to comment.