Skip to content
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.

Sign up for GitHub

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

AWS: support mixed plaintext/encrypted ports in ELBs via service.beta.kubernetes.io/aws-load-balancer-ssl-ports annotation #26976

Merged
merged 1 commit into from
Jun 10, 2016
+76 −16
Merged

AWS: support mixed plaintext/encrypted ports in ELBs via service.beta.kubernetes.io/aws-load-balancer-ssl-ports annotation #26976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 40 additions & 4 deletions pkg/cloudprovider/providers/aws/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net/url"
"os"
"regexp"
"strconv"
"strings"
"sync"
"time"
Expand All @@ -48,6 +49,7 @@ import (
"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api/service"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/util/sets"
)

const ProviderName = "aws"
Expand Down Expand Up @@ -79,6 +81,10 @@ const ServiceAnnotationLoadBalancerProxyProtocol = "service.beta.kubernetes.io/a
// CertARN is an IAM or CM certificate ARN, e.g. arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012
const ServiceAnnotationLoadBalancerCertificate = "service.beta.kubernetes.io/aws-load-balancer-ssl-cert"

// Service annotation specifying a comma-separated list of ports that will use SSL/HTTPS
// listeners. Defaults to '*' (all).
const ServiceAnnotationLoadBalancerSSLPorts = "service.beta.kubernetes.io/aws-load-balancer-ssl-ports"

// Service annotation specifying the protocol spoken by the backend (pod) behind a secure listener.
// Only inspected when `aws-load-balancer-ssl-cert` is used.
// If `http` (default) or `https`, an HTTPS listener that terminates the connection and parses headers is created.
Expand Down Expand Up @@ -2095,10 +2101,38 @@ func isSubnetPublic(rt []*ec2.RouteTable, subnetID string) (bool, error) {
return false, nil
}

type portSets struct {
names sets.String
numbers sets.Int64
}

// getPortSets returns a portSets structure representing port names and numbers
// that the comma-separated string describes. If the input is empty or equal to
// "*", a nil pointer is returned.
func getPortSets(annotation string) (ports *portSets) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we tend to avoid the named-retval style. But I'm just repeating what I've been told!

if annotation != "" && annotation != "*" {
ports = &portSets{
sets.NewString(),
sets.NewInt64(),
}
portStringSlice := strings.Split(annotation, ",")
for _, item := range portStringSlice {
port, err := strconv.Atoi(item)
if err != nil {
ports.names.Insert(item)
} else {
ports.numbers.Insert(int64(port))
}
}
}
return
}

