Skip to content

Commit

Permalink
feat: allowed ACM cert discovery to filter on CA ARNs (kubernetes-sig…
Browse files Browse the repository at this point in the history
  • Loading branch information
the-technat authored and shraddhabang committed Mar 19, 2024
1 parent 82da786 commit ef8bf22
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 15 deletions.
2 changes: 1 addition & 1 deletion controllers/ingress/group_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager, controllerConfig.FeatureGates,
cloud.VpcID(), controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, backendSGProvider, sgResolver,
controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), logger)
controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), logger)
stackMarshaller := deploy.NewDefaultStackMarshaller()
stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, elbv2TaggingManager,
controllerConfig, ingressTagPrefix, logger)
Expand Down
1 change: 1 addition & 0 deletions docs/deploy/configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
|aws-max-retries | int | 10 | Maximum retries for AWS APIs |
|aws-region | string | [instance metadata](#instance-metadata) | AWS Region for the kubernetes cluster |
|aws-vpc-id | string | [instance metadata](#instance-metadata) | AWS VPC ID for the Kubernetes cluster |
|allowed-certificate-authority-arns | stringList | [] | Specify an optional list of CA ARNs to filter on in cert discovery (empty means all CAs are allowed) |
|backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG|
|cluster-name | string | | Kubernetes cluster name|
|default-ssl-policy | string | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation |
Expand Down
3 changes: 3 additions & 0 deletions helm/aws-load-balancer-controller/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ spec:
{{- if .Values.serviceTargetENISGTags }}
- --service-target-eni-security-group-tags={{ .Values.serviceTargetENISGTags }}
{{- end }}
{{- if .Values.certDiscovery.allowedCertificateAuthorityARNs }}
- --allowed-certificate-authority-arns={{ .Values.certDiscovery.allowedCertificateAuthorityARNs }}
{{- end }}
{{- if or .Values.env .Values.envSecretName }}
env:
{{- if .Values.env}}
Expand Down
3 changes: 3 additions & 0 deletions helm/aws-load-balancer-controller/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ controllerConfig:
# NLBHealthCheckAdvancedConfig: true
# ALBSingleSubnet: false

certDiscovery:
allowedCertificateAuthorityARNs: "" # empty means all CAs are in scope

# objectSelector for webhook
objectSelector:
matchExpressions:
Expand Down
5 changes: 5 additions & 0 deletions pkg/config/ingress_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const (
flagIngressMaxConcurrentReconciles = "ingress-max-concurrent-reconciles"
flagTolerateNonExistentBackendService = "tolerate-non-existent-backend-service"
flagTolerateNonExistentBackendAction = "tolerate-non-existent-backend-action"
flagAllowedCAArns = "allowed-certificate-authority-arns"
defaultIngressClass = "alb"
defaultDisableIngressClassAnnotation = false
defaultDisableIngressGroupNameAnnotation = false
Expand Down Expand Up @@ -42,6 +43,9 @@ type IngressConfig struct {
// TolerateNonExistentBackendAction specifies whether to allow rules that reference a backend action that does not
// exist. In this case, requests to that rule will result in a 503 error.
TolerateNonExistentBackendAction bool

// AllowedCertificateAuthoritiyARNs contains a list of all CAs to consider when discovering certificates for ingress resources
AllowedCertificateAuthorityARNs []string
}

// BindFlags binds the command line flags to the fields in the config object
Expand All @@ -58,4 +62,5 @@ func (cfg *IngressConfig) BindFlags(fs *pflag.FlagSet) {
"Tolerate rules that specify a non-existent backend service")
fs.BoolVar(&cfg.TolerateNonExistentBackendAction, flagTolerateNonExistentBackendAction, defaultTolerateNonExistentBackendAction,
"Tolerate rules that specify a non-existent backend action")
fs.StringSliceVar(&cfg.AllowedCertificateAuthorityARNs, flagAllowedCAArns, []string{}, "Specify an optional list of CA ARNs to filter on in cert discovery")
}
38 changes: 26 additions & 12 deletions pkg/ingress/cert_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ package ingress

import (
"context"
"slices"
"strings"
"sync"
"time"

"github.com/aws/aws-sdk-go/aws"
awssdk "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/acm"
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
Expand All @@ -11,9 +17,6 @@ import (
"k8s.io/apimachinery/pkg/util/cache"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
"strings"
"sync"
"time"
)

const (
Expand All @@ -33,7 +36,7 @@ type CertDiscovery interface {
}

// NewACMCertDiscovery constructs new acmCertDiscovery
func NewACMCertDiscovery(acmClient services.ACM, logger logr.Logger) *acmCertDiscovery {
func NewACMCertDiscovery(acmClient services.ACM, allowedCAARNs []string, logger logr.Logger) *acmCertDiscovery {
return &acmCertDiscovery{
acmClient: acmClient,
logger: logger,
Expand All @@ -44,6 +47,7 @@ func NewACMCertDiscovery(acmClient services.ACM, logger logr.Logger) *acmCertDis
certDomainsCache: cache.NewExpiring(),
importedCertDomainsCacheTTL: defaultImportedCertDomainsCacheTTL,
privateCertDomainsCacheTTL: defaultPrivateCertDomainsCacheTTL,
allowedCAARNs: allowedCAARNs,
}
}

Expand All @@ -59,6 +63,7 @@ type acmCertDiscovery struct {
certARNsCache *cache.Expiring
certARNsCacheTTL time.Duration
certDomainsCache *cache.Expiring
allowedCAARNs []string
importedCertDomainsCacheTTL time.Duration
privateCertDomainsCacheTTL time.Duration
}
Expand Down Expand Up @@ -102,7 +107,10 @@ func (d *acmCertDiscovery) loadDomainsForAllCertificates(ctx context.Context) (m
if err != nil {
return nil, err
}
domainsByCertARN[certARN] = certDomains
if len(certDomains) > 0 {
domainsByCertARN[certARN] = certDomains
}

}
return domainsByCertARN, nil
}
Expand Down Expand Up @@ -143,14 +151,20 @@ func (d *acmCertDiscovery) loadDomainsForCertificate(ctx context.Context, certAR
return nil, err
}
certDetail := resp.Certificate
domains := sets.NewString(aws.StringValueSlice(certDetail.SubjectAlternativeNames)...)
switch aws.StringValue(certDetail.Type) {
case acm.CertificateTypeImported:
d.certDomainsCache.Set(certARN, domains, d.importedCertDomainsCacheTTL)
case acm.CertificateTypeAmazonIssued, acm.CertificateTypePrivate:
d.certDomainsCache.Set(certARN, domains, d.privateCertDomainsCacheTTL)

// check if cert is issued from an allowed CA
if len(d.allowedCAARNs) == 0 || slices.Contains(d.allowedCAARNs, awssdk.StringValue(certDetail.CertificateAuthorityArn)) {
domains := sets.NewString(aws.StringValueSlice(certDetail.SubjectAlternativeNames)...)
switch aws.StringValue(certDetail.Type) {
case acm.CertificateTypeImported:
d.certDomainsCache.Set(certARN, domains, d.importedCertDomainsCacheTTL)
case acm.CertificateTypeAmazonIssued, acm.CertificateTypePrivate:
d.certDomainsCache.Set(certARN, domains, d.privateCertDomainsCacheTTL)
}
return domains, nil
}
return domains, nil
return sets.String{}, nil

}

func (d *acmCertDiscovery) domainMatchesHost(domainName string, tlsHost string) bool {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ingress/model_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, featureGates config.FeatureGates,
vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string,
backendSGProvider networkingpkg.BackendSGProvider, sgResolver networkingpkg.SecurityGroupResolver,
enableBackendSG bool, disableRestrictedSGRules bool, enableIPTargetType bool, logger logr.Logger) *defaultModelBuilder {
certDiscovery := NewACMCertDiscovery(acmClient, logger)
enableBackendSG bool, disableRestrictedSGRules bool, allowedCAARNs []string, enableIPTargetType bool, logger logr.Logger) *defaultModelBuilder {
certDiscovery := NewACMCertDiscovery(acmClient, allowedCAARNs, logger)
ruleOptimizer := NewDefaultRuleOptimizer(logger)
return &defaultModelBuilder{
k8sClient: k8sClient,
Expand Down

0 comments on commit ef8bf22

Please sign in to comment.