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.

Sign up for GitHub

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

Jump to bottom

fix: annotation konghq.com/rewrite always works #6626

Merged
merged 2 commits into from
Nov 8, 2024
+15 −10
Merged

fix: annotation konghq.com/rewrite always works #6626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e5f883
fix: make bug reproducible in tests (ensure not random order of routes)
programmer04 Nov 7, 2024
22c892d
fix: annotation konghq.com/rewrite always works
programmer04 Nov 7, 2024
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
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,18 @@ Adding a new version? You'll need three changes:

## Unreleased

### Changed

- Bump version of Gateway API to `1.2.0`.
[#6571](https://github.com/Kong/kubernetes-ingress-controller/pull/6571)

### Fixed

- Fixed annotation `konghq.com/rewrite` that was not being applied sometimes
when `Ingress` without annotation and a different `Ingress` with annotation
pointed to the same `Service`.
[#6569](https://github.com/Kong/kubernetes-ingress-controller/pull/6626)

## [3.3.1]

> Release date: 2024-08-28
Expand Down
9 changes: 5 additions & 4 deletions internal/dataplane/translator/subtranslator/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package subtranslator
import (
"fmt"
"regexp"
"sort"
"strings"
"time"
"unicode"
Expand Down Expand Up @@ -235,7 +236,9 @@ func (i *ingressTranslationIndex) Translate() map[string]kongstate.Service {
route := meta.translateIntoKongRoute()
kongStateService.Routes = append(kongStateService.Routes, *route)
}

sort.SliceStable(kongStateService.Routes, func(i, j int) bool {
return *kongStateService.Routes[i].Name < *kongStateService.Routes[j].Name
})
kongStateServiceCache[kongServiceName] = kongStateService
}

Expand Down Expand Up @@ -629,12 +632,11 @@ func MaybeRewriteURI(service *kongstate.Service, rewriteURIEnable bool) error {

rewriteURI, exists := annotations.ExtractRewriteURI(route.Ingress.Annotations)
if !exists {
return nil
continue
}
if !rewriteURIEnable {
return fmt.Errorf("konghq.com/rewrite annotation not supported when rewrite uris disabled")
}

if rewriteURI == "" {
rewriteURI = "/"
}
Expand All @@ -651,7 +653,6 @@ func MaybeRewriteURI(service *kongstate.Service, rewriteURIEnable bool) error {
},
},
})

}
return nil
}
7 changes: 1 addition & 6 deletions test/integration/ingress_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,9 +1092,7 @@ func TestIngressRewriteURI(t *testing.T) {

t.Logf("creating an Ingress for service %s without rewrite annotation", service.Name)
const serviceDomainDirect = "direct.example"
ingressDirect := generators.NewIngressForService("/", map[string]string{
annotations.AnnotationPrefix + annotations.StripPathKey: "true",
}, service)
ingressDirect := generators.NewIngressForService("/", nil, service)
ingressDirect.Name += "-direct"
ingressDirect.Spec.IngressClassName = kong.String(consts.IngressClass)
for i := range ingressDirect.Spec.Rules {
Expand Down Expand Up @@ -1165,7 +1163,6 @@ func TestIngressRewriteURI(t *testing.T) {
t.Logf("creating an Ingress for service %s with rewrite annotation", service.Name)
const serviceDomainRewrite = "rewrite.example"
ingressRewrite := generators.NewIngressForService("/~/foo/(.*)", map[string]string{
annotations.AnnotationPrefix + annotations.StripPathKey: "true",
annotations.AnnotationPrefix + annotations.RewriteURIKey: "/image/$1",
}, service)
ingressRewrite.Name += "-rewrite"
Expand All @@ -1178,8 +1175,6 @@ func TestIngressRewriteURI(t *testing.T) {
require.NoError(t, err)
cleaner.Add(ingressRewrite)

t.Log("rewrite uri feature is enabled")

t.Log("try to access the ingress with valid capture group")
helpers.EventuallyGETPath(t, proxyHTTPURL, serviceDomainRewrite, "/foo/jpeg", nil, http.StatusOK, consts.JPEGMagicNumber, nil, ingressWait, waitTick)

Expand Down
Loading