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

Add addressgroup peer for in-cluster stretched NetworkPolicy enforcement #4432

Merged
merged 1 commit into from
Dec 18, 2022

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Dec 1, 2022

This PR is to fix stretched networkpolicy enforcement on Pod
to MC Service traffic when the MC Service endpoint resides in
the same cluster of the client. Under current datapath design,
the packet will loose tunnel information which contains the
client's label identity, thus bypassing stretched policy
enforcement. For more details on this issue, refer to #4431

As a result, stretched NetworkPolicy enforcement need to be
divided into two parts: for in-cluster access, the Antrea Controller
will create an addressgroup matching the clusterSet-scoped
selector, just like cluster-scoped selectors. For cross-cluster
policy enforcement, label identity will be used to match
selected peers.

Signed-off-by: Dyanngg dingyang@vmware.com

@Dyanngg Dyanngg added the area/multi-cluster Issues or PRs related to multi cluster. label Dec 1, 2022
@Dyanngg Dyanngg added this to the Antrea v1.10 release milestone Dec 1, 2022
@codecov
Copy link

codecov bot commented Dec 1, 2022

Codecov Report

Merging #4432 (a872430) into main (2f2d4d2) will decrease coverage by 0.97%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4432      +/-   ##
==========================================
- Coverage   67.70%   66.73%   -0.98%     
==========================================
  Files         402      402              
  Lines       57253    57254       +1     
==========================================
- Hits        38764    38209     -555     
- Misses      15818    16395     +577     
+ Partials     2671     2650      -21     
Flag Coverage Δ
integration-tests 34.57% <ø> (-0.08%) ⬇️
kind-e2e-tests 45.77% <0.00%> (-1.29%) ⬇️
unit-tests 56.43% <100.00%> (-0.03%) ⬇️
Impacted Files Coverage Δ
pkg/apis/controlplane/types.go 100.00% <ø> (ø)
pkg/controller/networkpolicy/crd_utils.go 93.57% <100.00%> (+0.02%) ⬆️
pkg/ipfix/ipfix_intermediate.go 0.00% <0.00%> (-90.91%) ⬇️
pkg/ipfix/ipfix_collector.go 0.00% <0.00%> (-83.34%) ⬇️
pkg/flowaggregator/certificate.go 0.00% <0.00%> (-75.53%) ⬇️
pkg/flowaggregator/exporter/utils.go 0.00% <0.00%> (-72.73%) ⬇️
pkg/flowaggregator/apiserver/apiserver.go 4.61% <0.00%> (-58.47%) ⬇️
pkg/ipfix/ipfix_process.go 31.25% <0.00%> (-50.00%) ⬇️
pkg/ipfix/ipfix_registry.go 66.66% <0.00%> (-33.34%) ⬇️
pkg/flowaggregator/flowaggregator.go 36.62% <0.00%> (-31.80%) ⬇️
... and 45 more

Comment on lines -180 to -193
} else if peer.Scope == v1alpha1.ScopeClusterSet {
clusterSetScopeSelectors = append(clusterSetScopeSelectors, antreatypes.NewGroupSelector(np.GetNamespace(), peer.PodSelector, peer.NamespaceSelector, nil, nil))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean it will create AddressGroup anyway no matter it's ClusterSet scope or not?
I am wondering how we can verify that the stretchedNetworkPolicy being spitted into two parts of NP correctly? Shall we mention this on the document?

Copy link
Contributor Author

@Dyanngg Dyanngg Dec 15, 2022

Choose a reason for hiding this comment

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

Does it mean it will create AddressGroup anyway no matter it's ClusterSet scope or not?

If there are any pod/nsSelectors, yes it will create AddressGroups because an AddressGroup will need to be created no matter the scope of these selectors.

I am wondering how we can verify that the stretchedNetworkPolicy being spitted into two parts of NP correctly?

E2E testcases should verify this.

@jianjuns jianjuns changed the title Add addressgroup peer for in-cluster stretched networkpolicy enforcement Add addressgroup peer for in-cluster stretched NetworkPolicy enforcement Dec 5, 2022
@luolanzone
Copy link
Contributor

@Dyanngg Does this depend on @GraysonWu 's agent PR 3914? or it can merged independently?
@tnqn @jianjuns could you take a look at this small PR? thanks.

@Dyanngg
Copy link
Contributor Author

Dyanngg commented Dec 15, 2022

@Dyanngg Does this depend on @GraysonWu 's agent PR 3914? or it can merged independently? @tnqn @jianjuns could you take a look at this small PR? thanks.

I think it can be merged independently.

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 Dec 18, 2022

/skip-all

@tnqn tnqn merged commit 113fe92 into antrea-io:main Dec 18, 2022
@Dyanngg Dyanngg deleted the in-cluster-snp branch December 19, 2022 21:12
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/multi-cluster Issues or PRs related to multi cluster.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants