-
Notifications
You must be signed in to change notification settings - Fork 4
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
Cloud API: Nexus Incoming/Outgoing Service Definitions #16
Changes from 13 commits
82dfc84
6336346
bdc5c85
ae200d8
0c888a4
a344890
ca41698
fd0e337
a5c6138
5a75261
9acae7e
57ff662
45c4d09
c622166
88e7ec3
968eae8
0f16196
b4aaedc
9ccf3fb
b62fc9c
bf9771e
db6d017
8bf55f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -7,6 +7,7 @@ option go_package = "go.temporal.io/api/cloud/cloudservice/v1;cloudservice"; | |||||||
import "temporal/api/cloud/operation/v1/message.proto"; | ||||||||
import "temporal/api/cloud/identity/v1/message.proto"; | ||||||||
import "temporal/api/cloud/namespace/v1/message.proto"; | ||||||||
import "temporal/api/cloud/nexus/v1/message.proto"; | ||||||||
import "temporal/api/cloud/region/v1/message.proto"; | ||||||||
|
||||||||
message GetUsersRequest { | ||||||||
|
@@ -229,3 +230,187 @@ message GetRegionResponse { | |||||||
// The temporal cloud region. | ||||||||
temporal.api.cloud.region.v1.Region region = 1; | ||||||||
} | ||||||||
|
||||||||
message GetIncomingServicesRequest { | ||||||||
// The requested size of the page to retrieve - optional. | ||||||||
// Cannot exceed 1000. Defaults to 100. | ||||||||
int32 page_size = 1; | ||||||||
|
||||||||
// The page token if this is continuing from another response - optional. | ||||||||
string page_token = 2; | ||||||||
|
||||||||
// Filter incoming services by the namespace they route to - optional. | ||||||||
string namespace = 3; | ||||||||
|
||||||||
// Filter incoming services by the task queue they route to - optional. | ||||||||
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. nit: Do we really need to support filtering with task_queue? Can't think of a usecase. 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. 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. spoke offline, we think this is useful to have this filter here |
||||||||
string task_queue = 4; | ||||||||
|
||||||||
// Filter incoming services by their name - optional. Specifying this will result in zero or one results. | ||||||||
string name = 5; | ||||||||
} | ||||||||
|
||||||||
message GetIncomingServicesResponse { | ||||||||
// The list of incoming services in ascending ids order | ||||||||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
repeated temporal.api.cloud.nexus.v1.IncomingService incoming_services = 1; | ||||||||
|
||||||||
// The next page's token | ||||||||
string next_page_token = 2; | ||||||||
} | ||||||||
|
||||||||
message GetIncomingServiceRequest { | ||||||||
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.
Suggested change
Just in case we want to add a "by name" request? 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. Can it simply be 'GetIncomingServiceRequest' and if/when we add by name, we'd add 'GetIncomingServiceByNameRequest'? Or is this against a convention for the 'ById' suffix we already use? I see that in
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. agreeing with sergey here. am inclined towards leaving this as is for now (e.g. GetIncomingServiceRequest) just to be consistent with existing cloud user apis the weirdness is that when we translate this to HTTP path, we would end up creating a different path at that time probably, but not the end of the world if we have to introduce that |
||||||||
// The id of the incoming service to get | ||||||||
string incoming_service_id = 1; | ||||||||
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. nit: can we just call this 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. we can. I was trying to be consistent with this one: https://github.com/temporalio/api-cloud/blob/main/temporal/api/cloud/cloudservice/v1/request_response.proto#L33 but I definitely wish that used will leave as-is for now unless there is strong feeling about it |
||||||||
} | ||||||||
|
||||||||
message GetIncomingServiceResponse { | ||||||||
// The incoming service | ||||||||
temporal.api.cloud.nexus.v1.IncomingService incoming_service = 1; | ||||||||
} | ||||||||
|
||||||||
message CreateIncomingServiceRequest { | ||||||||
// The spec for the incoming service | ||||||||
temporal.api.cloud.nexus.v1.IncomingServiceSpec spec = 1; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 2; | ||||||||
} | ||||||||
|
||||||||
message CreateIncomingServiceResponse { | ||||||||
// The id of the service that was created | ||||||||
string incoming_service_id = 1; | ||||||||
|
||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 2; | ||||||||
} | ||||||||
|
||||||||
message UpdateIncomingServiceRequest { | ||||||||
// The id of the incoming service to update | ||||||||
string incoming_service_id = 1; | ||||||||
|
||||||||
// The updated incoming service specification | ||||||||
temporal.api.cloud.nexus.v1.IncomingServiceSpec spec = 2; | ||||||||
|
||||||||
// The version of the incoming service for which this update is intended for | ||||||||
// The latest version can be found in the GetIncomingService operation response | ||||||||
string resource_version = 3; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 4; | ||||||||
} | ||||||||
|
||||||||
message UpdateIncomingServiceResponse { | ||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 1; | ||||||||
} | ||||||||
|
||||||||
message DeleteIncomingServiceRequest { | ||||||||
// The id of the incoming service to delete | ||||||||
string incoming_service_id = 1; | ||||||||
|
||||||||
// The version of the incoming service for which this delete is intended for | ||||||||
// The latest version can be found in the GetIncomingService operation response | ||||||||
string resource_version = 2; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 3; | ||||||||
} | ||||||||
|
||||||||
message DeleteIncomingServiceResponse { | ||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 1; | ||||||||
} | ||||||||
|
||||||||
message GetOutgoingServicesRequest { | ||||||||
// The requested size of the page to retrieve - optional. | ||||||||
// Cannot exceed 1000. Defaults to 100. | ||||||||
int32 page_size = 1; | ||||||||
|
||||||||
// The page token if this is continuing from another response - optional. | ||||||||
string page_token = 2; | ||||||||
|
||||||||
// The namespace to get all the outgoing services from | ||||||||
string namespace = 3; | ||||||||
|
||||||||
// The name of the outgoing service to filter on - optional. Specifying this will result in zero or one results. | ||||||||
string name = 4; | ||||||||
Comment on lines
+335
to
+336
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. Not really useful as a filter, this is basically a Get request. 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. right, this is what we did for our Users api (users have a static id, but you call GetUsersRequset and can supply an email address), so this is just an attempt to stay consistent. 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. @anekkanti - thoughts? we basically followed the users model, which relies on this |
||||||||
} | ||||||||
|
||||||||
message GetOutgoingServicesResponse { | ||||||||
// The list of outgoing services in ascending name order | ||||||||
repeated temporal.api.cloud.nexus.v1.OutgoingService outgoing_services = 1; | ||||||||
|
||||||||
// The next page's token | ||||||||
string next_page_token = 2; | ||||||||
} | ||||||||
|
||||||||
message GetOutgoingServiceRequest { | ||||||||
// The name of the namespace the outgoing service is registered on | ||||||||
string namespace = 1; | ||||||||
|
||||||||
// The name of the outgoing service | ||||||||
string name = 2; | ||||||||
} | ||||||||
|
||||||||
message GetOutgoingServiceResponse { | ||||||||
// The outgoing service | ||||||||
temporal.api.cloud.nexus.v1.OutgoingService outgoing_service = 1; | ||||||||
} | ||||||||
|
||||||||
message CreateOutgoingServiceRequest { | ||||||||
// The namespace to register this outgoing service request on | ||||||||
string namespace = 1; | ||||||||
|
||||||||
// The spec for the outgoing service | ||||||||
temporal.api.cloud.nexus.v1.OutgoingServiceSpec spec = 2; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 3; | ||||||||
} | ||||||||
|
||||||||
message CreateOutgoingServiceResponse { | ||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 1; | ||||||||
} | ||||||||
|
||||||||
message UpdateOutgoingServiceRequest { | ||||||||
// The name of the namespace the outgoing service is registered on | ||||||||
string namespace = 1; | ||||||||
|
||||||||
// The name of the outgoing service to update | ||||||||
string name = 2; | ||||||||
|
||||||||
// The updated outcoming service specification | ||||||||
temporal.api.cloud.nexus.v1.OutgoingServiceSpec spec = 3; | ||||||||
|
||||||||
// The version of the outgoing service for which this update is intended for | ||||||||
// The latest version can be found in the GetOutgoingService operation response | ||||||||
string resource_version = 4; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 5; | ||||||||
} | ||||||||
|
||||||||
message UpdateOutgoingServiceResponse { | ||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 1; | ||||||||
} | ||||||||
|
||||||||
message DeleteOutgoingServiceRequest { | ||||||||
// The id of the namespace to remove the outgoing service from | ||||||||
string namespace = 1; | ||||||||
|
||||||||
// The name outcoming service to delete | ||||||||
string name = 2; | ||||||||
|
||||||||
// The version of the outcoming service for which this delete is intended for | ||||||||
// The latest version can be found in the GetOutgoingService operation response | ||||||||
string resource_version = 3; | ||||||||
|
||||||||
// The id to use for this async operation - optional | ||||||||
string async_operation_id = 4; | ||||||||
} | ||||||||
|
||||||||
message DeleteOutgoingServiceResponse { | ||||||||
// The async operation | ||||||||
temporal.api.cloud.operation.v1.AsyncOperation async_operation = 1; | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,4 +118,78 @@ service CloudService { | |
get: "/api/v1/regions/{region}", | ||
}; | ||
} | ||
|
||
// Gets all known incoming services | ||
rpc GetIncomingServices(GetIncomingServicesRequest) returns (GetIncomingServicesResponse) { | ||
option (google.api.http) = { | ||
get: "/api/v1/nexus/incomingservices", | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
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. Can we add a 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. since I am following what we do for users, I've kept the same format (which doesn't have by-id or id either..). @anekkanti , thoughts? 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 think for the default behavior (by ID) we don't need to add a suffix. 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. yes, am inclined to leave as-is as well. can always add suffix later if needed |
||
}; | ||
} | ||
|
||
// Get an incoming service | ||
rpc GetIncomingService(GetIncomingServiceRequest) returns (GetIncomingServiceResponse) { | ||
option (google.api.http) = { | ||
get: "/api/v1/nexus/incomingservices/{incoming_service_id}", | ||
}; | ||
} | ||
|
||
// Create an incoming service | ||
rpc CreateIncomingService(CreateIncomingServiceRequest) returns (CreateIncomingServiceResponse) { | ||
option (google.api.http) = { | ||
post: "/api/v1/nexus/incomingservices", | ||
body: "*" | ||
}; | ||
} | ||
|
||
// Update an incoming service | ||
rpc UpdateIncomingService(UpdateIncomingServiceRequest) returns (UpdateIncomingServiceResponse) { | ||
option (google.api.http) = { | ||
post: "/api/v1/nexus/incomingservices/{incoming_service_id}", | ||
body: "*" | ||
}; | ||
} | ||
|
||
// Delete an incoming service | ||
rpc DeleteIncomingService(DeleteIncomingServiceRequest) returns (DeleteIncomingServiceResponse) { | ||
option (google.api.http) = { | ||
delete: "/api/v1/nexus/incomingservices/{incoming_service_id}", | ||
}; | ||
} | ||
|
||
// Gets all outgoing services for a namespace | ||
rpc GetOutgoingServices(GetOutgoingServicesRequest) returns (GetOutgoingServicesResponse) { | ||
option (google.api.http) = { | ||
get: "/api/v1/namespaces/{namespace}/nexus/outgoingservices", | ||
}; | ||
} | ||
|
||
// Get an outgoing service | ||
rpc GetOutgoingService(GetOutgoingServiceRequest) returns (GetOutgoingServiceResponse) { | ||
option (google.api.http) = { | ||
get: "/api/v1/namespaces/{namespace}/nexus/outgoingservices/{name}", | ||
}; | ||
} | ||
|
||
// Create an outgoing service | ||
rpc CreateOutgoingService(CreateOutgoingServiceRequest) returns (CreateOutgoingServiceResponse) { | ||
option (google.api.http) = { | ||
post: "/api/v1/namespaces/{namespace}/nexus/outgoingservices", | ||
body: "*" | ||
}; | ||
} | ||
|
||
// Update an outgoing service | ||
rpc UpdateOutgoingService(UpdateOutgoingServiceRequest) returns (UpdateOutgoingServiceResponse) { | ||
option (google.api.http) = { | ||
post: "/api/v1/namespaces/{namespace}/nexus/outgoingservices/{name}", | ||
body: "*" | ||
}; | ||
} | ||
|
||
// Delete an outgoing service | ||
rpc DeleteOutgoingService(DeleteOutgoingServiceRequest) returns (DeleteOutgoingServiceResponse) { | ||
option (google.api.http) = { | ||
delete: "/api/v1/namespaces/{namespace}/nexus/outgoingservices/{name}", | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
syntax = "proto3"; | ||
|
||
package temporal.api.cloud.nexus.v1; | ||
|
||
option go_package = "go.temporal.io/api/cloud/nexus/v1;nexus"; | ||
|
||
import "google/protobuf/timestamp.proto"; | ||
|
||
message IncomingServiceSpec { | ||
// The name of the incoming service. Must be unique within an account. | ||
// The name must match `[a-zA-Z_][a-zA-Z0-9_]*`. | ||
// This field is mutable. | ||
string name = 1; | ||
|
||
// The namespace to route requests to. This field is mutable. | ||
string namespace = 2; | ||
|
||
// The task queue to route requests to. This field is mutable. | ||
string task_queue = 3; | ||
|
||
// The set of authorization policies for the service. Each request's caller | ||
// must match with at least one of the specs to be accepted by the incoming service. | ||
repeated IncomingServiceAuthorizationSpec authorization_specs = 4; | ||
Comment on lines
+21
to
+23
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 think maybe instead of auth spec, we might want to have a general message to also add limits (rate, concurrency, etc..) and other properties for every caller (only namespaces for now). |
||
} | ||
|
||
message IncomingServiceAuthorizationSpec { | ||
// The policy set on this incoming service authorization spec. | ||
// Possible values are: allowed_cloud_namespace | ||
string policy_type = 1; | ||
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. Why not make this an enum, I'm curious? 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. @anekkanti can comment here. We appear to have stayed away from enums elsewhere (and are using strings instead), so this is just being consistent with the existing cloud APIs |
||
|
||
// A policy spec that allows one caller namespace to access the service. | ||
AllowedCloudNamespacePolicySpec allowed_cloud_namespace_policy_spec = 2; | ||
} | ||
|
||
message AllowedCloudNamespacePolicySpec { | ||
// The namespace that is allowed to call into this service. Calling namespace must be in same account as incoming service. | ||
string namespace = 1; | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// A binding to a cloud namespace and task queue for dispatching incoming Nexus requests. | ||
message IncomingService { | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// The id of the incoming service. This is generated by the server and is immutable. | ||
string id = 1; | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// The current version of the incoming service specification. | ||
// The next update operation must include this version. | ||
string resource_version = 2; | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// The incoming service specification. | ||
IncomingServiceSpec spec = 3; | ||
|
||
// The current state of the service. | ||
// Possible values: activating, activationfailed, active, updating, updatefailed, deleting, deletefailed, deleted | ||
// For any failed state, reach out to Temporal Cloud support for remediation. | ||
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. mention eventual consistency here? |
||
string state = 4; | ||
|
||
// The id of the async operation that is creating/updating/deleting the service, if any. | ||
string async_operation_id = 5; | ||
|
||
// The date and time when the service was created. | ||
google.protobuf.Timestamp created_time = 6; | ||
|
||
// The date and time when the service was last modified. | ||
// Will not be set if the service has never been modified. | ||
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. Curious why not default this to 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. yeah, this is just for consistency. could have definitely been defaulted to created_time here |
||
google.protobuf.Timestamp last_modified_time = 7; | ||
} | ||
|
||
message OutgoingServiceSpec { | ||
// The name of the outgoing service. Must be unique within the namespace. | ||
// The name must match `[a-zA-Z_][a-zA-Z0-9_]*`. | ||
// This name is immutable. | ||
string name = 1; | ||
Comment on lines
+69
to
+72
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. This is the identifier, doesn't belong in the spec AFAICT. 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. that seems to be what we have done for namespace and for outgoing service. it is a property a user can set (albeit only on create), so we happened to put it on spec. can try and move it off, but it does create some inconsistency with the other cloud apis |
||
|
||
// The target that the outgoing service will connect to. | ||
TargetSpec spec = 2; | ||
} | ||
|
||
message TargetSpec { | ||
// The type of target that is set in this spec. | ||
// Possible values are: cloud_incoming_service | ||
string target_type = 1; | ||
|
||
// A target spec for an incoming service within the same account as the outgoing service. | ||
TargetCloudIncomingServiceSpec target_cloud_incoming_service_spec = 2; | ||
} | ||
|
||
message TargetCloudIncomingServiceSpec { | ||
// The ID of the Incoming Service that the outgoing service will connect to. | ||
string incoming_service_id = 1; | ||
} | ||
|
||
// A per-namespace binding from service name to destination service to invoke Nexus requests | ||
// that are initiated by workflows. | ||
message OutgoingService { | ||
mastermanu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// The name of the outgoing service. | ||
string name = 1; | ||
|
||
// The namespace this outgoing service is attached to. | ||
string namespace = 2; | ||
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. Should this be 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. good point. reordered |
||
|
||
// The current version of the incoming service specification. | ||
// The next update operation must include this version. | ||
string resource_version = 3; | ||
|
||
// The incoming service specification. | ||
OutgoingServiceSpec spec = 4; | ||
|
||
// The current state of the service. | ||
// Possible values: activating, activationfailed, active, updating, updatefailed, deleting, deletefailed, deleted | ||
// For any failed state, reach out to Temporal Cloud support for remediation. | ||
string state = 5; | ||
|
||
// The id of the async operation that is creating/updating/deleting the service, if any. | ||
string async_operation_id = 6; | ||
|
||
// The date and time when the service was created. | ||
google.protobuf.Timestamp created_time = 7; | ||
|
||
// The date and time when the service was last modified. | ||
// Will not be set if the service has never been modified. | ||
google.protobuf.Timestamp last_modified_time = 8; | ||
} |
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.
Wonder why not call this "list" instead of "get", I find this confusing but I figure it's consistent with other cloud APIs.
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.
Please mention if these are AND or OR filters in the comment for this message.
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.
yeah the list vs get is for consistency with existing cloud apis. could have been list.
these are AND filters. will update comments