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

Separate out the security config from the rest of the transport config #37

Merged
merged 7 commits into from
Dec 4, 2024

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Dec 4, 2024

Motivation:

Users must explicitly state whether they want TLS or plaintext. This config for this is currently nested within the rest of the config for the transport. In many cases this results in users writing .defaults(transportSecurity: .tls(.defaults(...))) where .defaults(...) often includes various TLS config. This is verbose and a little bit finicky to use.

Modifications:

  • Seprate out the transport security from the rest of the config and have the client and server transport take a config and a transport security config.
  • This allows for APIs which compose better while also allowing for the transport config to just use a set of defaults.

Result:

Easier to use API.

Motivation:

Users must explicitly state whether they want TLS or plaintext. This
config for this is currently nested within the rest of the config for
the transport. In many cases this results in users writing
`.defaults(transportSecurity: .tls(.defaults(...)))` where
`.defaults(...)` often includes various TLS config.
This is verbose and a little bit finicky to use.

Modifications:

- Seprate out the transport security from the rest of the config and
  have the client and server transport take a config _and_ a transport
  security config.
- This allows for APIs which compose better while also allowing for the
  transport config to just use a set of defaults.

Result:

Easier to use API.
@glbrntt glbrntt added the ⚠️ semver/major Breaks existing public API. label Dec 4, 2024
Copy link
Collaborator

@gjcairo gjcairo left a comment

Choose a reason for hiding this comment

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

Nice refactor, thanks George!

@glbrntt glbrntt added ⚠️ semver/major Breaks existing public API. and removed ⚠️ semver/major Breaks existing public API. labels Dec 4, 2024
@glbrntt glbrntt merged commit 5152e8f into grpc:main Dec 4, 2024
32 of 36 checks passed
@glbrntt glbrntt deleted the config branch December 4, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants