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

Schema validation in db-less mode Kong 1.4 #5401

Closed
altexy opened this issue Jan 7, 2020 · 6 comments · Fixed by #5464
Closed

Schema validation in db-less mode Kong 1.4 #5401

altexy opened this issue Jan 7, 2020 · 6 comments · Fixed by #5464
Labels
task/needs-investigation Requires investigation and reproduction before classifying it as a bug or not.

Comments

@altexy
Copy link

altexy commented Jan 7, 2020

Summary

Schema validation silently ignored when starting Kong in db-less mode.

Steps To Reproduce

  1. Write db-less config with any plugin with missed required field.
  2. Start Kong in DB-less mode

Kong 1.3.x failed to start, the error log contains an appropriate message.

Kong 1.4.x silently start, no schema validation logs present.

Additional Details & Logs

  • Kong version official Docker images kong:1.4.x-alpine and kong:1.3.x-alpine
@bungle bungle added the task/needs-investigation Requires investigation and reproduction before classifying it as a bug or not. label Jan 7, 2020
@bungle
Copy link
Member

bungle commented Jan 7, 2020

@altexy, thanks for reporting. Possibly a bug.

@bungle
Copy link
Member

bungle commented Jan 11, 2020

@hishamhm, do you have an idea about this?

@bungle bungle added task/bug and removed task/needs-investigation Requires investigation and reproduction before classifying it as a bug or not. labels Jan 14, 2020
bungle added a commit that referenced this issue Jan 14, 2020
### Summary

### Issues Resolved

Fix #5401
bungle added a commit that referenced this issue Jan 15, 2020
### Summary

This was reported with #5401. When I added DAO transformations,
for some reason I ended up writing stupid code that just broke
the whole schema validation on declarative. This commit is an
attempt to fix that. Sorry for all the pain caused.

### Issues Resolved

Fix #5401
bungle added a commit that referenced this issue Jan 17, 2020
### Summary

This was reported with #5401. When I added DAO transformations,
for some reason I ended up writing stupid code that just broke
the whole schema validation on declarative. This commit is an
attempt to fix that. Sorry for all the pain caused.

### Issues Resolved

Fix #5401
bungle added a commit that referenced this issue Jan 20, 2020
### Summary

This was reported with #5401. When I added DAO transformations,
for some reason I ended up writing stupid code that just broke
the whole schema validation on declarative. This commit is an
attempt to fix that. Sorry for all the pain caused.

While fixing this, I also found out that original validation
didn't work correctly when you had entity validations that relied
on values of foreign keys. This is now also corrected.

### Issues Resolved

Fix #5401
hishamhm pushed a commit that referenced this issue Jan 20, 2020
### Summary

This was reported with #5401. The changes for adding DAO transformations ended up breaking schema validation on declarative mode. This commit fixes that.

While fixing this, I also found out that original validation didn't work correctly when you had entity validations that relied on values of foreign keys. This is now also corrected.

### Issues Resolved

Fix #5401
@hbagdi
Copy link
Member

hbagdi commented May 28, 2020

@hbagdi hbagdi reopened this May 28, 2020
@bungle
Copy link
Member

bungle commented Jun 1, 2020

I can confirm that this only happens with reload. So investigating it further.

E.g. start with:

_format_version: "1.1"

services:
  - name: sample_api
    host: kong.test
    port: 4000
KONG_DATABASE=off KONG_DECLARATIVE_CONFIG=b.yaml ./bin/kong start --vv

And you have:

{
    "data": [
        {
            "client_certificate": null,
            "connect_timeout": 60000,
            "created_at": 1590998526,
            "host": "kong.test",
            "id": "f43346f0-16f5-55b3-b815-f4dfa7cdb1e8",
            "name": "sample_api",
            "path": null,
            "port": 4000,
            "protocol": "http",
            "read_timeout": 60000,
            "retries": 5,
            "tags": null,
            "updated_at": 1590998526,
            "write_timeout": 60000
        }
    ],
    "next": null
}

Then

_format_version: "1.1"

services:
  - name: sample_api
    port: 4000
  - name: sample_api2
    host: another.test
    port: 4000
KONG_DATABASE=off KONG_DECLARATIVE_CONFIG=b.yaml ./bin/kong reload --vv

And you have:

{
    "data": [
        {
            "client_certificate": null,
            "connect_timeout": 60000,
            "created_at": 1590998526,
            "host": "kong.test",
            "id": "f43346f0-16f5-55b3-b815-f4dfa7cdb1e8",
            "name": "sample_api",
            "path": null,
            "port": 4000,
            "protocol": "http",
            "read_timeout": 60000,
            "retries": 5,
            "tags": null,
            "updated_at": 1590998526,
            "write_timeout": 60000
        }
    ],
    "next": null
}

Also:

./bin/kong config parse b.yaml
Error: Failed parsing:
in 'services':
  - in entry 1 of 'services':
    in 'host': required field missing

  Run with --v (verbose) or --vv (debug) for more details

So I think that perhaps the reload does not load declarative and you actually need restart to do so? So is this expected or not? @hishamhm?

@bungle bungle added task/needs-investigation Requires investigation and reproduction before classifying it as a bug or not. and removed task/bug labels Jun 1, 2020
@bungle
Copy link
Member

bungle commented Sep 16, 2020

I think it is expected. reload does not honor environment vars it uses the env vars that were used when Kong was started.

@bungle
Copy link
Member

bungle commented Sep 16, 2020

I am closing this now. Please reopen if you have any further info, or need more guidance.

@bungle bungle closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task/needs-investigation Requires investigation and reproduction before classifying it as a bug or not.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants