Skip to content

CCO-681: Add network policies with default deny #871

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jstuever
Copy link
Contributor

@jstuever jstuever commented Jun 6, 2025

This change adds the network policies necessary to maintain functionality of the operator while enforcing a default deny for ingress and egress traffic. It provides the following:

Default deny all ingress and egress
Allow egress from cloud-credential-operator to kube api
Allow egress from cloud-credential-operator to DNS
Allow egress from cloud-credential-operator to external cloud platforms
Allow egress from pod-identity-webhook to DNS
Allow egress from pod-identity-webhook to external cloud platforms
Allow ingress to cloud-credential-operator:8443 (metrics)
Allow ingress to cloud-credential-operator:6060 (pprof)
Allow ingress to pod-identity-webhook:9443 (webhook)

This change adds the network policies necessary to maintain
functionality of the operator while enforcing a default deny for ingress
and egress traffic. It provides the following:

Default deny all ingress and egress
Allow egress from cloud-credential-operator to kube api
Allow egress from cloud-credential-operator to DNS
Allow egress from cloud-credential-operator to external cloud platforms
Allow egress from pod-identity-webhook to DNS
Allow egress from pod-identity-webhook to external cloud platforms
Allow ingress to cloud-credential-operator:8443 (metrics)
Allow ingress to cloud-credential-operator:6060 (pprof)
Allow ingress to pod-identity-webhook:9443 (webhook)
@jstuever jstuever changed the title Add network policies with default deny CCO-681: Add network policies with default deny Jun 6, 2025
@openshift-ci openshift-ci bot requested review from 2uasimojo and dlom June 6, 2025 20:21
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 6, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 6, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 6, 2025

@jstuever: This pull request references CCO-681 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

In response to this:

This change adds the network policies necessary to maintain functionality of the operator while enforcing a default deny for ingress and egress traffic. It provides the following:

Default deny all ingress and egress
Allow egress from cloud-credential-operator to kube api
Allow egress from cloud-credential-operator to DNS
Allow egress from cloud-credential-operator to external cloud platforms
Allow egress from pod-identity-webhook to DNS
Allow egress from pod-identity-webhook to external cloud platforms
Allow ingress to cloud-credential-operator:8443 (metrics)
Allow ingress to cloud-credential-operator:6060 (pprof)
Allow ingress to pod-identity-webhook:9443 (webhook)

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 openshift-eng/jira-lifecycle-plugin repository.

@jstuever
Copy link
Contributor Author

jstuever commented Jun 6, 2025

/hold
This needs to be tested with all of the providers and could benefit from qe pre-merge testing.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 6, 2025
@jstuever
Copy link
Contributor Author

jstuever commented Jun 9, 2025

/test list

Copy link
Contributor

openshift-ci bot commented Jun 9, 2025

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

/test coverage
/test e2e-aws-ovn
/test e2e-azure-manual-oidc
/test e2e-hypershift
/test e2e-upgrade
/test images
/test security
/test unit
/test verify
/test verify-deps

The following commands are available to trigger optional jobs:

/test e2e-aws-manual-oidc
/test e2e-aws-qe
/test e2e-azure
/test e2e-azure-upgrade
/test e2e-gcp
/test e2e-gcp-manual-oidc
/test e2e-openstack
/test e2e-openstack-parallel
/test okd-scos-e2e-aws-ovn
/test okd-scos-images

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-cloud-credential-operator-master-e2e-aws-ovn
pull-ci-openshift-cloud-credential-operator-master-e2e-hypershift
pull-ci-openshift-cloud-credential-operator-master-e2e-upgrade
pull-ci-openshift-cloud-credential-operator-master-images
pull-ci-openshift-cloud-credential-operator-master-okd-scos-e2e-aws-ovn
pull-ci-openshift-cloud-credential-operator-master-security
pull-ci-openshift-cloud-credential-operator-master-unit
pull-ci-openshift-cloud-credential-operator-master-verify
pull-ci-openshift-cloud-credential-operator-master-verify-deps

In response to this:

/test list

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.

@jstuever
Copy link
Contributor Author

jstuever commented Jun 9, 2025

/test e2e-azure e2e-gcp e2e-openstack

@jstuever
Copy link
Contributor Author

jstuever commented Jun 9, 2025

/test e2e-aws-manual-oidc e2e-azure-manual-oidc e2e-gcp-manual-oidc

include.release.openshift.io/self-managed-high-availability: "true"
spec:
egress:
- ports:
Copy link

Choose a reason for hiding this comment

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

Why is this a large range? You know the ports that you want to allow traffic to... check out the example ones in: https://docs.google.com/document/d/1CDoGSRd-h8VT4PMrK_83Ro0YzYPjORbkxtfTJU1sN6Q/edit?tab=t.0#heading=h.atjxybk6bxes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because there is a use case where a user can specify the cloud api endpoint and we have no guarantee that it is limited to ports 80/443.

@jstuever
Copy link
Contributor Author

jstuever commented Jun 9, 2025

/retest

Copy link
Contributor

openshift-ci bot commented Jun 10, 2025

@jstuever: all tests passed!

Full PR test history. Your PR dashboard.

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.

@jstuever
Copy link
Contributor Author

/assign @dlom

@jstuever
Copy link
Contributor Author

/cc @huangmingxia
pre-merge testing

@openshift-ci openshift-ci bot requested a review from huangmingxia June 10, 2025 16:53
@dlom
Copy link
Contributor

dlom commented Jun 10, 2025

Looks very similar to what we discussed last thursday

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2025
Copy link
Contributor

openshift-ci bot commented Jun 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlom, jstuever

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

@huangmingxia
Copy link

Hi @jstuever I’ve performed some tests on the AWS STS cluster, and the results are recorded in test case OCP-82633. When you have time, could you please help review it? Thanks. I’ll proceed with testing on other platforms next.

Also, I noticed the following statement in the enhancements documentation:

Change the namespace admission controller so all namespaces with an openshift- prefix (since those are special anyway) are labeled with the security.openshift.io/openshift-namespace label so that network policy can address them

I’m not entirely sure if this requires any additional handling on our side during testing or implementation. Could you please help confirm whether we need to pay special attention to this? Thanks :)

@jstuever
Copy link
Contributor Author

@huangmingxia Test case OCP-82633 looks good to me for the AWS platform.

It is my understanding that the "namespace admission controller" requirements are outside the scope of the cloud credential operator epic. I believe the intent is to have the namespace admission controller label all of the openshift namespaces in order to facilitate a cluster-wide administrative network policy that will explicitly deny all ingress/egress traffic in all of the labelled openshift namespaces. This epic is focused on the effort of ensuring cloud-credential-operator has all of the required network policies to continue functioning after that cluster-wide policy is applied (at a later date).

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants