Skip to content

Commit

Permalink
enable http and https listener attributes (kubernetes-sigs#3948)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wweiwei-li @shraddhabang
wweiwei-li authored and shraddhabang committed Nov 22, 2024
1 parent 0b5dfef commit 5fec250
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 20 deletions.
9 changes: 9 additions & 0 deletions docs/guide/ingress/annotations.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ You can add annotations to kubernetes Ingress and Service objects to customize t
| [alb.ingress.kubernetes.io/target-node-labels](#target-node-labels) | stringMap |N/A| Ingress,Service | N/A |
| [alb.ingress.kubernetes.io/mutual-authentication](#mutual-authentication) | json |N/A| Ingress | Exclusive |
| [alb.ingress.kubernetes.io/multi-cluster-target-group](#multi-cluster-target-group) | boolean |N/A| Ingress, Service | N/A |
| [alb.ingress.kubernetes.io/listener-attributes.${Protocol}-${Port}](#listener-attributes) | stringMap |N/A| Ingress |Merge|
| [alb.ingress.kubernetes.io/minimum-load-balancer-capacity](#load-balancer-capacity-reservation) | stringMap |N/A| Ingress | Exclusive |

## IngressGroup
Expand Down Expand Up @@ -904,6 +905,14 @@ Custom attributes to LoadBalancers and TargetGroups can be controlled with follo
alb.ingress.kubernetes.io/multi-cluster-target-group: "true"
```

- <a name="listener-attributes">`alb.ingress.kubernetes.io/listener-attributes.${Protocol}-${Port}`</a> specifies Listener Attributes which should be applied to listener.

!!!example
- Server header enablement attribute
```
alb.ingress.kubernetes.io/listener-attributes.HTTP-80: routing.http.response.server.enabled=true
```


## Resource Tags
The AWS Load Balancer Controller automatically applies following tags to the AWS resources (ALB/TargetGroups/SecurityGroups/Listener/ListenerRule) it creates:
Expand Down
4 changes: 2 additions & 2 deletions pkg/deploy/elbv2/listener_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (
)

var PROTOCOLS_SUPPORTING_LISTENER_ATTRIBUTES = map[elbv2model.Protocol]bool{
elbv2model.ProtocolHTTP: false,
elbv2model.ProtocolHTTPS: false,
elbv2model.ProtocolHTTP: true,
elbv2model.ProtocolHTTPS: true,
elbv2model.ProtocolTCP: true,
elbv2model.ProtocolUDP: false,
elbv2model.ProtocolTLS: false,
Expand Down
29 changes: 21 additions & 8 deletions pkg/ingress/model_build_listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,14 @@ func (t *defaultModelBuildTask) fetchTrustStoreArnFromName(ctx context.Context,

func (t *defaultModelBuildTask) buildIngressGroupListenerAttributes(ctx context.Context, ingList []ClassifiedIngress, listenerProtocol elbv2model.Protocol, port int32) ([]elbv2model.ListenerAttribute, error) {
rawIngGrouplistenerAttributes := make(map[string]string)
ingClassAttributes := make(map[string]string)
if len(ingList) > 0 {
var err error
ingClassAttributes, err = t.buildIngressClassListenerAttributes(ingList[0].IngClassConfig, listenerProtocol, port)
if err != nil {
return nil, err
}
}
for _, ing := range ingList {
ingAttributes, err := t.buildIngressListenerAttributes(ctx, ing.Ing.Annotations, port, listenerProtocol)
if err != nil {
Expand All @@ -435,18 +443,23 @@ func (t *defaultModelBuildTask) buildIngressGroupListenerAttributes(ctx context.
attributeKey := attribute.Key
attributeValue := attribute.Value
if existingAttributeValue, exists := rawIngGrouplistenerAttributes[attributeKey]; exists && existingAttributeValue != attributeValue {
return nil, errors.Errorf("conflicting attributes %v: %v | %v", attributeKey, existingAttributeValue, attributeValue)
if ingClassValue, exists := ingClassAttributes[attributeKey]; exists {
// Conflict is resolved by ingClassAttributes, show a warning
t.logger.Info("listener attribute conflict resolved by ingress class",
"attributeKey", attributeKey,
"existingValue", existingAttributeValue,
"newValue", attributeValue,
"ingClassValue", ingClassValue)
} else {
// Conflict is not resolved by ingClassAttributes, return an error
return nil, errors.Errorf("conflicting listener attributes %v: %v | %v for ingress %s/%s",
attributeKey, existingAttributeValue, attributeValue, ing.Ing.Namespace, ing.Ing.Name)
}
}
rawIngGrouplistenerAttributes[attributeKey] = attributeValue
}
}
if len(ingList) > 0 {
ingClassAttributes, err := t.buildIngressClassListenerAttributes(ingList[0].IngClassConfig, listenerProtocol, port)
if err != nil {
return nil, err
}
rawIngGrouplistenerAttributes = algorithm.MergeStringMap(ingClassAttributes, rawIngGrouplistenerAttributes)
}
rawIngGrouplistenerAttributes = algorithm.MergeStringMap(ingClassAttributes, rawIngGrouplistenerAttributes)
attributes := make([]elbv2model.ListenerAttribute, 0, len(rawIngGrouplistenerAttributes))
for attrKey, attrValue := range rawIngGrouplistenerAttributes {
attributes = append(attributes, elbv2model.ListenerAttribute{
Expand Down
84 changes: 84 additions & 0 deletions pkg/ingress/model_build_listener_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"
networking "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
Expand Down Expand Up @@ -272,6 +273,89 @@ func Test_buildListenerAttributes(t *testing.T) {
},
},
},
{
name: "Ignore conflicting value when the key is specified by ingress class param",
fields: fields{
ingGroup: Group{
ID: GroupID{Name: "explicit-group"},
Members: []ClassifiedIngress{
{
Ing: &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "ing-6",
Annotations: map[string]string{
"alb.ingress.kubernetes.io/listen-ports": `[{"HTTP": 80}]`,
"alb.ingress.kubernetes.io/listener-attributes.HTTP-80": "attrKey1=attrValue1",
},
},
},
IngClassConfig: ClassConfiguration{
IngClassParams: &elbv2api.IngressClassParams{
ObjectMeta: metav1.ObjectMeta{
Name: "awesome-class",
},
Spec: elbv2api.IngressClassParamsSpec{
Listeners: []elbv2api.Listener{
{
Protocol: "HTTP",
Port: 80,
ListenerAttributes: []elbv2api.Attribute{
{
Key: "attrKey1",
Value: "attrValue1",
},
},
},
},
},
},
},
},
{
Ing: &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "ing-7",
Annotations: map[string]string{
"alb.ingress.kubernetes.io/listen-ports": `[{"HTTP": 80}]`,
"alb.ingress.kubernetes.io/listener-attributes.HTTP-80": "attrKey1=attrValue2",
},
},
},
IngClassConfig: ClassConfiguration{
IngClassParams: &elbv2api.IngressClassParams{
ObjectMeta: metav1.ObjectMeta{
Name: "awesome-class",
},
Spec: elbv2api.IngressClassParamsSpec{
Listeners: []elbv2api.Listener{
{
Protocol: "HTTP",
Port: 80,
ListenerAttributes: []elbv2api.Attribute{
{
Key: "attrKey1",
Value: "attrValue1",
},
},
},
},
},
},
},
},
},
},
},
wantErr: false,
wantValue: []elbv2model.ListenerAttribute{
{
Key: "attrKey1",
Value: "attrValue1",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
28 changes: 20 additions & 8 deletions pkg/ingress/model_build_load_balancer_attributes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import (
// buildIngressGroupLoadBalancerAttributes builds the LB attributes for a group of Ingresses.
func (t *defaultModelBuildTask) buildIngressGroupLoadBalancerAttributes(ingList []ClassifiedIngress) (map[string]string, error) {
ingGroupAttributes := make(map[string]string)
ingClassAttributes := make(map[string]string)
if len(ingList) > 0 {
var err error
ingClassAttributes, err = t.buildIngressClassLoadBalancerAttributes(ingList[0].IngClassConfig)
if err != nil {
return nil, err
}
}
for _, ing := range ingList {
ingAttributes, err := t.buildIngressLoadBalancerAttributes(ing)
if err != nil {
Expand All @@ -18,18 +26,22 @@ func (t *defaultModelBuildTask) buildIngressGroupLoadBalancerAttributes(ingList
for attrKey, attrValue := range ingAttributes {
existingAttrValue, exists := ingGroupAttributes[attrKey]
if exists && existingAttrValue != attrValue {
return nil, errors.Errorf("conflicting attributes %v: %v | %v", attrKey, existingAttrValue, attrValue)
if ingClassValue, exists := ingClassAttributes[attrKey]; exists {
// Conflict is resolved by ingClassAttributes, show a warning
t.logger.Info("load balancer attribute conflict resolved by ingress class",
"attributeKey", attrKey,
"existingValue", existingAttrValue,
"newValue", attrValue,
"ingClassValue", ingClassValue)
} else {
// Conflict is not resolved by ingClassAttributes, return an error
return nil, errors.Errorf("conflicting load balancer attributes %v: %v | %v", attrKey, existingAttrValue, attrValue)
}
}
ingGroupAttributes[attrKey] = attrValue
}
}
if len(ingList) > 0 {
ingClassAttributes, err := t.buildIngressClassLoadBalancerAttributes(ingList[0].IngClassConfig)
if err != nil {
return nil, err
}
return algorithm.MergeStringMap(ingClassAttributes, ingGroupAttributes), nil
}
ingGroupAttributes = algorithm.MergeStringMap(ingClassAttributes, ingGroupAttributes)
return ingGroupAttributes, nil
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/ingress/model_build_load_balancer_attributes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package ingress

import (
"fmt"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
networking "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
"testing"
)

func Test_defaultModelBuildTask_buildIngressGroupLoadBalancerAttributes(t *testing.T) {
Expand Down Expand Up @@ -82,7 +83,7 @@ func Test_defaultModelBuildTask_buildIngressGroupLoadBalancerAttributes(t *testi
},
},
},
wantErr: errors.New("conflicting attributes deletion_protection.enabled: true | false"),
wantErr: errors.New("conflicting load balancer attributes deletion_protection.enabled: true | false"),
},
{
name: "non-empty annotation attributes from single Ingress, non-empty IngressClass attributes - has overlap attributes",
Expand Down
62 changes: 62 additions & 0 deletions test/e2e/ingress/vanilla_ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/gavv/httpexpect/v2"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
networking "k8s.io/api/networking/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -777,6 +778,56 @@ var _ = Describe("vanilla ingress tests", func() {
}
})
})

Context("with `alb.ingress.kubernetes.io/listener-attributes.{Protocol}-{Port}` variant settings", func() {
It("with 'alb.ingress.kubernetes.io/listener-attributes.{Protocol}-{Port}' annotation explicitly specified, one ALB shall be created and functional", func() {
appBuilder := manifest.NewFixedResponseServiceBuilder()
ingBuilder := manifest.NewIngressBuilder()
dp, svc := appBuilder.Build(sandboxNS.Name, "app", tf.Options.TestImageRegistry)
ingBackend := networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: svc.Name,
Port: networking.ServiceBackendPort{
Number: 80,
},
},
}
annotation := map[string]string{
"kubernetes.io/ingress.class": "alb",
"alb.ingress.kubernetes.io/scheme": "internet-facing",
"alb.ingress.kubernetes.io/listen-ports": `[{"HTTP": 80}]`,
"alb.ingress.kubernetes.io/listener-attributes.HTTP-80": "routing.http.response.server.enabled=false",
}
if tf.Options.IPFamily == "IPv6" {
annotation["alb.ingress.kubernetes.io/ip-address-type"] = "dualstack"
annotation["alb.ingress.kubernetes.io/target-type"] = "ip"
}
ing := ingBuilder.
AddHTTPRoute("", networking.HTTPIngressPath{Path: "/path", PathType: &exact, Backend: ingBackend}).
WithAnnotations(annotation).Build(sandboxNS.Name, "ing")
resStack := fixture.NewK8SResourceStack(tf, dp, svc, ing)
err := resStack.Setup(ctx)
Expect(err).NotTo(HaveOccurred())

defer resStack.TearDown(ctx)

lbARN, lbDNS := ExpectOneLBProvisionedForIngress(ctx, tf, ing)
sdkListeners, err := tf.LBManager.GetLoadBalancerListeners(ctx, lbARN)

Eventually(func() bool {
return verifyListenerAttributes(ctx, tf, *sdkListeners[0].ListenerArn, map[string]string{
"routing.http.response.server.enabled": "false",
}) == nil
}, utils.PollTimeoutShort, utils.PollIntervalMedium).Should(BeTrue())

// test traffic
ExpectLBDNSBeAvailable(ctx, tf, lbARN, lbDNS)
httpExp := httpexpect.New(tf.LoggerReporter, fmt.Sprintf("http://%v", lbDNS))
httpExp.GET("/path").Expect().
Status(http.StatusOK).
Body().Equal("Hello World!")
})
})
})

// ExpectOneLBProvisionedForIngress expects one LoadBalancer provisioned for Ingress.
Expand Down Expand Up @@ -820,3 +871,14 @@ func ExpectLBDNSBeAvailable(ctx context.Context, tf *framework.Framework, lbARN
Expect(err).NotTo(HaveOccurred())
tf.Logger.Info("dns becomes available", "dns", lbDNS)
}

func verifyListenerAttributes(ctx context.Context, f *framework.Framework, lsARN string, expectedAttrs map[string]string) error {
lsAttrs, err := f.LBManager.GetListenerAttributes(ctx, lsARN)
Expect(err).NotTo(HaveOccurred())
for _, attr := range lsAttrs {
if val, ok := expectedAttrs[awssdk.ToString(attr.Key)]; ok && val != awssdk.ToString(attr.Value) {
return errors.Errorf("Attribute %v, expected %v, actual %v", awssdk.ToString(attr.Key), val, awssdk.ToString(attr.Value))
}
}
return nil
}

0 comments on commit 5fec250

Please sign in to comment.