diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go index d22b4839ccec..bfaa9a225284 100644 --- a/pkg/ccl/kvccl/kvtenantccl/connector.go +++ b/pkg/ccl/kvccl/kvtenantccl/connector.go @@ -460,6 +460,39 @@ func (c *Connector) WithTxn(context.Context, *kv.Txn) spanconfig.KVAccessor { panic("not applicable") } +// GetSystemSpanConfigEntries implements the spanconfig.KVAccessor interface. +func (c *Connector) GetSystemSpanConfigEntries( + ctx context.Context, +) (entries []roachpb.SystemSpanConfigEntry, _ error) { + if err := c.withClient(ctx, func(ctx context.Context, c *client) error { + resp, err := c.GetSystemSpanConfigs(ctx, &roachpb.GetSystemSpanConfigsRequest{}) + if err != nil { + return err + } + + entries = resp.SystemSpanConfigEntries + return nil + }); err != nil { + return nil, err + } + return entries, nil +} + +// UpdateSystemSpanConfigEntries implements the spanconfig.KVAccessor interface. +func (c *Connector) UpdateSystemSpanConfigEntries( + ctx context.Context, + toDelete []roachpb.SystemSpanConfigTarget, + toUpsert []roachpb.SystemSpanConfigEntry, +) error { + return c.withClient(ctx, func(ctx context.Context, c *client) error { + _, err := c.UpdateSystemSpanConfigs(ctx, &roachpb.UpdateSystemSpanConfigsRequest{ + ToDelete: toDelete, + ToUpsert: toUpsert, + }) + return err + }) +} + // withClient is a convenience wrapper that executes the given closure while // papering over InternalClient retrieval errors. func (c *Connector) withClient( diff --git a/pkg/ccl/kvccl/kvtenantccl/connector_test.go b/pkg/ccl/kvccl/kvtenantccl/connector_test.go index 12a2c7f617b6..2a2ebc00b7a2 100644 --- a/pkg/ccl/kvccl/kvtenantccl/connector_test.go +++ b/pkg/ccl/kvccl/kvtenantccl/connector_test.go @@ -100,6 +100,18 @@ func (m *mockServer) UpdateSpanConfigs( panic("unimplemented") } +func (m *mockServer) GetSystemSpanConfigs( + context.Context, *roachpb.GetSystemSpanConfigsRequest, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + +func (m *mockServer) UpdateSystemSpanConfigs( + context.Context, *roachpb.UpdateSystemSpanConfigsRequest, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + func (m *mockServer) TenantSettings( *roachpb.TenantSettingsRequest, roachpb.Internal_TenantSettingsServer, ) error { diff --git a/pkg/kv/kvclient/kvcoord/send_test.go b/pkg/kv/kvclient/kvcoord/send_test.go index 80a2b197c905..20fada847ebf 100644 --- a/pkg/kv/kvclient/kvcoord/send_test.go +++ b/pkg/kv/kvclient/kvcoord/send_test.go @@ -91,6 +91,18 @@ func (n Node) UpdateSpanConfigs( panic("unimplemented") } +func (n Node) GetSystemSpanConfigs( + _ context.Context, _ *roachpb.GetSystemSpanConfigsRequest, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + +func (n Node) UpdateSystemSpanConfigs( + _ context.Context, _ *roachpb.UpdateSystemSpanConfigsRequest, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + func (n Node) TenantSettings( *roachpb.TenantSettingsRequest, roachpb.Internal_TenantSettingsServer, ) error { diff --git a/pkg/kv/kvclient/kvcoord/transport_test.go b/pkg/kv/kvclient/kvcoord/transport_test.go index 51ff462b7ed5..6e8509365170 100644 --- a/pkg/kv/kvclient/kvcoord/transport_test.go +++ b/pkg/kv/kvclient/kvcoord/transport_test.go @@ -209,6 +209,18 @@ func (m *mockInternalClient) UpdateSpanConfigs( return nil, fmt.Errorf("unsupported UpdateSpanConfigs call") } +func (m *mockInternalClient) GetSystemSpanConfigs( + _ context.Context, _ *roachpb.GetSystemSpanConfigsRequest, _ ...grpc.CallOption, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + return nil, fmt.Errorf("unsupported GetSpanConfigs call") +} + +func (m *mockInternalClient) UpdateSystemSpanConfigs( + _ context.Context, _ *roachpb.UpdateSystemSpanConfigsRequest, _ ...grpc.CallOption, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + return nil, fmt.Errorf("unsupported UpdateSpanConfigs call") +} + func (m *mockInternalClient) TenantSettings( context.Context, *roachpb.TenantSettingsRequest, ...grpc.CallOption, ) (roachpb.Internal_TenantSettingsClient, error) { diff --git a/pkg/roachpb/api.proto b/pkg/roachpb/api.proto index 205d7a95d6a8..521dcdb307a8 100644 --- a/pkg/roachpb/api.proto +++ b/pkg/roachpb/api.proto @@ -2826,6 +2826,12 @@ service Internal { // keyspans. rpc UpdateSpanConfigs (UpdateSpanConfigsRequest) returns (UpdateSpanConfigsResponse) { } + // GetSystemSpanConfigs is used to fetch system span configurations. + rpc GetSystemSpanConfigs(GetSystemSpanConfigsRequest) returns (GetSystemSpanConfigsResponse) { } + + // UpdateSystemSpanConfigs is used to update system span configurations. + rpc UpdateSystemSpanConfigs (UpdateSystemSpanConfigsRequest) returns (UpdateSystemSpanConfigsResponse) {} + // TenantSettings is used by tenants to obtain and stay up to date with tenant // setting overrides. rpc TenantSettings (TenantSettingsRequest) returns (stream TenantSettingsEvent) { } diff --git a/pkg/roachpb/mocks_generated.go b/pkg/roachpb/mocks_generated.go index 1886c2e503dc..9aa7e547d2e7 100644 --- a/pkg/roachpb/mocks_generated.go +++ b/pkg/roachpb/mocks_generated.go @@ -76,6 +76,26 @@ func (mr *MockInternalClientMockRecorder) GetSpanConfigs(arg0, arg1 interface{}, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSpanConfigs", reflect.TypeOf((*MockInternalClient)(nil).GetSpanConfigs), varargs...) } +// GetSystemSpanConfigs mocks base method. +func (m *MockInternalClient) GetSystemSpanConfigs(arg0 context.Context, arg1 *GetSystemSpanConfigsRequest, arg2 ...grpc.CallOption) (*GetSystemSpanConfigsResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetSystemSpanConfigs", varargs...) + ret0, _ := ret[0].(*GetSystemSpanConfigsResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSystemSpanConfigs indicates an expected call of GetSystemSpanConfigs. +func (mr *MockInternalClientMockRecorder) GetSystemSpanConfigs(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSystemSpanConfigs", reflect.TypeOf((*MockInternalClient)(nil).GetSystemSpanConfigs), varargs...) +} + // GossipSubscription mocks base method. func (m *MockInternalClient) GossipSubscription(arg0 context.Context, arg1 *GossipSubscriptionRequest, arg2 ...grpc.CallOption) (Internal_GossipSubscriptionClient, error) { m.ctrl.T.Helper() @@ -236,6 +256,26 @@ func (mr *MockInternalClientMockRecorder) UpdateSpanConfigs(arg0, arg1 interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSpanConfigs", reflect.TypeOf((*MockInternalClient)(nil).UpdateSpanConfigs), varargs...) } +// UpdateSystemSpanConfigs mocks base method. +func (m *MockInternalClient) UpdateSystemSpanConfigs(arg0 context.Context, arg1 *UpdateSystemSpanConfigsRequest, arg2 ...grpc.CallOption) (*UpdateSystemSpanConfigsResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "UpdateSystemSpanConfigs", varargs...) + ret0, _ := ret[0].(*UpdateSystemSpanConfigsResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateSystemSpanConfigs indicates an expected call of UpdateSystemSpanConfigs. +func (mr *MockInternalClientMockRecorder) UpdateSystemSpanConfigs(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSystemSpanConfigs", reflect.TypeOf((*MockInternalClient)(nil).UpdateSystemSpanConfigs), varargs...) +} + // MockInternal_RangeFeedClient is a mock of Internal_RangeFeedClient interface. type MockInternal_RangeFeedClient struct { ctrl *gomock.Controller diff --git a/pkg/roachpb/span_config.proto b/pkg/roachpb/span_config.proto index ca0277bba56c..cdc28f62c8c2 100644 --- a/pkg/roachpb/span_config.proto +++ b/pkg/roachpb/span_config.proto @@ -182,9 +182,6 @@ message SystemSpanConfigTarget { // TenantID indicates the tenant ID of the logical cluster being targeted. // For secondary tenants this field is left unset. For the host we can use // this field to protect a specific secondary tenant. - // - // TODO(arul): Ensure that secondary tenants don't populate this field when - // we make use of these in the RPC. roachpb.TenantID tenant_id = 1 [(gogoproto.customname) = "TenantID", (gogoproto.nullable) = true]; } @@ -219,6 +216,18 @@ message GetSpanConfigsResponse { repeated SpanConfigEntry span_config_entries = 1 [(gogoproto.nullable) = false]; }; +// GetSystemSpanConfigsRequest is used to fetch all system span configurations +// installed by the requesting tenant. +message GetSystemSpanConfigsRequest {}; + +// GetSystemSpanConfigsResponse lists out all system span configurations that +// are installed by the requesting tenant. +message GetSystemSpanConfigsResponse { + // SystemSpanConfigEntries captures the system span configurations that have + // been set by the tenant. + repeated SystemSpanConfigEntry system_span_config_entries = 1 [(gogoproto.nullable) = false]; +}; + // UpdateSpanConfigsRequest is used to update the span configurations over the // given spans. // @@ -245,3 +254,20 @@ message UpdateSpanConfigsRequest { message UpdateSpanConfigsResponse { }; +// UpdateSystemSpanConfigsRequest is used to update system span configurations. + +// System span config targets being deleted are expected to have been present. +// Targets are not allowed to be duplicated in the same list or across lists; +// existing span configs should be updated by including in the upsert list +// without deleting their targets first. +message UpdateSystemSpanConfigsRequest { + // SystemSpanConfigsToDelete captures the targets of the system span + // configurations to delete. + repeated SystemSpanConfigTarget to_delete = 1 [(gogoproto.nullable) = false]; + + // SystemSpanConfigsToUpsert captures the system span configurations we want + // to upsert with. + repeated SystemSpanConfigEntry to_upsert = 2 [(gogoproto.nullable) = false]; +}; + +message UpdateSystemSpanConfigsResponse {}; diff --git a/pkg/rpc/auth_tenant.go b/pkg/rpc/auth_tenant.go index 2e0b8c55f46a..56078743ebad 100644 --- a/pkg/rpc/auth_tenant.go +++ b/pkg/rpc/auth_tenant.go @@ -109,6 +109,12 @@ func (a tenantAuthorizer) authorize( case "/cockroach.roachpb.Internal/UpdateSpanConfigs": return a.authUpdateSpanConfigs(tenID, req.(*roachpb.UpdateSpanConfigsRequest)) + case "/cockroach.roachpb.Internal/UpdateSystemSpanConfigs": + return a.authUpdateSystemSpanConfigs(tenID, req.(*roachpb.UpdateSystemSpanConfigsRequest)) + + case "/cockroach.roachpb.Internal/GetSystemSpanConfigs": + return a.authTenant(tenID) + default: return authErrorf("unknown method %q", fullMethod) } @@ -296,6 +302,40 @@ func (a tenantAuthorizer) authUpdateSpanConfigs( return nil } +func (a tenantAuthorizer) authUpdateSystemSpanConfigs( + tenID roachpb.TenantID, args *roachpb.UpdateSystemSpanConfigsRequest, +) error { + if err := a.authTenant(tenID); err != nil { + return err + } + + // The host tenant is allowed to target other secondary tenants, so we can + // skip validation checks below. + if tenID == roachpb.SystemTenantID { + return nil + } + + // Ensure a secondary tenant isn't being targeted. + validate := func(target roachpb.SystemSpanConfigTarget) error { + if target.TenantID != nil { + return authError("secondary tenants cannot target tenants for system span configurations") + } + return nil + } + + for _, target := range args.ToDelete { + if err := validate(target); err != nil { + return err + } + } + for _, entry := range args.ToUpsert { + if err := validate(entry.SystemSpanConfigTarget); err != nil { + return err + } + } + return nil +} + func contextWithTenant(ctx context.Context, tenID roachpb.TenantID) context.Context { ctx = roachpb.NewContextForTenant(ctx, tenID) ctx = logtags.AddTag(ctx, "tenant", tenID.String()) diff --git a/pkg/rpc/context.go b/pkg/rpc/context.go index 4cdd89f8e131..19c2a15548eb 100644 --- a/pkg/rpc/context.go +++ b/pkg/rpc/context.go @@ -607,6 +607,20 @@ func (a internalClientAdapter) UpdateSpanConfigs( return a.server.UpdateSpanConfigs(ctx, req) } +// GetSystemSpanConfigs is part of the roachpb.InternalClient interface. +func (a internalClientAdapter) GetSystemSpanConfigs( + ctx context.Context, req *roachpb.GetSystemSpanConfigsRequest, _ ...grpc.CallOption, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + return a.server.GetSystemSpanConfigs(ctx, req) +} + +// UpdateSystemSpanConfigs is part of the roachpb.InternalClient interface. +func (a internalClientAdapter) UpdateSystemSpanConfigs( + ctx context.Context, req *roachpb.UpdateSystemSpanConfigsRequest, _ ...grpc.CallOption, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + return a.server.UpdateSystemSpanConfigs(ctx, req) +} + type respStreamClientAdapter struct { ctx context.Context respC chan interface{} diff --git a/pkg/rpc/context_test.go b/pkg/rpc/context_test.go index 0fab06554512..9807247e14d4 100644 --- a/pkg/rpc/context_test.go +++ b/pkg/rpc/context_test.go @@ -270,6 +270,18 @@ func (*internalServer) GetSpanConfigs( panic("unimplemented") } +func (*internalServer) UpdateSystemSpanConfigs( + context.Context, *roachpb.UpdateSystemSpanConfigsRequest, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + +func (*internalServer) GetSystemSpanConfigs( + context.Context, *roachpb.GetSystemSpanConfigsRequest, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + func (*internalServer) UpdateSpanConfigs( context.Context, *roachpb.UpdateSpanConfigsRequest, ) (*roachpb.UpdateSpanConfigsResponse, error) { diff --git a/pkg/rpc/nodedialer/nodedialer_test.go b/pkg/rpc/nodedialer/nodedialer_test.go index 9f9eb5db8e3b..f5b2ba96a405 100644 --- a/pkg/rpc/nodedialer/nodedialer_test.go +++ b/pkg/rpc/nodedialer/nodedialer_test.go @@ -598,6 +598,18 @@ func (*internalServer) UpdateSpanConfigs( panic("unimplemented") } +func (*internalServer) GetSystemSpanConfigs( + context.Context, *roachpb.GetSystemSpanConfigsRequest, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + +func (*internalServer) UpdateSystemSpanConfigs( + context.Context, *roachpb.UpdateSystemSpanConfigsRequest, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + panic("unimplemented") +} + func (*internalServer) TenantSettings( *roachpb.TenantSettingsRequest, roachpb.Internal_TenantSettingsServer, ) error { diff --git a/pkg/server/node.go b/pkg/server/node.go index 1f22c1ad4642..c766ad0ac9f5 100644 --- a/pkg/server/node.go +++ b/pkg/server/node.go @@ -1518,3 +1518,26 @@ func (n *Node) UpdateSpanConfigs( } return &roachpb.UpdateSpanConfigsResponse{}, nil } + +// GetSystemSpanConfigs implements the roachpb.InternalServer interface. +func (n *Node) GetSystemSpanConfigs( + ctx context.Context, _ *roachpb.GetSystemSpanConfigsRequest, +) (*roachpb.GetSystemSpanConfigsResponse, error) { + entries, err := n.spanConfigAccessor.GetSystemSpanConfigEntries(ctx) + if err != nil { + return nil, err + } + + return &roachpb.GetSystemSpanConfigsResponse{SystemSpanConfigEntries: entries}, nil +} + +// UpdateSystemSpanConfigs implements the roachpb.InternalServer interface. +func (n *Node) UpdateSystemSpanConfigs( + ctx context.Context, req *roachpb.UpdateSystemSpanConfigsRequest, +) (*roachpb.UpdateSystemSpanConfigsResponse, error) { + err := n.spanConfigAccessor.UpdateSystemSpanConfigEntries(ctx, req.ToDelete, req.ToUpsert) + if err != nil { + return nil, err + } + return &roachpb.UpdateSystemSpanConfigsResponse{}, nil +} diff --git a/pkg/spanconfig/spanconfig.go b/pkg/spanconfig/spanconfig.go index 9d6aa5091cfd..60d430115d99 100644 --- a/pkg/spanconfig/spanconfig.go +++ b/pkg/spanconfig/spanconfig.go @@ -44,6 +44,20 @@ type KVAccessor interface { toUpsert []roachpb.SpanConfigEntry, ) error + // GetSystemSpanConfigEntries returns the system span config entries that + // have been installed by the tenant. + GetSystemSpanConfigEntries(ctx context.Context) ([]roachpb.SystemSpanConfigEntry, error) + + // UpdateSystemSpanConfigEntries updates system span configurations for the + // given targets. Targets for span config entries being deleted are expected + // to have been present; targets must be distinct within and across the two + // lists. + UpdateSystemSpanConfigEntries( + ctx context.Context, + toDelete []roachpb.SystemSpanConfigTarget, + toUpsert []roachpb.SystemSpanConfigEntry, + ) error + // WithTxn returns a KVAccessor that runs using the given transaction (with // its operations discarded if aborted, valid only if committed). If nil, a // transaction is created internally for every operation. diff --git a/pkg/spanconfig/spanconfigkvaccessor/dummy.go b/pkg/spanconfig/spanconfigkvaccessor/dummy.go index b6171758fd45..6df8701b3bc5 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/dummy.go +++ b/pkg/spanconfig/spanconfigkvaccessor/dummy.go @@ -53,6 +53,20 @@ func (k dummyKVAccessor) UpdateSpanConfigEntries( return k.error } +// GetSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (k dummyKVAccessor) GetSystemSpanConfigEntries( + context.Context, +) ([]roachpb.SystemSpanConfigEntry, error) { + return nil, k.error +} + +// UpdateSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (k dummyKVAccessor) UpdateSystemSpanConfigEntries( + context.Context, []roachpb.SystemSpanConfigTarget, []roachpb.SystemSpanConfigEntry, +) error { + return k.error +} + func (k dummyKVAccessor) WithTxn(context.Context, *kv.Txn) spanconfig.KVAccessor { return k } diff --git a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go index fba4f1397a48..5cdb011a6239 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go +++ b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go @@ -68,7 +68,7 @@ func (k *KVAccessor) WithTxn(ctx context.Context, txn *kv.Txn) spanconfig.KVAcce return newKVAccessor(k.db, k.ie, k.settings, k.configurationsTableFQN, txn) } -// GetSpanConfigEntriesFor is part of the KVAccessor interface. +// GetSpanConfigEntriesFor is part of the spanconfig.KVAccessor interface. func (k *KVAccessor) GetSpanConfigEntriesFor( ctx context.Context, spans []roachpb.Span, ) (resp []roachpb.SpanConfigEntry, retErr error) { @@ -116,7 +116,7 @@ func (k *KVAccessor) GetSpanConfigEntriesFor( return resp, nil } -// UpdateSpanConfigEntries is part of the KVAccessor interface. +// UpdateSpanConfigEntries is part of the spanconfig.KVAccessor interface. func (k *KVAccessor) UpdateSpanConfigEntries( ctx context.Context, toDelete []roachpb.Span, toUpsert []roachpb.SpanConfigEntry, ) error { @@ -129,6 +129,20 @@ func (k *KVAccessor) UpdateSpanConfigEntries( }) } +// GetSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (k *KVAccessor) GetSystemSpanConfigEntries( + context.Context, +) ([]roachpb.SystemSpanConfigEntry, error) { + return nil, errors.New("unimplemented") +} + +// UpdateSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (k *KVAccessor) UpdateSystemSpanConfigEntries( + context.Context, []roachpb.SystemSpanConfigTarget, []roachpb.SystemSpanConfigEntry, +) error { + return errors.New("unimplemented") +} + func newKVAccessor( db *kv.DB, ie sqlutil.InternalExecutor, diff --git a/pkg/spanconfig/spanconfigtestutils/recorder.go b/pkg/spanconfig/spanconfigtestutils/recorder.go index 0db4cf8db685..91e00e0f32c9 100644 --- a/pkg/spanconfig/spanconfigtestutils/recorder.go +++ b/pkg/spanconfig/spanconfigtestutils/recorder.go @@ -126,3 +126,17 @@ func (r *KVAccessorRecorder) Recording(clear bool) string { return output.String() } + +// GetSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (r *KVAccessorRecorder) GetSystemSpanConfigEntries( + context.Context, +) ([]roachpb.SystemSpanConfigEntry, error) { + panic("unimplemented") +} + +// UpdateSystemSpanConfigEntries is part of the spanconfig.KVAccessor interface. +func (r *KVAccessorRecorder) UpdateSystemSpanConfigEntries( + context.Context, []roachpb.SystemSpanConfigTarget, []roachpb.SystemSpanConfigEntry, +) error { + panic("unimplemented") +}