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

Implement RFD 19: Event Iteration API #6731

Merged
merged 32 commits into from
May 18, 2021
Merged

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented May 4, 2021

What

This PR implements the additional API endpoints described in RFD 19 and modifies event drivers to add this functionality to the various backends.

Details

This adds two additional endpoints onto the gRPC auth service in accordance with RFD 19 and rewrites the backend event drivers to support pagination.

Related Issues

Fixes #5435

Enterprise PR

This PR needs changes to be made to teleport.e. The pair PR is https://github.com/gravitational/teleport.e/pull/257

Other

This PR is based on the branch of #6583 and that PR must be merged before this is reviewed in order for the diff to make sense.

Needs a backport to branch/v6 after merge.

cc @kimlisa

@xacrimon xacrimon added cloud Cloud c-bi Internal Customer Reference c-ar Internal Customer Reference labels May 4, 2021
@xacrimon xacrimon added this to the 6.2 "Buffalo" milestone May 4, 2021
@xacrimon xacrimon self-assigned this May 4, 2021
@russjones
Copy link
Contributor

@fspmarshall @quinqu @andrejtokarcik You three are the assigned reviewers, can you review?

@xacrimon xacrimon force-pushed the joel/event-api-pagination branch 4 times, most recently from d080fe6 to d084f6b Compare May 10, 2021 14:08
@xacrimon xacrimon requested a review from alex-kovoy as a code owner May 10, 2021 16:43
rfd/0019-event-iteration-api.md Show resolved Hide resolved
api/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/auth/clt.go Outdated Show resolved Hide resolved
lib/events/filelog.go Show resolved Hide resolved
api/client/client.go Outdated Show resolved Hide resolved
@xacrimon xacrimon requested a review from quinqu May 10, 2021 19:07
@xacrimon xacrimon force-pushed the joel/event-api-pagination branch 3 times, most recently from 95f3b8d to 88f07de Compare May 10, 2021 19:32
@xacrimon
Copy link
Contributor Author

@fspmarshall @quinqu @andrejtokarcik PTAL

api/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/events/api.go Outdated Show resolved Hide resolved
api/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/auth/auth_with_roles.go Outdated Show resolved Hide resolved
lib/events/auditlog.go Outdated Show resolved Hide resolved
lib/events/dynamic.go Outdated Show resolved Hide resolved
lib/events/dynamic.go Outdated Show resolved Hide resolved
lib/events/dynamoevents/dynamoevents.go Outdated Show resolved Hide resolved
lib/events/dynamoevents/dynamoevents.go Show resolved Hide resolved
lib/events/forward.go Outdated Show resolved Hide resolved
lib/events/test/suite.go Outdated Show resolved Hide resolved
@@ -81,6 +84,10 @@ func (s *DynamoeventsSuite) SetUpTest(c *check.C) {
c.Assert(err, check.IsNil)
}

func (s *DynamoeventsSuite) TestPagination(c *check.C) {
s.EventPagination(c)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be too cumbersome to introduce this as a test based on the testing module in accordance with our new testing guidelines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is, in order to not introduce a lot of code duplication and weird logic it only really makes sense if we rewrite all of these suite tests which I'd rather not do in this already gigantic (in terms of delta) PR.

api/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/events/dynamoevents/dynamoevents.go Outdated Show resolved Hide resolved
lib/web/apiserver.go Outdated Show resolved Hide resolved
Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-required c-ar Internal Customer Reference c-bi Internal Customer Reference cloud Cloud
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix events iteration
5 participants