Merge pull request #4695 from janosi/secure-verify-ca-secret

Removing secure-verify-ca-secret support

docs/kubectl-plugin.md

@@ -121,11 +121,6 @@ $ kubectl ingress-nginx backends -n ingress-nginx
       }
     },
     "port": 0,
-    "secureCACert": {
-      "secret": "",
-      "caFilename": "",
-      "caSha": ""
-    },
     "sslPassthrough": false,
     "endpoints": [
       {

docs/user-guide/nginx-configuration/annotations.md

@@ -78,7 +78,6 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/enable-rewrite-log](#enable-rewrite-log)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/rewrite-target](#rewrite)|URI|
 |[nginx.ingress.kubernetes.io/satisfy](#satisfy)|string|
-|[nginx.ingress.kubernetes.io/secure-verify-ca-secret](#secure-backends)|string|
 |[nginx.ingress.kubernetes.io/server-alias](#server-alias)|string|
 |[nginx.ingress.kubernetes.io/server-snippet](#server-snippet)|string|
 |[nginx.ingress.kubernetes.io/service-upstream](#service-upstream)|"true" or "false"|

internal/ingress/annotations/annotations_test.go

@@ -110,41 +110,6 @@ func buildIngress() *networking.Ingress {
 	}
 }
 
-func TestSecureVerifyCACert(t *testing.T) {
-	ec := NewAnnotationExtractor(mockCfg{
-		MockSecrets: map[string]*apiv1.Secret{
-			"default/secure-verify-ca": {
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "secure-verify-ca",
-				},
-			},
-		},
-	})
-
-	anns := []struct {
-		it          int
-		annotations map[string]string
-		exists      bool
-	}{
-		{1, map[string]string{backendProtocol: "HTTPS", annotationSecureVerifyCACert: "not"}, false},
-		{2, map[string]string{backendProtocol: "HTTP", annotationSecureVerifyCACert: "secure-verify-ca"}, false},
-		{3, map[string]string{backendProtocol: "HTTPS", annotationSecureVerifyCACert: "secure-verify-ca"}, true},
-		{4, map[string]string{backendProtocol: "HTTPS", annotationSecureVerifyCACert + "_not": "secure-verify-ca"}, false},
-		{5, map[string]string{backendProtocol: "HTTPS"}, false},
-		{6, map[string]string{}, false},
-		{7, nil, false},
-	}
-
-	for _, ann := range anns {
-		ing := buildIngress()
-		ing.SetAnnotations(ann.annotations)
-		su := ec.Extract(ing).SecureUpstream
-		if (su.CACert.CAFileName != "") != ann.exists {
-			t.Errorf("Expected exists was %v on iteration %v", ann.exists, ann.it)
-		}
-	}
-}
-
 func TestSSLPassthrough(t *testing.T) {
 	ec := NewAnnotationExtractor(mockCfg{})
 	ing := buildIngress()

internal/ingress/annotations/secureupstream/main.go

@@ -17,10 +17,8 @@ limitations under the License.
 package secureupstream
 
 import (
-	"fmt"
-
-	"github.com/pkg/errors"
 	networking "k8s.io/api/networking/v1beta1"
+	"k8s.io/klog"
 
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
 	"k8s.io/ingress-nginx/internal/ingress/resolver"
@@ -42,28 +40,9 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
 
 // Parse parses the annotations contained in the ingress
 // rule used to indicate if the upstream servers should use SSL
-func (a su) Parse(ing *networking.Ingress) (interface{}, error) {
-	bp, _ := parser.GetStringAnnotation("backend-protocol", ing)
-	ca, _ := parser.GetStringAnnotation("secure-verify-ca-secret", ing)
-	secure := &Config{
-		CACert: resolver.AuthSSLCert{},
-	}
-
-	if (bp != "HTTPS" && bp != "GRPCS") && ca != "" {
-		return secure,
-			errors.Errorf("trying to use CA from secret %v/%v on a non secure backend", ing.Namespace, ca)
-	}
-	if ca == "" {
-		return secure, nil
-	}
-	caCert, err := a.r.GetAuthCertificate(fmt.Sprintf("%v/%v", ing.Namespace, ca))
-	if err != nil {
-		return secure, errors.Wrap(err, "error obtaining certificate")
-	}
-	if caCert == nil {
-		return secure, nil
+func (a su) Parse(ing *networking.Ingress) (secure interface{}, err error) {
+	if ca, _ := parser.GetStringAnnotation("secure-verify-ca-secret", ing); ca != "" {
+		klog.Errorf("NOTE! secure-verify-ca-secret is not suppored anymore. Please use proxy-ssl-secret instead")
 	}
-	return &Config{
-		CACert: *caCert,
-	}, nil
+	return
 }

internal/ingress/annotations/secureupstream/main_test.go

@@ -116,7 +116,7 @@ func TestSecretNotFound(t *testing.T) {
 	data[parser.GetAnnotationWithPrefix("secure-verify-ca-secret")] = "secure-verify-ca"
 	ing.SetAnnotations(data)
 	_, err := NewParser(mockCfg{}).Parse(ing)
-	if err == nil {
+	if err != nil {
 		t.Error("Expected secret not found error on ingress")
 	}
 }
@@ -132,7 +132,24 @@ func TestSecretOnNonSecure(t *testing.T) {
 			"default/secure-verify-ca": {},
 		},
 	}).Parse(ing)
-	if err == nil {
+	if err != nil {
 		t.Error("Expected CA secret on non secure backend error on ingress")
 	}
 }
+
+func TestUnsupportedAnnotation(t *testing.T) {
+	ing := buildIngress()
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("backend-protocol")] = "HTTPS"
+	data[parser.GetAnnotationWithPrefix("secure-verify-ca-secret")] = "secure-verify-ca"
+	ing.SetAnnotations(data)
+
+	_, err := NewParser(mockCfg{
+		certs: map[string]resolver.AuthSSLCert{
+			"default/secure-verify-ca": {},
+		},
+	}).Parse(ing)
+	if err != nil {
+		t.Errorf("Unexpected error on ingress: %v", err)
+	}
+}

internal/ingress/controller/controller.go

@@ -706,8 +706,6 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
 			klog.V(3).Infof("Creating upstream %q", defBackend)
 			upstreams[defBackend] = newUpstream(defBackend)
 
-			upstreams[defBackend].SecureCACert = anns.SecureUpstream.CACert
-
 			upstreams[defBackend].UpstreamHashBy.UpstreamHashBy = anns.UpstreamHashBy.UpstreamHashBy
 			upstreams[defBackend].UpstreamHashBy.UpstreamHashBySubset = anns.UpstreamHashBy.UpstreamHashBySubset
 			upstreams[defBackend].UpstreamHashBy.UpstreamHashBySubsetSize = anns.UpstreamHashBy.UpstreamHashBySubsetSize
@@ -771,8 +769,6 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
 				upstreams[name] = newUpstream(name)
 				upstreams[name].Port = path.Backend.ServicePort
 
-				upstreams[name].SecureCACert = anns.SecureUpstream.CACert
-
 				upstreams[name].UpstreamHashBy.UpstreamHashBy = anns.UpstreamHashBy.UpstreamHashBy
 				upstreams[name].UpstreamHashBy.UpstreamHashBySubset = anns.UpstreamHashBy.UpstreamHashBySubset
 				upstreams[name].UpstreamHashBy.UpstreamHashBySubsetSize = anns.UpstreamHashBy.UpstreamHashBySubsetSize

internal/ingress/types.go

@@ -39,7 +39,6 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/annotations/ratelimit"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/redirect"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/rewrite"
-	"k8s.io/ingress-nginx/internal/ingress/resolver"
 )
 
 var (
@@ -86,9 +85,6 @@ type Backend struct {
 	Name    string             `json:"name"`
 	Service *apiv1.Service     `json:"service,omitempty"`
 	Port    intstr.IntOrString `json:"port"`
-	// SecureCACert has the filename and SHA1 of the certificate authorities used to validate
-	// a secured connection to the backend
-	SecureCACert resolver.AuthSSLCert `json:"secureCACert"`
 	// SSLPassthrough indicates that Ingress controller will delegate TLS termination to the endpoints.
 	SSLPassthrough bool `json:"sslPassthrough"`
 	// Endpoints contains the list of endpoints currently running

internal/ingress/types_equals.go

@@ -113,9 +113,6 @@ func (b1 *Backend) Equal(b2 *Backend) bool {
 	if b1.Port != b2.Port {
 		return false
 	}
-	if !(&b1.SecureCACert).Equal(&b2.SecureCACert) {
-		return false
-	}
 	if b1.SSLPassthrough != b2.SSLPassthrough {
 		return false
 	}

rootfs/etc/nginx/lua/test/balancer_test.lua

@@ -33,7 +33,6 @@ local function reset_backends()
   backends = {
     {
       name = "access-router-production-web-80", port = "80", secure = false,
-      secureCACert = { secret = "", caFilename = "", caSha = "" },
       sslPassthrough = false,
       endpoints = {
         { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 },