Skip to content

Support ALB SslPolicy via a new annotation #415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/ingress-resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ alb.ingress.kubernetes.io/tags
alb.ingress.kubernetes.io/target-group-attributes
alb.ingress.kubernetes.io/ignore-host-header
alb.ingress.kubernetes.io/ip-address-type
alb.ingress.kubernetes.io/ssl-policy
```

Optional annotations are:
Expand Down Expand Up @@ -117,3 +118,5 @@ Optional annotations are:
- **ignore-host-header**: Creates routing rules without [Host Header Checks](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-listeners.html#host-conditions).

- **ip-address-type**: The IP address type thats used to either route IPv4 traffic only or to route both IPv4 and IPv6 traffic. Can be either `dualstack` or `ipv4`. When omitted `ipv4` is used.

- **ssl-policy**: Defines the [Security Policy](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/create-https-listener.html#describe-ssl-policies) that should be assigned to the ALB, allowing you to control the protocol and ciphers.
10 changes: 10 additions & 0 deletions pkg/alb/listener/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type NewDesiredListenerOptions struct {
Port annotations.PortData
CertificateArn *string
Logger *log.Logger
SslPolicy *string
}

type ReconcileOptions struct {
Expand Down Expand Up @@ -52,6 +53,10 @@ func NewDesiredListener(o *NewDesiredListenerOptions) *Listener {
listener.Protocol = aws.String("HTTPS")
}

if o.SslPolicy != nil && o.Port.Scheme == "HTTPS" {
listener.SslPolicy = o.SslPolicy
}

listenerT := &Listener{
Desired: listener,
Logger: o.Logger,
Expand Down Expand Up @@ -132,6 +137,7 @@ func (l *Listener) create(rOpts *ReconcileOptions) error {
LoadBalancerArn: l.Desired.LoadBalancerArn,
Protocol: l.Desired.Protocol,
Port: l.Desired.Port,
SslPolicy: l.Desired.SslPolicy,
DefaultActions: []*elbv2.Action{
{
Type: l.Desired.DefaultActions[0].Type,
Expand Down Expand Up @@ -221,6 +227,8 @@ func (l *Listener) NeedsModification(target *elbv2.Listener, rOpts *ReconcileOpt
return true
case !util.DeepEqual(l.Current.DefaultActions, target.DefaultActions):
return true
case !util.DeepEqual(l.Current.SslPolicy, target.SslPolicy):
return true
}
return false
}
Expand All @@ -241,6 +249,8 @@ func (l *Listener) NeedsModificationCheck(target *elbv2.Listener) bool {
return true
case !util.DeepEqual(l.Current.DefaultActions, target.DefaultActions):
return true
case !util.DeepEqual(l.Current.SslPolicy, target.SslPolicy):
return true
}
return false
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/alb/listener/listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func setup() {
Type: aws.String("default"),
TargetGroupArn: aws.String(newTg),
}},
SslPolicy: aws.String("ELBSecurityPolicy-TLS-1-2-2017-01"),
}
}

Expand Down Expand Up @@ -96,9 +97,11 @@ func TestNewHTTPListener(t *testing.T) {
func TestNewHTTPSListener(t *testing.T) {
desiredPort := int64(443)
desiredCertArn := aws.String("abc123")
desiredSslPolicy := aws.String("ELBSecurityPolicy-Test")
o := &NewDesiredListenerOptions{
Port: annotations.PortData{desiredPort, "HTTPS"},
CertificateArn: desiredCertArn,
SslPolicy: desiredSslPolicy,
Logger: logr,
}

Expand All @@ -118,6 +121,9 @@ func TestNewHTTPSListener(t *testing.T) {
case *l.Desired.Certificates[0].CertificateArn != *desiredCertArn:
t.Errorf("Invalid certificate ARN. Actual: %s | Expected: %s",
*l.Desired.Certificates[0].CertificateArn, *desiredCertArn)
case *l.Desired.SslPolicy != *desiredSslPolicy:
t.Errorf("Invalid certificate SSL Policy. Actual: %s | Expected: %s",
*l.Desired.SslPolicy, *desiredSslPolicy)
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/alb/listeners/listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func NewDesiredListeners(o *NewDesiredListenersOptions) (Listeners, error) {
Port: port,
CertificateArn: o.Annotations.CertificateArn,
Logger: o.Logger,
SslPolicy: o.Annotations.SslPolicy,
})

if thisListener != nil {
Expand Down
13 changes: 13 additions & 0 deletions pkg/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const (
inboundCidrsKey = "alb.ingress.kubernetes.io/security-group-inbound-cidrs"
portKey = "alb.ingress.kubernetes.io/listen-ports"
schemeKey = "alb.ingress.kubernetes.io/scheme"
sslPolicyKey = "alb.ingress.kubernetes.io/ssl-policy"
ipAddressTypeKey = "alb.ingress.kubernetes.io/ip-address-type"
securityGroupsKey = "alb.ingress.kubernetes.io/security-groups"
subnetsKey = "alb.ingress.kubernetes.io/subnets"
Expand Down Expand Up @@ -77,6 +78,7 @@ type Annotations struct {
Tags []*elbv2.Tag
IgnoreHostHeader *bool
TargetGroupAttributes albelbv2.TargetGroupAttributes
SslPolicy *string
VPCID *string
Attributes []*elbv2.LoadBalancerAttribute
}
Expand Down Expand Up @@ -142,6 +144,7 @@ func (vf ValidatingAnnotationFactory) ParseAnnotations(ingress *extensions.Ingre
a.setWafAclId(annotations, vf.validator),
a.setAttributes(annotations),
a.setTargetGroupAttributes(annotations),
a.setSslPolicy(annotations, vf.validator),
} {
if err != nil {
cache.Set(cacheKey, err, 1*time.Hour)
Expand Down Expand Up @@ -701,6 +704,16 @@ func (a *Annotations) setWafAclId(annotations map[string]string, validator Valid
return nil
}

func (a *Annotations) setSslPolicy(annotations map[string]string, validator Validator) error {
if sslPolicy, ok := annotations[sslPolicyKey]; ok {
a.SslPolicy = aws.String(sslPolicy)
if err := validator.ValidateSslPolicy(a); err != nil {
return err
}
}
return nil
}

func cacheLookup(key string) *ccache.Item {
i := cache.Get(key)
if i == nil || i.Expired() {
Expand Down
27 changes: 27 additions & 0 deletions pkg/annotations/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,33 @@ func TestSetIgnoreHostHeader(t *testing.T) {
}
}

func TestSetSslPolicy(t *testing.T) {
var tests = []struct {
SslPolicy string
expected string
pass bool
}{
{"", "", true}, // ip-address-type has a sane default
{"ELBSecurityPolicy-TLS-1-2-2017-01", "", false},
{"ELBSecurityPolicy-TLS-1-2-2017-01", "ELBSecurityPolicy-TLS-1-2-2017-01", true},
}

for _, tt := range tests {
a := &Annotations{}

err := a.setSslPolicy(map[string]string{sslPolicyKey: tt.SslPolicy}, fakeValidator())
if err != nil && tt.pass {
t.Errorf("setIpAddressType(%v): expected %v, actual %v", tt.SslPolicy, tt.pass, err)
}
if err == nil && tt.pass && tt.expected != *a.SslPolicy {
t.Errorf("setIpAddressType(%v): expected %v, actual %v", tt.SslPolicy, tt.expected, *a.SslPolicy)
}
if err == nil && !tt.pass && tt.expected == *a.SslPolicy {
t.Errorf("setIpAddressType(%v): expected %v, actual %v", tt.SslPolicy, tt.expected, *a.SslPolicy)
}
}
}

// Should fail to create due to healthchecktimeout being greater than HealthcheckIntervalSeconds
func TestHealthcheckSecondsValidation(t *testing.T) {
a := &Annotations{}
Expand Down
25 changes: 25 additions & 0 deletions pkg/annotations/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import (
"fmt"
"net"

"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/elbv2"

"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/acm"
albec2 "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/ec2"
albelbv2 "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/elbv2"
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/iam"
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/aws/waf"
"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/config"
Expand All @@ -22,6 +25,7 @@ type Validator interface {
ValidateInboundCidrs(a *Annotations) error
ValidateScheme(a *Annotations, ingressNamespace, ingressName string) bool
ValidateWafAclId(a *Annotations) error
ValidateSslPolicy(a *Annotations) error
}

type ConcreteValidator struct {
Expand Down Expand Up @@ -108,3 +112,24 @@ func (v ConcreteValidator) ValidateWafAclId(a *Annotations) error {
}
return nil
}

func (v ConcreteValidator) ValidateSslPolicy(a *Annotations) error {
in := &elbv2.DescribeSSLPoliciesInput{
Names: []*string{
a.SslPolicy,
},
}
if _, err := albelbv2.ELBV2svc.DescribeSSLPolicies(in); err != nil {
if aerr, ok := err.(awserr.Error); ok {
switch aerr.Code() {
case elbv2.ErrCodeSSLPolicyNotFoundException:
return fmt.Errorf("%s: %s", elbv2.ErrCodeSSLPolicyNotFoundException, aerr.Error())
default:
return fmt.Errorf("Error: %s", aerr.Error())
}
} else {
return fmt.Errorf("Error: %s", aerr.Error())
}
}
return nil
}
8 changes: 8 additions & 0 deletions pkg/annotations/validation_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type FakeValidator struct {
ValidateInboundCidrsDelegate func() error
ValidateSchemeDelegate func() bool
ValidateWafAclIdDelegate func() error
ValidateSslPolicyDelegate func() error
}

func (fv FakeValidator) ResolveVPCValidateSubnets(a *Annotations) error {
Expand Down Expand Up @@ -52,3 +53,10 @@ func (fv FakeValidator) ValidateWafAclId(a *Annotations) error {
}
return nil
}

func (fv FakeValidator) ValidateSslPolicy(a *Annotations) error {
if fv.ValidateSslPolicyDelegate != nil {
return fv.ValidateSslPolicyDelegate()
}
return nil
}