Add backend Service and Cache support for SPIFFE Federation resource #45054
Conversation
strideynet
force-pushed
the
strideynet/add-backend-cache-for-spiffe-federation
branch
from
0454551
to
6c02c18
Compare
strideynet
force-pushed
the
strideynet/add-backend-cache-for-spiffe-federation
branch
from
12a1f47
to
89f7816
Compare
strideynet
force-pushed
the
strideynet/add-backend-cache-for-spiffe-federation
branch
from
89f7816
to
7c8a4b8
Compare
|
Waiting on #45058 to merge to let us tidy this up a little more. |
strideynet
added
the
no-changelog
github-actions
bot
requested review from
espadolini,
marcoandredinis and
tigrato
| @@ -132,6 +132,7 @@ require ( | |||
| github.com/go-errors/errors v1.4.2 // indirect | |||
| github.com/go-gorp/gorp/v3 v3.1.0 // indirect | |||
| github.com/go-jose/go-jose/v3 v3.0.3 // indirect | |||
| github.com/go-jose/go-jose/v4 v4.0.2 // indirect | |||
| @@ -94,6 +94,7 @@ type Config struct { | |||
| SAMLIdPSession services.SAMLIdPSession | |||
| SecReports services.SecReports | |||
| SnowflakeSession services.SnowflakeSession | |||
| SPIFFEFederations cache.SPIFFEFederationReader | |||
There was a problem hiding this comment.
any special reason to store the interface in the cache package
There was a problem hiding this comment.
It's the cache's dependency. In this case, had I used services.SPIFFEFederations, I'd be requiring methods which are not required by the cache to function (e.g the write side)
I'd usually have this kind of interfaces unexported, but, there's a bit of a weird case here where the functionality in accesspoint really feels like it belongs elsewhere. It's essentially a weird extension of the cache package itself. I'd considered redeclaring SPIFFEFederationsReader within accesspoint, but, I think that there's a better solution down the road (e.g a accesspoint.UpstreamReaders struct which holds the interfaces that the cache uses to populate).
Largely, I'm trying to play with the idea that the services.FoosService interfaces are unnecessary and problematic. We've see the symptoms of this across our codebase: weirdly named interfaces with "Ext" or "Internal" and methods that just return NotImplemented hung off "wrapper" structs that embed some other type that only partially implements an interface.
I thought about having services.SPIFFEFederationsReader, but it seems to really be the same problem, we should be aiming to create interfaces where they are consumed, not where they are implemented, and to have these interfaces declare the minimum amount of methods. If we think it's not the right time to start solving this problem, then I'm happy to create services.SPIFFEFederationsReader and use that, but I feel like that's just prolonging us ever solving this problem.
public-teleport-github-review-bot
bot
removed the request for review
from timothyb89
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
strideynet
deleted the
strideynet/add-backend-cache-for-spiffe-federation
branch
|
@strideynet See the table below for backport results.
|
…45054) * Start adding `lib/services` content for SPIFFEFederation type * Add SPIFFEFederation resource to cache * Wire SPIFFEFederation into cache * Fix NewTestAuthServer * Start writing tests * Add more test cases to validation * Add tests to cache for SPIFFEFederation * More test coverage * Add test for DeleteSPIFFEFederations * Finish off tests for SPIFFEFederation resource * go mod tidy * Go mod tidy * Appease linter * Avoid forcing kind/version in MarshalSPIFFEFederation * more linter appeasal
…45054) * Start adding `lib/services` content for SPIFFEFederation type * Add SPIFFEFederation resource to cache * Wire SPIFFEFederation into cache * Fix NewTestAuthServer * Start writing tests * Add more test cases to validation * Add tests to cache for SPIFFEFederation * More test coverage * Add test for DeleteSPIFFEFederations * Finish off tests for SPIFFEFederation resource * go mod tidy * Go mod tidy * Appease linter * Avoid forcing kind/version in MarshalSPIFFEFederation * more linter appeasal
…ource (#45054) (#45384) * Add backend Service and Cache support for SPIFFE Federation resource (#45054) * Start adding `lib/services` content for SPIFFEFederation type * Add SPIFFEFederation resource to cache * Wire SPIFFEFederation into cache * Fix NewTestAuthServer * Start writing tests * Add more test cases to validation * Add tests to cache for SPIFFEFederation * More test coverage * Add test for DeleteSPIFFEFederations * Finish off tests for SPIFFEFederation resource * go mod tidy * Go mod tidy * Appease linter * Avoid forcing kind/version in MarshalSPIFFEFederation * more linter appeasal * Go mod tidily * Fix imports * Forgot to remove merge conflict boundary
…ource (gravitational#45054) (gravitational#45412) * Add backend Service and Cache support for SPIFFE Federation resource (gravitational#45054) * Start adding `lib/services` content for SPIFFEFederation type * Add SPIFFEFederation resource to cache * Wire SPIFFEFederation into cache * Fix NewTestAuthServer * Start writing tests * Add more test cases to validation * Add tests to cache for SPIFFEFederation * More test coverage * Add test for DeleteSPIFFEFederations * Finish off tests for SPIFFEFederation resource * go mod tidy * Go mod tidy * Appease linter * Avoid forcing kind/version in MarshalSPIFFEFederation * more linter appeasal * Ignore ID change in cache tests * Go mod tidy
Part of #44861
As per #43348
Adds the local service implementation for SPIFFE Federation and support for this in the Cache used by the Auth Server.
Notably, I've broken out the cache additions for this resource into it's own file rather than appending to the rather unwieldy
collections.go. I'm hoping we can kick off a bit of a trend here 😅