Split `Applications.Link` into separate namespaces

AaronCrawfis commented 2 years ago

Current State

Today there are a number of behaviors in Applications.Link RP we want to fix:

Many don't know what a Link is.
Today we have Applications.Connector/daprHttpInvokeRoute, which is not technically a connector. It's a route.
Additionally, we want to have Dapr stand out with its own branding. Customers are asking "do we need to use Dapr when adopting Radius?"

Proposal

Split Dapr resources out into their own namespace: Applications.Dapr. This way it stands on its own and is separate from Connectors/Links.
Split the OSS resources into separate namespaces.

Design doc

2023-04 LinkRP-Namespace Splitting.docx

Proposed name changes

Current	Proposed
`Applications.Link/daprStateStores`	`Applications.Dapr/stateStores`
`Applications.Link/daprPubSubBrokers`	`Applications.Dapr/pubSubBrokers`
`Applications.Link/daprSecretStores`	`Applications.Dapr/secretStores`
`Applications.Link/daprInvokeHttpRoutes`	`Applications.Dapr/invokeHttpRoutes`
`Applications.Link/redisCaches`	`Applications.Datastores/redisCaches`
`Applications.Link/mongoDatabases`	`Applications.Datastores/mongoDatabases`
`Applications.Link/sqlDatabases`	`Applications.Datastores/sqlDatabases`
`Applications.Link/rabbitMQMessageQueues`	`Applications.Messaging/rabbitMQQueues`
`Applications.Link/extenders`	`Applications.Custom/extenders`

AB#6499

youngbupark commented 1 year ago

In today’s planning meeting, I realized that we wanted to consider changing DE to solve this problem. Changing DE would make the solution difficult and result in the misalignment with ARM contract and the original version of ARM DE. Eventually we may be unable to use the existing ARM tool ecosystem.

To use the existing ARM tool ecosystem and conform ARM spec, solving the problem at UCP or LinkRP level seems right:

1. Refactor LinkRP to have multiple namespaces.
1. Make UCP support proxying from applications.link/xxxx -> applications.xxxx

cc/ @shalabhms @kachawla @vinayada1 @lakshmimsft @rynowak @AaronCrawfis

kachawla commented 1 year ago

In today’s planning meeting, I realized that we wanted to consider changing DE to solve this problem.

Just wanted to add this here to clear any potential misunderstanding - we weren't planning to change DE - my intention was to say I don't know if DE will need to change or any other layer for routing outside of LinkRP. So this was going to be a discussion with experts in UCP and DE area to build more clarity before starting any work.

shalabhms commented 1 year ago

Yes , this will be part of the brainstorming session when we start work on the issue and agree that approach would be to solve the problem at UCP seems right.

rynowak commented 1 year ago

Here's my feedback on the naming of these things. I'd like to see fewer categories/namespaces if possible to resolve future debates. If we have a ton of fine-grained categories, it will be hard to fit future things into them.

Are we allowed to make the namespace plural? I think Applications.Databases reads a lot better than Applications.Database

In particular some ideas:

Applications.Cache/redis -> Applications.Database/redis: Are there other things we'd put in caches? My heart says no. Redis is also used for all kinds of things beyond just being a cache. My justification, a cache is just a specialized use-case for a database.
Applications.Custom/extenders -> Applications.Core/extenders: Are there other things we'd put in custom? I really struggle to think of any. This feels like it should be in core. I agree extender isn't a good fit for databases or messaging.

AaronCrawfis commented 1 year ago

Applications.Cache/redis -> Applications.Database/redis: Are there other things we'd put in caches? My heart says no. Redis is also used for all kinds of things beyond just being a cache. My justification, a cache is just a specialized use-case for a database.

Yeah I'm good with this, will update the table above

Applications.Custom/extenders -> Applications.Core/extenders: Are there other things we'd put in custom? I really struggle to think of any. This feels like it should be in core. I agree extender isn't a good fit for databases or messaging.

I think this one only makes sense if we unify the RPs into one RP. Otherwise we're doing some crazy criss-crossing on behaviors and namespace labels

rynowak commented 1 year ago

Applications.Cache/redis -> Applications.Database/redis: Are there other things we'd put in caches? My heart says no. Redis is also used for all kinds of things beyond just being a cache. My justification, a cache is just a specialized use-case for a database.

Yeah I'm good with this, will update the table above

Applications.Custom/extenders -> Applications.Core/extenders: Are there other things we'd put in custom? I really struggle to think of any. This feels like it should be in core. I agree extender isn't a good fit for databases or messaging.

I think this one only makes sense if we unify the RPs into one RP. Otherwise we're doing some crazy criss-crossing on behaviors and namespace labels

Right. There's a few "ifs" required for this to work.

Top line for me is that I dislike "custom" 😆. I wanted to at least make a suggestion.

youngbupark commented 1 year ago

@AaronCrawfis FYI - in ARM, resource type name should be plural (K8s resource type name is also plural)

https://github.com/Azure/azure-resource-manager-rpc/blob/f68d204a6018d057e0aed041f43816a8d0bdcb07/v1.0/proxy-api-reference.md?plain=1#L91

youngbupark commented 1 year ago

Are we allowed to make the namespace plural? I think Applications.Databases reads a lot better than Applications.Database

@rynowak In ARM, we can use either singular or plural for namespace, but resource type name should be plural.

