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

Update HTTPRequestRedirectPolicy StatusCode to use RedirectResponseCode #6789

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 1 addition & 2 deletions apis/projectcontour/v1/httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,7 @@ type HTTPRequestRedirectPolicy struct {
// StatusCode is the HTTP status code to be used in response.
// +optional
// +kubebuilder:default=302
// +kubebuilder:validation:Enum=301;302
StatusCode *int `json:"statusCode,omitempty"`
StatusCode *RedirectResponseCode `json:"statusCode,omitempty"`

// Path allows for redirection to a different path from the
// original on the request. The path must start with a
Expand Down
2 changes: 1 addition & 1 deletion apis/projectcontour/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelogs/unreleased/6789-billyjs-small.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use RedirectResponseCode instead of just 301/302 for HTTPRequestRedirectPolicy StatusCode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use RedirectResponseCode instead of just 301/302 for HTTPRequestRedirectPolicy StatusCode
`HTTPProxy.spec.routes.requestRedirectPolicy.statusCode` now supports 303, 307 and 308 redirect status codes in addition to 301 and 302.

4 changes: 4 additions & 0 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6632,6 +6632,10 @@ spec:
enum:
- 301
- 302
- 303
- 307
- 308
format: int32
type: integer
type: object
responseHeadersPolicy:
Expand Down
4 changes: 4 additions & 0 deletions examples/render/contour-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6852,6 +6852,10 @@ spec:
enum:
- 301
- 302
- 303
- 307
- 308
format: int32
type: integer
type: object
responseHeadersPolicy:
Expand Down
4 changes: 4 additions & 0 deletions examples/render/contour-gateway-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6643,6 +6643,10 @@ spec:
enum:
- 301
- 302
- 303
- 307
- 308
format: int32
type: integer
type: object
responseHeadersPolicy:
Expand Down
4 changes: 4 additions & 0 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6668,6 +6668,10 @@ spec:
enum:
- 301
- 302
- 303
- 307
- 308
format: int32
type: integer
type: object
responseHeadersPolicy:
Expand Down
4 changes: 4 additions & 0 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6852,6 +6852,10 @@ spec:
enum:
- 301
- 302
- 303
- 307
- 308
format: int32
type: integer
type: object
responseHeadersPolicy:
Expand Down
8 changes: 4 additions & 4 deletions internal/dag/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12395,7 +12395,7 @@ func TestDAGInsert(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
},
}},
},
Expand Down Expand Up @@ -12438,7 +12438,7 @@ func TestDAGInsert(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
},
}},
},
Expand Down Expand Up @@ -12490,7 +12490,7 @@ func TestDAGInsert(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
},
}},
},
Expand Down Expand Up @@ -12643,7 +12643,7 @@ func TestDAGInsert(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ type DirectResponse struct {
Body string
}

// Redirect allows for a 301/302 redirect to be the response
// Redirect allows for a redirect to be the response
// to a route request vs. routing to an envoy cluster.
type Redirect struct {
// Hostname is the host name to redirect to.
Expand All @@ -230,7 +230,7 @@ type Redirect struct {
PortNumber uint32

// StatusCode is the HTTP response code to
// use. Valid options are 301 or 302.
// use. Valid options are 301, 302, 303, 307, or 308.
StatusCode int

// PathRewritePolicy is the policy for rewriting
Expand Down
2 changes: 1 addition & 1 deletion internal/dag/httpproxy_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1951,7 +1951,7 @@ func redirectRoutePolicy(redirect *contour_v1.HTTPRequestRedirectPolicy) (*Redir

var statusCode int
if redirect.StatusCode != nil {
statusCode = *redirect.StatusCode
statusCode = int(*redirect.StatusCode)
}

if redirect.Path != nil && redirect.Prefix != nil {
Expand Down
6 changes: 6 additions & 0 deletions internal/envoy/v3/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,12 @@
r.Redirect.ResponseCode = envoy_config_route_v3.RedirectAction_MOVED_PERMANENTLY
case http.StatusFound:
r.Redirect.ResponseCode = envoy_config_route_v3.RedirectAction_FOUND
case http.StatusSeeOther:
r.Redirect.ResponseCode = envoy_config_route_v3.RedirectAction_SEE_OTHER
case http.StatusTemporaryRedirect:
r.Redirect.ResponseCode = envoy_config_route_v3.RedirectAction_TEMPORARY_REDIRECT
case http.StatusPermanentRedirect:
r.Redirect.ResponseCode = envoy_config_route_v3.RedirectAction_PERMANENT_REDIRECT

Check warning on line 400 in internal/envoy/v3/route.go

View check run for this annotation

Codecov / codecov/patch

internal/envoy/v3/route.go#L395-L400

Added lines #L395 - L400 were not covered by tests
}

return r
Expand Down
2 changes: 1 addition & 1 deletion internal/envoy/v3/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2762,7 +2762,7 @@ func TestRouteRedirect(t *testing.T) {
},
"unsupported status code specified": {
redirect: &dag.Redirect{
StatusCode: 303,
StatusCode: 306,
},
want: &envoy_config_route_v3.Route_Redirect{
Redirect: &envoy_config_route_v3.RedirectAction{},
Expand Down
6 changes: 3 additions & 3 deletions internal/featuretests/v3/redirectroutepolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestRedirectResponsePolicy_HTTProxy(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
Path: ptr.To("/blog"),
},
}},
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestRedirectResponsePolicy_HTTProxy(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
Prefix: ptr.To("/blogprefix"),
},
}},
Expand Down Expand Up @@ -135,7 +135,7 @@ func TestRedirectResponsePolicy_HTTProxy(t *testing.T) {
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(443)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
Prefix: ptr.To("/blogprefix"),
Path: ptr.To("/blogprefix"),
},
Expand Down
7 changes: 5 additions & 2 deletions site/content/docs/main/config/api-reference.html
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,9 @@ <h3 id="projectcontour.io/v1.HTTPRequestRedirectPolicy">HTTPRequestRedirectPolic
<code>statusCode</code>
<br>
<em>
int
<a href="#projectcontour.io/v1.RedirectResponseCode">
RedirectResponseCode
</a>
</em>
</td>
<td>
Expand Down Expand Up @@ -3085,7 +3087,8 @@ <h3 id="projectcontour.io/v1.RedirectResponseCode">RedirectResponseCode
(<code>uint32</code> alias)</p></h3>
<p>
(<em>Appears on:</em>
<a href="#projectcontour.io/v1.HTTPInternalRedirectPolicy">HTTPInternalRedirectPolicy</a>)
<a href="#projectcontour.io/v1.HTTPInternalRedirectPolicy">HTTPInternalRedirectPolicy</a>,
<a href="#projectcontour.io/v1.HTTPRequestRedirectPolicy">HTTPRequestRedirectPolicy</a>)
</p>
<p>
<p>RedirectResponseCode is a uint32 type alias with validation to ensure that the value is valid.</p>
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/httpproxy/internal_redirect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func getInternalRedirectHTTPProxy(namespace string) *contour_v1.HTTPProxy {
Services: []contour_v1.Service{},
RequestRedirectPolicy: &contour_v1.HTTPRequestRedirectPolicy{
Hostname: ptr.To(fqdn),
StatusCode: ptr.To(302),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(302)),
Path: ptr.To("/echo"),
},
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/httpproxy/request_redirect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func getRedirectHTTPProxy(namespace string, removeServices bool) *contour_v1.HTT
Scheme: ptr.To("https"),
Hostname: ptr.To("envoyproxy.io"),
Port: ptr.To(int32(8080)),
StatusCode: ptr.To(301),
StatusCode: ptr.To(contour_v1.RedirectResponseCode(301)),
},
}, {
Conditions: []contour_v1.MatchCondition{{
Expand Down
Loading