Merge pull request #1118 from jcmoraisjr/jm-follow-ssl-redirect

Add ssl-always-follow-redirect option

docs/content/en/docs/configuration/keys.md

@@ -456,6 +456,7 @@ The table below describes all supported configuration keys.
 | [`slots-min-free`](#dynamic-scaling)                 | minimum number of free slots            | Backend | `0`                |
 | [`source-address-intf`](#source-address-intf)        | `<intf1>[,<intf2>...]`                  | Backend |                    |
 | [`ssl-always-add-https`](#ssl-always-add-https)      | [true\|false]                           | Host    | `false`            |
+| [`ssl-always-follow-redirect`](#ssl-always-add-https) | [true\|false]                          | Host    | `true`             |
 | [`ssl-cipher-suites`](#ssl-ciphers)                  | colon-separated list                    | Host    | [see description](#ssl-ciphers) |
 | [`ssl-cipher-suites-backend`](#ssl-ciphers)          | colon-separated list                    | Backend | [see description](#ssl-ciphers) |
 | [`ssl-ciphers`](#ssl-ciphers)                        | colon-separated list                    | Host    | [see description](#ssl-ciphers) |
@@ -2488,15 +2489,20 @@ See also:
 
 ## SSL always add HTTPS
 
-| Configuration key      | Scope | Default | Since   |
-|------------------------|-------|---------|---------|
-| `ssl-always-add-https` | Host  | `false` | v0.12.4 |
+| Configuration key            | Scope | Default | Since   |
+|------------------------------|-------|---------|---------|
+| `ssl-always-add-https`       | Host  | `false` | v0.12.4 |
+| `ssl-always-follow-redirect` | Host  | `true`  | v0.14.7 |
 
 Every hostname declared on an Ingress resource is added to an internal HTTP map. If at least one Ingress adds the hostname in the `tls` attribute, the hostname is also added to an internal HTTPS map and does ssl offload using the default certificate. A secret name can also be added in the `tls` attribute, overriding the certificate used in the TLS handshake.
 
 `ssl-always-add-https` asks the controller to always add the domain in the internal HTTP and HTTPS maps, even if the `tls` attribute isn't declared. If `false`, a missing `tls` attribute will only declare the domain in the HTTP map and `ssl-redirect` is ignored. If `true`, a missing `tls` attribute adds the domain in the HTTPS map, and the TLS handshake will use the default certificate. If `tls` attribute is used, this configuration is ignored.
 
-The default value is `false` since v0.13 to correctly implement Ingress spec. The default value can be globally changed in the global ConfigMap.
+`ssl-always-follow-redirect` configures how the `ssl-redirect` option should be used when the `tls` attribute is missing, but the host is added in the HTTPS map. When `false`, it makes the controller to mimic a v0.11 and older behavior by not redirecting to HTTPS if the ingress does not declare the `tls` attribute. When `true`, SSL redirect will happen if configured, regardless the presence of the `tls` attribute. This option is ignored if `ssl-always-add-https` is false.
+
+The default value for `ssl-always-add-https` is `false` since v0.13 to correctly implement Ingress spec. The default value can be globally changed in the global ConfigMap.
+
+These options are implemented to help teams upgrade from older controller versions without disruptions. It is suggested not to be changed, and if so, it is also suggested to evolve ingress resources to a state that does not depend on it in the mid term.
 
 ---
 

pkg/converters/ingress/annotations/backend.go

@@ -977,7 +977,7 @@ func (c *updater) buildBackendSSL(d *backData) {
 func (c *updater) buildBackendSSLRedirect(d *backData) {
 	noTLSRedir := utils.Split(d.mapper.Get(ingtypes.GlobalNoTLSRedirectLocations).Value, ",")
 	for _, path := range d.backend.Paths {
-		redir := path.Host != nil && path.Host.HasTLS() &&
+		redir := path.Host != nil && path.Host.UseTLS() &&
 			d.mapper.GetConfig(path.Link).Get(ingtypes.BackSSLRedirect).Bool()
 		if redir {
 			for _, noredir := range noTLSRedir {

pkg/converters/ingress/annotations/updater.go

@@ -213,6 +213,7 @@ func (c *updater) UpdateHostConfig(host *hatypes.Host, mapper *Mapper) {
 	host.Alias.AliasName = mapper.Get(ingtypes.HostServerAlias).Value
 	host.Alias.AliasRegex = mapper.Get(ingtypes.HostServerAliasRegex).Value
 	host.TLS.UseDefaultCrt = mapper.Get(ingtypes.HostSSLAlwaysAddHTTPS).Bool()
+	host.TLS.FollowRedirect = mapper.Get(ingtypes.HostSSLAlwaysFollowRedirect).Bool()
 	host.VarNamespace = mapper.Get(ingtypes.HostVarNamespace).Bool()
 	c.buildHostAuthExternal(data)
 	c.buildHostAuthTLS(data)

pkg/converters/ingress/annotations/updater_test.go

@@ -170,7 +170,7 @@ const testingHostname = "host.local"
 
 type hostResolver struct{}
 
-func (h *hostResolver) HasTLS() bool {
+func (h *hostResolver) UseTLS() bool {
 	return true
 }
 

pkg/converters/ingress/defaults.go

@@ -32,12 +32,13 @@ func createDefaults() map[string]string {
 	return map[string]string{
 		types.TCPTCPServiceLogFormat: "default",
 		//
-		types.HostAuthTLSStrict:     "true",
-		types.HostSSLAlwaysAddHTTPS: "false",
-		types.HostSSLCiphers:        defaultSSLCiphers,
-		types.HostSSLCipherSuites:   defaultSSLCipherSuites,
-		types.HostSSLOptionsHost:    "",
-		types.HostTLSALPN:           "h2,http/1.1",
+		types.HostAuthTLSStrict:           "true",
+		types.HostSSLAlwaysAddHTTPS:       "false",
+		types.HostSSLAlwaysFollowRedirect: "true",
+		types.HostSSLCiphers:              defaultSSLCiphers,
+		types.HostSSLCipherSuites:         defaultSSLCipherSuites,
+		types.HostSSLOptionsHost:          "",
+		types.HostTLSALPN:                 "h2,http/1.1",
 		//
 		types.BackAuthExternalPlacement:  "backend",
 		types.BackAuthHeadersFail:        "*",

pkg/converters/ingress/types/annotations.go

@@ -36,25 +36,26 @@ var (
 
 // Host Annotations
 const (
-	HostAcmePreferredChain     = "acme-preferred-chain"
-	HostAppRoot                = "app-root"
-	HostAuthTLSErrorPage       = "auth-tls-error-page"
-	HostAuthTLSSecret          = "auth-tls-secret"
-	HostAuthTLSStrict          = "auth-tls-strict"
-	HostAuthTLSVerifyClient    = "auth-tls-verify-client"
-	HostCertSigner             = "cert-signer"
-	HostRedirectFrom           = "redirect-from"
-	HostRedirectFromRegex      = "redirect-from-regex"
-	HostServerAlias            = "server-alias"
-	HostServerAliasRegex       = "server-alias-regex"
-	HostSSLAlwaysAddHTTPS      = "ssl-always-add-https"
-	HostSSLCiphers             = "ssl-ciphers"
-	HostSSLCipherSuites        = "ssl-cipher-suites"
-	HostSSLOptionsHost         = "ssl-options-host"
-	HostSSLPassthrough         = "ssl-passthrough"
-	HostSSLPassthroughHTTPPort = "ssl-passthrough-http-port"
-	HostTLSALPN                = "tls-alpn"
-	HostVarNamespace           = "var-namespace"
+	HostAcmePreferredChain      = "acme-preferred-chain"
+	HostAppRoot                 = "app-root"
+	HostAuthTLSErrorPage        = "auth-tls-error-page"
+	HostAuthTLSSecret           = "auth-tls-secret"
+	HostAuthTLSStrict           = "auth-tls-strict"
+	HostAuthTLSVerifyClient     = "auth-tls-verify-client"
+	HostCertSigner              = "cert-signer"
+	HostRedirectFrom            = "redirect-from"
+	HostRedirectFromRegex       = "redirect-from-regex"
+	HostServerAlias             = "server-alias"
+	HostServerAliasRegex        = "server-alias-regex"
+	HostSSLAlwaysAddHTTPS       = "ssl-always-add-https"
+	HostSSLAlwaysFollowRedirect = "ssl-always-follow-redirect"
+	HostSSLCiphers              = "ssl-ciphers"
+	HostSSLCipherSuites         = "ssl-cipher-suites"
+	HostSSLOptionsHost          = "ssl-options-host"
+	HostSSLPassthrough          = "ssl-passthrough"
+	HostSSLPassthroughHTTPPort  = "ssl-passthrough-http-port"
+	HostTLSALPN                 = "tls-alpn"
+	HostVarNamespace            = "var-namespace"
 )
 
 var (

pkg/haproxy/types/host.go

@@ -260,8 +260,9 @@ func (h *Host) AddRedirect(path string, match MatchType, redirTo string) {
 }
 
 type hostResolver struct {
-	useDefaultCrt *bool
-	crtFilename   *string
+	useDefaultCrt  *bool
+	followRedirect *bool
+	crtFilename    *string
 }
 
 func (h *Host) addPath(path string, match MatchType, backend *Backend, redirTo string) *HostPath {
@@ -281,8 +282,9 @@ func (h *Host) addLink(backend *Backend, link *PathLink, redirTo string) *HostPa
 		}
 		bpath := backend.AddBackendPath(link)
 		bpath.Host = &hostResolver{
-			useDefaultCrt: &h.TLS.UseDefaultCrt,
-			crtFilename:   &h.TLS.TLSFilename,
+			useDefaultCrt:  &h.TLS.UseDefaultCrt,
+			followRedirect: &h.TLS.FollowRedirect,
+			crtFilename:    &h.TLS.TLSFilename,
 		}
 	} else if redirTo == "" {
 		hback = HostBackend{ID: "_error404"}
@@ -327,8 +329,14 @@ func (h *Host) HasTLS() bool {
 	return h.TLS.UseDefaultCrt || h.TLS.TLSHash != ""
 }
 
-func (h *hostResolver) HasTLS() bool {
-	return *h.useDefaultCrt || *h.crtFilename != ""
+func (h *hostResolver) UseTLS() bool {
+	// whether the ingress resource has the `tls:` entry for the host
+	hasTLSEntry := *h.crtFilename != ""
+
+	// whether the user has globally or locally configured auto TLS for the host
+	autoTLSEnabled := *h.useDefaultCrt && *h.followRedirect
+
+	return hasTLSEntry || autoTLSEnabled
 }
 
 // HasTLSAuth ...

pkg/haproxy/types/types.go

@@ -572,8 +572,9 @@ type HostRedirectConfig struct {
 // HostTLSConfig ...
 type HostTLSConfig struct {
 	TLSConfig
-	CAErrorPage   string
-	UseDefaultCrt bool
+	CAErrorPage    string
+	UseDefaultCrt  bool
+	FollowRedirect bool
 }
 
 // EndpointNaming ...
@@ -693,7 +694,7 @@ type BackendPathItem struct {
 
 // HostResolver ...
 type HostResolver interface {
-	HasTLS() bool
+	UseTLS() bool
 }
 
 // BackendPath ...

tests/framework/framework.go

@@ -46,6 +46,11 @@ const (
 	PublishSvcName  = "default/publish"
 	PublishAddress  = "10.0.1.1"
 	PublishHostname = "ingress.local"
+
+	TestPortHTTP       = 28080
+	TestPortHTTPS      = 28443
+	TestPortStat       = 21936
+	TestPortTCPService = 25432
 )
 
 func NewFramework(ctx context.Context, t *testing.T, o ...options.Framework) *framework {
@@ -164,8 +169,9 @@ func (f *framework) StartController(ctx context.Context, t *testing.T) {
 	global.Namespace = "default"
 	global.Name = "ingress-controller"
 	global.Data = map[string]string{
-		"http-port":       "18080",
-		"https-port":      "18443",
+		"http-port":       strconv.Itoa(TestPortHTTP),
+		"https-port":      strconv.Itoa(TestPortHTTPS),
+		"stats-port":      strconv.Itoa(TestPortStat),
 		"max-connections": "20",
 	}
 	err = f.cli.Create(ctx, &global)
@@ -218,9 +224,9 @@ func (f *framework) Request(ctx context.Context, t *testing.T, method, host, pat
 	t.Logf("request method=%s host=%s path=%s\n", method, host, path)
 	opt := options.ParseRequestOptions(o...)
 
-	url := "http://127.0.0.1:18080"
+	url := fmt.Sprintf("http://127.0.0.1:%d", TestPortHTTP)
 	if opt.HTTPS {
-		url = "https://127.0.0.1:18443"
+		url = fmt.Sprintf("https://127.0.0.1:%d", TestPortHTTPS)
 	}
 	req, err := http.NewRequestWithContext(ctx, method, url, nil)
 	require.NoError(t, err)

tests/integration/integration_test.go

@@ -304,12 +304,12 @@ func TestIntegrationGateway(t *testing.T) {
 
 		t.Run("expose TCPRoute", func(t *testing.T) {
 			t.Parallel()
-			gw := f.CreateGatewayV1(ctx, t, gc, options.Listener("pgserver", "TCP", 15432))
+			gw := f.CreateGatewayV1(ctx, t, gc, options.Listener("pgserver", "TCP", framework.TestPortTCPService))
 			svc := f.CreateService(ctx, t, tcpServerPort)
 			_ = f.CreateTCPRouteA2(ctx, t, gw, svc)
-			res1 := f.TCPRequest(ctx, t, 15432, "ping")
+			res1 := f.TCPRequest(ctx, t, framework.TestPortTCPService, "ping")
 			assert.Equal(t, "ping", res1)
-			res2 := f.TCPRequest(ctx, t, 15432, "reply")
+			res2 := f.TCPRequest(ctx, t, framework.TestPortTCPService, "reply")
 			assert.Equal(t, "reply", res2)
 		})
 	})