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

userapi: rename grpc package and change relative proto path #1037

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elchead
Copy link

@elchead elchead commented Nov 28, 2024

The origin of this PR started in this discussion.
As Contrast is migrating towards providing an SDK, the userapi should adhere to best practices and avoid compatibility issues.
As part of integrating the current SDK draft into Continuum, two issues were discovered.

  1. The gRPC package name is not globally unique (see https://protobuf.dev/reference/go/faq#namespace-conflict), which leads to conflicts when the consumer imports other gRPC packages with the same name.
  2. The protoc compiler currently takes in a source path of the form "userapi.proto". This leads to import issues when another package imports a proto file with the same path. This happened in the Continuum project:
panic: proto: file "userapi.proto" is already registered
                previously from: "github.com/edgelesssys/continuum/internal/proto/attestation-service/userapi"
                currently from:  "github.com/edgelesssys/contrast/internal/userapi"
        See https://protobuf.dev/reference/go/faq#namespace-conflict

A solution was suggested here which ensures that the path names are unlikely to collide. Hence, I moved the protoc generation to include the proto file path relative to the root.

Note: This change breaks compatibility with the current gRPC version, but it should be fine to release this with the next minor.

@elchead elchead added the breaking change A user-affecting breaking change label Nov 28, 2024
@elchead elchead marked this pull request as ready for review November 28, 2024 10:53
@katexochen katexochen added no changelog PRs not listed in the release notes and removed breaking change A user-affecting breaking change labels Nov 28, 2024
@katexochen katexochen removed their request for review November 29, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants