-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add V4 Roles #7118
Merged
Merged
Add V4 Roles #7118
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
722558d
send client version in grpc metadata
nklaassen 77c704a
add v4 roles
nklaassen 8529d31
addressing review comments
nklaassen e71b843
revert version change to implicit role and RoleForUser
nklaassen 02bbc02
add mutex for globals in metadata.go
nklaassen 7c425e9
Merge branch 'master' into nklaassen/roles-v4
nklaassen 9ec2883
s/VersionFromContext/ClientVersionFromContext/
nklaassen 0152f85
avoid mutable globals in metadata package
nklaassen 064d90e
Merge branch 'master' into nklaassen/roles-v4
nklaassen ec38188
Merge branch 'master' into nklaassen/roles-v4
nklaassen 1a54414
Merge branch 'master' into nklaassen/roles-v4
nklaassen e288bcd
lint fix
nklaassen f1a463f
mention v4 roles in docs
nklaassen 1eca38f
docs update
nklaassen fecee1b
Merge branch 'master' into nklaassen/roles-v4
nklaassen 6dc90b3
copy role when downgrading instead of mutating
nklaassen b7e12b0
lint fix
nklaassen 37e4337
update min supported version to 6.2.4
nklaassen acb1a1c
Merge branch 'master' into nklaassen/roles-v4
nklaassen 2ffebb1
Rename RoleV3 to RoleV4 (#7132)
nklaassen d6187f4
cosmetic edits
nklaassen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
/* | ||
Copyright 2021 Gravitational, Inc. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package metadata | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/gravitational/teleport/api/constants" | ||
"google.golang.org/grpc" | ||
"google.golang.org/grpc/metadata" | ||
) | ||
|
||
const ( | ||
VersionKey = "version" | ||
) | ||
|
||
// defaultMetadata returns the default metadata which will be added to all outgoing calls. | ||
func defaultMetadata() map[string]string { | ||
return map[string]string{ | ||
VersionKey: constants.Version, | ||
} | ||
} | ||
|
||
// AddMetadataToContext returns a new context copied from ctx with the given | ||
// raw metadata added. Metadata already set on the given context for any key | ||
// will not be overridden, but new key/value pairs will always be added. | ||
func AddMetadataToContext(ctx context.Context, raw map[string]string) context.Context { | ||
md := metadata.New(raw) | ||
if existingMd, ok := metadata.FromOutgoingContext(ctx); ok { | ||
for key, vals := range existingMd { | ||
md.Set(key, vals...) | ||
} | ||
} | ||
return metadata.NewOutgoingContext(ctx, md) | ||
} | ||
|
||
// DisableInterceptors can be set on the client context with context.WithValue(ctx, DisableInterceptors{}, struct{}{}) | ||
// to stop the client interceptors from adding any metadata to the context (useful for testing). | ||
type DisableInterceptors struct{} | ||
|
||
// StreamClientInterceptor intercepts a GRPC client stream call and adds | ||
// default metadata to the context. | ||
func StreamClientInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { | ||
if disable := ctx.Value(DisableInterceptors{}); disable == nil { | ||
ctx = AddMetadataToContext(ctx, defaultMetadata()) | ||
} | ||
return streamer(ctx, desc, cc, method, opts...) | ||
} | ||
|
||
// UnaryClientInterceptor intercepts a GRPC client unary call and adds default | ||
// metadata to the context. | ||
func UnaryClientInterceptor(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { | ||
if disable := ctx.Value(DisableInterceptors{}); disable == nil { | ||
ctx = AddMetadataToContext(ctx, defaultMetadata()) | ||
} | ||
return invoker(ctx, method, req, reply, cc, opts...) | ||
} | ||
|
||
// ClientVersionFromContext can be called from a GRPC server method to return | ||
// the client version that was added to the GRPC metadata by | ||
// StreamClientInterceptor or UnaryClientInterceptor on the client. | ||
func ClientVersionFromContext(ctx context.Context) (string, bool) { | ||
md, ok := metadata.FromIncomingContext(ctx) | ||
if !ok { | ||
return "", false | ||
} | ||
versionList := md.Get(VersionKey) | ||
if len(versionList) != 1 { | ||
return "", false | ||
} | ||
return versionList[0], true | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ import ( | |
"github.com/gravitational/teleport" | ||
"github.com/gravitational/teleport/api/client/proto" | ||
"github.com/gravitational/teleport/api/constants" | ||
"github.com/gravitational/teleport/api/metadata" | ||
"github.com/gravitational/teleport/api/types" | ||
apievents "github.com/gravitational/teleport/api/types/events" | ||
"github.com/gravitational/teleport/lib/auth/u2f" | ||
|
@@ -35,6 +36,7 @@ import ( | |
"github.com/gravitational/teleport/lib/session" | ||
"github.com/gravitational/teleport/lib/utils" | ||
|
||
"github.com/coreos/go-semver/semver" | ||
"github.com/golang/protobuf/ptypes/empty" | ||
"github.com/gravitational/trace" | ||
"github.com/gravitational/trace/trail" | ||
|
@@ -293,7 +295,7 @@ func (g *GRPCServer) WatchEvents(watch *proto.Watch, stream proto.AuthService_Wa | |
case <-watcher.Done(): | ||
return trail.ToGRPC(watcher.Error()) | ||
case event := <-watcher.Events(): | ||
out, err := eventToGRPC(event) | ||
out, err := eventToGRPC(stream.Context(), event) | ||
if err != nil { | ||
return trail.ToGRPC(err) | ||
} | ||
|
@@ -305,7 +307,7 @@ func (g *GRPCServer) WatchEvents(watch *proto.Watch, stream proto.AuthService_Wa | |
} | ||
|
||
// eventToGRPC converts a types.Event to an proto.Event | ||
func eventToGRPC(in types.Event) (*proto.Event, error) { | ||
func eventToGRPC(ctx context.Context, in types.Event) (*proto.Event, error) { | ||
eventType, err := eventTypeToGRPC(in.Type) | ||
if err != nil { | ||
return nil, trace.Wrap(err) | ||
|
@@ -346,6 +348,9 @@ func eventToGRPC(in types.Event) (*proto.Event, error) { | |
User: r, | ||
} | ||
case *types.RoleV3: | ||
if err = downgradeRole(ctx, r); err != nil { | ||
return nil, trace.Wrap(err) | ||
} | ||
out.Resource = &proto.Event_Role{ | ||
Role: r, | ||
} | ||
|
@@ -1368,6 +1373,28 @@ func (g *GRPCServer) DeleteAllKubeServices(ctx context.Context, req *proto.Delet | |
return &empty.Empty{}, nil | ||
} | ||
|
||
// downgradeRole tests the client version passed through the GRPC metadata, and | ||
// downgrades the given role to V3 in-place if V4 roles are not known to be | ||
// supported (client version is unknown or < 6.3). | ||
func downgradeRole(ctx context.Context, role *types.RoleV3) error { | ||
var clientVersion *semver.Version | ||
clientVersionString, ok := metadata.ClientVersionFromContext(ctx) | ||
if ok { | ||
var err error | ||
clientVersion, err = semver.NewVersion(clientVersionString) | ||
if err != nil { | ||
return trace.BadParameter("unrecognized client version: %s is not a valid semver", clientVersionString) | ||
} | ||
} | ||
|
||
minSupportedVersionForV4Roles := semver.New("6.3.0-aa") // "aa" is included so that this compares before v6.3.0-alpha | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will need to update this to the first version which will support V4 roles before merging |
||
if clientVersion == nil || clientVersion.LessThan(*minSupportedVersionForV4Roles) { | ||
log.Debugf(`Client version "%s" is unknown or less than 6.3.0, converting role to v3`, clientVersionString) | ||
return trace.Wrap(services.DowngradeRoleToV3(role)) | ||
} | ||
return nil | ||
} | ||
|
||
// GetRole retrieves a role by name. | ||
func (g *GRPCServer) GetRole(ctx context.Context, req *proto.GetRoleRequest) (*types.RoleV3, error) { | ||
auth, err := g.authenticate(ctx) | ||
|
@@ -1382,6 +1409,9 @@ func (g *GRPCServer) GetRole(ctx context.Context, req *proto.GetRoleRequest) (*t | |
if !ok { | ||
return nil, trail.ToGRPC(trace.Errorf("encountered unexpected role type")) | ||
} | ||
if err = downgradeRole(ctx, roleV3); err != nil { | ||
return nil, trail.ToGRPC(err) | ||
} | ||
return roleV3, nil | ||
} | ||
|
||
|
@@ -1401,6 +1431,9 @@ func (g *GRPCServer) GetRoles(ctx context.Context, _ *empty.Empty) (*proto.GetRo | |
if !ok { | ||
return nil, trail.ToGRPC(trace.BadParameter("unexpected type %T", r)) | ||
} | ||
if err = downgradeRole(ctx, role); err != nil { | ||
return nil, trail.ToGRPC(err) | ||
} | ||
rolesV3 = append(rolesV3, role) | ||
} | ||
return &proto.GetRolesResponse{ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have version in
version.go
file. Is this in addition to that? Does this mean we'd need to bump version in 3 places now when doing releases (Makefile, version.go and here)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
teleport/api/
doesn't depend onteleport/
which is why I can't really use the value fromversion.go
. It sounds like @Joerger is working on something so thatapi/
will have access to the generated client version, the suggestion was to copy this here for now and let him fill in the generated value when his changes come in. I'm not sure how quickly those changes are coming or if we should put in something to get the generated value for this PR @Joerger ? fyi I think we want this going out in the 6.2 trainThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the PR for the client version generation - #7157
If this PR is merged before that one, I'll update it to use the new generated version in #7157