Allow models to specifiy conflict keys

[poelzi]

Summary

The upsert DAO function generated uses the primary key as an "ON CONFLICT" match. This might not be the case if a field uses a unique flag or if that field is also workgroupable. Postgresql requires an index to be specified.

I tried autodetecting the correct behavior but it failed on other models. Changing the primary key also did not work. I added a way to specify the conflict index.
It defaults to the primary key.

Checklist

• The Pull Request has tests
• A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #12288

[bungle]

This feels like a generic issue with workspaces, if this is true?

[poelzi]

Yes. And it kind of depends how the model is used. I tried to autodetect this by setting the unique_key from the first unique index encountered that has a workspace setting. This unfortunately failed on other plugins but for the key_auth it is the correct way to handle it.
We import a fixture when kong starts that imports a basic ruleset. Reimporting always fails on key_auth since the workspace key tuple is the important key. I tried fixing it by using this pair as primary key, but this caused many other problems.

[bungle]

Yes. And it kind of depends how the model is used. I tried to autodetect this by setting the unique_key from the first unique index encountered that has a workspace setting. This unfortunately failed on other plugins but for the key_auth it is the correct way to handle it. We import a fixture when kong starts that imports a basic ruleset. Reimporting always fails on key_auth since the workspace key tuple is the important key. I tried fixing it by using this pair as primary key, but this caused many other problems.

Thanks for proposal. I did spend some time to figure out a generic fix, but didn't yet find anything.

Did you try already the (or something similar to it):

conflict_list(use_ws_id, pk_escaped)

I feel we need to have the conflict_list called on every ON CONFLICT clause. I can also take a look at it. Seems quite complicated.

[poelzi]

I think the problem is here:

local conflict_key = unique_escaped
  if has_composite_cache_key and not unique_field.is_endpoint_key then
    conflict_key = escape_identifier(connector, "cache_key")
  end

In the key-auth plugin, "key" is the cache_key and also the endpoint_key.
In that case, conflict_key is not set to the tuple.
I don't understand enough of the DAO and the implications it has to change that. Should the plugin just not be the endpoint_key ?

[hanshuebner]

This PR will prevent upserts from working when trying to insert an entity with an existing ID, but changes in the columns the are in the secondary unique index. The conflict on the ID will no longer be detected and the insert will then fail. Given that INSERT only allows one ON CONFLICT clause, there seems to be no straightforward generic solution.

It may still be possible to handle some problem using triggers, but the general recommendation to solve the specific problem at hand (failing imports using kong config db_import) is to specify the IDs of the entities in the input file. That way, the existing ON CONFLICT clauses will trigger when the same file is imported multiple times.

[poelzi]

I tried to understand your reasoning and looked at the code again but I can't. The current implementation is broken at least since 3.4 or 3.2 on a workflow that worked before.

This is a example config:

_format_version: "1.1"

services:
- name: admin-api
  url: http://localhost:8002
  plugins:
  - name: acl
    config:
      allow:
      - admin-api-group
      hide_groups_header: true
    tags:
    - admin-api
  tags:
  - admin-api
  routes:
  - name: admin-api
    hosts:
    - admin-api
    paths:
    - /admin-api
    plugins:
    - name: key-auth
      config:
        hide_credentials: true
      tags:
      - admin-api
    tags:
    - admin-api

consumers:
- username: admin-api-user
  acls:
  - group: admin-api-group
  keyauth_credentials:
  - key: very-secret-key

Now, second run of the migration yields:

kong-database-1       | 2024-03-28 13:45:14.873 UTC [43] ERROR:  duplicate key value violates unique constraint "keyauth_credentials_ws_id_key_unique"
kong-database-1       | 2024-03-28 13:45:14.873 UTC [43] DETAIL:  Key (ws_id, key)=(856b7296-9393-4f3c-b36f-27c4412ba4dc, very-secret-key) already exists.
kong-database-1       | 2024-03-28 13:45:14.873 UTC [43] STATEMENT:  INSERT INTO "keyauth_credentials" ("id", "created_at", "consumer_id", "key", "tags", "ws_id", "ttl")
kong-database-1       |              VALUES ('efcedf92-b8ec-4205-84bc-2beaef24d09c', TO_TIMESTAMP(1711633514) AT TIME ZONE 'UTC', 'b703da88-a6ae-5c0e-b775-0643b04b74df', 'very-secret-key', NULL, '856b7296-9393-4f3c-b36f-27c4412ba4dc', NULL)
kong-database-1       |         ON CONFLICT ("id") DO UPDATE
kong-database-1       |                 SET "created_at" = EXCLUDED."created_at", "consumer_id" = EXCLUDED."consumer_id", "key" = EXCLUDED."key", "tags" = EXCLUDED."tags", "ws_id" = EXCLUDED."ws_id", "ttl" = EXCLUDED."ttl"
kong-database-1       |           RETURNING "id", EXTRACT(EPOCH FROM "created_at" AT TIME ZONE 'UTC') AS "created_at", "consumer_id", "key", "tags", "ws_id",FLOOR(EXTRACT(EPOCH FROM ("ttl" AT TIME ZONE 'UTC' - CURRENT_TIMESTAMP AT TIME ZONE 'UTC'))) AS "ttl";

