Skip to content

Add non-alphanumeric input validation test for vpcID and fix typo #3687

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 2 commits into from
May 10, 2024
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
4 changes: 2 additions & 2 deletions apis/elbv2/v1beta1/ingressclassparams_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ type Attribute struct {

// IngressClassParamsSpec defines the desired state of IngressClassParams
type IngressClassParamsSpec struct {
// CertificateARN specifies the ARN of the certificates for all Ingresses that belong to IngressClass with this IngressClassParams.
// CertificateArn specifies the ARN of the certificates for all Ingresses that belong to IngressClass with this IngressClassParams.
// +optional
CertficateArn []string `json:"certficateArn,omitempty"`
CertificateArn []string `json:"certificateArn,omitempty"`

// NamespaceSelector restrict the namespaces of Ingresses that are allowed to specify the IngressClass with this IngressClassParams.
// * if absent or present but empty, it selects all namespaces.
Expand Down
4 changes: 2 additions & 2 deletions apis/elbv2/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ spec:
spec:
description: IngressClassParamsSpec defines the desired state of IngressClassParams
properties:
certficateArn:
description: CertificateARN specifies the ARN of the certificates
certificateArn:
description: CertificateArn specifies the ARN of the certificates
for all Ingresses that belong to IngressClass with this IngressClassParams.
items:
type: string
Expand Down
2 changes: 1 addition & 1 deletion docs/guide/ingress/cert_discovery.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Certificate Discovery
TLS certificates for ALB Listeners can be automatically discovered with hostnames from Ingress resources if the [`alb.ingress.kubernetes.io/certificate-arn`](annotations.md#certificate-arn) annotation is not specified.
TLS certificates for ALB Listeners can be automatically discovered with hostnames from Ingress resources if the [`spec.certificateArn`](ingress_class.md#speccertificatearn) in [`IngressClassParams`](ingress_class.md#ingressclassparams-specification) or [`alb.ingress.kubernetes.io/certificate-arn`](annotations.md#certificate-arn) annotation is not specified.

The controller will attempt to discover TLS certificates from the `tls` field in Ingress and `host` field in Ingress rules.

Expand Down
11 changes: 10 additions & 1 deletion docs/guide/ingress/ingress_class.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ You can use IngressClassParams to enforce settings for a set of Ingresses.
- myVal0
- myVal1
```
- with certificateArn
```
apiVersion: elbv2.k8s.aws/v1beta1
kind: IngressClassParams
metadata:
name: class2048-config
spec:
certificateArn: ['arn:aws:acm:us-east-1:123456789:certificate/test-arn-1','arn:aws:acm:us-east-1:123456789:certificate/test-arn-2']
```

### IngressClassParams specification

Expand Down Expand Up @@ -167,7 +176,7 @@ Cluster administrators can use the `scheme` field to restrict the scheme for all
Cluster administrators can use the optional `inboundCIDRs` field to specify the CIDRs that are allowed to access the load balancers that belong to this IngressClass.
If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/inbound-cidrs` annotation.

### spec.certificateArn
#### spec.certificateArn
Cluster administrators can use the optional `certificateARN` field to specify the ARN of the certificates for all Ingresses that belong to IngressClass with this IngressClassParams.

If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/certificate-arn` annotation.
Expand Down
7 changes: 3 additions & 4 deletions helm/aws-load-balancer-controller/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ spec:
spec:
description: IngressClassParamsSpec defines the desired state of IngressClassParams
properties:
certficateArn:
description:
CertificateARN specifies the ARN of the certificates for
all Ingresses that belong to IngressClass with this IngressClassParams.
certificateArn:
description: CertificateArn specifies the ARN of the certificates
for all Ingresses that belong to IngressClass with this IngressClassParams.
items:
type: string
type: array
Expand Down
2 changes: 1 addition & 1 deletion pkg/annotations/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,5 @@ const (
SvcLBSuffixLoadBalancerSecurityGroups = "aws-load-balancer-security-groups"
SvcLBSuffixManageSGRules = "aws-load-balancer-manage-backend-security-group-rules"
SvcLBSuffixEnforceSGInboundRulesOnPrivateLinkTraffic = "aws-load-balancer-inbound-sg-rules-on-private-link-traffic"
SvcLBSuffixSecurityGroupPrefixLists = "aws-load-balancer-security-group-prefix-lists"
SvcLBSuffixSecurityGroupPrefixLists = "aws-load-balancer-security-group-prefix-lists"
)
4 changes: 2 additions & 2 deletions pkg/ingress/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context
}

func (t *defaultModelBuildTask) computeIngressExplicitTLSCertARNs(_ context.Context, ing *ClassifiedIngress) []string {
if ing.IngClassConfig.IngClassParams != nil && len(ing.IngClassConfig.IngClassParams.Spec.CertficateArn) != 0 {
return ing.IngClassConfig.IngClassParams.Spec.CertficateArn
if ing.IngClassConfig.IngClassParams != nil && len(ing.IngClassConfig.IngClassParams.Spec.CertificateArn) != 0 {
return ing.IngClassConfig.IngClassParams.Spec.CertificateArn
}
var rawTLSCertARNs []string
_ = t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixCertificateARN, &rawTLSCertARNs, ing.Ing.Annotations)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ingress/model_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
IngClassConfig: ClassConfiguration{
IngClassParams: &v1beta1.IngressClassParams{
Spec: v1beta1.IngressClassParamsSpec{
CertficateArn: []string{"arn:aws:acm:us-east-1:9999999:certificate/ingress-class-certificate-arn"},
CertificateArn: []string{"arn:aws:acm:us-east-1:9999999:certificate/ingress-class-certificate-arn"},
},
},
},
Expand Down
12 changes: 12 additions & 0 deletions webhooks/elbv2/targetgroupbinding_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,18 @@ func Test_targetGroupBindingValidator_checkTargetGroupVpcID(t *testing.T) {
},
wantErr: errors.New("ValidationError: vpcID vpcid-123 failed to satisfy constraint: VPC Id must begin with 'vpc-' followed by 8 or 17 lowercase letters (a-f) or numbers."),
},
{
name: "[err] vpcID is not valid - non alphanumeric value",
args: args{
obj: &elbv2api.TargetGroupBinding{
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-2",
VpcID: "vpcid-@34!dv",
},
},
},
wantErr: errors.New("ValidationError: vpcID vpcid-@34!dv failed to satisfy constraint: VPC Id must begin with 'vpc-' followed by 8 or 17 lowercase letters (a-f) or numbers."),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading