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.

Sign up for GitHub

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

Jump to bottom

Add source ports in Antrea-native policy API #4687

Merged
merged 1 commit into from
May 18, 2023
Merged

Add source ports in Antrea-native policy API #4687

merged 1 commit into from
May 18, 2023

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Mar 8, 2023 •
edited
Loading

Fixes #4653

This PR adds two optional fields, namely sourcePort and sourceEndPort to the NetworkPolicyPort definition of Antrea-native policies. When specified, the policy rule will only match traffic initiated from a specific port (if only sourcePort is provided) or from a specific port range (if both sourcePort and sourceEndPort are provided).
Supported protocols are TCP, UDP and SCTP.

@Dyanngg Dyanngg marked this pull request as draft March 8, 2023 07:18
@Dyanngg Dyanngg force-pushed the np-src-port branch 3 times, most recently from 406f75f to 4b9ab83 Compare March 17, 2023 17:06
@codecov
Copy link

codecov bot commented Mar 17, 2023 •
edited
Loading

Codecov Report

Merging #4687 (406f75f) into main (d2c4ef8) will increase coverage by 0.04%.
The diff coverage is 93.87%.

❗ Current head 406f75f differs from pull request most recent head d3eb0da. Consider uploading reports for the commit d3eb0da to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4687      +/-   ##
==========================================
+ Coverage   70.44%   70.48%   +0.04%     
==========================================
  Files         405      380      -25     
  Lines       60616    59269    -1347     
==========================================
- Hits        42698    41775     -923     
+ Misses      15031    14663     -368     
+ Partials     2887     2831      -56
Flag Coverage Δ *Carryforward flag
e2e-tests 38.30% <ø> (-0.09%) ⬇️ Carriedforward from 3ecf6fd
integration-tests 34.40% <59.09%> (+0.44%) ⬆️
kind-e2e-tests 41.28% <73.46%> (+0.92%) ⬆️
unit-tests 61.71% <87.75%> (-0.45%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
pkg/apis/controlplane/types.go 100.00% <ø> (ø)
pkg/agent/openflow/network_policy.go 81.55% <93.18%> (+0.04%) ⬆️
pkg/controller/networkpolicy/crd_utils.go 77.92% <100.00%> (-10.22%) ⬇️

... and 89 files with indirect coverage changes

@Dyanngg Dyanngg force-pushed the np-src-port branch 4 times, most recently from 30205eb to dc8ca81 Compare March 22, 2023 22:32
@Dyanngg Dyanngg marked this pull request as ready for review March 24, 2023 03:46
@Dyanngg Dyanngg changed the title [WIP] Add source ports in Antrea-native policy API Add source ports in Antrea-native policy API Mar 24, 2023
@vicky-liu vicky-liu added this to the Antrea v1.12 release milestone May 4, 2023
@tnqn tnqn added the area/network-policy Issues or PRs related to network policies. label May 9, 2023
tnqn
tnqn reviewed May 11, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM overall, some minor comments

pkg/agent/openflow/network_policy.go Show resolved Hide resolved
test/e2e/k8s_util.go Outdated Show resolved Hide resolved
@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label May 11, 2023
tnqn
tnqn reviewed May 11, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

I think it should validate srcEndPort can not be specified alone?

tnqn
tnqn previously approved these changes May 12, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, please squash the commits and add the description to the commit message.

test/e2e/k8s_util.go Outdated Show resolved Hide resolved
pkg/controller/networkpolicy/validate.go Outdated Show resolved Hide resolved
@Dyanngg
Copy link
Contributor Author

Dyanngg commented May 12, 2023

/test-all

tnqn
tnqn previously approved these changes May 15, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM. @GraysonWu please let us if you have other comments.

jianjuns
jianjuns reviewed May 15, 2023
View reviewed changes
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

In commit message, change "This PR" to "This commit"

@tnqn tnqn requested a review from GraysonWu May 16, 2023 05:52
@Dyanngg Dyanngg force-pushed the np-src-port branch 2 times, most recently from 148ba47 to a4cf092 Compare May 16, 2023 18:13
@Dyanngg
Add source ports in policy API
747fa6c 
This commit adds two optional fields, namely 'sourcePort' and
'sourceEndPort' to the NetworkPolicyPort definition of Antrea-native
policies.
When specified, the policy rule will only match traffic initiated
from a specific port (if only sourcePort is provided) or from a
specific port range (if both sourcePort and sourceEndPort are
provided). Supported protocols are TCP, UDP and SCTP.

Signed-off-by: Dyanngg <dingyang@vmware.com>
@Dyanngg Dyanngg requested review from GraysonWu and tnqn May 17, 2023 03:59
@Dyanngg
Copy link
Contributor Author

Dyanngg commented May 17, 2023

/test-all

tnqn
tnqn approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented May 18, 2023

@GraysonWu do you have other comments?

GraysonWu
GraysonWu approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor

@GraysonWu GraysonWu left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn tnqn merged commit f79c8ac into antrea-io:main May 18, 2023
ceclinux pushed a commit to ceclinux/antrea that referenced this pull request Jun 5, 2023
@Dyanngg @ceclinux
Add source ports in policy API (antrea-io#4687)
d29afe1 
This commit adds two optional fields, namely 'sourcePort' and
'sourceEndPort' to the NetworkPolicyPort definition of Antrea-native
policies.
When specified, the policy rule will only match traffic initiated
from a specific port (if only sourcePort is provided) or from a
specific port range (if both sourcePort and sourceEndPort are
provided). Supported protocols are TCP, UDP and SCTP.

Signed-off-by: Dyanngg <dingyang@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jianjuns jianjuns jianjuns left review comments

@tnqn tnqn tnqn approved these changes

@GraysonWu GraysonWu GraysonWu approved these changes

@edwardbadboy edwardbadboy Awaiting requested review from edwardbadboy

Assignees
No one assigned
Labels
action/release-note Indicates a PR that should be included in release notes. area/network-policy Issues or PRs related to network policies.
Projects
None yet
Milestone
Antrea v1.12 release
Development

Successfully merging this pull request may close these issues.

Antrea Policy Match Traffic Source Port
5 participants
@Dyanngg @tnqn @GraysonWu @jianjuns @vicky-liu