Kong / kong

🦍 The Cloud-Native API Gateway and AI Gateway.
https://konghq.com/install/#kong-community
Apache License 2.0
38.78k stars 4.77k forks source link

db_import with keyauth_credentials fails on re-run #12288

Closed ahalay closed 5 months ago

ahalay commented 8 months ago

Is there an existing issue for this?

Kong version ($ kong version)

Kong 3.4.0 (Kong 3.3.1 is not affected)

Current Behavior

When I try to run db_import a second time with the same config file, it fails with an error (I use it for initial configuration and run it in k8s init container before further configuration via decK)

errors:

kong container:

Error: Failed importing:
[postgres] UNIQUE violation detected on '{key="very-secret-key"}'

postgresql container:

ERROR:  duplicate key value violates unique constraint "keyauth_credentials_ws_id_key_unique"
DETAIL:  Key (ws_id, key)=(a6a09e1f-9c1f-415d-918f-5384f5cfdd0c, very-secret-key) already exists.
STATEMENT:  INSERT INTO "keyauth_credentials" ("id", "created_at", "consumer_id", "key", "tags", "ws_id", "ttl")
VALUES ('6a309a66-d35e-4c0a-a325-d8911bb32057', TO_TIMESTAMP(1704296692) AT TIME ZONE 'UTC', 'b703da88-a6ae-5c0e-b775-0643b04b74df', 'very-secret-key', NULL, 'a6a09e1f-9c1f-415d-918f-5384f5cfdd0c', NULL)
ON CONFLICT ("id") DO UPDATE
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"
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";

Expected Behavior

Kong version 3.3 allows you to run db_import multiple times with updating existing database records

Steps To Reproduce

Fails on a second run of kong config db_import kong.yaml

kong.yaml:

_format_version: "3.0"
_transform: true

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

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

Anything else?

No response

hanshuebner commented 6 months ago

@ahalay Thank you for reporting this issue. As you have seen in PR #12597, the underlying issue is difficult to fix. We think of db_import primarily as an import tool, with DecK being the solution for updates. As you're using DecK already, can you maybe avoid running the db_import step multiple times or reset the database before you run it?

ahalay commented 6 months ago

Thanks for the info @hanshuebner

Unfortunately this is something we'd really like to avoid, as we haven't found a more convenient way than db_import for our needs. We use it to initially configure route and keyauth for Kong's admin-api without exposing admin port, and then configure everything else through the admin-api route with decK. Dropping records from the database in our case can have unexpected consequences if there are multiple replicas (admin-api could left without keyauth or will not be recreated at all), as we run db_import as an initContainer before Kong's own container for each replica.

hanshuebner commented 6 months ago

Given that a complete solution is not in sight, would it be possible to perform a check on the database to see if it needs to receive its initial payload using db_import in your initContainer?

ahalay commented 6 months ago

Yes, it is possible, but it deprives us of the possibility of updating the value of this keyauth, whose uniqueness the function complains about.

BastiNBG85 commented 6 months ago

we have exactly the same problem. We also use db_import after the automated migration steps in order to overwrite the admin-api route and the admin credentials (this will avoid locking out the user on accidently removing the route / user / api-key). I just wanted to upgrade from 3.3.0 to 3.6.1 and had the same issues. Using deck would not be a solution as we would have a "chicken-and-egg-problem" when something is accidently removed. so this is a kind of bypass ensuring that we always have access to our productive system (we just need to reboot the container and have access again. we have no other access to the commandline in order to change something as this is managed by our oc4 provider).

hanshuebner commented 5 months ago

We'll see how we can address this issue. (KAG-4044)

hanshuebner commented 5 months ago

We have discussed this internally once more and also reviewed #12597. It is unfortunately not straightforward to completely fix the issue, given that PostgreSQL only allows one ON CONFLICT clause and furthermore, IDs of rows that are being inserted are created in Lua land. The latter, however, shows a way how the problem can be avoided: When the IDs of the entities that are inserted are specified, the existing ON CONFLICT clauses will fire and correctly convert the INSERT into an upsert if the entity already exists.

Would you be able to specify the IDs of your entities in your YAML files?

consumers:
- username: admin-api-user
  acls:
  - group: admin-api-group
  keyauth_credentials:
  - id: 01E83305-9489-4232-8E4E-D7DD5FC6901F
    key: very-secret-key
ahalay commented 5 months ago

Yes, I think this option should work, as far as I understand in case an existing record will have a different id it can be safely updated in the database itself before db_import. Thanks for your help.

StarlightIbuki commented 5 months ago

Closing as this is a non-issue and the reporter seems to have a workaround.