(X) Applications.Databases/mongo
(O) Applications.Databases/mongos - mongos seems weird.
(O) Applications.Databases/mongoDatabases

kachawla commented 1 year ago

@youngbupark Just curious, is this requirement for resource type to be plural enforced by ARM due to a technical limitation or for consistency?

youngbupark commented 1 year ago

@youngbupark Just curious, is this requirement for resource type to be plural enforced by ARM due to a technical limitation or for consistency?

Here is the requirement in ARM spec:

I recall that this was one of the errors when I first ran ARM swagger linter to build managed RP before, so I still remember. I cannot find the exact reason in official ARM document, but even K8s requires to use plural resource type name in CR design doc. (looks like openshift api guideline follows the same rule)

My assumption is that resource type represents the collections of resource and when we design restful api, plural name looks more natural.

GET /providers/Applications.Core/containers -- LIST resources (multiple resources)
PUT /providers/Applications.Core/containers/container1 -- create or update resource.
GET /providers/Applications.Core/containers/container1 -- get single resource

Also when you search "rest resource plural", you can find the similar guideline to represent entity or resource types in restful api design.

@rynowak may have more context regarding rest api design.

kachawla commented 1 year ago

I recall that this was one of the errors when I first ran ARM swagger linter to build managed RP before, so I still remember. I cannot find the exact reason in official ARM document, but even K8s requires to use plural resource type name in CR design doc. (looks like openshift api guideline follows the same rule)

My assumption is that resource type represents the collections of resource. So when we design restful api, plural name looks more natural.

GET /providers/Applications.Core/containers -- LIST resources (multiple resources)

PUT /providers/Applications.Core/containers/container1 -- create or update resource.

GET /providers/Applications.Core/containers/container1 -- get single resource

Also when you search "rest resource plural", you can find the similar guideline to represent entity or resource types in restful api design.

Thanks for the details Young Bu. Makes sense that plural sounds more natural for collection of resources.

My intent behind the question was if this limitation is backed by legacy design choices then we don't have a choice, if not, then I don't think just because it's been done this way in the past is the right reason for us to restrict our options based on for both namespace and resource type. From https://armwiki.azurewebsites.net/api_contracts/guidelines/openapi.html, looks like consistency is the reason why ARM expects resource type name to be plural.

From a quick look at the existing provider namespaces, doesn't look like every provider is following singular namespace approach either - https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/azure-services-resource-providers, Microsoft.CognitiveServices and a few others are not for example.

Should we just get inputs from the API review team to understand the hard requirements here before trying to fit into the assumed requirements? Specially since we envision enabling user defined namespaces, we will need to defines clear guidelines with reasoning on any requirements we plan to enforce, fewer restrictions on naming would be less frustrating for users, also if we don't have a way to block them in our validation then they will just end up being recommendations rather than requirements. @AaronCrawfis @rynowak

youngbupark commented 1 year ago

From a quick look at the existing provider namespaces, doesn't look like every provider is following singular namespace approach either - https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/azure-services-resource-providers, Microsoft.CognitiveServices and a few others are not for example.

As I mentioned above, namespace name can be either singular or plural because namespace name is the resource of /providers/ resource type like below. So it also follows the same rule.

PUT /providers/Applications.Core/Containers/container

Namespace resource type: providers (plural)
Namespace resource: Applications.Core (singular) it can be plural like Applications.Cores.

Unfortunately, plural resource type name is hard-requirement in Internal Resource Provider Guidelines and linter. But, I agree to double-check with API review team.

ytimocin commented 1 year ago

Here's my feedback on the naming of these things. I'd like to see fewer categories/namespaces if possible to resolve future debates. If we have a ton of fine-grained categories, it will be hard to fit future things into them.

Are we allowed to make the namespace plural? I think Applications.Databases reads a lot better than Applications.Database

In particular some ideas:

Applications.Cache/redis -> Applications.Database/redis: Are there other things we'd put in caches? My heart says no. Redis is also used for all kinds of things beyond just being a cache. My justification, a cache is just a specialized use-case for a database.

Applications.Custom/extenders -> Applications.Core/extenders: Are there other things we'd put in custom? I really struggle to think of any. This feels like it should be in core. I agree extender isn't a good fit for databases or messaging.

How about Applications.DataStores for all the databases and also other things that would store data like Redis?

lakshmimsft commented 1 year ago

LJ: Updating Proposed names after discussions with team. Pls refer to design doc: 2023-04 LinkRP-Namespace Splitting.docx

Current	Proposed
Applications.Link/daprStateStores	Applications.Dapr/stateStores
Applications.Link/daprPubSubBrokers	Applications.Dapr/pubSubBrokers
Applications.Link/daprSecretStores	Applications.Dapr/secretStores
Applications.Link/daprInvokeHttpRoutes	Applications.Dapr/invokeHttpRoutes
Applications.Link/redisCaches	Applications.Datastores/redisCaches
Applications.Link/mongoDatabases	Applications.Datastores/mongoDatabases
Applications.Link/sqlDatabases	Applications.Datastores/sqlDatabases
Applications.Link/rabbitMQMessageQueues	Applications.Messaging/rabbitMQQueues
Applications.Link/extenders	Applications.Custom/extenders

AaronCrawfis commented 1 year ago

This is now complete

radius-project / radius