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 EnrollmentUrl to Did document #203

Merged

Conversation

Izzzu
Copy link

@Izzzu Izzzu commented Jul 15, 2022

co-author: algattik

What this PR changes/adds

Include Registration service enrollment api endpoint in Dataspace Authority DID document.

Why it does that

The Enrollment API url should be resolved from the Dataspace DID. A subsequent PR will implement that.

Further notes

Example DID: https://mvd144dataspacedid.z16.web.core.windows.net/.well-known/did.json

Also:

  • Corrected some variable names "DID URL" into "DID URI"
  • Simplified Terraform outputs by having the "/authority" registration service URL prefix contained in the output registration service URL

Linked Issue(s)

Linked to #43

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly?

@Izzzu Izzzu force-pushed the feature/43/43-enrollment-url-in-did branch 2 times, most recently from f100e89 to f1cfebe Compare July 18, 2022 14:45
@Izzzu Izzzu force-pushed the feature/43/43-enrollment-url-in-did branch from f1cfebe to bee7c1c Compare July 19, 2022 08:05
@Izzzu Izzzu marked this pull request as ready for review July 20, 2022 08:07
@cpeeyush cpeeyush self-requested a review July 20, 2022 08:35
@chrislomonico chrislomonico self-requested a review July 20, 2022 08:40
Izzzu and others added 2 commits July 20, 2022 19:46
Co-authored-by: Peeyush Chandel <555114+cpeeyush@users.noreply.github.com>
@Izzzu Izzzu force-pushed the feature/43/43-enrollment-url-in-did branch from 3f56177 to 1bf056c Compare July 20, 2022 18:49
@Izzzu Izzzu marked this pull request as draft July 21, 2022 09:24
Use the new reg service cli from branch

Use the new reg service cli from branch

Update edc

Update reg service version
@Izzzu Izzzu force-pushed the feature/43/43-enrollment-url-in-did branch from 3ee535f to 7a98883 Compare July 21, 2022 13:08
@Izzzu Izzzu force-pushed the feature/43/43-enrollment-url-in-did branch from 7a98883 to fd3bf7e Compare July 21, 2022 13:31
@Izzzu Izzzu marked this pull request as ready for review July 21, 2022 13:32
@Izzzu Izzzu marked this pull request as draft July 21, 2022 14:51
* Use Registration Service CLI to onboard participants (#154)

* .

* .

* .

* .

* .

* .

* .

* .

* .

* PR comments

* Update cd.yaml

* adapted CLI call in VerifyLocalTests

* Update register-participants.sh

* Update register-participants.sh

* Fixed DID URLs for docker-compose

* Adapted for upstream changes

* Update JWT_AUDIENCE to localhost due to docker hosts and registration script differences

* Revert "Update JWT_AUDIENCE to localhost due to docker hosts and registration script differences"

This reverts commit ad83173.

* use docker for cli tools

Use docker for cli tools

add execute permission explicitly

* Simplified scripts

* Update doc to reflect cli-tools changes

* Update version number

* use latest reg service

Co-authored-by: Peeyush Chandel <555114+cpeeyush@users.noreply.github.com>
@algattik algattik changed the base branch from feature/43-enrollment-url-in-did to feature/14-rs-jws July 27, 2022 16:06
@algattik algattik force-pushed the feature/43/43-enrollment-url-in-did branch from 66c99c6 to e7cf15e Compare July 27, 2022 17:33
@algattik algattik changed the title Add EnrollmentUrl to Did document Add EnrollmentUrl to Did document [TODO: update target branch] Jul 28, 2022
@algattik algattik force-pushed the feature/43/43-enrollment-url-in-did branch from 4f1d09b to f94669f Compare July 28, 2022 04:28
@algattik algattik changed the base branch from feature/14-rs-jws to feature/43-enrollment-url-in-did July 28, 2022 06:44
@algattik algattik changed the title Add EnrollmentUrl to Did document [TODO: update target branch] Add EnrollmentUrl to Did document [REQUIRES UPSTREAM #31] Jul 28, 2022
@algattik algattik changed the title Add EnrollmentUrl to Did document [REQUIRES UPSTREAM #31] Add EnrollmentUrl to Did document Jul 28, 2022
@algattik algattik marked this pull request as ready for review July 28, 2022 15:09
@algattik algattik requested review from cpeeyush and removed request for chrislomonico and paullatzelsperger July 28, 2022 15:09
@@ -86,7 +86,7 @@ resource "azurerm_container_group" "registration-service" {

environment_variables = {
EDC_CONNECTOR_NAME = local.connector_name
JWT_AUDIENCE = "${local.registration_service_url}${local.registration_service_path_prefix}"
JWT_AUDIENCE = local.registration_service_url
Copy link

@cpeeyush cpeeyush Jul 28, 2022

Choose a reason for hiding this comment

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

we also need to modify it for local setup ?

JWT_AUDIENCE: http://registration-service:8184/authority

or we plan to do it in subsequent PR ?

Choose a reason for hiding this comment

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

No update needed, the URL still has /authority in it. We just assemble it differently in TF

Copy link
Author

@Izzzu Izzzu left a comment

Choose a reason for hiding this comment

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

@algattik approving

Copy link

@cpeeyush cpeeyush left a comment

Choose a reason for hiding this comment

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

Approved with suggestion

@algattik algattik merged commit dd2d445 into feature/43-enrollment-url-in-did Jul 29, 2022
marcgs pushed a commit that referenced this pull request Aug 2, 2022
@Izzzu Izzzu deleted the feature/43/43-enrollment-url-in-did branch August 16, 2022 11:28
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.

4 participants