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.

Sign up for GitHub

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

Jump to bottom

feat(fault-injection): support conditional fault injection using nginx variables #3363

Merged
merged 8 commits into from
Jan 25, 2021
Merged

feat(fault-injection): support conditional fault injection using nginx variables #3363

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7f29e97
feat(fault-injection): support conditional fault injection using Ngin…
Firstsawyou Jan 20, 2021
7027c36
update code and docs.
Firstsawyou Jan 22, 2021
e7f6293
Merge branch 'up' into fault-injection-support-vars
Firstsawyou Jan 22, 2021
8278a2d
add test ceses and fix ci faild.
Firstsawyou Jan 23, 2021
1f04d58
fix review.
Firstsawyou Jan 25, 2021
86cee2e
update docs.
Firstsawyou Jan 25, 2021
8ba5c70
update code.
Firstsawyou Jan 25, 2021
6c912b5
fix ci error.
Firstsawyou Jan 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 97 additions & 4 deletions apisix/plugins/fault-injection.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,66 @@
-- limitations under the License.
--
local core = require("apisix.core")
local expr = require("resty.expr.v1")

local sleep = core.sleep
local random = math.random
local ipairs = ipairs

local plugin_name = "fault-injection"


local vars_schema = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to remove the vars_schema.

  1. we can do the check better with expr.new when checking the schema.
  2. when not/or is supported directly in the expr, this schema is outdated.
  3. this schema contains many incorrect hardcoded limitations, like the maxLength of operator is 2 (not enough for has), and an expression may contain 4 elements if ! is used, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you don't remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just keep:

local vars_schema = {
    type = "array",
    maxItems = 20
}

is enough. The remain can be validated via expr.new.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let me try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

type = "array",
items = {
type = "array",
items = {
type = "array",
items = {
{
type = "string",
minLength = 1,
maxLength = 100
},
{
type = "string",
minLength = 1,
maxLength = 2
}
},
additionalItems = {
anyOf = {
{type = "string"},
{type = "number"},
{type = "boolean"},
{
type = "array",
items = {
anyOf = {
{
type = "string",
minLength = 1, maxLength = 100
},
{
type = "number"
},
{
type = "boolean"
}
}
},
uniqueItems = true
}
}
},
minItems = 0,
maxItems = 10
}
},
maxItems = 20
}


local schema = {
type = "object",
properties = {
Expand All @@ -30,15 +83,17 @@ local schema = {
properties = {
http_status = {type = "integer", minimum = 200},
body = {type = "string", minLength = 0},
percentage = {type = "integer", minimum = 0, maximum = 100}
percentage = {type = "integer", minimum = 0, maximum = 100},
vars = vars_schema
},
required = {"http_status"},
},
delay = {
type = "object",
properties = {
duration = {type = "number", minimum = 0},
percentage = {type = "integer", minimum = 0, maximum = 100}
percentage = {type = "integer", minimum = 0, maximum = 100},
vars = vars_schema
},
required = {"duration"},
}
Expand All @@ -64,6 +119,25 @@ local function sample_hit(percentage)
end


local function vars_match(vars, ctx)
local match_result
for _, var in ipairs(vars) do
local expr, err = expr.new(var)
if err then
core.log.error("vars expression does not match: ", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be failed to create vars expression, we haven't matched it yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

return nil, err
end

match_result = expr:eval(ctx.var)
if match_result then
break
end
end

return match_result, nil
end


function _M.check_schema(conf)
local ok, err = core.schema.check(schema, conf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to check the expression here and add a test for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean checking the expression via:

        local expr, err = expr.new(var)
        if err then
            core.log.error("failed to create vars expression: ", err)
            return nil, err
        end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added.

if not ok then
Expand All @@ -77,11 +151,30 @@ end
function _M.rewrite(conf, ctx)
core.log.info("plugin rewrite phase, conf: ", core.json.delay_encode(conf))

if conf.delay and sample_hit(conf.delay.percentage) then
local err
local abort_vars = true
if conf.abort and conf.abort.vars then
abort_vars, err = vars_match(conf.abort.vars, ctx)
if err then
return 500, err
Firstsawyou marked this conversation as resolved.
Show resolved Hide resolved
end
end
core.log.info("abort_vars: ", abort_vars)

local delay_vars = true
if conf.delay and conf.delay.vars then
delay_vars, err = vars_match(conf.delay.vars, ctx)
if err then
return 500, err
Firstsawyou marked this conversation as resolved.
Show resolved Hide resolved
end
end
core.log.info("delay_vars: ", delay_vars)

if conf.delay and sample_hit(conf.delay.percentage) and delay_vars then
sleep(conf.delay.duration)
end

if conf.abort and sample_hit(conf.abort.percentage) then
if conf.abort and sample_hit(conf.abort.percentage) and abort_vars then
return conf.abort.http_status, core.utils.resolve_var(conf.abort.body, ctx.var)
end
end
Expand Down
Loading