Skip to content

Conversation

@Acedus
Copy link
Contributor

@Acedus Acedus commented Oct 21, 2025

What this PR does / why we need it:
This PR extends CDI to allow an optional generation and deployment of network policies that generally allow only the traffic patterns required by CDI's internal components, those being:

  • cdi-deployment
  • cdi-apiserver
  • cdi-operator
  • cdi-uploadproxy
  • DataImportCron's poller pod

It introduces a new environment variable CDI_DEPLOY_NP which allows controlling the deployment of said network policies during cluster-sync.

A separate static network policy that allows all traffic patterns for CDI testing pods is created if the sync is ran with test-infra.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

Added example network policies as part of the manifest-generator and csv-generator tools.

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 21, 2025
@kubevirt-bot kubevirt-bot requested review from aglitke and awels October 21, 2025 08:09
@Acedus
Copy link
Contributor Author

Acedus commented Oct 21, 2025

/cc @akalenyu

@coveralls
Copy link

coveralls commented Oct 21, 2025

Coverage Status

coverage: 58.818% (-0.3%) from 59.076%
when pulling 31de2f6 on Acedus:dump-network-policies
into 4c3dc65 on kubevirt:main.

@Acedus Acedus force-pushed the dump-network-policies branch 3 times, most recently from 37ebc46 to 9196530 Compare October 23, 2025 08:12
@Acedus
Copy link
Contributor Author

Acedus commented Oct 29, 2025

@kubevirt-bot
Copy link
Contributor

@Acedus: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cdi-apidocs
/test pull-cdi-generate-verify
/test pull-cdi-unit-test
/test pull-cdi-verify-go-mod
/test pull-containerized-data-importer-e2e-ceph
/test pull-containerized-data-importer-e2e-ceph-wffc
/test pull-containerized-data-importer-e2e-destructive
/test pull-containerized-data-importer-e2e-hpp-latest
/test pull-containerized-data-importer-e2e-hpp-previous
/test pull-containerized-data-importer-e2e-istio
/test pull-containerized-data-importer-e2e-nfs
/test pull-containerized-data-importer-e2e-upg
/test pull-containerized-data-importer-fossa
/test pull-containerized-data-importer-non-csi-hpp

The following commands are available to trigger optional jobs:

/test pull-cdi-goveralls
/test pull-cdi-linter
/test pull-cdi-unit-test-s390x

Use /test all to run all jobs.

In response to this:

/test pull-containerized-data-importer-e2e-nfs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@Acedus
Copy link
Contributor Author

Acedus commented Oct 29, 2025

/test pull-containerized-data-importer-e2e-nfs

`np.kubevirt.io/allow-access-cluster-services` is a pod label to be set by
CDI components to indicate that they require access to cluster services otherwise
blocked by the strict network policy (NP).
This label will be applied to the following CDI pods:
- cdi-operator
- cdi-deployment
- cdi-apiserver
- cdi-uploadproxy
- poller (DataImportCron poller pods that run in the CDI namespace)
This label is then used as pod selector to create a NP to give the pods
access to cluster services (apiserver/dns).

Signed-off-by: Adi Aloni <[email protected]>
Co-authored-by: fossedihelm <[email protected]>
@Acedus Acedus force-pushed the dump-network-policies branch from 9196530 to 19a0374 Compare October 30, 2025 08:15
@Acedus Acedus changed the title WIP: Add network policies to CDI Add network policies to CDI Oct 30, 2025
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 30, 2025
@awels
Copy link
Member

awels commented Oct 30, 2025

/test pull-containerized-data-importer-e2e-istio

Copy link
Member

@awels awels left a comment

Choose a reason for hiding this comment

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

One minor comment, we can do it in another PR if we care enough.

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2025
The DIC poller which is used to sample the digest of a container image
from a remote registry currently lacks an identifier labels which can be
used by NetworkPolicies to control its traffic patterns, this commit
adds one.

Signed-off-by: Adi Aloni <[email protected]>
This commit adds an additional factory function that isn't included as
part of the standard create all resources to generate the network
policies required by CDI to function properly.

This new function will be used in following commits to generate network
policies as part of manifest-generator and csv-gen.

Signed-off-by: Adi Aloni <[email protected]>
A new environment variable CDI_DEPLOY_NP has been added with the
options of true and false (defaults to false) to control the deployment
of CDI's network policies.

The network policies deployed when CDI_DEPLOY_NP is set to true are the
ones generated by the createNetworkPolicies factory function as well as
static ones for denying all traffic in the namespace and allow traffic
to kube-apiserver and DNS for pods labeled with `np.kubevirt.io/allow-access-cluster-services`.

Signed-off-by: Adi Aloni <[email protected]>
This commit adds the -dump-network-policies optional flag to the
csv-generator tool in order to allow dumping CDI's required network
policies in case of a restrictive environment.

Signed-off-by: Adi Aloni <[email protected]>
@Acedus Acedus force-pushed the dump-network-policies branch from 19a0374 to 31de2f6 Compare October 30, 2025 14:23
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2025
Copy link
Member

@awels awels left a comment

Choose a reason for hiding this comment

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

/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2025
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2025
@Acedus
Copy link
Contributor Author

Acedus commented Oct 30, 2025

Change - Dropped commit that enables NetworkPolicy deployment (further testing will be done in a separate periodic job which will be introduced in a subsequent PR). Addressed @awels comment.

Copy link
Collaborator

@akalenyu akalenyu left a comment

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,17 @@
apiVersion: networking.k8s.io/v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to ship this to external entities running our func test suite, similarly to the SCC? Totally not a must for this PR, just something to consider.

Comment on lines +14 to +17
ingress:
- {}
egress:
- {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, what's this mostly about? http/registry test servers?

@akalenyu
Copy link
Collaborator

/test pull-cdi-goveralls
/test pull-cdi-unit-test

looks like timeouts

@kubevirt-bot
Copy link
Contributor

@Acedus: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cdi-unit-test 31de2f6 link true /test pull-cdi-unit-test
pull-cdi-goveralls 31de2f6 link false /test pull-cdi-goveralls
pull-containerized-data-importer-e2e-ceph 31de2f6 link true /test pull-containerized-data-importer-e2e-ceph

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants