Skip to content

DiscoveryConfig: init service and add resource to tctl #32399

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.

Sign up for GitHub

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

Merged
marcoandredinis merged 4 commits into from
Oct 11, 2023
Merged

DiscoveryConfig: init service and add resource to tctl #32399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d269bce
DiscoveryConfig: init service and add it to `tctl`
marcoandredinis Sep 28, 2023
c5df96d
add revision
marcoandredinis Sep 29, 2023
3ddd28d
add upsert method
marcoandredinis Sep 29, 2023
972995d
improve tctl -f command
marcoandredinis Oct 2, 2023
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
12 changes: 12 additions & 0 deletions api/client/discoveryconfig/discoveryconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ func (c *Client) UpdateDiscoveryConfig(ctx context.Context, discoveryConfig *dis
return dc, trace.Wrap(err)
}

// UpsertDiscoveryConfig creates or updates a DiscoveryConfig.
func (c *Client) UpsertDiscoveryConfig(ctx context.Context, discoveryConfig *discoveryconfig.DiscoveryConfig) (*discoveryconfig.DiscoveryConfig, error) {
resp, err := c.grpcClient.UpsertDiscoveryConfig(ctx, &discoveryconfigv1.UpsertDiscoveryConfigRequest{
DiscoveryConfig: conv.ToProto(discoveryConfig),
})
if err != nil {
return nil, trace.Wrap(err)
}
dc, err := conv.FromProto(resp)
return dc, trace.Wrap(err)
}

// DeleteDiscoveryConfig removes the specified DiscoveryConfig resource.
func (c *Client) DeleteDiscoveryConfig(ctx context.Context, name string) error {
_, err := c.grpcClient.DeleteDiscoveryConfig(ctx, &discoveryconfigv1.DeleteDiscoveryConfigRequest{
Expand Down
12 changes: 12 additions & 0 deletions api/client/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/accesslist"
accesslistv1conv "github.com/gravitational/teleport/api/types/accesslist/convert/v1"
"github.com/gravitational/teleport/api/types/discoveryconfig"
discoveryconfigv1conv "github.com/gravitational/teleport/api/types/discoveryconfig/convert/v1"
"github.com/gravitational/teleport/api/types/userloginstate"
userloginstatev1conv "github.com/gravitational/teleport/api/types/userloginstate/convert/v1"
)
Expand Down Expand Up @@ -226,6 +228,10 @@ func EventToGRPC(in types.Event) (*proto.Event, error) {
out.Resource = &proto.Event_AccessListMember{
AccessListMember: accesslistv1conv.ToMemberProto(r),
}
case *discoveryconfig.DiscoveryConfig:
out.Resource = &proto.Event_DiscoveryConfig{
DiscoveryConfig: discoveryconfigv1conv.ToProto(r),
}
default:
return nil, trace.BadParameter("resource type %T is not supported", in.Resource)
}
Expand Down Expand Up @@ -399,6 +405,12 @@ func EventFromGRPC(in *proto.Event) (*types.Event, error) {
return nil, trace.Wrap(err)
}
return &out, nil
} else if r := in.GetDiscoveryConfig(); r != nil {
out.Resource, err = discoveryconfigv1conv.FromProto(r)
if err != nil {
return nil, trace.Wrap(err)
}
return &out, nil
} else {
return nil, trace.BadParameter("received unsupported resource %T", in.Resource)
}
Expand Down
265 changes: 173 additions & 92 deletions api/gen/proto/go/teleport/discoveryconfig/v1/discoveryconfig_service.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions api/gen/proto/go/teleport/discoveryconfig/v1/discoveryconfig_service_grpc.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions api/proto/teleport/discoveryconfig/v1/discoveryconfig_service.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ service DiscoveryConfigService {
// UpdateDiscoveryConfig updates an existing DiscoveryConfig resource.
rpc UpdateDiscoveryConfig(UpdateDiscoveryConfigRequest) returns (DiscoveryConfig);

// UpsertDiscoveryConfig creates or updates a DiscoveryConfig resource.
rpc UpsertDiscoveryConfig(UpsertDiscoveryConfigRequest) returns (DiscoveryConfig);
Copy link
Copy Markdown
Contributor

@tigrato tigrato Oct 11, 2023

Choose a reason for hiding this comment

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

Will this require optimistic locking?

Copy link
Copy Markdown
Contributor Author

@marcoandredinis marcoandredinis Oct 11, 2023

Choose a reason for hiding this comment

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

I think all resources will use it.

This is being used in tctl create -f and cache maintenance.

Copy link
Copy Markdown
Contributor

@espadolini espadolini Oct 11, 2023

Choose a reason for hiding this comment

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

Upsert is always unconditional - we considered adding a primitive backend operation for "write if item doesn't exist or if it exists and it matches this revision" but we couldn't really think of uses for it. 🤷

Copy link
Copy Markdown
Contributor

@espadolini espadolini Oct 11, 2023

Choose a reason for hiding this comment

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

If you meant in general then yeah, unfortunately we have an implemented and released UpdateDiscoveryConfig that no one should use so the conditionally updating RPC used by tctl edit will probably be UpdateDiscoveryConfigV2 or something like that. 🥲

Copy link
Copy Markdown
Contributor

@tigrato tigrato Oct 11, 2023

Choose a reason for hiding this comment

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

I meant the Update endpoint but github doesn't let me add the comment to the correct place

Copy link
Copy Markdown
Contributor Author

@marcoandredinis marcoandredinis Oct 11, 2023

Choose a reason for hiding this comment

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

I'm not sure I'm following here.

tctl edit should use the UpdateDiscoveryConfig which conditionally updates the resource.
It's not doing that right now, but the flow is there:

teleport/tool/tctl/common/edit_command.go

Lines 159 to 172 in 435e7d4

// Use the UpdateHandler if the resource has one, otherwise fallback to using
// the CreateHandler. UpdateHandlers are preferred over CreateHandler because an update
// will not forcibly overwrite a resource unlike with create which requires the force
// flag to be set to update an existing resource.
updator, found := rc.UpdateHandlers[ResourceKind(raw.Kind)]
if found {
return trace.Wrap(updator(ctx, client, raw))
}
// TODO(tross) remove the fallback to CreateHandlers once all the resources
// have been updated to implement an UpdateHandler.
if creator, found := rc.CreateHandlers[ResourceKind(raw.Kind)]; found {
return trace.Wrap(creator(ctx, client, raw))
}


// DeleteDiscoveryConfig removes the specified DiscoveryConfig resource.
rpc DeleteDiscoveryConfig(DeleteDiscoveryConfigRequest) returns (google.protobuf.Empty);

Expand Down Expand Up @@ -83,6 +86,12 @@ message UpdateDiscoveryConfigRequest {
DiscoveryConfig discovery_config = 1;
}

// UpsertDiscoveryConfigRequest is the request to upsert the provided DiscoveryConfig.
message UpsertDiscoveryConfigRequest {
// DiscoveryConfig is the DiscoveryConfig to be upserted.
DiscoveryConfig discovery_config = 1;
}

// DeleteDiscoveryConfigRequest is a request for deleting a specific DiscoveryConfig resource.
message DeleteDiscoveryConfigRequest {
// Name is the name of the DiscoveryConfig to be deleted.
Expand Down
13 changes: 13 additions & 0 deletions lib/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,12 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {
return nil, trace.Wrap(err)
}
}
if cfg.DiscoveryConfigs == nil {
cfg.DiscoveryConfigs, err = local.NewDiscoveryConfigService(cfg.Backend)
if err != nil {
return nil, trace.Wrap(err)
}
}
if cfg.Embeddings == nil {
cfg.Embeddings = local.NewEmbeddingsService(cfg.Backend)
}
Expand Down Expand Up @@ -315,6 +321,7 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {
SessionTrackerService: cfg.SessionTrackerService,
ConnectionsDiagnostic: cfg.ConnectionsDiagnostic,
Integrations: cfg.Integrations,
DiscoveryConfigs: cfg.DiscoveryConfigs,
Embeddings: cfg.Embeddings,
Okta: cfg.Okta,
AccessLists: cfg.AccessLists,
Expand Down Expand Up @@ -451,6 +458,7 @@ type Services struct {
services.ConnectionsDiagnostic
services.StatusInternal
services.Integrations
services.DiscoveryConfigs
services.Okta
services.AccessLists
services.UserLoginStates
Expand Down Expand Up @@ -484,6 +492,11 @@ func (r *Services) AccessListClient() services.AccessLists {
return r
}

// DiscoveryConfigClient returns the DiscoveryConfig client.
func (r *Services) DiscoveryConfigClient() services.DiscoveryConfigs {
return r
}

// UserLoginStateClient returns the user login state client.
func (r *Services) UserLoginStateClient() services.UserLoginStates {
return r
Expand Down
10 changes: 10 additions & 0 deletions lib/auth/auth_with_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/gravitational/teleport/api"
"github.com/gravitational/teleport/api/client"
"github.com/gravitational/teleport/api/client/accesslist"
"github.com/gravitational/teleport/api/client/discoveryconfig"
"github.com/gravitational/teleport/api/client/okta"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/client/userloginstate"
Expand All @@ -44,6 +45,7 @@ import (
"github.com/gravitational/teleport/api/gen/proto/go/assist/v1"
accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1"
devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
discoveryconfigv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/discoveryconfig/v1"
integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
loginrulepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1"
oktapb "github.com/gravitational/teleport/api/gen/proto/go/teleport/okta/v1"
Expand Down Expand Up @@ -347,6 +349,14 @@ func (a *ServerWithRoles) AccessListClient() services.AccessLists {
utils.NewGRPCDummyClientConnection("AccessListClient() should not be called on ServerWithRoles")))
}

// DiscoveryConfigClient allows ServerWithRoles to implement ClientI.
// It should not be called through ServerWithRoles,
// as it returns a dummy client that will always respond with "not implemented".
func (a *ServerWithRoles) DiscoveryConfigClient() services.DiscoveryConfigs {
return discoveryconfig.NewClient(discoveryconfigv1.NewDiscoveryConfigServiceClient(
utils.NewGRPCDummyClientConnection("DiscoveryConfigClient() should not be called on ServerWithRoles")))
}

// ResourceUsageClient allows ServerWithRoles to implement ClientI.
// It should not be called through ServerWithRoles,
// as it returns a dummy client that will always respond with "not implemented".
Expand Down
11 changes: 11 additions & 0 deletions lib/auth/clt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,11 @@ func (c *Client) UpsertUser(ctx context.Context, user types.User) (types.User, e
return upserted, trace.Wrap(err)
}

// DiscoveryConfigClient returns a client for managing the DiscoveryConfig resource.
func (c *Client) DiscoveryConfigClient() services.DiscoveryConfigs {
return c.APIClient.DiscoveryConfigClient()
}

// WebService implements features used by Web UI clients
type WebService interface {
// GetWebSessionInfo checks if a web session is valid, returns session id in case if
Expand Down Expand Up @@ -866,6 +871,12 @@ type ClientI interface {
// (as per the default gRPC behavior).
UserLoginStateClient() services.UserLoginStates

// DiscoveryConfigClient returns a DiscoveryConfig client.
// Clients connecting to older Teleport versions, still get an DiscoveryConfig client
// when calling this method, but all RPCs will return "not implemented" errors
// (as per the default gRPC behavior).
DiscoveryConfigClient() services.DiscoveryConfigs

// ResourceUsageClient returns a resource usage service client.
// Clients connecting to non-Enterprise clusters, or older Teleport versions,
// still get a client when calling this method, but all RPCs will return
Expand Down
20 changes: 20 additions & 0 deletions lib/auth/discoveryconfig/discoveryconfigv1/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,26 @@ func (s *Service) UpdateDiscoveryConfig(ctx context.Context, req *discoveryconfi
return conv.ToProto(resp), nil
}

// UpsertDiscoveryConfig creates or updates a DiscoveryConfig.
func (s *Service) UpsertDiscoveryConfig(ctx context.Context, req *discoveryconfigv1.UpsertDiscoveryConfigRequest) (*discoveryconfigv1.DiscoveryConfig, error) {
_, err := authz.AuthorizeWithVerbs(ctx, s.log, s.authorizer, true, types.KindDiscoveryConfig, types.VerbCreate, types.VerbUpdate)
if err != nil {
return nil, trace.Wrap(err)
}

dc, err := conv.FromProto(req.GetDiscoveryConfig())
if err != nil {
return nil, trace.Wrap(err)
}

resp, err := s.backend.UpsertDiscoveryConfig(ctx, dc)
if err != nil {
return nil, trace.Wrap(err)
}

return conv.ToProto(resp), nil
}

// DeleteDiscoveryConfig removes the specified DiscoveryConfig resource.
func (s *Service) DeleteDiscoveryConfig(ctx context.Context, req *discoveryconfigv1.DeleteDiscoveryConfigRequest) (*emptypb.Empty, error) {
_, err := authz.AuthorizeWithVerbs(ctx, s.log, s.authorizer, true, types.KindDiscoveryConfig, types.VerbDelete)
Expand Down
37 changes: 37 additions & 0 deletions lib/auth/discoveryconfig/discoveryconfigv1/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,43 @@ func TestDiscoveryConfigCRUD(t *testing.T) {
ErrAssertion: require.NoError,
},

// Upsert
{
Name: "no access to upsert discovery config",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindDiscoveryConfig},
Verbs: []string{types.VerbUpdate}, // missing VerbCreate
}}},
},
Test: func(ctx context.Context, resourceSvc *Service, dcName string) error {
dc := sampleDiscoveryConfigFn(t, dcName)
_, err := resourceSvc.UpsertDiscoveryConfig(ctx, &discoveryconfigpb.UpsertDiscoveryConfigRequest{
DiscoveryConfig: convert.ToProto(dc),
})
return err
},
ErrAssertion: requireTraceErrorFn(trace.IsAccessDenied),
},
{
Name: "access to upsert discovery config",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindDiscoveryConfig},
Verbs: []string{types.VerbUpdate, types.VerbCreate},
}}},
},
Setup: func(t *testing.T, dcName string) {},
Test: func(ctx context.Context, resourceSvc *Service, dcName string) error {
dc := sampleDiscoveryConfigFn(t, dcName)
_, err := resourceSvc.UpsertDiscoveryConfig(ctx, &discoveryconfigpb.UpsertDiscoveryConfigRequest{
DiscoveryConfig: convert.ToProto(dc),
})
return err
},
ErrAssertion: require.NoError,
},

// Delete
{
Name: "no access to delete discovery config",
Expand Down
12 changes: 12 additions & 0 deletions lib/auth/grpcserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/gen/proto/go/assist/v1"
auditlogpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/auditlog/v1"
discoveryconfigpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/discoveryconfig/v1"
integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
loginrulepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1"
oktapb "github.com/gravitational/teleport/api/gen/proto/go/teleport/okta/v1"
Expand All @@ -59,6 +60,7 @@ import (
"github.com/gravitational/teleport/api/types/installers"
"github.com/gravitational/teleport/api/types/wrappers"
"github.com/gravitational/teleport/lib/auth/assist/assistv1"
"github.com/gravitational/teleport/lib/auth/discoveryconfig/discoveryconfigv1"
integrationService "github.com/gravitational/teleport/lib/auth/integration/integrationv1"
"github.com/gravitational/teleport/lib/auth/loginrule"
"github.com/gravitational/teleport/lib/auth/okta"
Expand Down Expand Up @@ -5449,6 +5451,16 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
}
integrationpb.RegisterIntegrationServiceServer(server, integrationServiceServer)

discoveryConfig, err := discoveryconfigv1.NewService(discoveryconfigv1.ServiceConfig{
Authorizer: cfg.Authorizer,
Backend: cfg.AuthServer.Services,
Clock: cfg.AuthServer.clock,
})
if err != nil {
return nil, trace.Wrap(err)
}
discoveryconfigpb.RegisterDiscoveryConfigServiceServer(server, discoveryConfig)

// Initialize and register the user preferences service.
userPreferencesSrv, err := userpreferencesv1.NewService(&userpreferencesv1.ServiceConfig{
Backend: cfg.AuthServer.Services,
Expand Down
3 changes: 3 additions & 0 deletions lib/auth/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ type InitConfig struct {
// Integrations is a service that manages Integrations.
Integrations services.Integrations

// DiscoveryConfigs is a service that manages DiscoveryConfigs.
DiscoveryConfigs services.DiscoveryConfigs

// Embeddings is a service that manages Embeddings
Embeddings services.Embeddings

Expand Down
1 change: 1 addition & 0 deletions lib/authz/permissions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ func definitionForBuiltinRole(clusterName string, recConfig types.SessionRecordi
types.NewRule(types.KindDatabase, services.RW()),
types.NewRule(types.KindServerInfo, services.RW()),
types.NewRule(types.KindApp, services.RW()),
types.NewRule(types.KindDiscoveryConfig, services.RO()),
},
// Discovery service should only access kubes/apps/dbs that originated from discovery.
KubernetesLabels: types.Labels{types.OriginLabel: []string{types.OriginCloud}},
Expand Down
Loading