// buildListener creates a new listener from the given port, adding an SSL certificate
// if indicated by the appropriate annotations.
func buildListener(port api.ServicePort, annotations map[string]string) (*elb.Listener, error) {
func buildListener(port api.ServicePort, annotations map[string]string, sslPorts *portSets) (*elb.Listener, error) {
loadBalancerPort := int64(port.Port)
portName := strings.ToLower(port.Name)
instancePort := int64(port.NodePort)
protocol := strings.ToLower(string(port.Protocol))
instanceProtocol := protocol
Expand All @@ -2107,7 +2141,7 @@ func buildListener(port api.ServicePort, annotations map[string]string) (*elb.Li
listener.InstancePort = &instancePort
listener.LoadBalancerPort = &loadBalancerPort
certID := annotations[ServiceAnnotationLoadBalancerCertificate]
if certID != "" {
if certID != "" && (sslPorts == nil || sslPorts.numbers.Has(loadBalancerPort) || sslPorts.names.Has(portName)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we could probably move this to a method on portSets (and also avoid the nil check by returning a special value), and also pass in api.ServicePort. But probably for 1.4...

instanceProtocol = annotations[ServiceAnnotationLoadBalancerBEProtocol]
if instanceProtocol == "" {
protocol = "ssl"
Expand All @@ -2128,8 +2162,9 @@ func buildListener(port api.ServicePort, annotations map[string]string) (*elb.Li

// EnsureLoadBalancer implements LoadBalancer.EnsureLoadBalancer
func (s *AWSCloud) EnsureLoadBalancer(apiService *api.Service, hosts []string) (*api.LoadBalancerStatus, error) {
annotations := apiService.Annotations
glog.V(2).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v, %v)",
apiService.Namespace, apiService.Name, s.region, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, apiService.Annotations)
apiService.Namespace, apiService.Name, s.region, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, annotations)

if apiService.Spec.SessionAffinity != api.ServiceAffinityNone {
// ELB supports sticky sessions, but only when configured for HTTP/HTTPS
Expand All @@ -2142,6 +2177,7 @@ func (s *AWSCloud) EnsureLoadBalancer(apiService *api.Service, hosts []string) (

// Figure out what mappings we want on the load balancer
listeners := []*elb.Listener{}
portList := getPortSets(annotations[ServiceAnnotationLoadBalancerSSLPorts])
for _, port := range apiService.Spec.Ports {
if port.Protocol != api.ProtocolTCP {
return nil, fmt.Errorf("Only TCP LoadBalancer is supported for AWS ELB")
Expand All @@ -2150,7 +2186,7 @@ func (s *AWSCloud) EnsureLoadBalancer(apiService *api.Service, hosts []string) (
glog.Errorf("Ignoring port without NodePort defined: %v", port)
continue
}
listener, err := buildListener(port, apiService.Annotations)
listener, err := buildListener(port, annotations, portList)
if err != nil {
return nil, err
}
Expand Down
48 changes: 36 additions & 12 deletions pkg/cloudprovider/providers/aws/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,11 @@ func TestBuildListener(t *testing.T) {
name string

lbPort int64
portName string
instancePort int64
backendProtocolAnnotation string
certAnnotation string
sslPortAnnotation string

expectError bool
lbProtocol string
Expand All @@ -1225,49 +1227,69 @@ func TestBuildListener(t *testing.T) {
}{
{
"No cert or BE protocol annotation, passthrough",
80, 7999, "", "",
80, "", 7999, "", "", "",
false, "tcp", "tcp", "",
},
{
"Cert annotation without BE protocol specified, SSL->TCP",
80, 8000, "", "cert",
80, "", 8000, "", "cert", "",
false, "ssl", "tcp", "cert",
},
{
"BE protocol without cert annotation, passthrough",
443, 8001, "https", "",
443, "", 8001, "https", "", "",
false, "tcp", "tcp", "",
},
{
"Invalid cert annotation, bogus backend protocol",
443, 8002, "bacon", "foo",
true, "tcp", "tcp", "cert",
443, "", 8002, "bacon", "foo", "",
true, "tcp", "tcp", "",
},
{
"Invalid cert annotation, protocol followed by equal sign",
443, 8003, "http=", "=",
true, "tcp", "tcp", "cert",
443, "", 8003, "http=", "=", "",
true, "tcp", "tcp", "",
},
{
"HTTPS->HTTPS",
443, 8004, "https", "cert",
443, "", 8004, "https", "cert", "",
false, "https", "https", "cert",
},
{
"HTTPS->HTTP",
443, 8005, "http", "cert",
443, "", 8005, "http", "cert", "",
false, "https", "http", "cert",
},
{
"SSL->SSL",
443, 8006, "ssl", "cert",
443, "", 8006, "ssl", "cert", "",
false, "ssl", "ssl", "cert",
},
{
"SSL->TCP",
443, 8007, "tcp", "cert",
443, "", 8007, "tcp", "cert", "",
false, "ssl", "tcp", "cert",
},
{
"Port in whitelist",
1234, "", 8008, "tcp", "cert", "1234,5678",
false, "ssl", "tcp", "cert",
},
{
"Port not in whitelist, passthrough",
443, "", 8009, "tcp", "cert", "1234,5678",
false, "tcp", "tcp", "",
},
{
"Named port in whitelist",
1234, "bar", 8010, "tcp", "cert", "foo,bar",
false, "ssl", "tcp", "cert",
},
{
"Named port not in whitelist, passthrough",
443, "", 8011, "tcp", "cert", "foo,bar",
false, "tcp", "tcp", "",
},
}

for _, test := range tests {
Expand All @@ -1279,11 +1301,13 @@ func TestBuildListener(t *testing.T) {
if test.certAnnotation != "" {
annotations[ServiceAnnotationLoadBalancerCertificate] = test.certAnnotation
}
ports := getPortSets(test.sslPortAnnotation)
l, err := buildListener(api.ServicePort{
NodePort: int32(test.instancePort),
Port: int32(test.lbPort),
Name: test.portName,
Protocol: api.Protocol("tcp"),
}, annotations)
}, annotations, ports)
if test.expectError {
if err == nil {
t.Errorf("Should error for case %s", test.name)
Expand Down