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

[vnet] remote app provider #51279

Merged
merged 10 commits into from
Jan 28, 2025
Merged

[vnet] remote app provider #51279

merged 10 commits into from
Jan 28, 2025

Conversation

nklaassen
Copy link
Contributor

@nklaassen nklaassen commented Jan 21, 2025

Part of RFD 195.

As a follow-up to #51216, this PR implements vnet.remoteAppProvider which exposes apps to the VNet Windows admin service over gRPC. I'm also adding a unit test that asserts that networking works when apps are provided via the remote app provider.

@nklaassen nklaassen force-pushed the nklaassen/remote-appprovider branch from 3d197ac to 7f8b0b3 Compare January 22, 2025 01:54
@nklaassen nklaassen added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v17 labels Jan 22, 2025
@nklaassen nklaassen force-pushed the nklaassen/remote-appprovider branch from 7f8b0b3 to 375602d Compare January 22, 2025 19:01
@nklaassen nklaassen added the vnet label Jan 23, 2025
@nklaassen nklaassen force-pushed the nklaassen/refactor-appprovider branch from a3a4c20 to 53684bd Compare January 23, 2025 23:01
@nklaassen nklaassen force-pushed the nklaassen/remote-appprovider branch from 5486ab6 to a80b639 Compare January 23, 2025 23:02
Base automatically changed from nklaassen/refactor-appprovider to master January 24, 2025 00:29
@nklaassen nklaassen force-pushed the nklaassen/remote-appprovider branch from a80b639 to 6086c59 Compare January 24, 2025 01:27
lib/vnet/client_application_service.go Outdated Show resolved Hide resolved
lib/vnet/client_application_service.go Outdated Show resolved Hide resolved
mu sync.Mutex
// appSignerCache caches the crypto.Signer for each certificate issued by
// ReissueAppCert so that SignForApp can later use that signer.
appSignerCache map[appKey]crypto.Signer
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the lifecycle for entries in the map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a comment, we never need to evict signers because when the cert expires, ReissueAppCert will overwrite the signer with a new one. We currently never clean up stale app proxies that haven't been used for a while, one day we should probably do that to free up memory, at that point we should clean up the signers too

lib/vnet/client_application_service_client.go Outdated Show resolved Hide resolved
lib/vnet/remote_app_provider.go Outdated Show resolved Hide resolved
@nklaassen nklaassen force-pushed the nklaassen/remote-appprovider branch from f34a5a6 to 7ad0064 Compare January 28, 2025 18:03
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from GavinFrazar January 28, 2025 18:03
@nklaassen nklaassen enabled auto-merge January 28, 2025 18:05
@nklaassen nklaassen added this pull request to the merge queue Jan 28, 2025
Merged via the queue into master with commit 87135fc Jan 28, 2025
41 checks passed
@nklaassen nklaassen deleted the nklaassen/remote-appprovider branch January 28, 2025 18:42
@public-teleport-github-review-bot

@nklaassen See the table below for backport results.

Branch Result
branch/v17 Failed

nklaassen added a commit that referenced this pull request Jan 28, 2025
Backport #51279 to branch/v17
nklaassen added a commit that referenced this pull request Jan 29, 2025
Backport #51279 to branch/v17
nklaassen added a commit that referenced this pull request Jan 29, 2025
Backport #51279 to branch/v17
github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/md vnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants