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

fix: gateway should not be network address and broadcast address #4043

Merged
merged 3 commits into from
May 23, 2024

Conversation

zcq98
Copy link
Member

@zcq98 zcq98 commented May 17, 2024

Pull Request

What type of this PR

Examples of user facing changes:

  • Bug fixes

Which issue(s) this PR fixes

Fixes #4041

@bobz965 bobz965 requested a review from zhangzujian May 20, 2024 06:49
@bobz965
Copy link
Collaborator

bobz965 commented May 20, 2024

@zhangzujian please help review, thanks!

@zhangzujian
Copy link
Member

CIDRContainIP is widely used in Kube-OVN. I think we should not change its behavior. In stead, we can create a new funciton to validate subnet gateway address.

@zcq98
Copy link
Member Author

zcq98 commented May 21, 2024

CIDRContainIP is widely used in Kube-OVN. I think we should not change its behavior. In stead, we can create a new funciton to validate subnet gateway address.

但是不仅是subnet的gateway校验有问题,其它能指定IP的地方都存在可以将ip指定为网络地址/广播地址的问题

@zhangzujian
Copy link
Member

CIDRContainIP is widely used in Kube-OVN. I think we should not change its behavior. In stead, we can create a new funciton to validate subnet gateway address.

但是不仅是subnet的gateway校验有问题,其它能指定IP的地方都存在可以将ip指定为网络地址/广播地址的问题

那我们就把这些地方找出来逐一修复掉。如果工作量比较大,这个 PR 可以先只修复 gateway 地址。

@zhangzujian zhangzujian added the enhancement Improve exist functions label May 21, 2024
@zcq98 zcq98 requested a review from bobz965 May 21, 2024 07:02
pkg/util/net.go Outdated Show resolved Hide resolved
@zcq98 zcq98 force-pushed the gateway-check branch 2 times, most recently from d29f89f to 4c29b3f Compare May 21, 2024 07:56
pkg/util/validator.go Outdated Show resolved Hide resolved
pkg/util/validator.go Outdated Show resolved Hide resolved
Copy link
Member

@zhangzujian zhangzujian left a comment

Choose a reason for hiding this comment

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

LGTM. Can be merged after the workflows succeed.

@bobz965 bobz965 merged commit 2a3cdbd into kubeovn:master May 23, 2024
10 of 11 checks passed
bobz965 pushed a commit that referenced this pull request May 23, 2024
* fix: gateway should not be network address and broadcast address
---------

Signed-off-by: zcq98 <[email protected]>
Signed-off-by: bobz965 <[email protected]>
@zcq98 zcq98 deleted the gateway-check branch May 23, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve exist functions need backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Gateway should not be the network address and the broadcast address
3 participants