Skip to content

feat: NLB Certificate autodiscovery using ssl-domains annotations #3269

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: NLB Certificate autodiscovery using ssl-domains annotations #3269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
195f6a2
Enable autodiscovery for ACM using SSL hosts annotation
Kavinraja-G Jul 1, 2023
48915c6
Refactor ACM auto-discovery logic
Kavinraja-G Jul 2, 2023
736e120
Added docs for NLB ssl-domains usage
Kavinraja-G Jul 2, 2023
16274a4
Fix docs and run goimports
Kavinraja-G Jul 3, 2023
a1ba0f8
Fix CertDiscovery logic for Service NLBs
Kavinraja-G Jul 6, 2023
a78c443
Fix imports and cleanup returns in certificates
Kavinraja-G Jul 7, 2023
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
4 changes: 3 additions & 1 deletion controllers/service/service_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy"
elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2"
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
"sigs.k8s.io/aws-load-balancer-controller/pkg/ingress"
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
Expand Down Expand Up @@ -43,10 +44,11 @@ func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorde
annotationParser := annotations.NewSuffixAnnotationParser(serviceAnnotationPrefix)
trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, controllerConfig.ClusterName)
serviceUtils := service.NewServiceUtils(annotationParser, serviceFinalizer, controllerConfig.ServiceConfig.LoadBalancerClass, controllerConfig.FeatureGates)
certDiscovery := ingress.NewACMCertDiscovery(cloud.ACM(), logger)
modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, cloud.VpcID(), trackingProvider,
elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils,
backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, logger)
backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, certDiscovery, logger)
stackMarshaller := deploy.NewDefaultStackMarshaller()
stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, elbv2TaggingManager, controllerConfig, serviceTagPrefix, logger)
return &serviceReconciler{
Expand Down
36 changes: 24 additions & 12 deletions docs/guide/service/annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
## Annotations
!!!warning
These annotations are specific to the kubernetes [service resources reconciled](#lb-type) by the AWS Load Balancer Controller. Although the list was initially derived from the k8s in-tree `kube-controller-manager`, this
documentation is not an accurate reference for the services reconciled by the in-tree controller.
documentation is not an accurate reference for the services reconciled by the in-tree controller.

| Name | Type | Default | Notes |
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can we fix the formatting issue by only add a new line instead of change existing lines.

|--------------------------------------------------------------------------------------------------|-------------------------|---------------------------|--------------------------------------------------------|
Expand All @@ -29,6 +29,7 @@
| [service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix](#deprecated-attributes)| string | | deprecated, in favor of [aws-load-balancer-attributes](#load-balancer-attributes)|
| [service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled](#deprecated-attributes)| boolean | false | deprecated, in favor of [aws-load-balancer-attributes](#load-balancer-attributes)|
| [service.beta.kubernetes.io/aws-load-balancer-ssl-cert](#ssl-cert) | stringList | | |
| [service.beta.kubernetes.io/aws-load-balancer-ssl-domains](#ssl-domains) | stringList | | |
| [service.beta.kubernetes.io/aws-load-balancer-ssl-ports](#ssl-ports) | stringList | | |
| [service.beta.kubernetes.io/aws-load-balancer-ssl-negotiation-policy](#ssl-negotiation-policy) | string | ELBSecurityPolicy-2016-08 | |
| [service.beta.kubernetes.io/aws-load-balancer-backend-protocol](#backend-protocol) | string | | |
Expand All @@ -49,9 +50,9 @@
| [service.beta.kubernetes.io/aws-load-balancer-alpn-policy](#alpn-policy) | string | | |
| [service.beta.kubernetes.io/aws-load-balancer-target-node-labels](#target-node-labels) | stringMap | | |
| [service.beta.kubernetes.io/aws-load-balancer-attributes](#load-balancer-attributes) | stringMap | | |
| [service.beta.kubernetes.io/aws-load-balancer-security-groups](#security-groups) | stringList | | |
| [service.beta.kubernetes.io/aws-load-balancer-security-groups](#security-groups) | stringList | | |
| [service.beta.kubernetes.io/aws-load-balancer-manage-backend-security-group-rules](#manage-backend-sg-rules) | boolean | true | If `service.beta.kubernetes.io/aws-load-balancer-security-groups` is specified, this must also be explicitly specified otherwise it defaults to `false`. |
| [service.beta.kubernetes.io/aws-load-balancer-inbound-sg-rules-on-private-link-traffic](#update-security-settings) | string | |
| [service.beta.kubernetes.io/aws-load-balancer-inbound-sg-rules-on-private-link-traffic](#update-security-settings) | string | |

## Traffic Routing
Traffic Routing can be controlled with following annotations:
Expand All @@ -70,7 +71,7 @@ Traffic Routing can be controlled with following annotations:

!!!tip
This annotation specifies the controller used to provision LoadBalancers (as specified in [legacy-cloud-provider](#legacy-cloud-provider)). Refer to [lb-scheme](#lb-scheme) to specify whether the LoadBalancer is internet-facing or internal.

!!!note ""
- [Deprecated] For type `nlb-ip`, the controller will provision an NLB with targets registered by IP address. This value is supported for backwards compatibility.
- For type `external`, the NLB target type depends on the [nlb-target-type](#nlb-target-type) annotation.
Expand Down Expand Up @@ -242,11 +243,11 @@ for proxy protocol v2 configuration.
!!!warning ""
Only attributes defined in the annotation will be updated. To unset any AWS defaults(e.g. Disabling access logs after having them enabled once), the values need to be explicitly set to the original values(`access_logs.s3.enabled=false`) and omitting them is not sufficient.
Custom attributes set in this annotation's config map will be overriden by annotation-specific attributes. For backwards compatibility, existing annotations for the individual load balancer attributes get precedence in case of ties.

!!!note ""
- If `deletion_protection.enabled=true` is in the annotation, the controller will not be able to delete the NLB during reconciliation. Once the attribute gets edited to `deletion_protection.enabled=false` during reconciliation, the deployer will force delete the resource.
- Please note, if the deletion protection is not enabled via annotation (e.g. via AWS console), the controller still deletes the underlying resource.

!!!example
- enable access log to s3
```
Expand All @@ -272,7 +273,7 @@ for proxy protocol v2 configuration.
service.beta.kubernetes.io/aws-load-balancer-access-log-enabled
service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-name
service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix
service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled
service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled
```


Expand Down Expand Up @@ -385,6 +386,17 @@ You can configure TLS support via the following annotations:
service.beta.kubernetes.io/aws-load-balancer-ssl-cert: arn:aws:acm:us-west-2:xxxxx:certificate/xxxxxxx
```

- <a name="ssl-domains">`service.beta.kubernetes.io/aws-load-balancer-ssl-domains`</a> specifies the domain names for which the controller will automatically discover TLS certificates.

!!!note ""
The `service.beta.kubernetes.io/aws-load-balancer-ssl-cert` annotation takes precedence
over the `service.beta.kubernetes.io/aws-load-balancer-ssl-domains` annotation.

!!!example
```
service.beta.kubernetes.io/aws-load-balancer-ssl-domains: my-nlb.example.com
```

- <a name="ssl-ports">`service.beta.kubernetes.io/aws-load-balancer-ssl-ports`</a> specifies the frontend ports with TLS listeners.

!!!note ""
Expand Down Expand Up @@ -434,7 +446,7 @@ Load balancer access can be controlled via following annotations:
This annotation will be ignored in case preserve client IP is not enabled.
- preserve client IP is disabled by default for `IP` targets
- preserve client IP is enabled by default for `instance` targets

!!!warning ""
Preserve client IP has no effect on traffic converted from IPv4 to IPv6 and on traffic converted from IPv6 to IPv4. The source IP of this type of traffic is always the private IP address of the Network Load Balancer.
- This could cause the clients that have their traffic converted to bypass the specified CIDRs that are allowed to access the NLB.
Expand Down Expand Up @@ -495,7 +507,7 @@ Load balancer access can be controlled via following annotations:
```
service.beta.kubernetes.io/aws-load-balancer-security-groups: sg-xxxx, nameOfSg1, nameOfSg2
```

- <a name="manage-backend-sg-rules">`service.beta.kubernetes.io/aws-load-balancer-manage-backend-security-group-rules`</a> specifies whether the controller should automatically add the ingress rules to the instance/ENI security group.

!!!warning ""
Expand All @@ -506,7 +518,7 @@ Load balancer access can be controlled via following annotations:
service.beta.kubernetes.io/aws-load-balancer-manage-backend-security-group-rules: "false"
```

- <a name="update-security-settings">`service.beta.kubernetes.io/aws-load-balancer-inbound-sg-rules-on-private-link-traffic`</a> specifies whether to apply security group rules to traffic sent to the load balancer through AWS PrivateLink.
- <a name="update-security-settings">`service.beta.kubernetes.io/aws-load-balancer-inbound-sg-rules-on-private-link-traffic`</a> specifies whether to apply security group rules to traffic sent to the load balancer through AWS PrivateLink.

!!!example
```
Expand All @@ -517,8 +529,8 @@ Load balancer access can be controlled via following annotations:
## Legacy Cloud Provider
The AWS Load Balancer Controller manages Kubernetes Services in a compatible way with the AWS cloud provider's legacy service controller.

- For users on v2.5.0+, The AWS LBC provides a mutating webhook for service resources to set the `spec.loadBalancerCLass` field for Serive of type LoadBalancer, effectively making the AWS LBC the default controller for Service of type LoadBalancer.
- For users on v2.5.0+, The AWS LBC provides a mutating webhook for service resources to set the `spec.loadBalancerCLass` field for Serive of type LoadBalancer, effectively making the AWS LBC the default controller for Service of type LoadBalancer.
Users can disable this feature and revert to using the AWS Cloud Controller Manager as the default service controller by setting the helm chart value `enableServiceMutatorWebhook` to false with `--set enableServiceMutatorWebhook=false` .
- For users on older versions, the annotation `service.beta.kubernetes.io/aws-load-balancer-type` is used to determine which controller reconciles the service. If the annotation value is `nlb-ip` or `external`,
recent versions of the legacy cloud provider ignore the Service resource so that the AWS LBC can take over. For all other values of the annotation, the legacy cloud provider will handle the service.
recent versions of the legacy cloud provider ignore the Service resource so that the AWS LBC can take over. For all other values of the annotation, the legacy cloud provider will handle the service.
Note that this annotation should be specified during service creation and not edited later. Support for the annotation was added to the legacy cloud provider in Kubernetes v1.20, and is backported to v1.18.18+ and v1.19.10+.
1 change: 1 addition & 0 deletions pkg/annotations/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const (
SvcLBSuffixAccessLogS3BucketName = "aws-load-balancer-access-log-s3-bucket-name"
SvcLBSuffixAccessLogS3BucketPrefix = "aws-load-balancer-access-log-s3-bucket-prefix"
SvcLBSuffixCrossZoneLoadBalancingEnabled = "aws-load-balancer-cross-zone-load-balancing-enabled"
SvcLBSuffixSSLDomains = "aws-load-balancer-ssl-domains"
SvcLBSuffixSSLCertificate = "aws-load-balancer-ssl-cert"
SvcLBSuffixSSLPorts = "aws-load-balancer-ssl-ports"
SvcLBSuffixSSLNegotiationPolicy = "aws-load-balancer-ssl-negotiation-policy"
Expand Down
2 changes: 1 addition & 1 deletion pkg/ingress/model_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type defaultModelBuilder struct {
logger logr.Logger
}

// build mode stack for a IngressGroup.
// Build builds mode stack for a IngressGroup.
func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group) (core.Stack, *elbv2model.LoadBalancer, []types.NamespacedName, bool, error) {
stack := core.NewDefaultStack(core.StackID(ingGroup.ID))
task := &defaultModelBuildTask{
Expand Down
35 changes: 28 additions & 7 deletions pkg/service/model_build_listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,32 @@ func (t *defaultModelBuildTask) buildSSLNegotiationPolicy(_ context.Context) *st
return &t.defaultSSLPolicy
}

func (t *defaultModelBuildTask) buildListenerCertificates(_ context.Context) []elbv2model.Certificate {
func (t *defaultModelBuildTask) buildListenerCertificates(ctx context.Context) ([]elbv2model.Certificate, error) {
var rawCertificateARNs []string
_ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLCertificate, &rawCertificateARNs, t.service.Annotations)

var rawSSLDomains []string
var certificates []elbv2model.Certificate
for _, cert := range rawCertificateARNs {
certificates = append(certificates, elbv2model.Certificate{CertificateARN: aws.String(cert)})

if t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLCertificate, &rawCertificateARNs, t.service.Annotations) {
for _, cert := range rawCertificateARNs {
certificates = append(certificates, elbv2model.Certificate{CertificateARN: aws.String(cert)})
}
return certificates, nil
}

// auto-discover ACM certs only if the ssl-domains annotation exists ssl-cert annotations is not present
// which means ssl-cert takes precedence over the auto-discovered cert/ss-domains annotation
if t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLDomains, &rawSSLDomains, t.service.Annotations) {
autoDiscoveredCertARNs, err := t.certDiscovery.Discover(ctx, rawSSLDomains)
if err != nil {
return nil, err
}
for _, cert := range autoDiscoveredCertARNs {
certificates = append(certificates, elbv2model.Certificate{
CertificateARN: aws.String(cert),
})
}
}
return certificates
return certificates, nil
}

func validateTLSPortsSet(rawTLSPorts []string, ports []corev1.ServicePort) error {
Expand Down Expand Up @@ -192,7 +209,11 @@ type listenerConfig struct {
}

func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) (*listenerConfig, error) {
certificates := t.buildListenerCertificates(ctx)
certificates, err := t.buildListenerCertificates(ctx)
if err != nil {
return nil, err
}

tlsPortsSet, err := t.buildTLSPortsSet(ctx)
if err != nil {
return nil, err
Expand Down
6 changes: 5 additions & 1 deletion pkg/service/model_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2"
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
"sigs.k8s.io/aws-load-balancer-controller/pkg/ingress"
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
Expand All @@ -41,7 +42,7 @@ func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver
elbv2TaggingManager elbv2deploy.TaggingManager, ec2Client services.EC2, featureGates config.FeatureGates, clusterName string, defaultTags map[string]string,
externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, enableIPTargetType bool, serviceUtils ServiceUtils,
backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, enableBackendSG bool,
disableRestrictedSGRules bool, logger logr.Logger) *defaultModelBuilder {
disableRestrictedSGRules bool, certDiscovery ingress.CertDiscovery, logger logr.Logger) *defaultModelBuilder {
return &defaultModelBuilder{
annotationParser: annotationParser,
subnetsResolver: subnetsResolver,
Expand All @@ -50,6 +51,7 @@ func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver
elbv2TaggingManager: elbv2TaggingManager,
featureGates: featureGates,
serviceUtils: serviceUtils,
certDiscovery: certDiscovery,
clusterName: clusterName,
vpcID: vpcID,
defaultTags: defaultTags,
Expand Down Expand Up @@ -78,6 +80,7 @@ type defaultModelBuilder struct {
elbv2TaggingManager elbv2deploy.TaggingManager
featureGates config.FeatureGates
serviceUtils ServiceUtils
certDiscovery ingress.CertDiscovery
ec2Client services.EC2
enableBackendSG bool
disableRestrictedSGRules bool
Expand Down Expand Up @@ -165,6 +168,7 @@ type defaultModelBuildTask struct {
featureGates config.FeatureGates
serviceUtils ServiceUtils
enableIPTargetType bool
certDiscovery ingress.CertDiscovery
ec2Client services.EC2
logger logr.Logger

Expand Down
4 changes: 3 additions & 1 deletion pkg/service/model_builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy"
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2"
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
"sigs.k8s.io/aws-load-balancer-controller/pkg/ingress"
"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
"sigs.k8s.io/controller-runtime/pkg/log"
)
Expand Down Expand Up @@ -6459,9 +6460,10 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
} else {
enableIPTargetType = *tt.enableIPTargetType
}
certDiscovery := ingress.NewMockCertDiscovery(ctrl)
builder := NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, "vpc-xxx", trackingProvider, elbv2TaggingManager, ec2Client, featureGates,
"my-cluster", nil, nil, "ELBSecurityPolicy-2016-08", defaultTargetType, enableIPTargetType, serviceUtils,
backendSGProvider, sgResolver, tt.enableBackendSG, tt.disableRestrictedSGRules, logr.New(&log.NullLogSink{}))
backendSGProvider, sgResolver, tt.enableBackendSG, tt.disableRestrictedSGRules, certDiscovery, logr.New(&log.NullLogSink{}))
ctx := context.Background()
stack, _, _, err := builder.Build(ctx, tt.svc)
if tt.wantError {
Expand Down