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

Add blobserve router under ide-proxy #10513

Merged
merged 3 commits into from
Jun 8, 2022
Merged

Add blobserve router under ide-proxy #10513

merged 3 commits into from
Jun 8, 2022

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Jun 8, 2022

Description

This PR is 1st part of move blobserve behind ide proxy #7986

This PR contain follow things

  1. remove blobserve read only mode see comment for detail move blobserve behind ide proxy #7986 (comment), simple not execute, didn't remove logic, in case we need add this back
  2. change blobserve owner to team IDE
  3. add cache policy, if header is not contain X-BlobServe-InlineVars and HTTP method is not GET, set cache max-age to 31536000, otherwise is no-cache
  4. add HEAD method for warm-up cache

Related Issue(s)

Relate #7986

How to test

You can simple test this PR #10514, it contains the effect of the change in this PR

  1. open a workspace, see workspace can open, this does not affect what was there before
  2. check the following URL, it should be work, and check the response header, Cache-Control should be set public, max-age=31536000
https://ide.pd-move-blob.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada/__files__/index.html

https://ide.pd-move-blob.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada/index.html

https://ide.pd-move-blob.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada
  1. check X-BlobServe-InlineVars works, diff response body with remove X-BlobServe-InlineVars header, the difference should like https://www.diffchecker.com/iBzZcjRj ,this is because
    InlineStatic: []blobserve.InlineReplacement{{
    Search: "${window.location.origin}",
    Replacement: ".",
    }, {
    Search: "value.startsWith(window.location.origin)",
    Replacement: "value.startsWith(window.location.origin) || value.startsWith('${ide}')",
    }, {
    Search: "./out",
    Replacement: "${ide}/out",
    }, {
    Search: "./node_modules",
    Replacement: "${ide}/node_modules",
    }, {
    Search: "/_supervisor/frontend",
    Replacement: "${supervisor}",
    }},
# using the following command test  and check the response header, `Cache-Control` should be set `no-cache` 
curl -v "https://ide.pd-move-blob-2.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada/index.html" \
     -H 'X-BlobServe-InlineVars: {"ide": "_____IDE__URL____STRING___", "supervisor": "____SUPERVISOR____URL"}'
  1. try curl -v --head "https://ide.pd-move-blob-2.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada/index.html", it should not response body, only HTTP status

Release Notes

NONE

Documentation

@iQQBot iQQBot requested review from a team June 8, 2022 05:31
@github-actions github-actions bot added team: IDE team: workspace Issue belongs to the Workspace team labels Jun 8, 2022
@roboquat roboquat added the size/M label Jun 8, 2022
@iQQBot iQQBot marked this pull request as draft June 8, 2022 05:35
@iQQBot iQQBot marked this pull request as ready for review June 8, 2022 06:20
@mustard-mh mustard-mh self-assigned this Jun 8, 2022
Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

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

check the following URL, it should be work, and check the response header, Cache-Control should be set public, max-age=31536000

Why we should (can only) test those three URLs below?

@mustard-mh
Copy link
Contributor

mustard-mh commented Jun 8, 2022

  1. check the following URL, it should be work, and check the response header, Cache-Control should be set public, max-age=31536000

We should filter Network with blobserve

Replace ones URL from blobserve.ws.xxx/eu.gcr.io/xxx to ide.xxx/blobserve/eu.gcr.io/xxx

To check if new blobserve alias (const in the future) works

Image
image

@iQQBot
Copy link
Contributor Author

iQQBot commented Jun 8, 2022

Why we should (can only) test those three URLs below?

These are just an example, in fact I'm just giving the format here, just make sure the various formats above are compatible

PS: You can simple test this PR #10514, This contains the effect of the change

@iQQBot
Copy link
Contributor Author

iQQBot commented Jun 8, 2022

  1. check the following URL, it should be work, and check the response header, Cache-Control should be set public, max-age=31536000

We should filter Network with blobserve

Replace ones URL from blobserve.ws.xxx/eu.gcr.io/xxx to ide.xxx/blobserve/eu.gcr.io/xxx

To check if new blobserve alias (const in the future) works

Image
image

Thank you for your clarification 🙏

Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

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

Code LGTM

ide. alias works

before with alias
image image

Diff while checking X-BlobServe-InlineVars as expected

Img
image

https://ide.pd-move-blob-2.preview.gitpod-dev.com/blobserve/eu.gcr.io/gitpod-core-dev/build/ide/code:commit-80d9b1ebfd826fd0db25320ba94d762b51887ada/index.html only respond header

@roboquat roboquat merged commit c36ca91 into main Jun 8, 2022
@roboquat roboquat deleted the pd/move-blob branch June 8, 2022 10:44
@roboquat roboquat added the deployed: IDE IDE change is running in production label Jun 9, 2022
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/M team: IDE team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants