JSON API indicates it supports POST/PATCH/DELETE of config entity types, but that's impossible

I don't think we should do anything here. In complex frameworks we work basing our work on APIs. We use them and we trust they do what they advertise they do. If there are bugs they need to be addressed in the system powering the API, I don't think that systems using that API should monitor open issues (or any of its dependencies).

Is there anything that you think should happen here? If this is a means to gauge impact, in order gather support for fixing the underlying issue, I'm happy to leave this open.

It might be worth noting on the project page that support for config entities is potentially vague due to the following core bugs ... It feels like as such this issue should be maybe more of a plan/task than a concrete bug.

I like that idea @dawehner.

@Wim Leers feel free to revert the status if needed.

I agree in principle, but in practice I don't

What do you think we should do?

@e0ipso, thoughts on #8?

Am I mistaken to think that many config entities are working correctly as is? If they are, do we want to keep them working even if others aren't? The issue linked in the project page is fixed now… but I'm unsure about the whole state of this. Can you jog my memory on why we can't support ['POST', 'PUT', 'DELETE', 'PATCH'] nowadays?

Sorry for the multiple questions, I'm confused here.

The main reason core does not support config entities right now is that we don't provide any validation for the values you set on those. This results into a potentially broken Drupal site if you give someone access to update it (aka. every admin user, which might have a broken browser).

@dawehner last time we had this conversation I remember there was another issue that was making this impossible. If anything, the validation issue would make this not advisable, but certainly not impossible.

I still argue that there are many different ways a priviledged user can broke their site. While I strongly think validation is a highly important, I don't think we should cripple our abilities to do things while we wait.

Since this module is being targeted for core, and since there's disagreement among maintainers, seems like this could benefit from a core framework manager review.

In order to facilitate this, it might be good for the issue summary to reflect the point of disagreement, along with whatever relevant examples.

Not sure if this is enough of a framework manager review, so not removing the tag yet, but my first thought on this is that at least in core, we should have no pathways by which to have invalid config (i.e., config that would fail validation if we were to export and reimport it). So if jsonapi currently allows a POST, PATCH, or DELETE that would be rejected as invalid during configuration import, then at a minimum, that would need to be fixed before enabling that entity type's API endpoint in core.

For example, deleting the field.field.node.article.body FieldConfig entity will break pretty much the entire front end. It's impossible to delete that via the UI.

Is it possible to do through a config import? If it is, then in what way is your site then broken after doing it?

In addition to rejecting all modifications that can't be done through a config import, I think we'd also need to restrict some modifications to a highly privileged permission, if we allow it at all. For example, perhaps a really simple MVP might be to allow modifications of config entities via jsonapi only if both 'import configuration' and 'synchronize configuration' permissions are both allowed. Perhaps we could relax this to merely 'administer site configuration' or the other 'administer *' permissions once we're confident that enough of the things that are validated in the UI are also validated for jsonapi.

Deleting config entities

We should take in mind that deleting a config entity is its own special case, and is not the same as deleting a content entity.
Coming from the configuration system deleting a single configuration entity isn't really something which is supported, but rather you delete and entry and as such update the entire configuration dependency tree, which might result into more deletions. The UI has the concept of confirmations, but I'm not sure whether this model can be mapped to a REST API.

When I talked with @alexpott about this specific issue on Drupalcon he recommended to postpone the deletion of config entities via REST, given its complexity. I guess we could map deleting to setting the status to 0.

Adding/updating config entities

In addition to rejecting all modifications that can't be done through a config import, I think we'd also need to restrict some modifications to a highly privileged permission, if we allow it at all. For example, perhaps a really simple MVP might be to allow modifications of config entities via jsonapi only if both 'import configuration' and 'synchronize configuration' permissions are both allowed. Perhaps we could relax this to merely 'administer site configuration' or the other 'administer *' permissions once we're confident that enough of the things that are validated in the UI are also validated for jsonapi.

I have a question. The existing UI doesn't use this kind of mental model yet. You can add/update configuration as you want. The import and export then are separate steps, should this be adapted as well?

