From 074a5c79ab921881b12c6dcde06af423bc020169 Mon Sep 17 00:00:00 2001 From: shipperizer Date: Fri, 5 Apr 2024 18:04:58 +0200 Subject: [PATCH 1/6] fix: add page tokens to the response --- internal/http/types/generic.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/http/types/generic.go b/internal/http/types/generic.go index 252133d79..0584d24cb 100644 --- a/internal/http/types/generic.go +++ b/internal/http/types/generic.go @@ -16,14 +16,19 @@ type Response struct { } type Pagination struct { - Page int64 `json:"page"` - Size int64 `json:"size"` + Page int64 `json:"page"` + PageToken string `json:"page_token"` + Size int64 `json:"size"` + + Next string `json:"next,omitempty"` + Prev string `json:"prev,omitempty"` + First string `json:"first,omitempty"` } func NewPaginationWithDefaults() *Pagination { p := new(Pagination) - p.Page = 1 + p.PageToken = "" p.Size = 100 return p @@ -33,13 +38,14 @@ func ParsePagination(q url.Values) *Pagination { p := NewPaginationWithDefaults() - if page, err := strconv.ParseInt(q.Get("page"), 10, 64); err == nil && page > 1 { - p.Page = page - } - + // TODO @shipperizer introduce go-playground/validator if size, err := strconv.ParseInt(q.Get("size"), 10, 64); err == nil && size > 0 { p.Size = size } + if token := q.Get("page_token"); token != "" { + p.PageToken = token + } + return p } From e2b00691df0cb01bcea52aab8deb7c03a7735346 Mon Sep 17 00:00:00 2001 From: shipperizer Date: Fri, 5 Apr 2024 18:05:37 +0200 Subject: [PATCH 2/6] feat: parse and expose link header from hydra --- pkg/identities/interfaces.go | 2 +- pkg/identities/service.go | 72 +++++++++++-- pkg/identities/service_test.go | 184 ++++++++++++++++++--------------- 3 files changed, 160 insertions(+), 98 deletions(-) diff --git a/pkg/identities/interfaces.go b/pkg/identities/interfaces.go index a805de5b7..890e90cb0 100644 --- a/pkg/identities/interfaces.go +++ b/pkg/identities/interfaces.go @@ -10,7 +10,7 @@ import ( ) type ServiceInterface interface { - ListIdentities(context.Context, int64, int64, string) (*IdentityData, error) + ListIdentities(context.Context, int64, string, string) (*IdentityData, error) GetIdentity(context.Context, string) (*IdentityData, error) CreateIdentity(context.Context, *kClient.CreateIdentityBody) (*IdentityData, error) UpdateIdentity(context.Context, string, *kClient.UpdateIdentityBody) (*IdentityData, error) diff --git a/pkg/identities/service.go b/pkg/identities/service.go index 4f7b20ad0..4d66532cc 100644 --- a/pkg/identities/service.go +++ b/pkg/identities/service.go @@ -9,8 +9,10 @@ import ( "fmt" "io" "net/http" + "net/url" kClient "github.com/ory/kratos-client-go" + "github.com/tomnomnom/linkheader" "go.opentelemetry.io/otel/trace" "github.com/canonical/identity-platform-admin-ui/internal/logging" @@ -18,15 +20,23 @@ import ( ) type Service struct { - kratos kClient.IdentityApi + kratos kClient.IdentityAPI tracer trace.Tracer monitor monitoring.MonitorInterface logger logging.LoggerInterface } +// TODO @shipperizer worth offloading to a different place as it's going to be reused +type PaginationTokens struct { + First string + Prev string + Next string +} + type IdentityData struct { Identities []kClient.Identity + Tokens PaginationTokens Error *kClient.GenericError } @@ -35,8 +45,8 @@ type KratosError struct { Error *kClient.GenericError `json:"error,omitempty"` } -func (s *Service) buildListRequest(ctx context.Context, page, size int64, credID string) kClient.IdentityApiListIdentitiesRequest { - r := s.kratos.ListIdentities(ctx).Page(page).PerPage(size) +func (s *Service) buildListRequest(ctx context.Context, size int64, token, credID string) kClient.IdentityAPIListIdentitiesRequest { + r := s.kratos.ListIdentities(ctx).PageToken(token).PageSize(size) if credID != "" { r = r.CredentialsIdentifier(credID) @@ -45,6 +55,36 @@ func (s *Service) buildListRequest(ctx context.Context, page, size int64, credID return r } +func (s *Service) parseLinkURL(linkURL string) string { + u, err := url.Parse(linkURL) + + if err != nil { + s.logger.Errorf("failed to parse link header successfully: %s", err) + return "" + } + + return u.Query().Get("page_token") +} + +func (s *Service) parsePagination(r *http.Response) PaginationTokens { + links := linkheader.Parse(r.Header.Get("Link")) + + pagination := PaginationTokens{} + + for _, link := range links { + switch link.Rel { + case "first": + pagination.First = s.parseLinkURL(link.URL) + case "next": + pagination.Next = s.parseLinkURL(link.URL) + case "prev": + pagination.Prev = s.parseLinkURL(link.URL) + } + } + + return pagination +} + func (s *Service) parseError(r *http.Response) *kClient.GenericError { gerr := KratosError{Error: kClient.NewGenericErrorWithDefaults()} @@ -60,12 +100,21 @@ func (s *Service) parseError(r *http.Response) *kClient.GenericError { } // TODO @shipperizer fix pagination -func (s *Service) ListIdentities(ctx context.Context, page, size int64, credID string) (*IdentityData, error) { - ctx, span := s.tracer.Start(ctx, "kratos.IdentityApi.ListIdentities") +// shipperizer in ~/shipperizer/identity-platform-admin-ui on IAM-782 ● ● λ http :18000/admin/identities +// HTTP/1.1 200 OK +// Cache-Control: private, no-cache, no-store, must-revalidate +// Content-Length: 3 +// Content-Type: application/json; charset=utf-8 +// Date: Thu, 04 Apr 2024 14:08:03 GMT +// Link: ; rel="first",; rel="next",; rel="prev" +// X-Total-Count: 0 +// [] +func (s *Service) ListIdentities(ctx context.Context, size int64, token, credID string) (*IdentityData, error) { + ctx, span := s.tracer.Start(ctx, "kratos.IdentityAPI.ListIdentities") defer span.End() identities, rr, err := s.kratos.ListIdentitiesExecute( - s.buildListRequest(ctx, page, size, credID), + s.buildListRequest(ctx, size, token, credID), ) data := new(IdentityData) @@ -75,6 +124,7 @@ func (s *Service) ListIdentities(ctx context.Context, page, size int64, credID s data.Error = s.parseError(rr) } + data.Tokens = s.parsePagination(rr) data.Identities = identities // TODO @shipperizer check if identities is defaulting to empty slice inside kratos-client @@ -86,7 +136,7 @@ func (s *Service) ListIdentities(ctx context.Context, page, size int64, credID s } func (s *Service) GetIdentity(ctx context.Context, ID string) (*IdentityData, error) { - ctx, span := s.tracer.Start(ctx, "kratos.IdentityApi.GetIdentity") + ctx, span := s.tracer.Start(ctx, "kratos.IdentityAPI.GetIdentity") defer span.End() identity, rr, err := s.kratos.GetIdentityExecute( @@ -110,7 +160,7 @@ func (s *Service) GetIdentity(ctx context.Context, ID string) (*IdentityData, er } func (s *Service) CreateIdentity(ctx context.Context, bodyID *kClient.CreateIdentityBody) (*IdentityData, error) { - ctx, span := s.tracer.Start(ctx, "kratos.IdentityApi.CreateIdentity") + ctx, span := s.tracer.Start(ctx, "kratos.IdentityAPI.CreateIdentity") defer span.End() if bodyID == nil { @@ -147,7 +197,7 @@ func (s *Service) CreateIdentity(ctx context.Context, bodyID *kClient.CreateIden } func (s *Service) UpdateIdentity(ctx context.Context, ID string, bodyID *kClient.UpdateIdentityBody) (*IdentityData, error) { - ctx, span := s.tracer.Start(ctx, "kratos.IdentityApi.UpdateIdentity") + ctx, span := s.tracer.Start(ctx, "kratos.IdentityAPI.UpdateIdentity") defer span.End() if ID == "" { err := fmt.Errorf("no identity ID passed") @@ -196,7 +246,7 @@ func (s *Service) UpdateIdentity(ctx context.Context, ID string, bodyID *kClient } func (s *Service) DeleteIdentity(ctx context.Context, ID string) (*IdentityData, error) { - ctx, span := s.tracer.Start(ctx, "kratos.IdentityApi.DeleteIdentity") + ctx, span := s.tracer.Start(ctx, "kratos.IdentityAPI.DeleteIdentity") defer span.End() rr, err := s.kratos.DeleteIdentityExecute( @@ -215,7 +265,7 @@ func (s *Service) DeleteIdentity(ctx context.Context, ID string) (*IdentityData, return data, err } -func NewService(kratos kClient.IdentityApi, tracer trace.Tracer, monitor monitoring.MonitorInterface, logger logging.LoggerInterface) *Service { +func NewService(kratos kClient.IdentityAPI, tracer trace.Tracer, monitor monitoring.MonitorInterface, logger logging.LoggerInterface) *Service { s := new(Service) s.kratos = kratos diff --git a/pkg/identities/service_test.go b/pkg/identities/service_test.go index 007ec962c..aa1474532 100644 --- a/pkg/identities/service_test.go +++ b/pkg/identities/service_test.go @@ -21,7 +21,7 @@ import ( //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_interfaces.go -source=./interfaces.go //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_monitor.go -source=../../internal/monitoring/interfaces.go //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_tracing.go go.opentelemetry.io/otel/trace Tracer -//go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityApi +//go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityAPI func TestListIdentitiesSuccess(t *testing.T) { ctrl := gomock.NewController(t) @@ -30,12 +30,12 @@ func TestListIdentitiesSuccess(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() - identityRequest := kClient.IdentityApiListIdentitiesRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIListIdentitiesRequest{ + ApiService: mockKratosIdentityAPI, } identities := make([]kClient.Identity, 0) @@ -44,17 +44,17 @@ func TestListIdentitiesSuccess(t *testing.T) { identities = append(identities, *kClient.NewIdentity(fmt.Sprintf("test-%v", i), "test.json", "https://test.com/test.json", map[string]string{"name": "name"})) } - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.ListIdentities").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().ListIdentities(ctx).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().ListIdentitiesExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiListIdentitiesRequest) ([]kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.ListIdentities").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().ListIdentities(ctx).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().ListIdentitiesExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIListIdentitiesRequest) ([]kClient.Identity, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times - if page := (*int64)(reflect.ValueOf(r).FieldByName("page").UnsafePointer()); *page != 2 { - t.Fatalf("expected page as 2, got %v", *page) + if pageToken := (*string)(reflect.ValueOf(r).FieldByName("pageToken").UnsafePointer()); *pageToken != "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ" { + t.Fatalf("expected pageToken as eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ, got %v", *pageToken) } - if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("perPage").UnsafePointer()); *pageSize != 10 { + if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("pageSize").UnsafePointer()); *pageSize != 10 { t.Fatalf("expected page size as 10, got %v", *pageSize) } @@ -62,15 +62,27 @@ func TestListIdentitiesSuccess(t *testing.T) { t.Fatalf("expected credential id to be empty, got %v", *credID) } - return identities, new(http.Response), nil + rr := new(http.Response) + rr.Header = make(http.Header) + rr.Header.Set("Link", `; rel="first",; rel="next",; rel="prev`) + + return identities, rr, nil }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).ListIdentities(ctx, 2, 10, "") + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).ListIdentities(ctx, 10, "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ", "") if !reflect.DeepEqual(ids.Identities, identities) { t.Fatalf("expected identities to be %v not %v", identities, ids.Identities) } + + if !reflect.DeepEqual( + []string{ids.Tokens.First, ids.Tokens.Next, ids.Tokens.Prev}, + []string{"eyJvZmZzZXQiOiIwIiwidiI6Mn0", "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ", "eyJvZmZzZXQiOiItMjUwIiwidiI6Mn0"}, + ) { + t.Fatalf("expected tokens to be set, not %v", ids.Tokens) + } + if err != nil { t.Fatalf("expected error to be nil not %v", err) } @@ -83,28 +95,28 @@ func TestListIdentitiesFails(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() - identityRequest := kClient.IdentityApiListIdentitiesRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIListIdentitiesRequest{ + ApiService: mockKratosIdentityAPI, } identities := make([]kClient.Identity, 0) mockLogger.EXPECT().Error(gomock.Any()).Times(1) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.ListIdentities").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().ListIdentities(ctx).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().ListIdentitiesExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiListIdentitiesRequest) ([]kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.ListIdentities").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().ListIdentities(ctx).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().ListIdentitiesExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIListIdentitiesRequest) ([]kClient.Identity, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times - if page := (*int64)(reflect.ValueOf(r).FieldByName("page").UnsafePointer()); *page != 2 { - t.Fatalf("expected page as 2, got %v", *page) + if pageToken := (*string)(reflect.ValueOf(r).FieldByName("pageToken").UnsafePointer()); *pageToken != "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ" { + t.Fatalf("expected pageToken as eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ, got %v", *pageToken) } - if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("perPage").UnsafePointer()); *pageSize != 10 { + if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("pageSize").UnsafePointer()); *pageSize != 10 { t.Fatalf("expected page size as 10, got %v", *pageSize) } @@ -135,7 +147,7 @@ func TestListIdentitiesFails(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).ListIdentities(ctx, 2, 10, "test") + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).ListIdentities(ctx, 10, "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ", "test") if !reflect.DeepEqual(ids.Identities, identities) { t.Fatalf("expected identities to be empty not %v", ids.Identities) @@ -161,22 +173,22 @@ func TestGetIdentitySuccess(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() credID := "test-1" - identityRequest := kClient.IdentityApiGetIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIGetIdentityRequest{ + ApiService: mockKratosIdentityAPI, } identity := kClient.NewIdentity(credID, "test.json", "https://test.com/test.json", map[string]string{"name": "name"}) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.GetIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().GetIdentity(ctx, credID).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().GetIdentityExecute(gomock.Any()).Times(1).Return(identity, new(http.Response), nil) + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.GetIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().GetIdentity(ctx, credID).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().GetIdentityExecute(gomock.Any()).Times(1).Return(identity, new(http.Response), nil) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).GetIdentity(ctx, credID) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).GetIdentity(ctx, credID) if !reflect.DeepEqual(ids.Identities, []kClient.Identity{*identity}) { t.Fatalf("expected identities to be %v not %v", *identity, ids.Identities) @@ -193,20 +205,20 @@ func TestGetIdentityFails(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() credID := "test" - identityRequest := kClient.IdentityApiGetIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIGetIdentityRequest{ + ApiService: mockKratosIdentityAPI, } mockLogger.EXPECT().Error(gomock.Any()).Times(1) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.GetIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().GetIdentity(ctx, credID).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().GetIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiGetIdentityRequest) (*kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.GetIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().GetIdentity(ctx, credID).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().GetIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIGetIdentityRequest) (*kClient.Identity, *http.Response, error) { rr := httptest.NewRecorder() rr.Header().Set("Content-Type", "application/json") rr.WriteHeader(http.StatusNotFound) @@ -230,7 +242,7 @@ func TestGetIdentityFails(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).GetIdentity(ctx, credID) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).GetIdentity(ctx, credID) if !reflect.DeepEqual(ids.Identities, make([]kClient.Identity, 0)) { t.Fatalf("expected identities to be empty not %v", ids.Identities) @@ -256,12 +268,12 @@ func TestCreateIdentitySuccess(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() - identityRequest := kClient.IdentityApiCreateIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPICreateIdentityRequest{ + ApiService: mockKratosIdentityAPI, } identity := kClient.NewIdentity("test", "test.json", "https://test.com/test.json", map[string]string{"name": "name"}) @@ -269,10 +281,10 @@ func TestCreateIdentitySuccess(t *testing.T) { identityBody := kClient.NewCreateIdentityBody("test.json", map[string]interface{}{"name": "name"}) identityBody.SetCredentials(*credentials) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.CreateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().CreateIdentity(ctx).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().CreateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiCreateIdentityRequest) (*kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.CreateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().CreateIdentity(ctx).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().CreateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPICreateIdentityRequest) (*kClient.Identity, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times if IDBody := (*kClient.CreateIdentityBody)(reflect.ValueOf(r).FieldByName("createIdentityBody").UnsafePointer()); !reflect.DeepEqual(*IDBody, *identityBody) { @@ -283,7 +295,7 @@ func TestCreateIdentitySuccess(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).CreateIdentity(ctx, identityBody) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).CreateIdentity(ctx, identityBody) if !reflect.DeepEqual(ids.Identities, []kClient.Identity{*identity}) { t.Fatalf("expected identities to be %v not %v", *identity, ids.Identities) @@ -301,12 +313,12 @@ func TestCreateIdentityFails(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() - identityRequest := kClient.IdentityApiCreateIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPICreateIdentityRequest{ + ApiService: mockKratosIdentityAPI, } credentials := kClient.NewIdentityWithCredentialsWithDefaults() @@ -314,10 +326,10 @@ func TestCreateIdentityFails(t *testing.T) { identityBody.SetCredentials(*credentials) mockLogger.EXPECT().Error(gomock.Any()).Times(1) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.CreateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().CreateIdentity(ctx).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().CreateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiCreateIdentityRequest) (*kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.CreateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().CreateIdentity(ctx).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().CreateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPICreateIdentityRequest) (*kClient.Identity, *http.Response, error) { rr := httptest.NewRecorder() rr.Header().Set("Content-Type", "application/json") rr.WriteHeader(http.StatusInternalServerError) @@ -341,7 +353,7 @@ func TestCreateIdentityFails(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).CreateIdentity(ctx, identityBody) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).CreateIdentity(ctx, identityBody) if !reflect.DeepEqual(ids.Identities, make([]kClient.Identity, 0)) { t.Fatalf("expected identities to be empty not %v", ids.Identities) @@ -367,12 +379,12 @@ func TestUpdateIdentitySuccess(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() - identityRequest := kClient.IdentityApiUpdateIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIUpdateIdentityRequest{ + ApiService: mockKratosIdentityAPI, } identity := kClient.NewIdentity("test", "test.json", "https://test.com/test.json", map[string]string{"name": "name"}) @@ -381,10 +393,10 @@ func TestUpdateIdentitySuccess(t *testing.T) { identityBody.SetTraits(map[string]interface{}{"name": "name"}) identityBody.SetCredentials(*credentials) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.UpdateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().UpdateIdentity(ctx, identity.Id).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().UpdateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiUpdateIdentityRequest) (*kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.UpdateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().UpdateIdentity(ctx, identity.Id).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().UpdateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIUpdateIdentityRequest) (*kClient.Identity, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times if IDBody := (*kClient.UpdateIdentityBody)(reflect.ValueOf(r).FieldByName("updateIdentityBody").UnsafePointer()); !reflect.DeepEqual(*IDBody, *identityBody) { @@ -395,7 +407,7 @@ func TestUpdateIdentitySuccess(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).UpdateIdentity(ctx, identity.Id, identityBody) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).UpdateIdentity(ctx, identity.Id, identityBody) if !reflect.DeepEqual(ids.Identities, []kClient.Identity{*identity}) { t.Fatalf("expected identities to be %v not %v", *identity, ids.Identities) @@ -413,14 +425,14 @@ func TestUpdateIdentityFails(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() credID := "test" - identityRequest := kClient.IdentityApiUpdateIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIUpdateIdentityRequest{ + ApiService: mockKratosIdentityAPI, } credentials := kClient.NewIdentityWithCredentialsWithDefaults() @@ -429,10 +441,10 @@ func TestUpdateIdentityFails(t *testing.T) { identityBody.SetCredentials(*credentials) mockLogger.EXPECT().Error(gomock.Any()).Times(1) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.UpdateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().UpdateIdentity(ctx, credID).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().UpdateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiUpdateIdentityRequest) (*kClient.Identity, *http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.UpdateIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().UpdateIdentity(ctx, credID).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().UpdateIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIUpdateIdentityRequest) (*kClient.Identity, *http.Response, error) { rr := httptest.NewRecorder() rr.Header().Set("Content-Type", "application/json") rr.WriteHeader(http.StatusConflict) @@ -456,7 +468,7 @@ func TestUpdateIdentityFails(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).UpdateIdentity(ctx, credID, identityBody) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).UpdateIdentity(ctx, credID, identityBody) if !reflect.DeepEqual(ids.Identities, make([]kClient.Identity, 0)) { t.Fatalf("expected identities to be empty not %v", ids.Identities) @@ -482,20 +494,20 @@ func TestDeleteIdentitySuccess(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() credID := "test-1" - identityRequest := kClient.IdentityApiDeleteIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIDeleteIdentityRequest{ + ApiService: mockKratosIdentityAPI, } - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.DeleteIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().DeleteIdentity(ctx, credID).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().DeleteIdentityExecute(gomock.Any()).Times(1).Return(new(http.Response), nil) + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.DeleteIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().DeleteIdentity(ctx, credID).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().DeleteIdentityExecute(gomock.Any()).Times(1).Return(new(http.Response), nil) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).DeleteIdentity(ctx, credID) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).DeleteIdentity(ctx, credID) if len(ids.Identities) > 0 { t.Fatalf("invalid result, expected no identities, got %v", ids.Identities) @@ -513,20 +525,20 @@ func TestDeleteIdentityFails(t *testing.T) { mockLogger := NewMockLoggerInterface(ctrl) mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() credID := "test-1" - identityRequest := kClient.IdentityApiDeleteIdentityRequest{ - ApiService: mockKratosIdentityApi, + identityRequest := kClient.IdentityAPIDeleteIdentityRequest{ + ApiService: mockKratosIdentityAPI, } mockLogger.EXPECT().Error(gomock.Any()).Times(1) - mockTracer.EXPECT().Start(ctx, "kratos.IdentityApi.DeleteIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().DeleteIdentity(ctx, credID).Times(1).Return(identityRequest) - mockKratosIdentityApi.EXPECT().DeleteIdentityExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiDeleteIdentityRequest) (*http.Response, error) { + mockTracer.EXPECT().Start(ctx, "kratos.IdentityAPI.DeleteIdentity").Times(1).Return(ctx, trace.SpanFromContext(ctx)) + mockKratosIdentityAPI.EXPECT().DeleteIdentity(ctx, credID).Times(1).Return(identityRequest) + mockKratosIdentityAPI.EXPECT().DeleteIdentityExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIDeleteIdentityRequest) (*http.Response, error) { rr := httptest.NewRecorder() rr.Header().Set("Content-Type", "application/json") rr.WriteHeader(http.StatusNotFound) @@ -550,7 +562,7 @@ func TestDeleteIdentityFails(t *testing.T) { }, ) - ids, err := NewService(mockKratosIdentityApi, mockTracer, mockMonitor, mockLogger).DeleteIdentity(ctx, credID) + ids, err := NewService(mockKratosIdentityAPI, mockTracer, mockMonitor, mockLogger).DeleteIdentity(ctx, credID) if !reflect.DeepEqual(ids.Identities, make([]kClient.Identity, 0)) { t.Fatalf("expected identities to be empty not %v", ids.Identities) From f596fbe9c54da1fd8d84a5cb79c11832d6ce967c Mon Sep 17 00:00:00 2001 From: shipperizer Date: Fri, 5 Apr 2024 18:06:27 +0200 Subject: [PATCH 3/6] feat: adjust identity api to accept page token closes #256 --- cmd/serve.go | 2 +- go.mod | 3 +- go.sum | 6 ++-- internal/kratos/client.go | 4 +-- pkg/identities/handlers.go | 7 ++++- pkg/identities/handlers_test.go | 49 +++++++++++++++++++-------------- pkg/web/router.go | 2 +- 7 files changed, 45 insertions(+), 28 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 9bdb63c01..8ad7630f4 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -100,7 +100,7 @@ func serve() { schemasConfig := &schemas.Config{ K8s: k8sCoreV1, - Kratos: extCfg.KratosPublic().IdentityApi(), + Kratos: extCfg.KratosPublic().IdentityAPI(), Name: specs.SchemasConfigMapName, Namespace: specs.SchemasConfigMapNamespace, } diff --git a/go.mod b/go.mod index 05bcdf5b2..a2929600f 100644 --- a/go.mod +++ b/go.mod @@ -13,11 +13,12 @@ require ( github.com/openfga/go-sdk v0.3.4 github.com/openfga/language/pkg/go v0.0.0-20240122114256-aaa86ab89379 github.com/ory/hydra-client-go/v2 v2.1.1 - github.com/ory/kratos-client-go v1.0.0 + github.com/ory/kratos-client-go v1.1.0 github.com/ory/oathkeeper-client-go v0.40.6 github.com/prometheus/client_golang v1.17.0 github.com/spf13/cobra v1.8.0 github.com/stretchr/testify v1.8.4 + github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.45.0 go.opentelemetry.io/contrib/propagators/jaeger v1.20.0 go.opentelemetry.io/otel v1.19.0 diff --git a/go.sum b/go.sum index 04629d1c2..704f11188 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/openfga/language/pkg/go v0.0.0-20240122114256-aaa86ab89379 h1:j42rKsj github.com/openfga/language/pkg/go v0.0.0-20240122114256-aaa86ab89379/go.mod h1:dHJaJ7H5tViBCPidTsfl3IOd152FhYxWFQmZXOhZ2pw= github.com/ory/hydra-client-go/v2 v2.1.1 h1:3JatU9uFbw5XhF3lgPCas1l1Kok2v5Mq1p26zZwGHNg= github.com/ory/hydra-client-go/v2 v2.1.1/go.mod h1:IiIwChp/9wRvPoyFQblqPvg78uVishCCrV9+/M7Pl34= -github.com/ory/kratos-client-go v1.0.0 h1:mm32FMJrt4pBv2KEuhuNtiewJApc8c1Kmz0+WFHhOMA= -github.com/ory/kratos-client-go v1.0.0/go.mod h1:a2Tl4cgQAxsjR59w3EfnH5hengabjXUHiEVDzdqiZI0= +github.com/ory/kratos-client-go v1.1.0 h1:mCk5wxNTxjYq/sbZfoEY/JcxuBtuixStHD14Y0sU1E8= +github.com/ory/kratos-client-go v1.1.0/go.mod h1:ultwfjWsBxshnZgopqQ3DrKOe/t6SXsM+KKOd21PaTQ= github.com/ory/oathkeeper-client-go v0.40.6 h1:Qm/odusPn6DTJ8mXXElNzfXYpcD5wqmb/k3dOYKUfTg= github.com/ory/oathkeeper-client-go v0.40.6/go.mod h1:9JUPR04XPH3e53TYbfu+KveCnTIYtlSTJiQ23rEQLJI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -147,6 +147,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 h1:nrZ3ySNYwJbSpD6ce9duiP+QkD3JuLCcWkdaehUS/3Y= +github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80/go.mod h1:iFyPdL66DjUD96XmzVL3ZntbzcflLnznH0fr99w5VqE= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/internal/kratos/client.go b/internal/kratos/client.go index c4a14cb5b..ba39282f4 100644 --- a/internal/kratos/client.go +++ b/internal/kratos/client.go @@ -14,8 +14,8 @@ type Client struct { c *client.APIClient } -func (c *Client) IdentityApi() client.IdentityApi { - return c.c.IdentityApi +func (c *Client) IdentityAPI() client.IdentityAPI { + return c.c.IdentityAPI } func NewClient(url string, debug bool) *Client { diff --git a/pkg/identities/handlers.go b/pkg/identities/handlers.go index 80e26a6f6..7056f6f72 100644 --- a/pkg/identities/handlers.go +++ b/pkg/identities/handlers.go @@ -64,7 +64,7 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { credID := r.URL.Query().Get("credID") - ids, err := a.service.ListIdentities(r.Context(), pagination.Page, pagination.Size, credID) + ids, err := a.service.ListIdentities(r.Context(), pagination.Size, pagination.PageToken, credID) if err != nil { rr := a.error(ids.Error) @@ -75,6 +75,11 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { return } + // TODO @shipperizer improve on this, see if better to stick with link headers + pagination.Next = ids.Tokens.Next + pagination.Prev = ids.Tokens.Prev + pagination.First = ids.Tokens.First + w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode( types.Response{ diff --git a/pkg/identities/handlers_test.go b/pkg/identities/handlers_test.go index cf4cdd08d..cb48fed17 100644 --- a/pkg/identities/handlers_test.go +++ b/pkg/identities/handlers_test.go @@ -7,7 +7,6 @@ import ( "bytes" "encoding/json" "fmt" - "io/ioutil" "net/http" "net/http/httptest" "reflect" @@ -19,6 +18,8 @@ import ( "github.com/canonical/identity-platform-admin-ui/internal/http/types" + "io" + kClient "github.com/ory/kratos-client-go" ) @@ -26,7 +27,7 @@ import ( //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_interfaces.go -source=./interfaces.go //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_monitor.go -source=../../internal/monitoring/interfaces.go //go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_tracing.go go.opentelemetry.io/otel/trace Tracer -//go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityApi +//go:generate mockgen -build_flags=--mod=mod -package identities -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityAPI func TestHandleListSuccess(t *testing.T) { ctrl := gomock.NewController(t) @@ -43,11 +44,20 @@ func TestHandleListSuccess(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v0/identities", nil) values := req.URL.Query() - values.Add("page", "1") values.Add("size", "100") req.URL.RawQuery = values.Encode() - mockService.EXPECT().ListIdentities(gomock.Any(), int64(1), int64(100), "").Return(&IdentityData{Identities: identities}, nil) + mockService.EXPECT().ListIdentities(gomock.Any(), int64(100), "", "").Return( + &IdentityData{ + Identities: identities, + Tokens: PaginationTokens{ + Next: "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ", + First: "eyJvZmZzZXQiOiIwIiwidiI6Mn0", + Prev: "eyJvZmZzZXQiOiItMjUwIiwidiI6Mn0", + }, + }, + nil, + ) w := httptest.NewRecorder() mux := chi.NewMux() @@ -57,7 +67,7 @@ func TestHandleListSuccess(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -113,7 +123,6 @@ func TestHandleListFailAndPropagatesKratosError(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v0/identities", nil) values := req.URL.Query() - values.Add("page", "1") values.Add("size", "100") req.URL.RawQuery = values.Encode() @@ -122,7 +131,7 @@ func TestHandleListFailAndPropagatesKratosError(t *testing.T) { gerr.SetMessage("teapot error") gerr.SetReason("teapot is broken") - mockService.EXPECT().ListIdentities(gomock.Any(), int64(1), int64(100), "").Return(&IdentityData{Identities: make([]kClient.Identity, 0), Error: gerr}, fmt.Errorf("error")) + mockService.EXPECT().ListIdentities(gomock.Any(), int64(100), "", "").Return(&IdentityData{Identities: make([]kClient.Identity, 0), Error: gerr}, fmt.Errorf("error")) w := httptest.NewRecorder() mux := chi.NewMux() @@ -132,7 +141,7 @@ func TestHandleListFailAndPropagatesKratosError(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -178,7 +187,7 @@ func TestHandleDetailSuccess(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -254,7 +263,7 @@ func TestHandleDetailFailAndPropagatesKratosError(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -304,7 +313,7 @@ func TestHandleCreateSuccess(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -385,7 +394,7 @@ func TestHandleCreateFailAndPropagatesKratosError(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -426,7 +435,7 @@ func TestHandleCreateFailBadRequest(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -457,7 +466,7 @@ func TestHandleUpdateSuccess(t *testing.T) { identity := kClient.NewIdentity(credID, "test.json", "https://test.com/test.json", map[string]string{"name": "name"}) identityBody := kClient.NewUpdateIdentityBodyWithDefaults() identityBody.SchemaId = identity.SchemaId - identityBody.SetState(kClient.IDENTITYSTATE_ACTIVE) + identityBody.SetState("active") identityBody.Traits = map[string]interface{}{"name": "name"} identityBody.AdditionalProperties = map[string]interface{}{"name": "name"} @@ -475,7 +484,7 @@ func TestHandleUpdateSuccess(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -536,7 +545,7 @@ func TestHandleUpdateFailAndPropagatesKratosError(t *testing.T) { identityBody := kClient.NewUpdateIdentityBodyWithDefaults() identityBody.SchemaId = "test.json" identityBody.Traits = map[string]interface{}{"name": "name"} - identityBody.SetState(kClient.IDENTITYSTATE_ACTIVE) + identityBody.SetState("active") identityBody.AdditionalProperties = map[string]interface{}{"name": "name"} payload, err := json.Marshal(identityBody) @@ -557,7 +566,7 @@ func TestHandleUpdateFailAndPropagatesKratosError(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -598,7 +607,7 @@ func TestHandleUpdateFailBadRequest(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -639,7 +648,7 @@ func TestHandleRemoveSuccess(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) @@ -687,7 +696,7 @@ func TestHandleRemoveFailAndPropagatesKratosError(t *testing.T) { res := w.Result() defer res.Body.Close() - data, err := ioutil.ReadAll(res.Body) + data, err := io.ReadAll(io.Reader(res.Body)) if err != nil { t.Errorf("expected error to be nil got %v", err) diff --git a/pkg/web/router.go b/pkg/web/router.go index 050dda8be..b64727330 100644 --- a/pkg/web/router.go +++ b/pkg/web/router.go @@ -65,7 +65,7 @@ func NewRouter(idpConfig *idp.Config, schemasConfig *schemas.Config, rulesConfig metrics.NewAPI(logger).RegisterEndpoints(router) identitiesAPI := identities.NewAPI( - identities.NewService(externalConfig.KratosAdmin().IdentityApi(), tracer, monitor, logger), + identities.NewService(externalConfig.KratosAdmin().IdentityAPI(), tracer, monitor, logger), logger, ) identitiesAPI.RegisterEndpoints(router) From 018774c3f5e91280f486fb48f643c00de9e8ab69 Mon Sep 17 00:00:00 2001 From: shipperizer Date: Fri, 5 Apr 2024 22:49:11 +0200 Subject: [PATCH 4/6] feat: adjust pagination for schemas endpoints closes #44 --- pkg/schemas/handlers.go | 7 +- pkg/schemas/handlers_test.go | 11 +- pkg/schemas/interfaces.go | 2 +- pkg/schemas/service.go | 51 +++++++++- pkg/schemas/service_test.go | 190 ++++++++++++++++++----------------- 5 files changed, 158 insertions(+), 103 deletions(-) diff --git a/pkg/schemas/handlers.go b/pkg/schemas/handlers.go index 9f2f9a864..d8e94ea82 100644 --- a/pkg/schemas/handlers.go +++ b/pkg/schemas/handlers.go @@ -52,7 +52,7 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { pagination := types.ParsePagination(r.URL.Query()) - schemas, err := a.service.ListSchemas(r.Context(), pagination.Page, pagination.Size) + schemas, err := a.service.ListSchemas(r.Context(), pagination.Size, pagination.PageToken) if err != nil { rr := a.error(schemas.Error) @@ -63,6 +63,11 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { return } + // TODO @shipperizer improve on this, see if better to stick with link headers + pagination.Next = schemas.Tokens.Next + pagination.Prev = schemas.Tokens.Prev + pagination.First = schemas.Tokens.First + w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode( types.Response{ diff --git a/pkg/schemas/handlers_test.go b/pkg/schemas/handlers_test.go index 5f2e3ebeb..6d590c299 100644 --- a/pkg/schemas/handlers_test.go +++ b/pkg/schemas/handlers_test.go @@ -28,7 +28,7 @@ import ( //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_monitor.go -source=../../internal/monitoring/interfaces.go //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_tracing.go go.opentelemetry.io/otel/trace Tracer //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_corev1.go k8s.io/client-go/kubernetes/typed/core/v1 CoreV1Interface,ConfigMapInterface -//go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityApi +//go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityAPI func TestHandleListSuccess(t *testing.T) { ctrl := gomock.NewController(t) @@ -102,11 +102,16 @@ func TestHandleListSuccess(t *testing.T) { Schema: v1Schema, }, }, + Tokens: PaginationTokens{ + Next: "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ", + First: "eyJvZmZzZXQiOiIwIiwidiI6Mn0", + Prev: "eyJvZmZzZXQiOiItMjUwIiwidiI6Mn0", + }, } req := httptest.NewRequest(http.MethodGet, "/api/v0/schemas", nil) - mockService.EXPECT().ListSchemas(gomock.Any(), int64(1), int64(100)).Return(&schemas, nil) + mockService.EXPECT().ListSchemas(gomock.Any(), int64(100), "").Return(&schemas, nil) w := httptest.NewRecorder() mux := chi.NewMux() @@ -161,7 +166,7 @@ func TestHandleListFails(t *testing.T) { gerr.SetMessage("teapot error") gerr.SetReason("teapot is broken") - mockService.EXPECT().ListSchemas(gomock.Any(), int64(1), int64(100)).Return(&IdentitySchemaData{IdentitySchemas: make([]kClient.IdentitySchemaContainer, 0), Error: gerr}, fmt.Errorf("error")) + mockService.EXPECT().ListSchemas(gomock.Any(), int64(100), "").Return(&IdentitySchemaData{IdentitySchemas: make([]kClient.IdentitySchemaContainer, 0), Error: gerr}, fmt.Errorf("error")) w := httptest.NewRecorder() mux := chi.NewMux() diff --git a/pkg/schemas/interfaces.go b/pkg/schemas/interfaces.go index 8d71c7522..535f85135 100644 --- a/pkg/schemas/interfaces.go +++ b/pkg/schemas/interfaces.go @@ -10,7 +10,7 @@ import ( ) type ServiceInterface interface { - ListSchemas(context.Context, int64, int64) (*IdentitySchemaData, error) + ListSchemas(context.Context, int64, string) (*IdentitySchemaData, error) GetSchema(context.Context, string) (*IdentitySchemaData, error) EditSchema(context.Context, string, *kClient.IdentitySchemaContainer) (*IdentitySchemaData, error) CreateSchema(context.Context, *kClient.IdentitySchemaContainer) (*IdentitySchemaData, error) diff --git a/pkg/schemas/service.go b/pkg/schemas/service.go index 5c27aa093..1447e78f0 100644 --- a/pkg/schemas/service.go +++ b/pkg/schemas/service.go @@ -9,9 +9,11 @@ import ( "fmt" "io" "net/http" + "net/url" "github.com/google/uuid" kClient "github.com/ory/kratos-client-go" + "github.com/tomnomnom/linkheader" "go.opentelemetry.io/otel/trace" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" coreV1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -26,11 +28,19 @@ type Config struct { Name string Namespace string K8s coreV1.CoreV1Interface - Kratos kClient.IdentityApi + Kratos kClient.IdentityAPI +} + +// TODO @shipperizer worth offloading to a different place as it's going to be reused +type PaginationTokens struct { + First string + Prev string + Next string } type IdentitySchemaData struct { IdentitySchemas []kClient.IdentitySchemaContainer + Tokens PaginationTokens Error *kClient.GenericError } @@ -48,15 +58,45 @@ type Service struct { cmNamespace string k8s coreV1.CoreV1Interface - kratos kClient.IdentityApi + kratos kClient.IdentityAPI tracer trace.Tracer monitor monitoring.MonitorInterface logger logging.LoggerInterface } +func (s *Service) parseLinkURL(linkURL string) string { + u, err := url.Parse(linkURL) + + if err != nil { + s.logger.Errorf("failed to parse link header successfully: %s", err) + return "" + } + + return u.Query().Get("page_token") +} + +func (s *Service) parsePagination(r *http.Response) PaginationTokens { + links := linkheader.Parse(r.Header.Get("Link")) + + pagination := PaginationTokens{} + + for _, link := range links { + switch link.Rel { + case "first": + pagination.First = s.parseLinkURL(link.URL) + case "next": + pagination.Next = s.parseLinkURL(link.URL) + case "prev": + pagination.Prev = s.parseLinkURL(link.URL) + } + } + + return pagination +} + func (s *Service) parseError(ctx context.Context, r *http.Response) *kClient.GenericError { - ctx, span := s.tracer.Start(ctx, "schemas.Service.parseError") + _, span := s.tracer.Start(ctx, "schemas.Service.parseError") defer span.End() gerr := KratosError{Error: kClient.NewGenericErrorWithDefaults()} @@ -72,12 +112,12 @@ func (s *Service) parseError(ctx context.Context, r *http.Response) *kClient.Gen return gerr.Error } -func (s *Service) ListSchemas(ctx context.Context, page, size int64) (*IdentitySchemaData, error) { +func (s *Service) ListSchemas(ctx context.Context, size int64, token string) (*IdentitySchemaData, error) { ctx, span := s.tracer.Start(ctx, "schemas.Service.ListSchemas") defer span.End() schemas, rr, err := s.kratos.ListIdentitySchemasExecute( - s.kratos.ListIdentitySchemas(ctx).Page(page).PerPage(size), + s.kratos.ListIdentitySchemas(ctx).PageToken(token).PageSize(size), ) data := new(IdentitySchemaData) @@ -87,6 +127,7 @@ func (s *Service) ListSchemas(ctx context.Context, page, size int64) (*IdentityS data.Error = s.parseError(ctx, rr) } + data.Tokens = s.parsePagination(rr) data.IdentitySchemas = schemas // TODO @shipperizer check if schemas is defaulting to empty slice inside kratos-client diff --git a/pkg/schemas/service_test.go b/pkg/schemas/service_test.go index 2175697c6..5a350de10 100644 --- a/pkg/schemas/service_test.go +++ b/pkg/schemas/service_test.go @@ -24,7 +24,7 @@ import ( //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_monitor.go -source=../../internal/monitoring/interfaces.go //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_tracing.go go.opentelemetry.io/otel/trace Tracer //go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_corev1.go k8s.io/client-go/kubernetes/typed/core/v1 CoreV1Interface,ConfigMapInterface -//go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityApi +//go:generate mockgen -build_flags=--mod=mod -package schemas -destination ./mock_kratos.go github.com/ory/kratos-client-go IdentityAPI func TestListSchemasSuccess(t *testing.T) { ctrl := gomock.NewController(t) @@ -34,12 +34,12 @@ func TestListSchemasSuccess(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -108,28 +108,32 @@ func TestListSchemasSuccess(t *testing.T) { }, } - identitySchemaRequest := kClient.IdentityApiListIdentitySchemasRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIListIdentitySchemasRequest{ + ApiService: mockKratosIdentityAPI, } mockTracer.EXPECT().Start(ctx, "schemas.Service.ListSchemas").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { + mockKratosIdentityAPI.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times - if page := (*int64)(reflect.ValueOf(r).FieldByName("page").UnsafePointer()); *page != 1 { - t.Fatalf("expected page as 1, got %v", *page) + if pageToken := (*string)(reflect.ValueOf(r).FieldByName("pageToken").UnsafePointer()); *pageToken != "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ" { + t.Fatalf("expected pageToken as eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ, got %v", *pageToken) } - if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("perPage").UnsafePointer()); *pageSize != 10 { + if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("pageSize").UnsafePointer()); *pageSize != 10 { t.Fatalf("expected page size as 10, got %v", *pageSize) } - return schemas, new(http.Response), nil + rr := new(http.Response) + rr.Header = make(http.Header) + rr.Header.Set("Link", `; rel="first",; rel="next",; rel="prev`) + + return schemas, rr, nil }, ) - is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 1, 10) + is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 10, "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ") if !reflect.DeepEqual(is.IdentitySchemas, schemas) { t.Fatalf("expected schemas to be %v not %v", schemas, is.IdentitySchemas) @@ -149,34 +153,34 @@ func TestListSchemasFails(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" - identitySchemaRequest := kClient.IdentityApiListIdentitySchemasRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIListIdentitySchemasRequest{ + ApiService: mockKratosIdentityAPI, } schemas := make([]kClient.IdentitySchemaContainer, 0) - mockLogger.EXPECT().Error(gomock.Any()).Times(1) + mockLogger.EXPECT().Error(gomock.Any()).AnyTimes() mockTracer.EXPECT().Start(ctx, "schemas.Service.ListSchemas").Times(1).Return(ctx, trace.SpanFromContext(ctx)) mockTracer.EXPECT().Start(ctx, "schemas.Service.parseError").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { + mockKratosIdentityAPI.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times - if page := (*int64)(reflect.ValueOf(r).FieldByName("page").UnsafePointer()); *page != 1 { - t.Fatalf("expected page as 1, got %v", *page) + if pageToken := (*string)(reflect.ValueOf(r).FieldByName("pageToken").UnsafePointer()); *pageToken != "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ" { + t.Fatalf("expected pageToken as eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ, got %v", *pageToken) } - if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("perPage").UnsafePointer()); *pageSize != 10 { + if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("pageSize").UnsafePointer()); *pageSize != 10 { t.Fatalf("expected page size as 10, got %v", *pageSize) } @@ -202,7 +206,7 @@ func TestListSchemasFails(t *testing.T) { return schemas, rr.Result(), fmt.Errorf("error") }, ) - is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 1, 10) + is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 10, "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ") if is.Error == nil { t.Fatal("expected ids.Error to be not nil") @@ -225,39 +229,39 @@ func TestListSchemasSuccessButEmpty(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" schemas := []kClient.IdentitySchemaContainer{} - identitySchemaRequest := kClient.IdentityApiListIdentitySchemasRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIListIdentitySchemasRequest{ + ApiService: mockKratosIdentityAPI, } mockTracer.EXPECT().Start(ctx, "schemas.Service.ListSchemas").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { + mockKratosIdentityAPI.EXPECT().ListIdentitySchemas(ctx).Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().ListIdentitySchemasExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIListIdentitySchemasRequest) ([]kClient.IdentitySchemaContainer, *http.Response, error) { // use reflect as attributes are private, also are pointers so need to cast it multiple times - if page := (*int64)(reflect.ValueOf(r).FieldByName("page").UnsafePointer()); *page != 1 { - t.Fatalf("expected page as 1, got %v", *page) + if pageToken := (*string)(reflect.ValueOf(r).FieldByName("pageToken").UnsafePointer()); *pageToken != "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ" { + t.Fatalf("expected pageToken as eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ, got %v", *pageToken) } - if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("perPage").UnsafePointer()); *pageSize != 10 { + if pageSize := (*int64)(reflect.ValueOf(r).FieldByName("pageSize").UnsafePointer()); *pageSize != 10 { t.Fatalf("expected page size as 10, got %v", *pageSize) } return schemas, new(http.Response), nil }, ) - is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 1, 10) + is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).ListSchemas(ctx, 10, "eyJvZmZzZXQiOiIyNTAiLCJ2IjoyfQ") if !reflect.DeepEqual(is.IdentitySchemas, schemas) { t.Fatalf("expected schemas to be %v not %v", schemas, is.IdentitySchemas) @@ -277,17 +281,17 @@ func TestGetSchemaSuccess(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" - identitySchemaRequest := kClient.IdentityApiGetIdentitySchemaRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIGetIdentitySchemaRequest{ + ApiService: mockKratosIdentityAPI, } v0Schema := map[string]interface{}{ @@ -323,8 +327,8 @@ func TestGetSchemaSuccess(t *testing.T) { Schema: v0Schema, } mockTracer.EXPECT().Start(ctx, "schemas.Service.GetSchema").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().GetIdentitySchema(ctx, v0ID).Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).Return(schema.Schema, new(http.Response), nil) + mockKratosIdentityAPI.EXPECT().GetIdentitySchema(ctx, v0ID).Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).Return(schema.Schema, new(http.Response), nil) is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).GetSchema(ctx, v0ID) @@ -344,22 +348,22 @@ func TestGetSchemaSuccessButEmpty(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" - identitySchemaRequest := kClient.IdentityApiGetIdentitySchemaRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIGetIdentitySchemaRequest{ + ApiService: mockKratosIdentityAPI, } mockTracer.EXPECT().Start(ctx, "schemas.Service.GetSchema").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().GetIdentitySchema(ctx, "test").Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).Return(nil, new(http.Response), nil) + mockKratosIdentityAPI.EXPECT().GetIdentitySchema(ctx, "test").Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).Return(nil, new(http.Response), nil) is, err := NewService(cfg, mockTracer, mockMonitor, mockLogger).GetSchema(ctx, "test") @@ -379,25 +383,25 @@ func TestGetSchemaFails(t *testing.T) { mockTracer := NewMockTracer(ctrl) mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" - identitySchemaRequest := kClient.IdentityApiGetIdentitySchemaRequest{ - ApiService: mockKratosIdentityApi, + identitySchemaRequest := kClient.IdentityAPIGetIdentitySchemaRequest{ + ApiService: mockKratosIdentityAPI, } mockLogger.EXPECT().Error(gomock.Any()).Times(1) mockTracer.EXPECT().Start(ctx, "schemas.Service.GetSchema").Times(1).Return(ctx, trace.SpanFromContext(ctx)) mockTracer.EXPECT().Start(ctx, "schemas.Service.parseError").Times(1).Return(ctx, trace.SpanFromContext(ctx)) - mockKratosIdentityApi.EXPECT().GetIdentitySchema(ctx, "fake").Times(1).Return(identitySchemaRequest) - mockKratosIdentityApi.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).DoAndReturn( - func(r kClient.IdentityApiGetIdentitySchemaRequest) (map[string]interface{}, *http.Response, error) { + mockKratosIdentityAPI.EXPECT().GetIdentitySchema(ctx, "fake").Times(1).Return(identitySchemaRequest) + mockKratosIdentityAPI.EXPECT().GetIdentitySchemaExecute(gomock.Any()).Times(1).DoAndReturn( + func(r kClient.IdentityAPIGetIdentitySchemaRequest) (map[string]interface{}, *http.Response, error) { rr := httptest.NewRecorder() rr.Header().Set("Content-Type", "application/json") rr.WriteHeader(http.StatusNotFound) @@ -449,12 +453,12 @@ func TestEdiSchemaSuccess(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -563,12 +567,12 @@ func TestEditSchemaNotfound(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -677,12 +681,12 @@ func TestEditSchemaFails(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -750,12 +754,12 @@ func TestCreateSchemaSuccess(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -842,12 +846,12 @@ func TestCreateSchemaSuccessWithEmptyConfigmap(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -933,12 +937,12 @@ func TestCreateSchemaSuccessIfIDIsMissing(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1025,12 +1029,12 @@ func TestCreateSchemaFailsConflict(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1096,12 +1100,12 @@ func TestCreateSchemaFails(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1183,12 +1187,12 @@ func TestDeleteSchemaSuccess(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1294,12 +1298,12 @@ func TestDeleteSchemaNotFound(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1395,12 +1399,12 @@ func TestDeleteSchemaFailsIfDefault(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1498,12 +1502,12 @@ func TestDeleteSchemaFails(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1604,12 +1608,12 @@ func TestGetDefaultSchemaSuccess(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1645,12 +1649,12 @@ func TestGetDefaultSchemaNoDefaultSchema(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1688,12 +1692,12 @@ func TestGetDefaultSchemaFails(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1722,12 +1726,12 @@ func TestUpdateDefaultSchemaSuccess(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1798,12 +1802,12 @@ func TestUpdateDefaultSchemaIdNotFound(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1841,12 +1845,12 @@ func TestUpdateDefaultSchemaIdIsDefaultKey(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" @@ -1884,12 +1888,12 @@ func TestUpdateDefaultSchemaFails(t *testing.T) { mockMonitor := NewMockMonitorInterface(ctrl) mockCoreV1 := NewMockCoreV1Interface(ctrl) mockConfigMapV1 := NewMockConfigMapInterface(ctrl) - mockKratosIdentityApi := NewMockIdentityApi(ctrl) + mockKratosIdentityAPI := NewMockIdentityAPI(ctrl) ctx := context.Background() cfg := new(Config) cfg.K8s = mockCoreV1 - cfg.Kratos = mockKratosIdentityApi + cfg.Kratos = mockKratosIdentityAPI cfg.Name = "schemas" cfg.Namespace = "default" From 93e9d9b76bd19e8458030612794737ff95434cdd Mon Sep 17 00:00:00 2001 From: shipperizer Date: Mon, 8 Apr 2024 10:54:02 +0200 Subject: [PATCH 5/6] fix: adjust page offset for oathkeeper apis --- pkg/rules/handlers.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/rules/handlers.go b/pkg/rules/handlers.go index 8c9519208..b8ec1ac28 100644 --- a/pkg/rules/handlers.go +++ b/pkg/rules/handlers.go @@ -50,6 +50,10 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { pagination := types.ParsePagination(r.URL.Query()) + if pagination.Page < 1 { + pagination.Page = 1 + } + rules, err := a.service.ListRules(r.Context(), pagination.Page, pagination.Size) if err != nil { From baf20e7c57ae3609645339c94c5eca272262894f Mon Sep 17 00:00:00 2001 From: shipperizer Date: Mon, 8 Apr 2024 11:14:26 +0200 Subject: [PATCH 6/6] feat: WIP use page token in rules API listRules closes #268 --- pkg/rules/handlers.go | 67 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/pkg/rules/handlers.go b/pkg/rules/handlers.go index b8ec1ac28..e1e9283ff 100644 --- a/pkg/rules/handlers.go +++ b/pkg/rules/handlers.go @@ -4,10 +4,12 @@ package rules import ( + "encoding/base64" "encoding/json" "fmt" "io" "net/http" + "strconv" "github.com/go-playground/validator/v10" @@ -19,6 +21,10 @@ import ( oathkeeper "github.com/ory/oathkeeper-client-go" ) +type PageToken struct { + Offset string `json:"offset" validate:"required"` +} + type API struct { service ServiceInterface validator *validator.Validate @@ -45,16 +51,65 @@ func (a *API) validatingFunc(r *http.Request) validator.ValidationErrors { return nil } +func (a *API) pageDecode(pageToken string, size int) int { + if pageToken == "" { + return 0 + } + + pt := new(PageToken) + + rawPt, err := base64.StdEncoding.DecodeString(pageToken) + if err != nil { + a.logger.Warnf("bad page token encoding, defaulting to an empty one: %s", err) + + return 0 + } + + if err := json.Unmarshal(rawPt, pt); err != nil { + a.logger.Warnf("bad page token format, defaulting to an empty one: %s", err) + + return 0 + } + + if err := a.validator.Struct(c); err != nil { + a.logger.Warnf("invalid page token, defaulting ot an empty one: %s", err) + + return 0 + } + + offset, err := strconv.Atoi(pt.Offset) + + if err != nil { + a.logger.Warnf("invalid offset, default to 0: %s", err) + + return 0 + } + + return offset / size +} + +func (a *API) pageTokenEncode(page, size int) string { + pt := new(PageToken) + pt.Offset = fmt.Sprintf("%v", page*size) + + token, err := json.Marshal(pt) + if err != nil { + a.logger.Warnf("bad page token encoding, defaulting to an empty one: %s", err) + + return "" + } + + return base64.StdEncoding.EncodeToString(token) +} + func (a *API) handleList(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") pagination := types.ParsePagination(r.URL.Query()) - if pagination.Page < 1 { - pagination.Page = 1 - } + page := a.pageDecode(pagination.PageToken, int(pagination.Size)) - rules, err := a.service.ListRules(r.Context(), pagination.Page, pagination.Size) + rules, err := a.service.ListRules(r.Context(), int64(page), pagination.Size) if err != nil { @@ -75,6 +130,10 @@ func (a *API) handleList(w http.ResponseWriter, r *http.Request) { Data: rules, Message: "List of rules", Status: http.StatusOK, + Meta: &types.Pagination{ + Next: a.pageTokenEncode(page+1, int(pagination.Size)), + Size: pagination.Size, + }, }, ) }