C2-2432: implement create namespace (database) command #88
Conversation
| @Consumes(MediaType.APPLICATION_JSON) | ||
| @SecurityRequirement(name = OpenApiConstants.SecuritySchemes.TOKEN) | ||
| @Tag(ref = "General") | ||
| public class GeneralResource { |
There was a problem hiding this comment.
Can we name this as NamespaceResource? General resource doesn't convey what it does.
There was a problem hiding this comment.
Does not fit, we already have a DatabaseResource I followed the logic:
/v1- GeneralResurce/v1/{database}- DatabaseResource/v1/{database}/{collection}- CollectionResource
That would mean that we would have database resource and namespace resource, but they are both actually the same..
There was a problem hiding this comment.
it should be
/v1 - GeneralResource
/v1/{namespace} - NamespaceResource
/v1/{namespace}/{collection} - CollectionResource
There was a problem hiding this comment.
OK but this database to namespace change is huge and needs a separate PR imo.. WIll create a ticket..
| @Override | ||
| public Operation resolveCommand(CommandContext ctx, CreateDatabaseCommand command) { | ||
| String replicationMap = getReplicationMap(command.options()); | ||
| return new CreateDatabaseOperation(command.name(), replicationMap); |
There was a problem hiding this comment.
This will fail for Astra DB because create key space is disabled, can we have this Operation injected, so we can create custom implementation for Astra?
There was a problem hiding this comment.
we don't need to have it injected we can have if/else here based on the prop, just return different op for astra
There was a problem hiding this comment.
how do we handle this in other APIs? Do we actually have Astra-specific code in those APIs?
There was a problem hiding this comment.
No, operation simply fails on Astra with an error message. But I think our ITs must be excluding keyspace creation tests.
If I remember correctly, integration tests also cannot actually reliably detect running on Astra/CNDB; it is considered "DSE" as far as C-3.11/C-4/DSE classification is concerned. So we mostly have longer exclusion lists in maven pom.xmls on cndb side (or similar CI yaml files) to skip what needs to be skipped.
There was a problem hiding this comment.
I checked and our other APIs do not implement any logic to restrict access to creating keyspaces. The enforcement of this in Astra is through permission checks, such that any request to create a keyspace currently gets an "unauthorized" error. This is a function of the authorization provider at a lower tier of the architecture (i.e. enforced in coordinator).
Therefore what has been implemented here is fine. We do not need or want a custom implementation of this operation for Astra. If Astra changes its policy in the future to allow keyspace creation, the operation will succeed.
There was a problem hiding this comment.
In CNDB integration tests we have -Dstargate.auth.astra_allowed_orgs=testOrganization.. The whitelisted organizations can create keyspaces..
While not a full specification, there is at least a partial one: https://github.com/stargate/jsonapi/blob/main/docs/jsonapi-network-spec.textile#namespace-endpoint |
Referencing my other comment above, no changes are required. |
@vkarpov15 To be addressed in #93 |
@jeffreyscarpenter The spec you pointed are for the |
Implementation of the create namespace (database) command, allows creation of the new Cassandra keyspace.
Since there were no specifications for this command, I created a general endpoint that is a simple
v1. I also did a small refactoring of the command hierarchy, so that is clear that each endpoint can get specific command (currently you could send a create collection command to a documents endpoint btw).Other than that, it's pretty simple with a lot of tests and swagger updates.