3rd run:

kong-database-1       | 2024-03-28 14:55:17.931 UTC [42] ERROR:  duplicate key value violates unique constraint "keyauth_credentials_ws_id_key_unique"
kong-database-1       | 2024-03-28 14:55:17.931 UTC [42] DETAIL:  Key (ws_id, key)=(856b7296-9393-4f3c-b36f-27c4412ba4dc, very-secret-key) already exists.
kong-database-1       | 2024-03-28 14:55:17.931 UTC [42] STATEMENT:  INSERT INTO "keyauth_credentials" ("id", "created_at", "consumer_id", "key", "tags", "ws_id", "ttl")
kong-database-1       |              VALUES ('c0d42d3e-b9d7-40dc-ad56-4097e5f40754', TO_TIMESTAMP(1711637717) AT TIME ZONE 'UTC', 'b703da88-a6ae-5c0e-b775-0643b04b74df', 'very-secret-key', NULL, '856b7296-9393-4f3c-b36f-27c4412ba4dc', NULL)
kong-database-1       |         ON CONFLICT ("id") DO UPDATE
kong-database-1       |                 SET "created_at" = EXCLUDED."created_at", "consumer_id" = EXCLUDED."consumer_id", "key" = EXCLUDED."key", "tags" = EXCLUDED."tags", "ws_id" = EXCLUDED."ws_id", "ttl" = EXCLUDED."ttl"
kong-database-1       |           RETURNING "id", EXTRACT(EPOCH FROM "created_at" AT TIME ZONE 'UTC') AS "created_at", "consumer_id", "key", "tags", "ws_id",FLOOR(EXTRACT(EPOCH FROM ("ttl" AT TIME ZONE 'UTC' - CURRENT_TIMESTAMP AT TIME ZONE 'UTC'))) AS "ttl";
kong-init-config-1-1  | Error: 
kong-init-config-1-1  | /usr/local/share/lua/5.1/kong/cmd/config.lua:116: Failed importing:
kong-init-config-1-1  | [postgres] UNIQUE violation detected on '{key="very-secret-key"}'
kong-init-config-1-1  | stack traceback:
kong-init-config-1-1  |         [C]: in function 'error'
kong-init-config-1-1  |         /usr/local/share/lua/5.1/kong/cmd/config.lua:116: in function 'cmd_exec'
kong-init-config-1-1  |         /usr/local/share/lua/5.1/kong/cmd/init.lua:31: in function </usr/local/share/lua/5.1/kong/cmd/init.lua:31>
kong-init-config-1-1  |         [C]: in function 'xpcall'
kong-init-config-1-1  |         /usr/local/share/lua/5.1/kong/cmd/init.lua:31: in function </usr/local/share/lua/5.1/kong/cmd/init.lua:15>
kong-init-config-1-1  |         (command line -e):7: in function 'inline_gen'
kong-init-config-1-1  |         init_worker_by_lua:44: in function <init_worker_by_lua:43>
kong-init-config-1-1  |         [C]: in function 'xpcall'
kong-init-config-1-1  |         init_worker_by_lua:52: in function <init_worker_by_lua:50>
kong-init-config-1-1  | 2024/03/28 14:55:17 [info] parse successful, beginning import
kong-1                | 2024/03/28 14:55:18 [warn] 1#0: the "user" directive makes sense only if the master process runs with super-user privileges, ignored in /usr/local/kong/nginx.conf:7
kong-1                | nginx: [warn] the "user" directive makes sense only if the master process runs with super-user privileges, ignored in /usr/local/kong/nginx.conf:7
kong-init-config-1-1 exited with code 0

The ID is clearly autogenerated and the important unique key is (ws_id, key) which yields this error.
In my patch I explicitly made this behavior plugin independent and the plugin must decide the conflict key. I only added it tho the keyauth plugin because I don't know other plugins.
It is clearly broken as shown in #12288 which we still suffer from.

[poelzi]

ping @hanshuebner

[hanshuebner]

ping @hanshuebner

Why can't you pre-generate the IDs for all entities in your configuration file?