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: traffic split plugin not validating upstream_id #10008

Merged
merged 8 commits into from
Aug 17, 2023

Conversation

shreemaan-abhishek
Copy link
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)

@jiangfucheng
Copy link
Member

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

@moonming moonming merged commit 790d322 into apache:master Aug 17, 2023
31 checks passed
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
Copy link

Nobilta commented Sep 12, 2023

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

@Nobilta
Copy link

Nobilta commented Sep 12, 2023

@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
Copy link

Nobilta commented Sep 12, 2023

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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