Page MenuHomePhabricator
Create Task
Maniphest T329074

Test routing PCS through API Gateway
Closed, ResolvedPublic
Actions

Description

Background Information

In order to be able to test PCS responses without restbase, we need a sandbox with routing through API gateway to get an idea on how close we are to a response from restbase.

What

Split this work in two phases

  1. Internal Connections: add a routing URL that doesn't go through RESTBase
  2. Public Connection: final test with api gateway, e. g. https://api.wikimedia.org/api/test/[...]

Acceptance Criteria

  • Be able to request a PCS page in an internal request without passing through RESTBase - API Gateway as routing
  • Be able to request a PCS page in an public request in a sandbox environment without passing through RESTBase - API Gateway as routing

Details

SubjectRepoBranchLines +/-
rest-gateway: don't append when setting headersoperations/deployment-chartsmaster+9 -1
rest-gateway: add helmfile, enable mobileappsoperations/deployment-chartsmaster+132 -0
Add service records for rest-gatewayoperations/dnsmaster+6 -2
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedhnowlanT329049 Configure REST Gateway
 ResolvedhnowlanT329074 Test routing PCS through API Gateway

Event Timeline

MSantos created this task.Feb 7 2023, 4:34 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 7 2023, 4:34 PM
MSantos triaged this task as High priority.Feb 7 2023, 4:34 PM
DAlangi_WMF subscribed.Feb 10 2023, 12:53 PM
hnowlan added a comment.Feb 10 2023, 12:54 PM

Could you give me the desired query URL please for testing? I'm assuming this is being routed to mobileapps.discovery.wmnet, correct?

Jgiannelos added a comment.EditedFeb 10 2023, 12:58 PM

I think a good starting point to test PCS related endpoints is to setup the following routes for mobileapps:

  • <domain>/v1/page/mobile-html/<title>
  • <domain>/v1/page/mobile-html/<title>/<revision>
  • <domain>/v1/page/summary/<title>
  • <domain>/v1/page/summary/<title>/<revision>

The backend is mobileapps.discovery.wmnet.

The URL mapping that we use in RESTBase is the following:

  • For mobile-html
    • without revision specified
      • restbase:port/<domain>/v1/page/mobile-html/<title>
      • mobileapps:port/<domain>/v1/page/mobile-html/<title>
    • with revision specified
      • restbase:port/<domain>/v1/page/mobile-html/<title>
      • mobileapps:port/<domain>/v1/page/mobile-html/<title>
  • For summary
    • without revision specified
      • restbase:port/<domain>/v1/page/summary/<title>
      • mobileapps:port/<domain>/v1/page/summary/<title>
    • with revision specified
      • restbase:port/<domain>/v1/page/summary/<title>/<revision>
        • note: this is something not exposed currently in restbase but is a known feature that we never implemented because of restbase maintenance status
      • mobileapps:port/<domain>/v1/page/summary/<title>/<revision>
Jgiannelos moved this task from Backlog to In Progress on the Content-Transform-Team-WIP board.Feb 10 2023, 1:31 PM
MSantos assigned this task to hnowlan.Feb 13 2023, 4:42 PM
hnowlan added a parent task: T329049: Configure REST Gateway.Feb 14 2023, 11:24 AM
gerritbot added a comment.Mar 7 2023, 5:05 PM

Change 895327 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] rest-gateway: add helmfile, enable mobileapps

https://gerrit.wikimedia.org/r/895327

gerritbot added a project: Patch-For-Review.Mar 7 2023, 5:05 PM
daniel added a comment.Mar 7 2023, 5:54 PM

Be able to request a PCS page in an internal request without passing through RESTBase - API Gateway as routing

In my mind, internal requests should go through the service mesh, not through the gateway. Did I get that wrong?

• JMcLeod_WMF moved this task from In Progress to Blocked on the Content-Transform-Team-WIP board.Mar 13 2023, 3:18 PM
gerritbot added a comment.Mar 30 2023, 10:28 AM

Change 904493 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/dns@master] Add service records for rest-gateway

https://gerrit.wikimedia.org/r/904493

gerritbot added a comment.Mar 30 2023, 11:05 AM

Change 904493 merged by Hnowlan:

[operations/dns@master] Add service records for rest-gateway

https://gerrit.wikimedia.org/r/904493

gerritbot added a comment.Apr 11 2023, 3:58 PM

Change 895327 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway: add helmfile, enable mobileapps

https://gerrit.wikimedia.org/r/895327

Maintenance_bot removed a project: Patch-For-Review.Apr 11 2023, 4:11 PM
hnowlan added a comment.Apr 12 2023, 4:08 PM

There is now an instance of the rest gateway routing mobileapps running in staging for testing. Mobileapps routes can be tested via something like curl https://staging.svc.eqiad.wmnet:4113/en.wikipedia.org/v1/page/mobile-html/Dublin.

