Skip to content

fix: traffic split plugin not validating upstream_id#10008

Merged
moonming merged 8 commits into
apache:masterfrom
shreemaan-abhishek:fix/tfsp
Aug 17, 2023
Merged

fix: traffic split plugin not validating upstream_id#10008
moonming merged 8 commits into
apache:masterfrom
shreemaan-abhishek:fix/tfsp

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

Fixes #9996

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

Comment thread apisix/plugins/traffic-split.lua Outdated
Comment thread t/plugin/traffic-split.t
@jiangfucheng

Copy link
Copy Markdown
Member

@shreemaan-abhishek Please take a look the failed CI.

@moonming
moonming merged commit 790d322 into apache:master Aug 17, 2023
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Aug 22, 2023
* upstream/master: (77 commits)
  docs: Update admin-api.md (apache#10056)
  ci: fix a bug that can not open nginx.pid (apache#10061)
  feat: remove rust dependency by rollback lua-resty-ldap on master (apache#9936)
  docs: fix grpc-transcode.md error (apache#10059)
  feat: upgrade lua dependencies (apache#10051)
  fix: rollback lua-resty-session to 3.10 (apache#10046)
  feat: upgrade resty-redis-cluster from  1.02-4->1.05-1 (apache#10041)
  feat: update lua library (apache#10037)
  fix: worker not exited when executing quit or reload command (apache#9909)
  fix: traffic split plugin not validating upstream_id (apache#10008)
  ci: update the timeout value in cli.yml (apache#10026)
  fix(tencent-cloud-cls): DNS parsing failure (apache#9843)
  chore(deps): bump actions/setup-node from 3.7.0 to 3.8.0 (apache#10025)
  feat(openid-connect): add proxy_opts attribute (apache#9948)
  perf(log-rotate): replace string.sub with string.byte (apache#9984)
  fix(ci): replace github action in update-labels.yml (apache#9987)
  fix: can't sync etcd data if key has special character (apache#9967)
  perf(aws-lambda): cache the index of the array (apache#9944)
  fix: add support for dependency installation on endeavouros (apache#9985)
  chore(ci): automate management of unresponded issues (apache#9927)
  ...
@Nobilta

Nobilta commented Sep 12, 2023

Copy link
Copy Markdown

@shreemaan-abhishek @jiangfucheng Hi,everyone,when i test this function in apisix3.5,this error is still exist
image

@Nobilta

Nobilta commented Sep 12, 2023

Copy link
Copy Markdown

@shreemaan-abhishek @jiangfucheng Hi,everyone,when i test this function in apisix3.5,this error is still exist

image

In both the patch and put methods

image

@Nobilta

Nobilta commented Sep 12, 2023

Copy link
Copy Markdown

@shreemaan-abhishek @jiangfucheng we can modify check_schema function in traffic-split.lua and add the validating function.
image

function _M.check_schema(conf)
    local ok, err = core.schema.check(schema, conf)
    if not ok then
        return false, err
    end
    if conf.rules then
        for _, rule in ipairs(conf.rules) do
            if rule.match then
                for _, m in ipairs(rule.match) do
                    local ok, err = expr.new(m.vars)
                    if not ok then
                        return false, "failed to validate the 'vars' expression: " .. err
                    end
                end
            end
            if rule.weighted_upstreams then
                for _, wupstream in ipairs(rule.weighted_upstreams) do
                    local ups_id = wupstream.upstream_id
                    if ups_id then
                        local ups = upstream.get_by_id(ups_id)
                        core.log.error("upstream:", ups)
                        if not ups then
                            return false, "failed to fetch upstream info by "
                                        .. "upstream id: " .. ups_id
                        end
                    end
                end
            end
        end
    end
    return true
end

@shreemaan-abhishek
shreemaan-abhishek deleted the fix/tfsp branch September 18, 2023 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: traffic-split plugin does not verify the UpstreamID is valid

4 participants