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(kuma-cp): implement MeshTrafficPermisson for ExternalServices with ZoneEgress #7061

Merged
merged 2 commits into from
Jun 20, 2023

Conversation

lukidzi
Copy link
Contributor

@lukidzi lukidzi commented Jun 20, 2023

Checklist prior to review

  • Link to relevant issue as well as docs and UI issues --
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Tests (Unit test, E2E tests, manual test on universal and k8s) --
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? --
  • Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
@lukidzi lukidzi requested review from a team, slonka and jakubdyszkiewicz and removed request for a team June 20, 2023 07:52
@michaelbeaumont
Copy link
Contributor

michaelbeaumont commented Jun 20, 2023

I don't get it, isn't this implementing MeshTrafficPermission for ExternalService (assuming ZoneEgress of course, which I believe is meant to become required)? Isn't that the feature here?

@lukidzi
Copy link
Contributor Author

lukidzi commented Jun 20, 2023

I don't get it, isn't this implementing MeshTrafficPermission for ExternalService (assuming ZoneEgress of course, which I believe is meant to become required)? Isn't that the feature here?

It's more support for ZoneEgress with ExternalService than ExternalService support. If I understand correctly MeshTrafficPermisson is an inbound policy so even if you have ExternalServices but no ZoneEgress policy won't have an effect.

@michaelbeaumont
Copy link
Contributor

Yeah I know it requires ZoneEgress it's just that for a user the feature is "you can use MeshTrafficPermission with ExternalServices". I mean you targetRef the ExternalService, not a ZoneEgress proxy. I just want the changelog to be useful to users

Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
@lobkovilya
Copy link
Contributor

If I understand correctly MeshTrafficPermisson is an inbound policy so even if you have ExternalServices but no ZoneEgress policy won't have an effect.

Speaking of this, have you had a chance to check MeshTrafficPermission with ZoneEgress when there are no TrafficPermissions? We have the issue #6589, but I'm not sure if it affects deployments with ZoneEgress

@lukidzi
Copy link
Contributor Author

lukidzi commented Jun 20, 2023

If I understand correctly MeshTrafficPermisson is an inbound policy so even if you have ExternalServices but no ZoneEgress policy won't have an effect.

Speaking of this, have you had a chance to check MeshTrafficPermission with ZoneEgress when there are no TrafficPermissions? We have the issue #6589, but I'm not sure if it affects deployments with ZoneEgress

We are testing if traffic is blocked without TrafficPermission so I feel like that issue is still relevant.

@lobkovilya
Copy link
Contributor

Right, I just wanted to make sure the issue doesn't affect the case with ZoneEgress

@michaelbeaumont michaelbeaumont changed the title feat(kuma-cp): implement MeshTrafficPermisson for ZoneEgress feat(kuma-cp): implement MeshTrafficPermisson for ExternalServices with ZoneEgress Jun 20, 2023
@lukidzi lukidzi merged commit 19e952c into kumahq:release-2.3 Jun 20, 2023
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.

3 participants