dr0ptp4kt moved this task from Blocked to To Verify on the Content-Transform-Team-WIP board.Apr 17 2023, 3:22 PM
MSantos added a comment.Apr 25 2023, 3:17 PM

@hnowlan is the API gateway overriding the response headers? or it's not forwarding headers to the services? I'm saying that because there's a difference in the headers for the requests, see a comparison with current production through restbase:

"中華民國國軍": {
       "added": {
           "x-envoy-upstream-service-time": "836"
       },
       "deleted": {},
       "updated": {
           "access-control-allow-origin": "*,*",
           "access-control-allow-headers": "accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding,accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding",
           "access-control-expose-headers": "etag,etag",
           "x-xss-protection": "1; mode=block, 1; mode=block",
           "x-content-type-options": "nosniff, nosniff",
           "x-frame-options": "SAMEORIGIN, SAMEORIGIN",
           "access-control-allow-methods": "GET,HEAD,GET,HEAD",
           "referrer-policy": "origin-when-cross-origin, origin-when-cross-origin"
       }
   }

This differences could be either API Gateway not forwarding the header 'x-restbase-compat': 'true' to PCS or overriding the headers. The difference doesn't happen requesting PCS directly.

hnowlan added a comment.Apr 28 2023, 4:01 PM
In T329074#8804974, @MSantos wrote:

@hnowlan is the API gateway overriding the response headers? or it's not forwarding headers to the services? I'm saying that because there's a difference in the headers for the requests, see a comparison with current production through restbase:

"中華民國國軍": {
       "added": {
           "x-envoy-upstream-service-time": "836"
       },
       "deleted": {},
       "updated": {
           "access-control-allow-origin": "*,*",
           "access-control-allow-headers": "accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding,accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding",
           "access-control-expose-headers": "etag,etag",
           "x-xss-protection": "1; mode=block, 1; mode=block",
           "x-content-type-options": "nosniff, nosniff",
           "x-frame-options": "SAMEORIGIN, SAMEORIGIN",
           "access-control-allow-methods": "GET,HEAD,GET,HEAD",
           "referrer-policy": "origin-when-cross-origin, origin-when-cross-origin"
       }
   }

This differences could be either API Gateway not forwarding the header 'x-restbase-compat': 'true' to PCS or overriding the headers. The difference doesn't happen requesting PCS directly.

Interesting - it appears the gateway is appending headers rather than replacing. Not ideal as it will require us to add this logic into our Lua layer, but this isn't a critical issue.

However, the gateway *shouldn't* be removing upstream headers. I see content-type being set correctly, and things like x-webkit-csp that we explicitly don't touch in the gateway.

MSantos added a comment.May 2 2023, 12:23 PM

So, how can I debug what is the incoming header in PCS when routing through the API Gateway? More specifically, does the API Gateway send anything in the payload that can make it possible to identify which requests came through API Gateway and the headers requested?

MSantos added a project: Page Content Service.May 8 2023, 7:34 AM
Restricted Application added a project: Product-Infrastructure-Team-Backlog-Deprecated. · View Herald TranscriptMay 8 2023, 7:34 AM
MSantos moved this task from Needs Triage to In Progress on the Page Content Service board.May 8 2023, 7:41 AM
hnowlan added a comment.May 8 2023, 9:51 AM
In T329074#8819973, @MSantos wrote:

So, how can I debug what is the incoming header in PCS when routing through the API Gateway? More specifically, does the API Gateway send anything in the payload that can make it possible to identify which requests came through API Gateway and the headers requested?

Envoy will set x-envoy-internal when a request is being sent to a service. Here are the full headers sent/received via a simple curl:

2023-05-08 09:44:33.906][85][debug][http] [source/common/http/conn_manager_impl.cc:882] [C43097][S11737610104135090600] request headers complete (end_stream=true):
':authority', 'staging.svc.eqiad.wmnet:4113'
':path', '/en.wikipedia.org/v1/page/mobile-html/Duck'
':method', 'GET'
'user-agent', 'curl/7.64.0'
'accept', '*/*'

[2023-05-08 09:44:33.906][85][debug][http] [source/common/http/filter_manager.cc:779] [C43097][S11737610104135090600] request end stream
[2023-05-08 09:44:33.906][85][debug][router] [source/common/router/router.cc:445] [C43097][S11737610104135090600] cluster 'mobileapps_cluster' match for URL '/en.wikipedia.org/v1/page/mobile-html/Duck'