Coming from the configuration system deleting a single configuration entity isn't really something which is supported

Sure it is. You can export your site's entire config, delete a YAML file from it, and import that directory. If the import then proceeds without validation errors, then you successfully deleted that single config entity. I think it's at least theoretically fine to allow jsonapi to invoke a DELETE operation that accomplishes the same result. However, if the import validation would have rejected such a change, then I think it needs to also be rejected by jsonapi.

When I talked with @alexpott about this specific issue on Drupalcon he recommended to postpone the deletion of config entities via REST

If it's too expensive (either in developer time or runtime) to determine whether or not that would be an allowed change, then I think not supporting deletions at all until we're ready to would be fine too, per @alexpott's recommendation.

The existing UI doesn't use this kind of mental model yet. You can add/update configuration as you want. The import and export then are separate steps, should this be adapted as well?

The UI, however, implements validation that config import does not. For example, editing a node type from the UI, because of FAPI #options validation, you can only set preview_mode to 0, 1, or 2. In a config export/import, you can set it to other things, like 5. Doing so might create problems on your site that we currently don't know about. Because people with 'import configuration'/'synchronize configuration' permission can already do this, I think it's fine if those same people can make the same change via jsonapi. However, people with merely 'administer content types' permission can't currently make such changes, and so I don't think we should start allowing them to via jsonapi. Once those FAPI validations are moved into config validations, then I think editing config entity types via jsonapi can use the same permissions as doing it via the UI.

Just to be clear, when/if JSON API is added to Drupal core the policy on this will match the one in REST core (whatever that policy ends up being). I think this clarifies some confusion.

The discussion here affects only the JSON API contrib module. I said that a site owner can break their site in many different ways, and I didn't only mean via the UI or API, but also not making backups, misconfiguring their webserver, creating a custom resource that exposes internal APIs, committing faulty files, … There is a line where we (Drupal maintainers) have to stop the hand-holding, that's what I'm debating here. Something very different would be if non-privileged users could break your site, which is not the case because the entity access it ensured here.

This sounds like a great compromise for me!

@Wim Leers I am good with the approach. Although that may or may not be easy to do with the current tool set in jsonapi_extras.

How do we deal with synchronized updates? Also, this will require a BC break, no tip toeing like with the normalizers lockdown.

Re #21, Why not create a new jsonapi_config_entities module rather than putting it into the existing extras module? JSON API Extras already seems to wear a lot of disparate hats :)

JSON API Extras already seems to wear a lot of disparate hats

I think that's totally OK. You seem to disagree. Care to elaborate a bit on that?

Just because it leads to weird modules like ctools which don't really do what they are named and add a little bit of confusion for people. Also, maybe a site wants to be able to interact with config entities via the API but doesn't want the other features included in JSON API Extras.

Naming is hard, but don't let me start a bikeshed :) Big +1 to the general idea of moving into a separate (sub)module - whatever it may be.

Awesome! Thanks for the clarification @balsama.

When trying to add the ability to save role that have updated permissions (for the decoupled admin interface), JSON API clearly fails,

Drupal\jsonapi\Controller\EntityResource->createIndividual(Object, Object)
call_user_func_array(Array, Array) (Line: 89)
Drupal\jsonapi\Controller\RequestHandler->Drupal\jsonapi\Controller\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 90)
Drupal\jsonapi\Controller\RequestHandler->handle(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 40)
Drupal\jsonapi\StackMiddleware\FormatSetter->handle(Object, 1, 1) (Line: 49)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Re: is somebody actually using this? And if so, how/with which entity type?

While it wasn't something we supported, Lightning API did have an Entity CRUD test that tested creating and updating a taxonomy vocab config entity:
https://cgit.drupalcode.org/lightning_api/tree/modules/api_test/tests/sr...

...which we removed after updating to JSON API 1.10.

We should create an issue to cleanly separate these features to a separate module. I'm keeping this issue as is to help conversation going. See #2957474: Move the write functionality of config entities to a sub-module in preparation for removal

@drpal can you please create a separate ticket for that issue?