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

Prevent deprecation of AppServerOrSAMLIdPServiceProvider from being a breaking change #41047

Merged
merged 2 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/auth/assist/assistv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func assembleSearchResponse(ctx context.Context, a *Service, documents []*ai.Doc
resources = append(resources, resource)
}

paginated, err := services.MakePaginatedResources(types.KindUnifiedResource, resources, nil /* requestable map */)
paginated, err := services.MakePaginatedResources(ctx, types.KindUnifiedResource, resources, nil /* requestable map */)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,7 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
}
}

paginatedResources, err := services.MakePaginatedResources(types.KindUnifiedResource, unifiedResources, resourceAccess.requestableMap)
paginatedResources, err := services.MakePaginatedResources(ctx, types.KindUnifiedResource, unifiedResources, resourceAccess.requestableMap)
if err != nil {
return nil, trace.Wrap(err, "making paginated unified resources")
}
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/grpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4310,7 +4310,7 @@ func (g *GRPCServer) ListResources(ctx context.Context, req *authpb.ListResource
return nil, trace.Wrap(err)
}

paginatedResources, err := services.MakePaginatedResources(req.ResourceType, resp.Resources, nil /* requestable map */)
paginatedResources, err := services.MakePaginatedResources(ctx, req.ResourceType, resp.Resources, nil /* requestable map */)
if err != nil {
return nil, trace.Wrap(err, "making paginated resources")
}
Expand Down
40 changes: 36 additions & 4 deletions lib/services/unified_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sync"
"time"

"github.com/coreos/go-semver/semver"
"github.com/google/btree"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
Expand All @@ -33,6 +34,7 @@ import (
"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/metadata"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/utils"
Expand Down Expand Up @@ -742,7 +744,7 @@ const (
)

// MakePaginatedResource converts a resource into a paginated proto representation.
func MakePaginatedResource(requestType string, r types.ResourceWithLabels, requiresRequest bool) (*proto.PaginatedResource, error) {
func MakePaginatedResource(ctx context.Context, requestType string, r types.ResourceWithLabels, requiresRequest bool) (*proto.PaginatedResource, error) {
var protoResource *proto.PaginatedResource
resourceKind := requestType
if requestType == types.KindUnifiedResource {
Expand Down Expand Up @@ -850,7 +852,37 @@ func MakePaginatedResource(requestType string, r types.ResourceWithLabels, requi
return nil, trace.BadParameter("%s has invalid type %T", resourceKind, resource)
}

protoResource = &proto.PaginatedResource{Resource: &proto.PaginatedResource_SAMLIdPServiceProvider{SAMLIdPServiceProvider: serviceProvider}, RequiresRequest: requiresRequest}
// TODO(gzdunek): DELETE IN 17.0
// This is needed to maintain backward compatibility between v16 server and v15 client.
clientVersion, versionExists := metadata.ClientVersionFromContext(ctx)
isClientNotSupportingSAMLIdPServiceProviderResource := false
if versionExists {
version, err := semver.NewVersion(clientVersion)
if err == nil && version.Major < 16 {
isClientNotSupportingSAMLIdPServiceProviderResource = true
}
}

if isClientNotSupportingSAMLIdPServiceProviderResource {
protoResource = &proto.PaginatedResource{
Resource: &proto.PaginatedResource_AppServerOrSAMLIdPServiceProvider{
//nolint:staticcheck // SA1019. TODO(gzdunek): DELETE IN 17.0
AppServerOrSAMLIdPServiceProvider: &types.AppServerOrSAMLIdPServiceProviderV1{
Resource: &types.AppServerOrSAMLIdPServiceProviderV1_SAMLIdPServiceProvider{
SAMLIdPServiceProvider: serviceProvider,
},
},
},
RequiresRequest: requiresRequest,
}
} else {
protoResource = &proto.PaginatedResource{
Resource: &proto.PaginatedResource_SAMLIdPServiceProvider{
SAMLIdPServiceProvider: serviceProvider,
},
RequiresRequest: requiresRequest,
}
}
default:
return nil, trace.NotImplemented("resource type %s doesn't support pagination", resource.GetKind())
}
Expand All @@ -859,11 +891,11 @@ func MakePaginatedResource(requestType string, r types.ResourceWithLabels, requi
}

// MakePaginatedResources converts a list of resources into a list of paginated proto representations.
func MakePaginatedResources(requestType string, resources []types.ResourceWithLabels, requestableMap map[string]struct{}) ([]*proto.PaginatedResource, error) {
func MakePaginatedResources(ctx context.Context, requestType string, resources []types.ResourceWithLabels, requestableMap map[string]struct{}) ([]*proto.PaginatedResource, error) {
paginatedResources := make([]*proto.PaginatedResource, 0, len(resources))
for _, r := range resources {
_, requiresRequest := requestableMap[r.GetName()]
protoResource, err := MakePaginatedResource(requestType, r, requiresRequest)
protoResource, err := MakePaginatedResource(ctx, requestType, r, requiresRequest)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
44 changes: 44 additions & 0 deletions lib/services/unified_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/defaults"
apimetadata "github.com/gravitational/teleport/api/metadata"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/header"
"github.com/gravitational/teleport/lib/backend/memory"
Expand Down Expand Up @@ -382,6 +385,47 @@ func TestUnifiedResourceWatcher_DeleteEvent(t *testing.T) {
}, 5*time.Second, 10*time.Millisecond, "Timed out waiting for unified resources to be deleted")
}

func Test_PaginatedResourcesSAMLIdPServiceProviderCompatibility(t *testing.T) {
samlApp, err := types.NewSAMLIdPServiceProvider(
types.Metadata{
Name: "sp1",
},
types.SAMLIdPServiceProviderSpecV1{
EntityDescriptor: newTestEntityDescriptor("sp1"),
EntityID: "sp1",
},
)
require.NoError(t, err)

// for a v15 client, expect AppServerOrSAMLIdPServiceProvider response
v15ctx := metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{apimetadata.VersionKey: "15.0.0"}))
v15response, err := services.MakePaginatedResources(v15ctx, types.KindUnifiedResource, []types.ResourceWithLabels{samlApp}, map[string]struct{}{})
require.NoError(t, err)
require.Equal(t,
&proto.PaginatedResource{
Resource: &proto.PaginatedResource_AppServerOrSAMLIdPServiceProvider{
//nolint:staticcheck // SA1019. TODO(gzdunek): DELETE IN 17.0 (with the entire test)
AppServerOrSAMLIdPServiceProvider: &types.AppServerOrSAMLIdPServiceProviderV1{
Resource: &types.AppServerOrSAMLIdPServiceProviderV1_SAMLIdPServiceProvider{
SAMLIdPServiceProvider: samlApp.(*types.SAMLIdPServiceProviderV1),
},
}}},
v15response[0],
)

// for a v16 client, expect SAMLIdPServiceProvider response
v16ctx := metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{apimetadata.VersionKey: "16.0.0"}))
v16response, err := services.MakePaginatedResources(v16ctx, types.KindUnifiedResource, []types.ResourceWithLabels{samlApp}, map[string]struct{}{})
require.NoError(t, err)
require.Equal(t,
&proto.PaginatedResource{
Resource: &proto.PaginatedResource_SAMLIdPServiceProvider{
SAMLIdPServiceProvider: samlApp.(*types.SAMLIdPServiceProviderV1),
}},
v16response[0],
)
}

func newTestEntityDescriptor(entityID string) string {
return fmt.Sprintf(testEntityDescriptor, entityID)
}
Expand Down
Loading