[2023-05-08 09:44:33.906][85][debug][router] [source/common/router/router.cc:631] [C43097][S11737610104135090600] router decoding headers:
':authority', 'mobileapps_cluster'
':path', '/en.wikipedia.org/v1/page/mobile-html/Duck'
':method', 'GET'
':scheme', 'https'
'user-agent', 'curl/7.64.0'
'accept', '*/*'
'x-forwarded-for', '10.2.2.69'
'x-forwarded-proto', 'https'
'x-envoy-internal', 'true'
'x-request-id', '2b9c3069-0c24-4153-a9bc-c70a0d02a1f3'
'x-envoy-expected-rq-timeout-ms', '15000'
'x-envoy-original-path', '/en.wikipedia.org/v1/page/mobile-html/Duck'


[2023-05-08 09:44:34.152][85][debug][http] [source/common/http/conn_manager_impl.cc:1469] [C43097][S11737610104135090600] encoding headers via codec (end_stream=false):
':status', '200'
'access-control-allow-origin', '*,*'
'access-control-allow-headers', 'accept, x-requested-with, content-type,accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding'
'access-control-expose-headers', 'etag,etag'
'x-xss-protection', '1; mode=block'
'x-content-type-options', 'nosniff'
'x-frame-options', 'SAMEORIGIN'
'x-content-security-policy', 'default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self'' 'x-webkit-csp', 'default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self''
'content-type', 'text/html; charset=utf-8; profile="https://www.mediawiki.org/wiki/Specs/Mobile-HTML/1.2.2"'
'etag', '"1136340390/f04d7ef0-ed84-11ed-9432-4d2f262a8f39"'
'content-language', 'en'
'content-length', '151307'
'vary', 'Accept-Encoding'
'date', 'Mon, 08 May 2023 09:44:34 GMT'
'x-envoy-upstream-service-time', '244'
'server', 'envoy'
'access-control-allow-methods', 'GET,HEAD'
'x-content-type-options', 'nosniff'
'x-frame-options', 'SAMEORIGIN'
'referrer-policy', 'origin-when-cross-origin'
'x-xss-protection', '1; mode=block'
'content-security-policy', 'default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self''
MSantos closed this task as Resolved.May 8 2023, 12:37 PM
MSantos added a subscriber: VirginiaPoundstone.
In T329074#8813735, @hnowlan wrote:

Interesting - it appears the gateway is appending headers rather than replacing. Not ideal as it will require us to add this logic into our Lua layer, but this isn't a critical issue.

Thanks for helping me debug this. I can now confirm that everything works as expected and in fact, my diff tests were false positives because of the gateway appending the headers. Should we create a task for that and prioritize when necessary? cc/ @VirginiaPoundstone it's not a blocker but we should look into it in the future.

Saying that I will close the task, thanks @hnowlan!

hnowlan added a comment.May 8 2023, 1:53 PM
In T329074#8833263, @MSantos wrote:
In T329074#8813735, @hnowlan wrote:

Interesting - it appears the gateway is appending headers rather than replacing. Not ideal as it will require us to add this logic into our Lua layer, but this isn't a critical issue.

Thanks for helping me debug this. I can now confirm that everything works as expected and in fact, my diff tests were false positives because of the gateway appending the headers. Should we create a task for that and prioritize when necessary? cc/ @VirginiaPoundstone it's not a blocker but we should look into it in the future.

Saying that I will close the task, thanks @hnowlan!

There are two routes towards fixing this - ideally we can get Envoy to not append (which I am testing atm). If this is not available in the current version then we can accomplish it by expanding our CSP lua to iterate over a list of headers and defaults to set if they are absent.

gerritbot added a comment.May 8 2023, 2:02 PM

Change 917340 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] rest-gateway: don't append when setting headers

https://gerrit.wikimedia.org/r/917340

gerritbot added a project: Patch-For-Review.May 8 2023, 2:02 PM
gerritbot added a comment.May 16 2023, 3:19 PM

Change 917340 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway: don't append when setting headers

https://gerrit.wikimedia.org/r/917340

Maintenance_bot removed a project: Patch-For-Review.May 16 2023, 3:29 PM
hnowlan added a comment.May 16 2023, 3:36 PM

Header duplication issue is resolved.

However, I have noted that some CSP headers are different (and the deprecated headers can probably be removed anyway):

< x-content-security-policy: default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self'
< x-webkit-csp: default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self'
< content-security-policy: default-src 'none'; connect-src app://*.wikipedia.org https://*.wikipedia.org; media-src app://upload.wikimedia.org https://upload.wikimedia.org 'self'; img-src app://*.wikimedia.org https://*.wikimedia.org app://wikimedia.org https://wikimedia.org 'self' data:; object-src 'none'; script-src app://meta.wikimedia.org https://meta.wikimedia.org 'unsafe-inline'; style-src app://meta.wikimedia.org https://meta.wikimedia.org app://*.wikipedia.org https://*.wikipedia.org 'self' 'unsafe-inline'; frame-ancestors 'self'
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL