migrated all code to envoyproxy/envoy and keep e2e only here

[wbpcode]

What this PR does / why we need it:

Now, all source code will be hold by the envoyproxy/envoy.

The are mainly two points I want to address by this change:

• For Istio self,to avoid constant merging and fixing when upgrading upstream Envoy. There's no stable ABI, so it can be breaking frequently.
• For users/developers who care Envoy based mesh and gateway, they now could follow the unified singe repo for data plane.

There are only 3-4 extensions in this repo and migrating them to upstream's repo have no any effect to our users or our CI/release procedures because extensions's position doesn't affect the compiled result.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

[istio-policy-bot]

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

[istio-policy-bot]

😊 Welcome @wbpcode! This is either your first contribution to the Istio proxy repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[istio-testing]

Hi @wbpcode. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[wbpcode]

Waiting #6726 to be merged first because we need to ensure the CI works fine before this cleanup.

[wbpcode]

cc @kyessenov cc @zirain

[zirain]

/ok-to-test

[keithmattix]

Thanks for the PR. This is a pretty big change that I hadn’t been made aware of. We definitely will need some more discussion here

[wbpcode]

Thanks for the PR. This is a pretty big change that I hadn’t been made aware of. We definitely will need some more discussion here

Sure. This change comes from envoyproxy/envoy#29681 to free us/distributors out from constant merge and CI problems.

And at envoyproxy/envoy#42785, we have migrated all proxy extensions to envoy repo, without change any logic or API. (We updated Envoy's tool to make sure it could accept istio's API style.)

This PR try to build proxy with code in the Envoy repo and remove the code that have been migrated.

Ideally, there should no any aware-able change for users.

[wbpcode]

/retest

[kyessenov]

is this making some build changes?
CC @krinkinmu

[krinkinmu]

I'm probably overlooking something, but calling envoy_toolchains() + llvm_register_toolchains() should be enough here as far as I understand.

FWIW, I did the same in my PR migrating to hermetic toolchain, but I think it was just an artifact from my experiments that I didn't cleanup.

[kyessenov]

SG, I think it would be good to add some other people to contrib/istio owners in Envoy (@keithmattix ?). I don't think the location of the code matters that much.

[wbpcode]

SG, I think it would be good to add some other people to contrib/istio owners in Envoy (@keithmattix ?). I don't think the location of the code matters that much.

Sure. I will update the code owner list.

[keithmattix]

Need a rebase here and need to discuss again with stakeholders. We talked about it in last week's community call, and I think the biggest questions were around the implications of having istio code owned by those who aren't istio maintainers plus having different degrees of testing in different places (istio/proxy, envoy, and istio/istio)

[zirain]

Need a rebase here and need to discuss again with stakeholders. We talked about it in last week's community call, and I think the biggest questions were around the implications of having istio code owned by those who aren't istio maintainers plus having different degrees of testing in different places (istio/proxy, envoy, and istio/istio)

having istio code owned by those who aren't istio maintainers

I think we already add all the guys from E&T WG into codeowners in envoy group.

having different degrees of testing in different places

I would say new feature and new test cases will be added in envoy repo, if there're some addtional e2e test case, it could be happen here.

[wbpcode]

/retest

[keithmattix]

Please let's bring this up one last time in a community meeting before moving forward

[zirain]

Please let's bring this up one last time in a community meeting before moving forward

don't worry, there's a DNM, it won't be merged.

[istio-testing]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[keithmattix]

FYI, we discussed this in the latest WG meeting; I captured the high level points here: https://docs.google.com/document/d/1wsa06GGiq1LEGwhkiPP0FKIZJqdAiue-VeBonWAzAyk/edit?pli=1&tab=t.0#heading=h.gp28tf83n5tk

[wbpcode]

Consensus: need to answer if the problem we’re solving is worth it, but generally felt like folks were on board if the integration tests could be ported to envoy as well

cc @keithmattix

Although I think even we never move the e2e, this still make sense to simplify our and distributors' life. But sure I completely agree it would make more sense to avoid all these e2e here.

But I think we may needn't to do this in one punch? Rather than the migration, my plan is keep the e2e here, and at next 1-2 release cycle, we will add more unit/integration tests in the Envoy, to ensure all these filters in the Envoy could have similar test quality with other core filters (96.6% coverage).
Then, another 1-2 release cycles, we can remove all e2e tests here also?

By this way, this whole process will be more smoothy and we can have more confidence to the change?

[dhawton]

Consensus: need to answer if the problem we’re solving is worth it, but generally felt like folks were on board if the integration tests could be ported to envoy as well

cc @keithmattix

Although I think even we never move the e2e, this still make sense to simplify our and distributors' life. But sure I completely agree it would make more sense to avoid all these e2e here.

But I think we may needn't to do this in one punch? Rather than the migration, my plan is keep the e2e here, and at next 1-2 release cycle, we will add more unit/integration tests in the Envoy, to ensure all these filters in the Envoy could have similar test quality with other core filters (96.6% coverage). Then, another 1-2 release cycles, we can remove all e2e tests here also?

By this way, this whole process will be more smoothy and we can have more confidence to the change?

But this doesn't really identify the problem, at least for me. What problem are we solving?

From what John said, the original intent on this change was a step toward the effort to migrate to using a straight Envoy binary, but that is no longer the plan.

[keithmattix]

this still make sense to simplify our and distributors' life.

This feels like the problem statement; can we get more details on how this makes things simpler? And are there other benefits?

[wbpcode]

cc @dhawton

Hello, @dhawton, What problem are we solving? This problem actually has been discussed before in this PR. So I focused on the e2e.

But maybe there is too much context and I should put related information at the PR description.

The are mainly two points I want to address:

• For Istio self,to avoid constant merging and fixing when upgrading upstream Envoy. There's no stable ABI, so it can be breaking frequently.
• For users/developers who care Envoy based mesh and gateway, they now could follow the unified singe repo for data plane.

There are only 3-4 extensions in this repo and migrating them to upstream's repo have no any effect to our users or our CI/release procedures because extensions's position doesn't affect the compiled result.

This change could make the cooperation a little more smoothy and have no side effect. So, from my perspective, it's more like a why not?

[wbpcode]

this still make sense to simplify our and distributors' life.

This feels like the problem statement; can we get more details on how this makes things simpler? And are there other benefits?

Hello, @keithmattix , sorry I missed your comment before. But I think the #6748 (comment) also responded your question, hope that make sense for you.

Is there anything still block this change? If it make sense, we actually need to remigrate recent change to Envoy repo and resolve all these conflict.

[keithmattix]

This change could make the cooperation a little more smoothy and have no side effect. So, from my perspective, it's more like a why not?

I think the hesitation I have is that there is a side effect as it stands today: the code and the tests get separated. That's why where I am is that I'm ok with the move if and only if e2e tests move too. I totally agree with the benefits, but we need to weigh the costs

[zirain]

This change could make the cooperation a little more smoothy and have no side effect. So, from my perspective, it's more like a why not?

I think the hesitation I have is that there is a side effect as it stands today: the code and the tests get separated. That's why where I am is that I'm ok with the move if and only if e2e tests move too. I totally agree with the benefits, but we need to weigh the costs

I think the migration of all the e2e tests is impossible today, and I don't think anyone have bandwidth to do it.
It sounds good to me that we keep all the existing e2e here, and new features will require UT and e2e on envoy usptream.
do this make sense to you?

If not, I think the best choice is keep it as today, and we maybe need to remove all the code from envoy upstream to make user not too confuse.

[wbpcode]

cc @keithmattix

But I think we may needn't to do this in one punch? Rather than the migration, my plan is 
keep the e2e here, 
and at next 1-2 release cycle, we will add more unit/integration tests in the Envoy, to ensure
all these filters
in the Envoy could have similar test quality with other core filters (96.6% coverage).
Then, another 1-2 release cycles, we can remove all e2e tests here also?

is this way make sense for you? Keep the e2e here for a while and finally, all these e2e tests will be replaced by the unit tests and integration tests in the Envoy. Migrating the e2e self is impossible for Envoy because we have no plan to accept another testing system.

If we finally could done it. All distributors and developers could get a unique place to follow data plane source code. And data plane upgrading, CVE fixing for Istio would be easier.

[keithmattix]

To be clear: we don't have to use the current testing framework; we just need to cover the same functionality

[wbpcode]

To be clear: we don't have to use the current testing framework; we just need to cover the same functionality

This sound possible. And if you want all these tests be there before migration. I can give it a try.

[krinkinmu]

This sound possible. And if you want all these tests be there before migration. I can give it a try.

For me at least it does not matter if we will have the same tests coverage right from the start in the Envoy repo, but I'd like us to agree on a feasible path forward. The plan of phasing out the current tests and replacing them with equivalent tests in the envoy repo sounds ok to me.

I'd also be fine with patching in Istio CI into the Envoy CI for Istio filters and not migrating the tests, though I'm not sure if that's possible currently.

[wbpcode]

I'd also be fine with patching in Istio CI into the Envoy CI for Istio filters and not migrating the tests, though I'm not sure if that's possible currently.

This is a little hard I think 🤔

For me at least it does not matter if we will have the same tests coverage right from the start in the Envoy repo, but I'd like us to agree on a feasible path forward. The plan of phasing out the current tests and replacing them with equivalent tests in the envoy repo sounds ok to me.

Thanks very much. I also prefer gradually replacement of the tests. And I think even we have added same tests in Envoy, the e2e here still should be kept for one or tow release cycles to ensure nothing is broken.

Anyway, I can try to add more tests first to address @keithmattix 's concern.

[keithmattix]

If we could start with just a POC of a couple of tests to prove it can be done, I think that would be all I need personally.

@dhawton @jaellio @howardjohn from the product security side, would this help alleviate some concern? I think from the security side, we could always fix with patches right (since I don't think contrib is under envoy security)