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: traffic-split plugin support https #9115

Merged
merged 29 commits into from
Sep 27, 2023

Conversation

TenYearsIn
Copy link
Contributor

@TenYearsIn TenYearsIn commented Mar 20, 2023

Description

Fixes #8996

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)

@monkeyDluffy6017
Copy link
Contributor

Hi @TenYearsIn, test cases are needed and please make the ci pass

@TenYearsIn
Copy link
Contributor Author

The Test case (traffic-split2.t) 11 seems that there was some problem, I used the original code to test it, but it still fails

@soulbird
Copy link
Contributor

soulbird commented Apr 3, 2023

Without your modification, I can also run the test case through.
In general, adding new functionality should not change existing test cases.

@shreemaan-abhishek
Copy link
Contributor

@TenYearsIn
Copy link
Contributor Author

@shreemaan-abhishek
Copy link
Contributor

@TenYearsIn, please provide access to me to your forked repository.

@TenYearsIn
Copy link
Contributor Author

@shreemaan-abhishek done

Revolyssup
Revolyssup previously approved these changes Sep 21, 2023
@monkeyDluffy6017 monkeyDluffy6017 changed the title feat: traffic-split plugin can't proxy virtual upstream that schema is https feat: traffic-split plugin support https Sep 27, 2023



=== TEST 21: use upstream with https scheme
Copy link
Contributor

Choose a reason for hiding this comment

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

The test case can pass without your pr

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like using upstream_id supports https but using real upstream is not supported, I have pushed my changes now, please review.

@monkeyDluffy6017 monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label Sep 27, 2023
@monkeyDluffy6017 monkeyDluffy6017 added approved and removed wait for update wait for the author's response in this issue/PR labels Sep 27, 2023
@monkeyDluffy6017 monkeyDluffy6017 merged commit 869754b into apache:master Sep 27, 2023
34 checks passed
Zhenye-Na pushed a commit to Zhenye-Na/apisix that referenced this pull request Oct 1, 2023
Zhenye-Na pushed a commit to Zhenye-Na/apisix that referenced this pull request Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants