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

Verify that the EIP within the IP address ranges configured in the ippools of the egressgateway #959

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

bzsuni
Copy link
Collaborator

@bzsuni bzsuni commented Nov 8, 2023

Fix: #871
Fix: #1017

This pr fixed the two issues below:

  1. Verify that the Egress IP (EIP) falls within the IP address range configured in the IP pools of the Egress Gateway when creating the Egress Policy.
  2. Allow the creation of a single IPv4 or IPv6 IP pool when the Egress Gateway ConfigMap is set to dual mode.

@bzsuni bzsuni requested a review from weizhoublue as a code owner November 8, 2023 15:26
@bzsuni bzsuni added the pr/not-ready not ready for merging label Nov 8, 2023
@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch from 2cbf57d to 4138314 Compare November 8, 2023 15:29
@weizhoublue weizhoublue temporarily deployed to commit November 8, 2023 15:31 Inactive
@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch from 4138314 to f928f69 Compare November 9, 2023 05:37
@weizhoublue weizhoublue temporarily deployed to commit November 9, 2023 05:38 Inactive
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #959 (2f45a1a) into main (3531f23) will increase coverage by 1.08%.
The diff coverage is 71.71%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #959      +/-   ##
==========================================
+ Coverage   43.36%   44.44%   +1.08%     
==========================================
  Files          21       21              
  Lines        3166     3260      +94     
==========================================
+ Hits         1373     1449      +76     
- Misses       1592     1605      +13     
- Partials      201      206       +5     
Flag Coverage Δ
unittests 44.44% <71.71%> (+1.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
pkg/controller/webhook/validate.go 60.21% <65.78%> (+2.54%) ⬆️
pkg/utils/ip/ip.go 68.47% <75.40%> (+2.26%) ⬆️

@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch 3 times, most recently from ca02adc to eda6834 Compare November 10, 2023 11:27
@bzsuni bzsuni added release/none should not recoded in new release and removed pr/not-ready not ready for merging labels Nov 10, 2023
@bzsuni bzsuni requested review from lou-lan and biqiangwu November 10, 2023 11:29
lou-lan
lou-lan previously approved these changes Nov 10, 2023
@weizhoublue
Copy link
Collaborator

在双栈环境下,如果 yaml 中只写了 ipv4,不写 ipv6 ,现在行为是什么

@bzsuni
Copy link
Collaborator Author

bzsuni commented Nov 16, 2023

在双栈环境下,如果 yaml 中只写了 ipv4,不写 ipv6 ,现在行为是什么

目前情况是:
如果 egressgateway cm 中是双栈,创建 单栈 的 gateway 会报错;
如果 gateway 是双栈,创建单栈的 policy 会报错。

image

@weizhoublue
Copy link
Collaborator

在双栈环境下,如果 policy 的 yaml 中指定了 ipv4,不指定 ipv6 ,现在行为是什么

@bzsuni
Copy link
Collaborator Author

bzsuni commented Nov 17, 2023

在双栈环境下,如果 policy 的 yaml 中指定了 ipv4,不指定 ipv6 ,现在行为是什么

如上所述,如果是双栈, policy yaml 中指定了一个 ipv4 的 eip, 现在是会报错的。
如截图,有对应的 校验代码在处理

@weizhoublue
Copy link
Collaborator

在双栈环境下,如果 policy 的 yaml 中指定了 ipv4,不指定 ipv6 ,现在行为是什么

如上所述,如果是双栈, policy yaml 中指定了一个 ipv4 的 eip, 现在是会报错的。 如截图,有对应的 校验代码在处理

这个要求安装时,安装人要强感知是否是双栈,是否可能做个优化,自动检测集群是否在双栈下,从而自动设置

@biqiangwu
Copy link
Contributor

在双栈环境下,如果 yaml 中只写了 ipv4,不写 ipv6 ,现在行为是什么

目前情况是: 如果 egressgateway cm 中是双栈,创建 单栈 的 gateway 会报错; 如果 gateway 是双栈,创建单栈的 policy 会报错。

image

最初的期望是双栈的 gateway,可以创建只写 ipv4 或者 ipv6的 policy,报错是代码逻辑有问题

@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch from eda6834 to 1280a22 Compare November 29, 2023 13:33
@weizhoublue weizhoublue temporarily deployed to commit November 29, 2023 13:34 Inactive
@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch 3 times, most recently from cd598ce to a5ca579 Compare November 30, 2023 02:25
@bzsuni bzsuni force-pushed the bz/fix/check/policy-eip-range branch from a5ca579 to 2f45a1a Compare December 5, 2023 03:06
@weizhoublue weizhoublue temporarily deployed to commit December 5, 2023 03:06 Inactive
@lou-lan lou-lan merged commit 5fc42b5 into main Dec 5, 2023
34 checks passed
@lou-lan lou-lan deleted the bz/fix/check/policy-eip-range branch December 5, 2023 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/none should not recoded in new release
Projects
None yet
4 participants