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

Earthid driver integration #441

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

vaibhavahire99
Copy link

This pull request removes the driver-did-earthid directory from the Universal Resolver repository to comply with best practices of separating the driver code from the Universal Resolver core. The changes made in this pull request ensure that the Universal Resolver configuration references the EarthID driver Docker image hosted externally, rather than including the driver code in this repository.

Key Changes:
Removed driver-did-earthid directory:

The directory containing the EarthID driver code has been removed from the Universal Resolver repository.
This ensures that the driver implementation can evolve independently and is maintained separately.
Updated docker-compose.yml and configuration files:

The configuration now points to the vibhi09/driver-did-earthid:latest Docker image for resolving did:earthid identifiers.
The resolver setup will continue to support EarthID DIDs by referencing the external Docker image.

@peacekeeper
Copy link
Member

Thanks @vaibhavahire99 for your effort. This looks much better, but still one request.. This PR seems to add a lot of newlines to the docker-compose.yml file. Do you think you could also make this small change, so that you actually only add your driver and don't change other driver entries?

Also, this PR doesn't seem to add your driver to application.yml and .env and README.md, as described here: https://github.com/decentralized-identity/universal-resolver/blob/main/docs/driver-development.md

@vaibhavahire99
Copy link
Author

Hi thanks @peacekeeper. I've made the changes as mentioned. Can you please take a look and let me know if works.

@BernhardFuchs
Copy link
Member

Hi,
I've just tested the driver and on startup it throws following error:

exec /usr/local/bin/docker-entrypoint.sh: exec format error

@@ -374,3 +376,11 @@ services:
image: ghcr.io/iden3/driver-did-iden3:v0.0.4
ports:
- "8152:8080"

driver-did-earthid:
image: vibhi09/driver-did-earthid:latest
Copy link
Member

Choose a reason for hiding this comment

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

Please use a stable version instead of "latest"

Copy link
Author

Choose a reason for hiding this comment

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

Sure let me retest and update the request.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @peacekeeper , I've updated the repo with a stable version and updated port number kindly review my latest commit in the current pull request and let me know if any changes required.
Thank you!

environment:
baseUrl: "https://did.myearth.id/v1/resolve"
ports:
- "8153:8080"
Copy link
Contributor

@FabioPinheiro FabioPinheiro Oct 24, 2024

Choose a reason for hiding this comment

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

We are using 8153 for the driver-did-prism

Suggested change
- "8153:8080"
- "8154:8080"

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the port number as suggested. Please let me know if any changes.

@peacekeeper
Copy link
Member

@vaibhavahire99 It seems there are merge conflicts now that need to be resolved..

@vaibhavahire99
Copy link
Author

Hi @peacekeeper, I've resolved the conflicts can you please take a look and let me know.
Thank you

@@ -380,3 +381,10 @@ services:
image: ghcr.io/fabiopinheiro/uni-resolver-driver-did-prism:0.2
ports:
- "8153:9090"
driver-did-earthid:
Copy link
Member

Choose a reason for hiding this comment

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

Pls fix issue with indendation

@@ -341,4 +341,7 @@ uniresolver:
- did:prism:c36cd59bbc62dee1925e1343a8fed051416e417116d6169d060746f1e6816cd4
- did:prism:0d8481c41b654794f02922601f84811763c655dcfc376acf841eb996846d5e68 # deactivated
- did:prism:52e163e8e53466b808e53df870bccd0a066aa4d05af9b689f5c73edcbe23d625 # with updates

- pattern: "^(did:earthid:.+)$"
Copy link
Member

Choose a reason for hiding this comment

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

Pls fix issue with indendation

@BernhardFuchs
Copy link
Member

Pls fix the issues with indendation and resolve new merge conflicts. We added a new driver in the meantime and that created new conflicts

@vaibhavahire99
Copy link
Author

Hi @BernhardFuchs, I've resolved the conflicts and the indendation issues. Can you please check and let me know if any changes.

@BernhardFuchs
Copy link
Member

There is still following error thrown:

exec /usr/local/bin/docker-entrypoint.sh: exec format error

@vaibhavahire99
Copy link
Author

Hi @BernhardFuchs, I've updated the platform dependency. Can you please check now and let me know if any changes.
Thanks!

@BernhardFuchs
Copy link
Member

Thx for the update but the driver still throws the same error on startup:

exec /usr/local/bin/docker-entrypoint.sh: exec format error

