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

coordinator: don't share CA instance with gRPC servers #520

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

burgerdev
Copy link
Contributor

@burgerdev burgerdev commented Jun 3, 2024

At a high-level, this PR

  • moves and renames the meshAuthority to authority.Authority
  • takes the permanent ca.CA instance away from the gRPC servers

The idea is that the methods called by the gRPC servers on the CA instance are always coupled to the current active manifest, thus getting the manifest from meshAuthority and the certificates from CA is inherently racy (think of concurrent calls to SetManifest and NewMeshCert, for example). This is a first step towards tighter coupling of the two, by handing out CA objects when required and removing the permanent members.

In the meshAPI server, I'm moving all certificate operations to the ValidateCallback, and all certificate operations happen on the same CA instance. This does not make a practical difference now, but allows to swap out the permanent CA object with one per manifest generation.

In the userAPI server, I'm adding the CA as a return to the manifest retrieval operation, also in preparation to swap it out in the future.

The API of the Authority should be considered transitory while we're working on the persistence. I could imagine it returning a struct with manifest and CA combined.

The race conditions are still present (see associated TODOs), but it should be easier to mitigate them going forward.

@burgerdev burgerdev requested a review from katexochen June 3, 2024 14:32
@burgerdev burgerdev force-pushed the burgerdev/ca-ctx branch 2 times, most recently from eb0ed6b to de66752 Compare June 4, 2024 19:13
@burgerdev burgerdev added the no changelog PRs not listed in the release notes label Jun 4, 2024
@burgerdev burgerdev marked this pull request as ready for review June 4, 2024 19:14
@burgerdev burgerdev force-pushed the burgerdev/ca-ctx branch 3 times, most recently from 5a10cbd to b9779c5 Compare June 6, 2024 08:34
internal/authority/authority.go Outdated Show resolved Hide resolved
internal/ca/bundle.go Outdated Show resolved Hide resolved
coordinator/internal/authority/authority.go Outdated Show resolved Hide resolved
coordinator/meshapi.go Outdated Show resolved Hide resolved
coordinator/userapi.go Outdated Show resolved Hide resolved
@burgerdev burgerdev merged commit 7f7deaf into main Jun 6, 2024
8 checks passed
@burgerdev burgerdev deleted the burgerdev/ca-ctx branch June 6, 2024 13:15
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