Bug: --default-ssl-certificate does not raise an error if the certificate is not found

[akx]

NGINX Ingress controller version:
0.20.0 git-e8d8103

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"10", GitVersion:"v1.10.7", GitCommit:"0c38c362511b20a098d7cd855f1314dad92c2780", GitTreeState:"clean", BuildDate:"2018-08-20T10:09:03Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.3-eks", GitCommit:"58c199a59046dbf0a13a387d3491a39213be53df", GitTreeState:"clean", BuildDate:"2018-09-21T21:00:04Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}

Environment:

• Cloud provider or hardware configuration: Amazon EKS

What happened:

I set --default-ssl-certificate accidentally to point to a Certificate object created by cert-manager, not the associated Secret.

The controller started happily and served things with the default dummy self-signed certificate.

What you expected to happen:

An error to occur on startup, or a warning to be logged.

Changing the value to the correct secret reference fixes this, and I see

backend_ssl.go:42] Syncing Secret "somenamespace/cert-name"
ssl.go:59] Creating temp file /etc/ingress-controller/ssl/cert-manager-example-com-tls.pem313862785 for Keypair: cert-manager-example-com-tls.pem
ssl.go:113] parsing ssl certificate extensions
backend_ssl.go:119] Configuring Secret "somenamespace/cert-name" for TLS encryption (CN: [*.example.com example.com])
backend_ssl.go:68] Adding Secret "somenamespace/cert-name" to the local store

in the logs.

How to reproduce it (as minimally and precisely as possible):

Add --default-ssl-certificate=default/jndfghkjs or something.

Anything else we need to know:

I may be wrong, but I assume this happens due to the way secrets are handled somewhere around

ingress-nginx/internal/ingress/controller/store/store.go

Lines 357 to 359 in 8eb47ad

	if store.defaultSSLCertificate == key {
	store.syncSecret(store.defaultSSLCertificate)
	}

.

Since no secret's name ever matches the cfg.defaultSslCertificate, that branch is never met, and the secret is never synced.

This is exacerbated by the fact that the segment at

ingress-nginx/internal/ingress/controller/controller.go

Lines 752 to 761 in 8eb47ad

	// generated on Start() with createDefaultSSLCertificate()
	defaultPemFileName := n.cfg.FakeCertificatePath
	defaultPemSHA := n.cfg.FakeCertificateSHA
	// read custom default SSL certificate, fall back to generated default certificate
	defaultCertificate, err := n.store.GetLocalSSLCert(n.cfg.DefaultSSLCertificate)
	if err == nil {
	defaultPemFileName = defaultCertificate.PemFileName
	defaultPemSHA = defaultCertificate.PemSHA
	}

ignores any errors, so even if n.cfg.DefaultSSLCertificate is specified, but it can not be loaded for a reason or another, we silently fall back to the dummy certificate.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[akx]

This bug still exists as of master:

ingress-nginx/internal/ingress/controller/controller.go

Lines 910 to 915 in 3c83d22

	// read custom default SSL certificate, fall back to generated default certificate
	defaultCertificate, err := n.store.GetLocalSSLCert(n.cfg.DefaultSSLCertificate)
	if err == nil {
	defaultPemFileName = defaultCertificate.PemFileName
	defaultPemSHA = defaultCertificate.PemSHA
	}