-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: make subdomain testing possible #9
Merged
Merged
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
3cfb66d
feat: make subdomain testing possible
laurentsenta 0cc1e52
feat: subdomain with cleaner API
laurentsenta 8a17a7f
feat: improve sugar with url rewrites, clearer
laurentsenta 3a17974
feat: use fixtures
laurentsenta ea6db6c
feat: reporting + test localhost like 114 does
laurentsenta 9c2fa4c
fix: improve reporting & fix test
laurentsenta 37be4fb
wip: show alternatives
laurentsenta 399b1d4
feat: add the localhost domain
laurentsenta e264baf
refactor: follow-up call w/Lidel
laurentsenta ba714b7
feat: fix 127.0.0.1 test in 114
laurentsenta 8e60e46
feat: add missing sugar and implement 50% of 114
laurentsenta 649f9a8
feat: nicer body with hint
laurentsenta d1ba558
feat: clarify tunnel and update todos
laurentsenta a9964a3
chore: fixes
laurentsenta f94c7e9
feat: subdomain-url in entrypoint
laurentsenta 9981d52
feat: add subdomain url to gh
laurentsenta e8b7750
ci: enable subdomain tests
laurentsenta 33db81f
tweak
laurentsenta adcd947
feat: add query and missing tests
laurentsenta e215caf
fix: clean fixture in 114
laurentsenta 84f0dab
chore: last fixes
laurentsenta 79eb4f1
fix: drop api tests (kubo legacy)
laurentsenta a928677
chore: remove comments after review
laurentsenta d1b336e
chore: remove comments
laurentsenta b36c487
doc: add subdomain-url doc
laurentsenta 7ab250c
fix: drop specs & simplif makefile
laurentsenta 9679a8a
refactor: reorg makefile
laurentsenta 5f7479a
chore: rename ipfs to kubo
laurentsenta 2dfced1
chore: use same defaults in action and main
laurentsenta 50d7cb0
fix: remove dead code
laurentsenta 68d3c50
chore: remove dead code
laurentsenta 9bb23f4
chore: Url -> URL
laurentsenta fe51641
fix: Makefile disable gateway spec
laurentsenta 93f5156
chore: rename maradona
laurentsenta 05087c3
fix: ci script name
laurentsenta 48c8935
fix: move readme sections
laurentsenta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#! /usr/bin/env bash | ||
ipfs config --json Gateway.PublicGateways '{ | ||
"example.com": { | ||
"UseSubdomains": true, | ||
"Paths": ["/ipfs", "/ipns", "/api"] | ||
}, | ||
"localhost": { | ||
"UseSubdomains": true, | ||
"InlineDNSLink": true, | ||
"Paths": ["/ipfs", "/ipns", "/api"] | ||
} | ||
}' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we being clear enough with the name/description here? I'm afraid this creates an impression that I, as an implementer, should set up something accessible at
http://example.com
. I think I'd assume I have to set up another gateway instance that supports the subdomain gateway spec and expose it atsubdomain-url
.Also, do we think we might have non-subdomain-related tests benefitting from the proxying in the future?
Wouldn't something like
local-gateway-proxy-url
orinternal-gateway-proxy-url
reflect the intention better?I don't want to block this, but I wanted to raise my concerns with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a comment in the README, there's no real proxying anymore, we use that value to overwrite the request sent to the gateway, it's really subdomain-related.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks for adding the comment. It makes things clearer. Few more thoughts that it sparked with me.
Wouldn't it make sense to always use what we now call
subdomain-url
. As in, all the test should talk togateway-url
in a way that thinks it's calledsubdomain-url
? Theoretically, wouldn't that be more correct? So really, every test case should be executed 3 times (with host header, proxy, proxy with tunneling) as long assubdomain-url
andgateway-url
are different.Looking more closely at how we execute the test cases with host header, I think we're not using the protocol part of
gateway-url
. Is that correct?If so, isn't what we really need from the user these two pieces of info:
That would come together to something like this - main...gateway-host (note that I intentionally skipped modifying test/config code; I just wanted to get the idea across).
There's absolutely no rush with replying to this. And it's completely possible that I'm missing the point a bit - still trying to wrap my head fully around this. I just want to understand what might be most straight-forward way for those running the tests to provide us all the information we need.