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

feat(kas): Allow setting DialOptions for KAS / multi KAS. #753

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

Conversation

ntrevino-virtru
Copy link
Contributor

This allows setting DialOptions for KAS. This is especially important for decrypt with external servers.

Previously an error such as this would be output:
msg="unable to verify parse token" err="jwt.Parse: no keys for verification are provided (use jwt.WithVerify(false) to explicitly skip)"
@ntrevino-virtru ntrevino-virtru requested review from a team as code owners May 6, 2024 23:26
@ntrevino-virtru ntrevino-virtru changed the title CCR-2825: Allow setting DialOptions for KAS / multi KAS. feat(CCR-2825): Allow setting DialOptions for KAS / multi KAS. May 6, 2024
@@ -30,6 +32,8 @@ const kHTTPOk = 200

// KASInfo contains Key Access Server information.
type KASInfo struct {
// Can be empty. If empty, it will inherit the global SDK DialOptions.
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to have separate dialoptions for KAS or perhaps every service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could see that making sense. KAS definitely.

@ntrevino-virtru ntrevino-virtru changed the title feat(CCR-2825): Allow setting DialOptions for KAS / multi KAS. feat(kas): Allow setting DialOptions for KAS / multi KAS. May 7, 2024
@ntrevino-virtru ntrevino-virtru force-pushed the feature/ccr-2825/kas-dial-options branch from 3c80777 to 2109303 Compare May 8, 2024 15:11
@jrschumacher
Copy link
Member

jrschumacher commented May 10, 2024

The original intention of the SDK abstraction of the services was for convenience when you have a monolith running. Since we haven't yet gotten to a place where performance concerns require the splitting of services from an OpenTDF perspective could we take a step back for a moment and consider our options. (Reconsider if the abstraction is necessary to propagate)

At minimum I feel that one option (which requires no development) would be spinning up multiple clients and pointing each one at a different platform. Since we don't have the concept of a central platform that negotiates the connections to other instances it will work, but causes confusion since each instance of the SDK will invoke a separate abstraction to every service even when that service isn't running within that instance of the platform.

This would beg the question if you should specify the service abstractions you want to define when invoking the SDK. (Light development)

Alternatively if you require multiple KAS or a have a deployment model that is not monolithic maybe the utility feature isn't for you and you are required to dial every service? (Solved through documentation)

Lastly, if we had the concept of a core platform and further extended the well known then the platform could communicate all the dialoptions required or proxy requests to the required service. (Service development)

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.

None yet

3 participants