Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dbless): nest duplicate entity primary key errors for children #12797

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Mar 28, 2024

summary

This makes a change to the shape/structure of dbless errors. Before this change, an upstream with duplicate targets would yield a response like this:

{
  "code": 14,
  "fields": {
    "targets": [
      null,
      "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared"
    ]
  },
  "message": "declarative config is invalid: {targets={[2]=\"uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared\"}}",
  "name": "invalid declarative configuration"
}

This is bad because the error-flattening code cannot associate this error with the input. The linkage between the upstream->target from the input has been lost.

After this change, the errors are nested under the parent upstream:

{
  "code": 14,
  "fields": {
    "upstreams": [
      null,
      {
        "targets": [
          null,
          "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared"
        ]
      }
    ]
  },
  "message": "declarative config is invalid: {upstreams={[2]={targets={[2]=\"uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared\"}}}}",
  "name": "invalid declarative configuration"
}

As a result, the error can now be properly mapped to the input target ($input.upstreams[2].targets[2]), so POST /config?flatten_errors=1 returns a helpful result, including the [generated] target ID and the upstream ID:

{
  "code": 14,
  "fields": {},
  "flattened_errors": [
    {
      "entity": {
        "id": "48322e4a-b3b0-591b-8ed6-fd95a6d75019",
        "tags": [
          "target-2"
        ],
        "target": "10.244.0.12:80",
        "upstream": {
          "id": "f9792964-6797-482c-bfdf-08220a4f6839"
        },
        "weight": 1
      },
      "entity_id": "48322e4a-b3b0-591b-8ed6-fd95a6d75019",
      "entity_tags": [
        "target-2"
      ],
      "entity_type": "target",
      "errors": [
        {
          "message": "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared",
          "type": "entity"
        }
      ]
    }
  ],
  "message": "declarative config is invalid: {}",
  "name": "invalid declarative configuration"
}

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong

Issue reference

KAG-4144

@github-actions github-actions bot added core/db schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Mar 28, 2024
@flrgh flrgh force-pushed the fix/declarative-nested-unique-error branch 5 times, most recently from 723e333 to ca38510 Compare April 4, 2024 20:54
@flrgh flrgh added this to the 3.7.0 milestone Apr 9, 2024
@flrgh
Copy link
Contributor Author

flrgh commented Apr 9, 2024

This fixes some buggy behavior negatively impacting Konnect/KIC, so although unplanned I think we should prioritize merging it for 3.7.

This makes a change to the shape/structure of dbless errors. Before this
change, an upstream with duplicate targets would yield a response like
this:

```
{
  "code": 14,
  "fields": {
    "targets": [
      null,
      "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared"
    ]
  },
  "message": "declarative config is invalid: {targets={[2]=\"uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared\"}}",
  "name": "invalid declarative configuration"
}
```

After this change, the errors are nested under the parent upstream:

```
{
  "code": 14,
  "fields": {
    "upstreams": [
      null,
      {
        "targets": [
          null,
          "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared"
        ]
      }
    ]
  },
  "message": "declarative config is invalid: {upstreams={[2]={targets={[2]=\"uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared\"}}}}",
  "name": "invalid declarative configuration"
}
```

As a result, the error can now be properly mapped to the input target,
so `POST /config?flatten_errors=1` returns a helpful result:

```
{
  "code": 14,
  "fields": {},
  "flattened_errors": [
    {
      "entity": {
        "id": "48322e4a-b3b0-591b-8ed6-fd95a6d75019",
        "tags": [
          "target-2"
        ],
        "target": "10.244.0.12:80",
        "upstream": {
          "id": "f9792964-6797-482c-bfdf-08220a4f6839"
        },
        "weight": 1
      },
      "entity_id": "48322e4a-b3b0-591b-8ed6-fd95a6d75019",
      "entity_tags": [
        "target-2"
      ],
      "entity_type": "target",
      "errors": [
        {
          "message": "uniqueness violation: 'targets' entity with primary key set to '48322e4a-b3b0-591b-8ed6-fd95a6d75019' already declared",
          "type": "entity"
        }
      ]
    }
  ],
  "message": "declarative config is invalid: {}",
  "name": "invalid declarative configuration"
}
```
@flrgh flrgh force-pushed the fix/declarative-nested-unique-error branch from ca38510 to 518df6d Compare April 10, 2024 16:40
@kikito kikito requested a review from jschmid1 April 23, 2024 09:07
@jschmid1 jschmid1 merged commit b8891eb into master Apr 23, 2024
25 checks passed
@jschmid1 jschmid1 deleted the fix/declarative-nested-unique-error branch April 23, 2024 09:28
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/db schema-change-noteworthy size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants