Skips an invalid Ingress.spec.tls entry instead of invalidating the whole Ingress

Signed-off-by: irbekrm <irbekrm@gmail.com>
This commit is contained in:
irbekrm 2021-02-01 19:32:36 +00:00
parent 33f1881190
commit bb99260365
2 changed files with 84 additions and 20 deletions

View File

@ -106,18 +106,13 @@ func (c *controller) Sync(ctx context.Context, ing *networkingv1beta1.Ingress) e
}
func (c *controller) validateIngress(ing *networkingv1beta1.Ingress) []error {
// check for duplicate values of networkingv1beta1.IngressTLS.SecretName
var errs []error
namedSecrets := make(map[string]int)
for i, tls := range ing.Spec.TLS {
namedSecrets[tls.SecretName] += 1
// validate the ingress TLS block
if len(tls.Hosts) == 0 {
errs = append(errs, fmt.Errorf("Secret %q for ingress TLS has no hosts specified", tls.SecretName))
}
if tls.SecretName == "" {
errs = append(errs, fmt.Errorf("TLS entry %d for hosts %v must specify a secretName", i, tls.Hosts))
}
for _, tls := range ing.Spec.TLS {
namedSecrets[tls.SecretName]++
}
// not doing this in the previous for-loop to avoid erroring more than once for the same SecretName
for name, n := range namedSecrets {
if n > 1 {
errs = append(errs, fmt.Errorf("Duplicate TLS entry for secretName %q", name))
@ -126,13 +121,33 @@ func (c *controller) validateIngress(ing *networkingv1beta1.Ingress) []error {
return errs
}
func validateIngressTLSBlock(tlsBlock networkingv1beta1.IngressTLS) []error {
// unlikely that _both_ SecretName and Hosts would be empty, but still returning []error for consistency
var errs []error
if len(tlsBlock.Hosts) == 0 {
errs = append(errs, fmt.Errorf("secret %q for ingress TLS has no hosts specified", tlsBlock.SecretName))
}
if tlsBlock.SecretName == "" {
errs = append(errs, fmt.Errorf("TLS entry for hosts %v must specify a secretName", tlsBlock.Hosts))
}
return errs
}
func (c *controller) buildCertificates(ctx context.Context, ing *networkingv1beta1.Ingress,
issuerName, issuerKind, issuerGroup string) (new, update []*cmapi.Certificate, _ error) {
log := logs.FromContext(ctx)
var newCrts []*cmapi.Certificate
var updateCrts []*cmapi.Certificate
for _, tls := range ing.Spec.TLS {
for i, tls := range ing.Spec.TLS {
errs := validateIngressTLSBlock(tls)
// if this tls entry is invalid, record an error event on Ingress object and continue to the next tls entry
if len(errs) > 0 {
errMsg := utilerrors.NewAggregate(errs).Error()
c.recorder.Eventf(ing, corev1.EventTypeWarning, "BadConfig", fmt.Sprintf("TLS entry %d is invalid: %s", i, errMsg))
continue
}
existingCrt, err := c.certificateLister.Certificates(ing.Namespace).Get(tls.SecretName)
if !apierrors.IsNotFound(err) && err != nil {
return nil, nil, err

View File

@ -513,11 +513,14 @@ func TestSync(t *testing.T) {
},
},
{
Name: "should return an error when no TLS hosts are specified",
Issuer: acmeIssuer,
IssuerLister: []runtime.Object{acmeIssuer},
Err: true,
ExpectedEvents: []string{`Warning BadConfig Secret "example-com-tls" for ingress TLS has no hosts specified`},
Name: "should skip an invalid TLS entry (no TLS hosts specified)",
Issuer: acmeIssuer,
IssuerLister: []runtime.Object{acmeIssuer},
Err: true,
ExpectedEvents: []string{
`Warning BadConfig TLS entry 0 is invalid: secret "example-com-tls-invalid" for ingress TLS has no hosts specified`,
`Normal CreateCertificate Successfully created Certificate "example-com-tls"`,
},
Ingress: &networkingv1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ingress-name",
@ -529,18 +532,44 @@ func TestSync(t *testing.T) {
},
Spec: networkingv1beta1.IngressSpec{
TLS: []networkingv1beta1.IngressTLS{
{
SecretName: "example-com-tls-invalid",
},
{
SecretName: "example-com-tls",
Hosts: []string{"example.com", "www.example.com"},
},
},
},
},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
Name: "example-com-tls",
Namespace: gen.DefaultTestNamespace,
OwnerReferences: buildOwnerReferences("ingress-name", gen.DefaultTestNamespace),
},
Spec: cmapi.CertificateSpec{
DNSNames: []string{"example.com", "www.example.com"},
SecretName: "example-com-tls",
IssuerRef: cmmeta.ObjectReference{
Name: "issuer-name",
Kind: "Issuer",
},
},
},
},
},
{
Name: "should return an error when no TLS secret name is specified",
Issuer: acmeIssuer,
Err: true,
ExpectedEvents: []string{`Warning BadConfig TLS entry 0 for hosts [example.com] must specify a secretName`},
Name: "should skip an invalid TLS entry (no TLS secret name specified)",
Issuer: acmeIssuer,
Err: true,
IssuerLister: []runtime.Object{acmeIssuer},
ExpectedEvents: []string{
`Warning BadConfig TLS entry 0 is invalid: TLS entry for hosts [example.com] must specify a secretName`,
`Normal CreateCertificate Successfully created Certificate "example-com-tls"`,
},
Ingress: &networkingv1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ingress-name",
@ -555,10 +584,30 @@ func TestSync(t *testing.T) {
{
Hosts: []string{"example.com"},
},
{
Hosts: []string{"example.com", "www.example.com"},
SecretName: "example-com-tls",
},
},
},
},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
Name: "example-com-tls",
Namespace: gen.DefaultTestNamespace,
OwnerReferences: buildOwnerReferences("ingress-name", gen.DefaultTestNamespace),
},
Spec: cmapi.CertificateSpec{
DNSNames: []string{"example.com", "www.example.com"},
SecretName: "example-com-tls",
IssuerRef: cmmeta.ObjectReference{
Name: "issuer-name",
Kind: "Issuer",
},
},
},
},
IssuerLister: []runtime.Object{acmeIssuer},
},
{
Name: "should error if the specified issuer is not found",