Merge pull request #2611 from munnerz/ingress-shim-duplicate-secretname

Only allow a single TLS entry per secret name in an Ingress resource
This commit is contained in:
jetstack-bot 2020-02-21 14:27:22 +00:00 committed by GitHub
commit 56673bab40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 65 additions and 7 deletions

View File

@ -109,7 +109,9 @@ func (c *controller) Sync(ctx context.Context, ing *extv1beta1.Ingress) error {
func (c *controller) validateIngress(ing *extv1beta1.Ingress) []error {
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))
@ -118,6 +120,11 @@ func (c *controller) validateIngress(ing *extv1beta1.Ingress) []error {
errs = append(errs, fmt.Errorf("TLS entry %d for hosts %v must specify a secretName", i, tls.Hosts))
}
}
for name, n := range namedSecrets {
if n > 1 {
errs = append(errs, fmt.Errorf("Duplicate TLS entry for secretName %q", name))
}
}
return errs
}

View File

@ -97,6 +97,7 @@ func TestSync(t *testing.T) {
ExpectedCreate []*cmapi.Certificate
ExpectedUpdate []*cmapi.Certificate
ExpectedDelete []*cmapi.Certificate
ExpectedEvents []string
}
tests := []testT{
{
@ -125,6 +126,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -176,6 +178,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -223,6 +226,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -264,6 +268,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -306,6 +311,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -351,6 +357,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -392,6 +399,7 @@ func TestSync(t *testing.T) {
},
},
ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -435,6 +443,7 @@ func TestSync(t *testing.T) {
},
},
},
ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`},
ExpectedCreate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -455,10 +464,11 @@ func TestSync(t *testing.T) {
},
},
{
Name: "should return an error when no TLS hosts are specified",
Issuer: acmeIssuer,
IssuerLister: []runtime.Object{acmeIssuer},
Err: true,
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`},
Ingress: &extv1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ingress-name",
@ -478,9 +488,10 @@ func TestSync(t *testing.T) {
},
},
{
Name: "should return an error when no TLS secret name is specified",
Issuer: acmeIssuer,
Err: true,
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`},
Ingress: &extv1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ingress-name",
@ -586,6 +597,7 @@ func TestSync(t *testing.T) {
),
},
DefaultIssuerKind: "Issuer",
ExpectedEvents: []string{`Normal UpdateCertificate Successfully updated Certificate "existing-crt"`},
ExpectedUpdate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -650,6 +662,7 @@ func TestSync(t *testing.T) {
},
},
},
ExpectedEvents: []string{`Normal UpdateCertificate Successfully updated Certificate "cert-secret-name"`},
ExpectedUpdate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -784,6 +797,7 @@ func TestSync(t *testing.T) {
},
},
},
ExpectedEvents: []string{`Normal DeleteCertificate Successfully deleted unrequired Certificate "existing-crt"`},
ExpectedDelete: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -845,6 +859,7 @@ func TestSync(t *testing.T) {
},
},
},
ExpectedEvents: []string{`Normal UpdateCertificate Successfully updated Certificate "example-com-tls"`},
ExpectedUpdate: []*cmapi.Certificate{
{
ObjectMeta: metav1.ObjectMeta{
@ -864,6 +879,38 @@ func TestSync(t *testing.T) {
},
},
},
{
Name: "if an ingress contains multiple tls entires that specify the same secretName, an error should be logged and no action taken",
Issuer: acmeIssuer,
IssuerLister: []runtime.Object{acmeIssuer},
ExpectedEvents: []string{
`Warning BadConfig Duplicate TLS entry for secretName "example-com-tls"`,
},
Ingress: &extv1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ingress-name",
Namespace: gen.DefaultTestNamespace,
Annotations: map[string]string{
cmapi.IngressIssuerNameAnnotationKey: "issuer-name",
cmapi.IssuerKindAnnotationKey: "Issuer",
cmapi.IssuerGroupAnnotationKey: "cert-manager.io",
},
UID: types.UID("ingress-name"),
},
Spec: extv1beta1.IngressSpec{
TLS: []extv1beta1.IngressTLS{
{
Hosts: []string{"example.com"},
SecretName: "example-com-tls",
},
{
Hosts: []string{"notexample.com"},
SecretName: "example-com-tls",
},
},
},
},
},
}
testFn := func(test testT) func(t *testing.T) {
return func(t *testing.T) {
@ -902,6 +949,7 @@ func TestSync(t *testing.T) {
T: t,
CertManagerObjects: allCMObjects,
ExpectedActions: expectedActions,
ExpectedEvents: test.ExpectedEvents,
}
b.Init()
defer b.Stop()
@ -927,6 +975,9 @@ func TestSync(t *testing.T) {
t.Errorf("Expected no error, but got: %s", err)
}
if err := b.AllEventsCalled(); err != nil {
t.Error(err)
}
if err := b.AllReactorsCalled(); err != nil {
t.Errorf("Not all expected reactors were called: %v", err)
}