Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure https redirect happens before root redirect #1117

Merged
merged 1 commit into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/converters/ingress/annotations/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ func (c *updater) buildGlobalSSL(d *globalData) {
ssl.ModeAsync = d.mapper.Get(ingtypes.GlobalSSLModeAsync).Bool()
ssl.Options = d.mapper.Get(ingtypes.GlobalSSLOptions).Value
ssl.RedirectCode = d.mapper.Get(ingtypes.GlobalSSLRedirectCode).Int()
ssl.SSLRedirect = d.mapper.Get(ingtypes.BackSSLRedirect).Bool()
}

func (c *updater) buildGlobalHTTPStoHTTP(d *globalData) {
Expand Down
21 changes: 21 additions & 0 deletions pkg/haproxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func (c *config) WriteFrontendMaps() error {
//
RedirFromRootMap: mapBuilder.AddMap(mapsDir + "/_front_redir_fromroot.map"),
RedirFromMap: mapBuilder.AddMap(mapsDir + "/_front_redir_from.map"),
RedirRootSSLMap: mapBuilder.AddMap(mapsDir + "/_front_redir_root_ssl.map"),
RedirToMap: mapBuilder.AddMap(mapsDir + "/_front_redir_to.map"),
SSLPassthroughMap: mapBuilder.AddMap(mapsDir + "/_front_sslpassthrough.map"),
VarNamespaceMap: mapBuilder.AddMap(mapsDir + "/_front_namespace.map"),
Expand Down Expand Up @@ -274,6 +275,26 @@ func (c *config) WriteFrontendMaps() error {
// TODO wildcard/alias/alias-regex hostname can overlap
// a configured domain which doesn't have rootRedirect
if host.RootRedirect != "" {
// looking for root path configuration - if ssl redirect is enabled,
// we need to redirect to https before redirect the path.
redirectssl := func() bool {
redir := c.global.SSL.SSLRedirect
for _, path := range host.FindPath("/") {
if backend := c.backends.Items()[path.Backend.ID]; backend != nil {
if bpath := backend.FindBackendPath(path.Link); bpath != nil {
redir = bpath.SSLRedirect
if !path.Link.ComposeMatch() {
// give precedence for root path without method, header or cookie matching
return redir
}
}
}
}
return redir
}
if redirectssl() {
fmaps.RedirRootSSLMap.AddHostnameMapping(host.Hostname, "")
}
fmaps.RedirFromRootMap.AddHostnameMapping(host.Hostname, host.RootRedirect)
}
//
Expand Down
6 changes: 6 additions & 0 deletions pkg/haproxy/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4042,6 +4042,8 @@ func TestInstanceRootRedirect(t *testing.T) {
c := setup(t)
defer c.teardown()

c.config.global.SSL.SSLRedirect = true

var h *hatypes.Host
var b = c.config.Backends().AcquireBackend("d1", "app", "8080")
h = c.config.Hosts().AcquireHost("*.d1.local")
Expand Down Expand Up @@ -4081,6 +4083,7 @@ frontend _front_http
mode http
bind :80
<<set-req-base>>
http-request redirect scheme https if { path / } { var(req.host) -i -m str -f /etc/haproxy/maps/_front_redir_root_ssl__exact.map }
http-request set-var(req.rootredir) var(req.host),map_str(/etc/haproxy/maps/_front_redir_fromroot__exact.map)
http-request set-var(req.rootredir) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_fromroot__regex.map) if !{ var(req.rootredir) -m found }
http-request redirect location %[var(req.rootredir)] if { path / } { var(req.rootredir) -m found }
Expand Down Expand Up @@ -4116,6 +4119,9 @@ d2.local#/app1 d2_app_8080
`)
c.checkMap("_front_redir_fromroot__exact.map", `
d2.local /app1
`)
c.checkMap("_front_redir_root_ssl__exact.map", `
d2.local
`)
c.checkMap("_front_redir_fromroot__regex.map", `
^[^.]+\.d1\.local$ /app
Expand Down
6 changes: 6 additions & 0 deletions pkg/haproxy/types/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,12 @@ func (l *PathLink) Equals(other *PathLink) bool {
return l.hash == other.hash
}

// ComposeMatch returns true if the pathLink has composing match,
// by adding method, header or cookie match.
func (l *PathLink) ComposeMatch() bool {
return len(l.headers) > 0
}

// WithHostname ...
func (l *PathLink) WithHostname(hostname string) *PathLink {
l.hostname = hostname
Expand Down
2 changes: 2 additions & 0 deletions pkg/haproxy/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ type SSLConfig struct {
ModeAsync bool
Options string
RedirectCode int
SSLRedirect bool
}

// DHParamConfig ...
Expand Down Expand Up @@ -406,6 +407,7 @@ type FrontendMaps struct {
HTTPSSNIMap *HostsMap
//
RedirFromRootMap *HostsMap
RedirRootSSLMap *HostsMap
RedirFromMap *HostsMap
RedirToMap *HostsMap
SSLPassthroughMap *HostsMap
Expand Down
13 changes: 12 additions & 1 deletion rootfs/etc/templates/haproxy/haproxy.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1156,8 +1156,19 @@ frontend {{ $proxy__front_http }}
http-request set-var(req.host) hdr(host),field(1,:),lower
http-request set-var(req.base) var(req.host),concat(\#,req.path)

{{- /*------------------------------------*/}}
{{- $acmeexclusive := and $global.Acme.Enabled (not $global.Acme.Shared) }}

{{- /*------------------------------------*/}}
{{- if $fmaps.RedirRootSSLMap.HasHost }}
{{- range $match := $fmaps.RedirRootSSLMap.MatchFiles }}
http-request redirect scheme https
{{- if $global.SSL.RedirectCode }} code {{ $global.SSL.RedirectCode }}{{ end }}
{{- "" }} if{{ if $acmeexclusive }} !acme-challenge{{ end }}
{{- "" }} { path / } { var(req.host) -i -m {{ $match.Method }} -f {{ $match.Filename }} }
{{- end }}
{{- end }}

{{- /*------------------------------------*/}}
{{- if $fmaps.RedirFromRootMap.HasHost }}
{{- range $match := $fmaps.RedirFromRootMap.MatchFiles }}
http-request set-var(req.rootredir) var(req.host)
Expand Down
6 changes: 2 additions & 4 deletions tests/framework/options/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ import (

type Object func(o *objectOpt)

func AddConfigKeyAnnotations(ann map[string]string) Object {
func AddConfigKeyAnnotation(key, value string) Object {
annprefix := "haproxy-ingress.github.io/"
return func(o *objectOpt) {
if o.Ann == nil {
o.Ann = make(map[string]string)
}
for k, v := range ann {
o.Ann[annprefix+k] = v
}
o.Ann[annprefix+key] = value
}
}

Expand Down
22 changes: 20 additions & 2 deletions tests/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestIntegrationIngress(t *testing.T) {
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotations(map[string]string{ingtypes.BackSSLRedirect: "false"}),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "false"),
)
res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusOK))
assert.True(t, res.EchoResponse)
Expand All @@ -61,7 +61,7 @@ func TestIntegrationIngress(t *testing.T) {
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotations(map[string]string{ingtypes.BackSSLRedirect: "true"}),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "true"),
)
res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusFound))
assert.False(t, res.EchoResponse)
Expand Down Expand Up @@ -103,6 +103,24 @@ func TestIntegrationIngress(t *testing.T) {
assert.Equal(t, reqHeaders, res.ReqHeaders)
})

t.Run("should redirect to https before app-root", func(t *testing.T) {
t.Parallel()
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "true"),
options.AddConfigKeyAnnotation(ingtypes.HostAppRoot, "/app"),
)

res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusFound))
assert.False(t, res.EchoResponse)
assert.Equal(t, fmt.Sprintf("https://%s/", hostname), res.HTTPResponse.Header.Get("location"))

res = f.Request(ctx, t, http.MethodGet, hostname, "/", options.HTTPSRequest(true))
assert.False(t, res.EchoResponse)
assert.Equal(t, "/app", res.HTTPResponse.Header.Get("location"))
})

t.Run("should take leader", func(t *testing.T) {
t.Parallel()
assert.EventuallyWithT(t, func(collect *assert.CollectT) {
Expand Down
Loading