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

feat(llm): add auth.allow_override option for llm auth functionality #13493

Merged
merged 8 commits into from
Aug 15, 2024

Conversation

oowl
Copy link
Member

@oowl oowl commented Aug 12, 2024

Summary

Add allow_override option to allow overriding the upstream model auth parameter or header from the caller's request.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

/AG-92

kong/llm/schemas/init.lua Outdated Show resolved Hide resolved
Copy link
Contributor

@outsinre outsinre left a comment

Choose a reason for hiding this comment

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

No test?

Seems if this vale is true, means auth info from the request is used, and ignore configured auth?

kong/llm/schemas/init.lua Outdated Show resolved Hide resolved
kong/llm/schemas/init.lua Outdated Show resolved Hide resolved
@team-eng-enablement team-eng-enablement added author/community PRs from the open-source community (not Kong Inc) and removed author/community PRs from the open-source community (not Kong Inc) labels Aug 13, 2024
@oowl oowl changed the title feat(llm): add auth.can_override option for llm auth functionality feat(llm): add auth.allow_override option for llm auth functionality Aug 14, 2024
@oowl oowl added the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Aug 14, 2024
Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

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

I would name it "override_request_auth" as we are overriding the request. But anyway LGTM.

@StarlightIbuki
Copy link
Contributor

The failing test seems relevant

@tysoekong
Copy link
Contributor

tysoekong commented Aug 14, 2024

I think this works fine! But 1 failing test with random 500.

@oowl oowl removed the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Aug 15, 2024
@ms2008 ms2008 merged commit 700b3b0 into master Aug 15, 2024
29 checks passed
@ms2008 ms2008 deleted the feat/ai-proxy-overide branch August 15, 2024 03:06
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-13493-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-13493-to-master-to-upstream
git checkout -b cherry-pick-13493-to-master-to-upstream
ancref=$(git merge-base 9bc3deb30041dbc45b1fe44fe8069f21ab69caaa 1b9314d6cd28e2fca660f3db625619b9f6032e5e)
git cherry-pick -x $ancref..1b9314d6cd28e2fca660f3db625619b9f6032e5e

@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Aug 15, 2024
@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Aug 27, 2024
curiositycasualty pushed a commit that referenced this pull request Oct 15, 2024
…13493)

Add `allow_override` option to allow overriding the upstream model auth
parameter or header from the caller's request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.