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

Migrating GraphQL API to GMS #3131

Merged
merged 10 commits into from
Aug 20, 2021

Conversation

jjoyce0510
Copy link
Collaborator

Status
Ready for review.

Changes
This PR includes a migration of the GraphQL API from the datahub-frontend server (ui web server) to the DataHub backend service (GMS). This consolidation is being done for a few reasons:

  1. Centralization of Auth: Soon with RBAC datahub services will be secured via Authentication. In the hopes of minimizing the surface area of authentication & authorization, we feel it is best to colocate the 2 APIs we currently provide (GraphQL and Rest.li)
  2. Making GraphQL the "de-facto" public API of DataHub. Going forward, we intend to evolve the GraphQL API to be able to service all programmatic interactions with DataHub, from the UI & beyond. To move in this direction, we think it best to extract GraphQL from it's confines as a UI-serving API to a more easily accessible API sitting in GMS.
  3. Simplifying our Web Server: We intend to leverage our Play web server as a simple web server, used for fronting web resources & bundles, basic SSO handling, and proxying API requests. In doing so we will lighten our dependency on Play, which does not provide native Gradle support and is built on a Scala core.

In doing so, we've added an additional API Spring MVC servlet at the GMS layer. In the near future, we intend for this to be the primary place for requesting API resources from DataHub.

In addition to moving the GraphQL API, we've also begun to rebrand "GMS" as the "Metadata Service", a much more comprehensible name for service responsible for persisting and serving the Metadata Graph.

Compatibility
Luckily, no changes in this PR are necessarily backwards incompatible. The frontend Play server will simply proxy to the GraphQL API hoisted at GMS, using the exact same GraphQL model and components for resolving the Graph. The UI will continue to speak to the datahub-frontend web server directly.

The GraphQL infrastructure will continue to speak directly to general-purpose Rest.li endpoints (entities, aspects, relationships), only now that call will be local to the same process.

The Future
The future is bright for the Metadata Service! Coming soon with be a rich Authentication model at the API layer along with fine-grained access controls dictating who can do what on the DataHub platform. Stay tuned fo exciting work coming in the weeks ahead!

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@Security.Authenticated(Authenticator.class)
public CompletableFuture<Result> proxy(String path) throws ExecutionException, InterruptedException {
final String resolvedUri = PATH_REMAP.getOrDefault(request().uri(), request().uri());
return _ws.url(String.format("http://%s:%s%s", GMS_HOST, GMS_PORT, resolvedUri))
Copy link
Contributor

Choose a reason for hiding this comment

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

will this always be http?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See comment above^ in order to serve mutual auth, we'll have to do some additional client side configuration work. I don't want to include that here. Think I should create a ticket?

Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM. Let's fix follow up issues if they occur.

@shirshanka shirshanka merged commit 2c5edd8 into datahub-project:master Aug 20, 2021
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.

2 participants