Skip to content

Commit

Permalink
Merge pull request #1117 from jcmoraisjr/jm-https-before-root-ctx
Browse files Browse the repository at this point in the history
Ensure https redirect happens before root redirect
  • Loading branch information
jcmoraisjr authored Jun 3, 2024
2 parents 0a1b2af + 4846c3c commit 5594eb9
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 7 deletions.
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

0 comments on commit 5594eb9

Please sign in to comment.