Skip to content

feat: add dnslink support and update t0109#34

Merged
laurentsenta merged 9 commits intomainfrom
feat/add-t0109-with-dnslink
May 9, 2023
Merged

feat: add dnslink support and update t0109#34
laurentsenta merged 9 commits intomainfrom
feat/add-t0109-with-dnslink

Conversation

@laurentsenta
Copy link
Copy Markdown
Contributor

@laurentsenta laurentsenta commented Apr 5, 2023 •
edited
Loading

Contributes to #26
Closes #22

This tests adds dnslink support:

We introduce a fixture type which contains a dnslink configuration:

dnslinks:
  custom-dnslink:
    subdomain: dnslink-enabled-on-fqdn
    path: /ipfs/QmYBhLYDwVFvxos9h8CGU2ibaY66QNgv8hpfewxaQrPiZj

Test writer uses the fixture with:

dnsLinks := dnslink.MustOpenDNSLink("t0109-dnslink.yml")
dnsLink := dnsLinks.Get("custom-dnslink")

// then use it in your subdomain gateway URL test:
gatewayURL := SubdomainGatewayURL
u, _ := url.Parse(gatewayURL)

// dnsLinkBaseUrl = "http://dnslink-enabled-on-fqdn.foo.bar.example.com"
dnsLinkBaseUrl := fmt.Sprintf("%s://%s%s", u.Scheme, dnsLink, u.Host)

Test user calls the export CLI to get the yml files. We also generate a dnslinks.json.

There is a demo in kubo-config.example.sh that shows how to import the fixtures:

IPFS_NS_MAP=$(cat ./dnslinks.json | jq -r 'to_entries | map("\(.key).example.com:\(.value)") | join(",")')

echo "Set the following IPFS_NS_MAP before starting the kubo daemon:"
echo "IPFS_NS_MAP=${IPFS_NS_MAP}"

Todo

  • Clarify the API and how we want to expose this to users
  • helper libs reusable
  • finalize in CI (import the env when the daemon starts)

@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch from 1ee3082 to 560bbea Compare April 5, 2023 12:05
@laurentsenta laurentsenta changed the base branch from main to feat/add-t0122 April 5, 2023 12:06
@laurentsenta laurentsenta changed the base branch from feat/add-t0122 to main April 5, 2023 12:06
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch from 560bbea to 266cdb6 Compare April 5, 2023 12:21
@laurentsenta laurentsenta requested a review from galargh April 5, 2023 12:23
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch 5 times, most recently from 279e24a to d2757ba Compare April 7, 2023 07:54
@galargh galargh removed their request for review April 7, 2023 07:58
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch 2 times, most recently from a40f2e0 to 9fd3ab5 Compare April 7, 2023 09:06
@laurentsenta laurentsenta mentioned this pull request Apr 7, 2023
Closed
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch 5 times, most recently from 66a3982 to 0fa2fe7 Compare April 7, 2023 09:53
@laurentsenta laurentsenta mentioned this pull request Apr 7, 2023
Closed
20 tasks
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch 2 times, most recently from 928d79d to ed96dd0 Compare April 7, 2023 10:14
@laurentsenta laurentsenta requested a review from galargh April 10, 2023 13:34
@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch from ed96dd0 to fbce0ea Compare May 9, 2023 08:53
@laurentsenta
Copy link
Copy Markdown
Contributor Author

laurentsenta commented May 9, 2023

failing test on purpose: https://github.com/ipfs/gateway-conformance/actions/runs/4924297430

@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch from 3933b9e to 1be71ab Compare May 9, 2023 09:04
galargh
galargh approved these changes May 9, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@galargh galargh left a comment

Choose a reason for hiding this comment

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

Nothing blocking 🥇

@laurentsenta laurentsenta force-pushed the feat/add-t0109-with-dnslink branch from 1be71ab to 83c417c Compare May 9, 2023 14:47
@laurentsenta laurentsenta merged commit 9e419ec into main May 9, 2023
@laurentsenta laurentsenta deleted the feat/add-t0109-with-dnslink branch May 9, 2023 14:57
@laurentsenta laurentsenta mentioned this pull request May 9, 2023
Merged
14 tasks
lidel
lidel reviewed May 9, 2023
View reviewed changes
fixtures/t0109-dnslink.yml
# yaml-language-server: $schema=fixture.schema.json
dnslinks:
custom-dnslink:
subdomain: dnslink-enabled-on-fqdn
Copy link
Copy Markdown
Member

@lidel lidel May 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(github was down and missed oportunity to review earlier)

@laurentsenta i am bit confused – whgy is this "subdomain" specific?

DNSLink TXT records can be present on subdomains OR the top domains right after TLD.
Should this be renamed to fqdn or plain domain?

Copy link
Copy Markdown
Contributor Author

@laurentsenta laurentsenta May 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(github was down and missed oportunity to review earlier)

👍 thanks for reviewing anyway; happy to fix if the approach is lacking, @hacdias had a similar question when we reviewed it.

DNSLink TXT records can be present on subdomains OR the top domains right after TLD.

We want to configure the top domain + tld during test EXECUTION, not at test definition: two users might run that test on different subdomain gateway URLs, example.com and dweb.link for example.

This means we want to be able to construct URLs with {dns-link.subdomain}.{gateway_url} at run time (actual code).

Thinking about it, I guess we might want to define a fqdn/domain, like

dnslinks:
    wikipedia:
        domain: wikipedia-on-ipfs.org
        path: /ipfs/somethingsomething

but that would be a different test, we do need to keep the subdomain option to port most of the tests like 114 and 109.

That's my understanding so far, let me know if I'm missing something!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidel lidel lidel left review comments

@galargh galargh galargh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add dnslink support

3 participants

@laurentsenta @lidel @galargh