Skip to content

Commit

Permalink
fix(scheme): deprecated shorthand fields precedence
Browse files Browse the repository at this point in the history
### Summary

The shorthand fields e.g. as used in (the url is a shorthand) take
precedence:
```
http PUT :8001/services/test url=http://test.org/ port=2345
```

That means that `port=2345` will be overwritten by http default
port of `80` that is implicit in shorthand field `url`.

This is how it has been for a long long time.

In PR #12686 we added `deprecation` field to shorthand fields definition.

Some of our automatic migrations without database migrations magic use
this functionality, but the precedence there does not good:

```
http -f PUT :8001/plugins/x name=x config.new=test config.old=should_be_ignored
```

In above the `config.old` is a shorthand field with a deprecation property.
Thus it should not overwrite the `config.new`, but in current code base it
does. This PR changes it so that it doesn't do it anymore.

KAG-5134 and https://kongstrong.slack.com/archives/C07AQH7SAF8/p1722589141558609

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
  • Loading branch information
bungle committed Aug 9, 2024
1 parent d4d73dc commit 711b888
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 12 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/fix-deprecate-shorthands-precedence.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Deprecated shorthand fields don't take precedence over replacement fields when both are specified.
type: bugfix
scope: Core
2 changes: 0 additions & 2 deletions kong/api/routes/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ local function patch_plugin(self, db, _, parent)
return kong.response.exit(404, { message = "Not found" })
end

plugin = plugin or {}

post.name = post.name or plugin.name

-- Only now we can decode the 'config' table for form-encoded values
Expand Down
8 changes: 0 additions & 8 deletions kong/db/dao/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,10 @@ end


function Plugins:update(primary_key, entity, options)
options = options or {}
options.hide_shorthands = true
local rbw_entity = self.super.select(self, primary_key, options) -- ignore errors
if rbw_entity then
entity = self.schema:merge_values(entity, rbw_entity)
end
local ok, err, err_t = check_protocols_match(self, entity)
if not ok then
return nil, err, err_t
end

options.hide_shorthands = false
return self.super.update(self, primary_key, entity, options)
end

Expand Down
6 changes: 4 additions & 2 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1743,10 +1743,12 @@ function Schema:process_auto_fields(data, context, nulls, opts)
data[sname] = nil
local new_values = sdata.func(value)
if new_values then
local deprecation = sdata.deprecation
for k, v in pairs(new_values) do
if type(v) == "table" then
data[k] = table_merge(data[k] or {}, v)
else
data[k] = deprecation and table_merge(v, data[k])
or table_merge(data[k] or {}, v)
elseif not deprecation or data[k] == nil then
data[k] = v
end
end
Expand Down
54 changes: 54 additions & 0 deletions spec/01-unit/01-db/01-schema/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4110,6 +4110,60 @@ describe("schema", function()
assert.same({ name = "test1" }, output)
end)

it("takes precedence", function()
local TestSchema = Schema.new({
name = "test",
fields = {
{ name = { type = "string" } },
},
shorthand_fields = {
{
username = {
type = "string",
func = function(value)
return {
name = value
}
end,
},
},
},
})

local input = { username = "test1", name = "ignored" }
local output, _ = TestSchema:process_auto_fields(input)
assert.same({ name = "test1" }, output)
end)

it("does not take precedence if deprecated", function()
local TestSchema = Schema.new({
name = "test",
fields = {
{ name = { type = "string" } },
},
shorthand_fields = {
{
username = {
type = "string",
func = function(value)
return {
name = value
}
end,
deprecation = {
message = "username is deprecated, please use name instead",
removal_in_version = "4.0",
},
},
},
},
})

local input = { username = "ignored", name = "test1" }
local output, _ = TestSchema:process_auto_fields(input)
assert.same({ name = "test1" }, output)
end)

it("can produce multiple fields", function()
local TestSchema = Schema.new({
name = "test",
Expand Down

0 comments on commit 711b888

Please sign in to comment.