Restructure (6): Store interface for local truststore access#590
Merged
roosterfish merged 11 commits intocanonical:v3from Jan 20, 2026
Merged
Restructure (6): Store interface for local truststore access#590roosterfish merged 11 commits intocanonical:v3from
Store interface for local truststore access#590roosterfish merged 11 commits intocanonical:v3from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the truststore interface to expose it properly through public types rather than internal implementation types. The changes align with a broader effort to restructure microcluster packages and enable better testing through mockable interfaces.
Changes:
- Introduced a new
Storeinterface inmicrocluster/types/truststore.goto represent a local truststore - Moved
RemoteandLocationtypes frominternal/trusttomicrocluster/typespackage - Renamed the
Remotes()method toTruststore()in the State interface and updated all method names for consistency (e.g.,Addresses()→RemoteAddresses(),Cluster()→RemoteClients())
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| microcluster/types/truststore.go | New file defining the Store interface and Remote/Location types for public API |
| internal/trust/truststore.go | Updated to use types.Remote instead of internal Remote type |
| internal/trust/remotes.go | Removed duplicate type definitions; updated to use types.Remote and types.Location; renamed methods for consistency |
| internal/state/state.go | Changed Remotes() to Truststore() returning types.Store interface; updated SetConfig signature |
| internal/rest/rest.go | Updated to call Truststore().RemoteCertificatesNative() |
| internal/rest/resources/truststore.go | Updated to use types.Remote/types.Location and Truststore() method |
| internal/rest/resources/tokens.go | Updated to call Truststore().RemoteAddresses() |
| internal/rest/resources/heartbeat.go | Updated to call Truststore().Replace() |
| internal/rest/resources/daemon.go | Updated to use Truststore() method |
| internal/rest/resources/control.go | Updated to use types.Remote/types.Location and Truststore() method |
| internal/rest/resources/cluster.go | Updated to use types.Remote/types.Location and Truststore() method |
| internal/recover/recover.go | Updated to call RemoteClients() instead of Cluster() |
| internal/daemon/daemon.go | Updated to use types.Remote/types.Location and updated method references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Store interface allows masking the internal truststore implementation and offers helpful utilities to query truststore information purely in memory (except the add/replace operations). By having the interface in a the public types package we can replace the Remotes func in the public State interface. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
…tore entries Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Also change the wording a little bit to represent the actual truststore. Entries in the truststore are called remotes therefore those are returned/used by the various funcs of the Store interface. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
…tore Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
1efd93f to
9d988f4
Compare
tugbataluy
approved these changes
Jan 20, 2026
Contributor
tugbataluy
left a comment
There was a problem hiding this comment.
I think your implementation is solid, looks great as-is, ready to ship!
markylaing
reviewed
Jan 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the sixth PR taken from a single commit in #534 (even though in this case the actual changes are very different from the original commit).
The final package layout will be different but to allow reviewing the changes in smaller chunks I will split this PR by its commits.
In this PR the
Remotesfunc from theStateinterface is replaced with an interface to not anymore return an implementation of the truststore with types located inside theinternal/package.Furthermore the naming is made a bit more consistent:
Storerepresents a truststoreRemoterepresents an entry in the truststoreThe initial idea was to just hide the truststore access. However it's a very inexpensive way of querying for the existence and details of cluster members as the truststore is already located in memory.