Skip to content

Commit

Permalink
Merge pull request #1200 from lsst-sqre/tickets/DM-48326
Browse files Browse the repository at this point in the history
DM-48326: Stop escaping `:` in Ingress auth-url
  • Loading branch information
rra authored Jan 8, 2025
2 parents c0751dc + b811dcf commit 71d700d
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 9 deletions.
3 changes: 3 additions & 0 deletions changelog.d/20250107_161611_rra_DM_48326.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Bug fixes

- Do not escape `:` characters in the `auth-url` `Ingress` annotation. ingress-nginx 4.12.0 has added a restrictive regex filter to acceptable URLs that disallows `%` and therefore all escaped characters.
2 changes: 1 addition & 1 deletion src/gafaelfawr/services/kubernetes.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def _build_annotations(self, ingress: GafaelfawrIngress) -> dict[str, str]:
auth_url = (
str(self._config.base_internal_url).rstrip("/")
+ "/ingress/auth?"
+ urlencode(ingress.config.to_auth_query())
+ urlencode(ingress.config.to_auth_query(), safe=":/")
)
base_url = ingress.config.base_url or str(self._config.base_url)
snippet_key = "nginx.ingress.kubernetes.io/configuration-snippet"
Expand Down
12 changes: 6 additions & 6 deletions tests/data/kubernetes/output/ingresses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/auth-method: GET
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&service=tap"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&service=tap"
nginx.ingress.kubernetes.io/configuration-snippet: |
{snippet}
creationTimestamp: {any}
Expand Down Expand Up @@ -50,7 +50,7 @@ metadata:
nginx.ingress.kubernetes.io/auth-method: "GET"
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-signin: "https://foo.example.com/login"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&satisfy=any&notebook=true&minimum_lifetime=600"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&satisfy=any&notebook=true&minimum_lifetime=600"
nginx.ingress.kubernetes.io/configuration-snippet: |
{snippet}
creationTimestamp: {any}
Expand Down Expand Up @@ -95,7 +95,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/auth-method: GET
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&scope=read%3Asome&delegate_to=some-service&delegate_scope=read%3Aall%2Cread%3Asome&auth_type=basic"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&scope=read:some&delegate_to=some-service&delegate_scope=read:all%2Cread:some&auth_type=basic"
nginx.ingress.kubernetes.io/configuration-snippet: |
{snippet}
creationTimestamp: {any}
Expand Down Expand Up @@ -135,7 +135,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/auth-method: GET
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&delegate_to=some-service&delegate_scope=read%3Aall&use_authorization=true"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&delegate_to=some-service&delegate_scope=read:all&use_authorization=true"
nginx.ingress.kubernetes.io/configuration-snippet: |
add_header "X-Foo" "bar";
{snippet}
Expand Down Expand Up @@ -215,7 +215,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/auth-method: GET
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&username=some-user"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&username=some-user"
nginx.ingress.kubernetes.io/configuration-snippet: |
{snippet}
creationTimestamp: {any}
Expand Down Expand Up @@ -255,7 +255,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/auth-method: GET
nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&only_service=portal&only_service=vo-cutouts&service=uws"
nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&only_service=portal&only_service=vo-cutouts&service=uws"
nginx.ingress.kubernetes.io/configuration-snippet: |
{snippet}
creationTimestamp: {any}
Expand Down
4 changes: 2 additions & 2 deletions tests/operator/ingress_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async def test_replace(

expected_url = (
"http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth"
"?scope=read%3Aall&service=tap&auth_type=basic"
"?scope=read:all&service=tap&auth_type=basic"
)
expected["metadata"]["annotations"][
"nginx.ingress.kubernetes.io/auth-url"
Expand Down Expand Up @@ -111,7 +111,7 @@ async def test_replace(

expected_url = (
"http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth"
"?scope=read%3Aall&service=tap&auth_type=bearer"
"?scope=read:all&service=tap&auth_type=bearer"
)
expected["metadata"]["annotations"][
"nginx.ingress.kubernetes.io/auth-url"
Expand Down

0 comments on commit 71d700d

Please sign in to comment.