@peacekeeper
Copy link
Member

I also tried it and got the same error as @BernhardFuchs .

@vaibhavahire99 Have you tried testing your own driver locally according to https://github.com/decentralized-identity/universal-resolver/blob/main/docs/driver-development.md#how-to-test-a-driver-locally ?

@vaibhavahire99
Copy link
Author

Hi @peacekeeper and @BernhardFuchs , kindly let me review this and get back with updated commit.
Thank you!

@ankurdotb ankurdotb mentioned this pull request Feb 6, 2025
@peacekeeper
Copy link
Member

@vaibhavahire99 It looks like there are some merge conflicts in files now, could you fix them?

@vaibhavahire99
Copy link
Author

Hi @peacekeeper, sure let me take a latest pull and resolve conflicts and update the commit.

@vaibhavahire99
Copy link
Author

Hi @peacekeeper, I've resolved the conflicts and updated the port. Please kindly let me know if any changes.
Thank You!

@vaibhavahire99
Copy link
Author

Hi @peacekeeper, can you please check this commit and let me know if any update.

@peacekeeper
Copy link
Member

@BernhardFuchs Could you check this when you have time

@vaibhavahire99
Copy link
Author

Hi @peacekeeper and @BernhardFuchs, any updates?

@vaibhavahire99
Copy link
Author

Hi @peacekeeper, can you please let me know if any updates?
Thanks!

@peacekeeper
Copy link
Member

Hello @vaibhavahire99 I'm sorry please wait a few more days, @BernhardFuchs is currently away, and we will look at it asap.

@BernhardFuchs
Copy link
Member

Just checked the driver and it seems that the image is behind a login.
docker pull docker.io/earthid/services:driver-did-earthid-v1.0 throws following error:

Error response from daemon: pull access denied for earthid/services, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

@vaibhavahire99
Copy link
Author

Hi @BernhardFuchs, Thanks for the update.
I've updated the image visiblity to public access now. You can try pulling with:
docker pull earthid/driver-did-earthid-v1.0:latest
Let me know if any changes.
Thanks!

@BernhardFuchs
Copy link
Member

Tried it and when resolving the example DID the resolver throws this error:

Resolve problem for did:earthid:testnet:6XXVvDkgXCjD6fy62dWaQdyk5368yE1QFdkDpoCFS1d1:0.0.5032772: 404 Not Found (<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /1.0/identifiers/1.0/identifiers/did:earthid:testnet:6XXVvDkgXCjD6fy62dWaQdyk5368yE1QFdkDpoCFS1d1:0.0.5032772</pre>
</body>
</html>
)
uniresolver.ResolutionException: 404 Not Found (<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /1.0/identifiers/1.0/identifiers/did:earthid:testnet:6XXVvDkgXCjD6fy62dWaQdyk5368yE1QFdkDpoCFS1d1:0.0.5032772</pre>
</body>
</html>
)
	at uniresolver.driver.http.HttpDriver.resolve(HttpDriver.java:155)
	at uniresolver.local.LocalUniResolver.resolveWithDrivers(LocalUniResolver.java:155)
	at uniresolver.local.LocalUniResolver.resolve(LocalUniResolver.java:108)
	at uniresolver.local.LocalUniResolver.resolve(LocalUniResolver.java:57)
	at uniresolver.web.WebUniResolver.resolve(WebUniResolver.java:58)
	at uniresolver.web.servlet.ResolveServlet.doGet(ResolveServlet.java:108)

@vaibhavahire99 Does this work for you locally?

@peacekeeper
Copy link
Member

The /1.0/identifiers/ part seems to be added twice:

Cannot GET /1.0/identifiers/1.0/identifiers/did:earthid:testnet:6XXVvDkgXCjD6fy62dWaQdyk5368yE1QFdkDpoCFS1d1:0.0.5032772

Try removing the duplicate path segments.

@vaibhavahire99
Copy link
Author

Hi @peacekeeper and @BernhardFuchs, that's correct try using:
http://localhost:8080/1.0/identifiers/did:earthid:testnet:6XXVvDkgXCjD6fy62dWaQdyk5368yE1QFdkDpoCFS1d1:0.0.5032772
this should work.
Let me know if any query.
Thanks!

@peacekeeper
Copy link
Member

@vaibhavahire99 You need to update the PR with the correct path.

Please first try it yourself locally. We cannot do your work for you. You need to fix the configuration, try it locally, and if it works, update the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants