ESO:418 updated the enhancement proposal to include the network policy for proxy by siddhibhor-56 · Pull Request #1998 · openshift/enhancements

siddhibhor-56 · 2026-05-05T12:26:34Z

Updated external-secrets-network-policy enhancement to add proxy egress NetworkPolicy support; spec.appConfig.proxy.networkPolicyAllowProxyEgressAll controls automatic creation of eso-sys-proxy-egress-core, built at runtime using the port from getProxyConfiguration().
Introduce spec.appConfig.proxy.networkPolicyAllowProxyEgressAll field to control whether the operator auto-creates the proxy egress policy with managed and unmanaged options default to managed.
Add migration helper cleanupMigratedNetworkPolicies() to prune legacy unprefixed NetworkPolicy names on upgrade, driven by label and name-based listing and a one-time migration-complete annotation on the CR.

…y for proxy

openshift-ci · 2026-05-05T12:26:51Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign trilokgeer for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

enhancements/external-secrets-operator/OWNERS

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

bharath-b-rh

Also, please bring in the changes suggested in analysis doc to here.

bharath-b-rh · 2026-05-07T09:43:50Z

 tracking-link: 
  - https://issues.redhat.com/browse/ESO-165
  - https://issues.redhat.com/browse/ESO-70
+  - https://redhat.atlassian.net/browse/RFE-8516


nit: Why not https://issues.redhat.com/browse/ESO-418?

bharath-b-rh · 2026-05-07T09:47:51Z

 - As an administrator, I want to configure and manage egress rules for `external-secrets` operands via the operator API or CRDs, so I can control which external services they are allowed to access.
+- As an administrator on a proxy-configured cluster, I want the operator to automatically allow ESO pods to reach the proxy server, so I do not have to manually create egress NetworkPolicies after every install or upgrade.
+- As an administrator who manages proxy egress manually, I want to set `spec.appConfig.proxy.networkPolicyAllowProxyEgressAll: Unmanaged` to disable the operator's automatic proxy egress policy, so I retain full control over proxy traffic rules.
+- As a cluster administrator upgrading from an older release, I want the operator to clean up legacy unprefixed NetworkPolicy objects after it has applied the new prefixed ones, so there are no stale or duplicate policies left behind.


Suggested change

- As a cluster administrator upgrading from an older release, I want the operator to clean up legacy unprefixed NetworkPolicy objects after it has applied the new prefixed ones, so there are no stale or duplicate policies left behind.

- As a developer after upgrading from an older release, I want the operator to clean up legacy unprefixed NetworkPolicy objects after it has applied the new prefixed ones, so there are no stale or redundant policies left behind.

bharath-b-rh · 2026-05-07T09:50:39Z

 - Ensure that metrics collection for all components remains functional.
 - Ensure the API server can communicate with the `webhook` for admission control.
+- Automatically create a proxy-egress allow policy when an effective proxy is configured, controlled by `spec.appConfig.proxy.networkPolicyAllowProxyEgressAll` (default `Managed`).
+- Introduce a stable `eso-sys-` / `eso-user-` naming scheme for all operator-managed NetworkPolicy objects to enable unambiguous ownership and safe pruning.


Suggested change

- Introduce a stable `eso-sys-` / `eso-user-` naming scheme for all operator-managed NetworkPolicy objects to enable unambiguous ownership and safe pruning.

- Introduce a stable naming scheme (eso-sys- / eso-user-) for all operator-managed NetworkPolicy objects. This ensures unambiguous ownership and prevents naming overlaps with default policies, which currently lead to reconciliation conflicts

bharath-b-rh · 2026-05-07T09:52:34Z

 - Ensure the API server can communicate with the `webhook` for admission control.
+- Automatically create a proxy-egress allow policy when an effective proxy is configured, controlled by `spec.appConfig.proxy.networkPolicyAllowProxyEgressAll` (default `Managed`).
+- Introduce a stable `eso-sys-` / `eso-user-` naming scheme for all operator-managed NetworkPolicy objects to enable unambiguous ownership and safe pruning.
+- Migrate legacy unprefixed NetworkPolicy names to the new scheme on upgrade and clean up stale objects without leaving gaps in coverage.


Suggested change

- Migrate legacy unprefixed NetworkPolicy names to the new scheme on upgrade and clean up stale objects without leaving gaps in coverage.

- Handle the migration of legacy, unprefixed NetworkPolicies to the new naming scheme during upgrades. This includes cleaning up stale objects while ensuring no gaps in traffic coverage during the transition.

bharath-b-rh · 2026-05-07T09:53:18Z


 - This enhancement does not propose creating a generic, cluster-wide policy management solution. The policies are specific to `external-secrets`.
 - We are not introducing AdminNetworkPolicy at this stage, as standard NetworkPolicy objects are sufficient for this scope and can be managed directly by the operator.
- Creating a user-facing option to disable these policies is not in scope, as they represent a baseline security posture.


Any reason why this was removed?

because we can disable the proxy Np through the CR , hence removed it.

bharath-b-rh · 2026-05-07T09:54:04Z

 - This enhancement does not propose creating a generic, cluster-wide policy management solution. The policies are specific to `external-secrets`.
 - We are not introducing AdminNetworkPolicy at this stage, as standard NetworkPolicy objects are sufficient for this scope and can be managed directly by the operator.
- Creating a user-facing option to disable these policies is not in scope, as they represent a baseline security posture.
+- `spec.appConfig.proxy.networkPolicyAllowProxyEgressAll` does not affect non-proxy clusters — `getProxyConfiguration()` returns nil there and no proxy policy is ever created regardless of the field value.


Why is this a non-goal?

This can be removed from non-goals

bharath-b-rh · 2026-05-07T09:55:29Z

+4.  **Proxy Egress Policy (conditional):** After applying the static operand policies, the reconciler evaluates whether an automatic proxy egress allow policy should be created in the `external-secrets` namespace. The policy is created only when **both** conditions hold:
+
+    * `spec.appConfig.proxy.networkPolicyAllowProxyEgressAll` is `Managed` (the default).
+    * `getProxyConfiguration()` returns a non-nil result - i.e., an effective proxy is actually configured on the cluster.


Please rephrase accordingly

Suggested change

* `getProxyConfiguration()` returns a non-nil result - i.e., an effective proxy is actually configured on the cluster.

* Proxy is configured in ExternalSecretsManager or ExternalSecretsConfig or in the OpenShift cluster.

bharath-b-rh · 2026-05-07T10:00:47Z

+      egress:
+        - ports:
+            - protocol: TCP
+              port: <proxy-port>   # set at reconcile time from getProxyConfiguration()


The EP should also talk about, how the port will be arrived at like when an URL is provided without port, and arriving at 443 as default for https and 80 as default for http.

Add a section for this

Adds a new 'Proxy Egress NetworkPolicy' context to the E2E suite covering EP #1998 controller behavior: - No proxy configured → eso-sys-proxy-egress-core absent - Proxy + Managed → eso-sys-proxy-egress-core created with correct port - Managed → Unmanaged transition → NP deleted - User-defined NP names get eso-user- prefix in K8s object name - Migration complete annotation set after first reconcile Ref: openshift/enhancements#1998 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…etworkPolicyAllowProxyEgressAll - Move 5 ESC proxy egress tests from onUpdate to onCreate (they lacked required 'updated' field, causing 'Object Kind is missing' errors) - Add logLevel: 1 to expected in ESC onUpdate proxy egress test (API server defaults logLevel when appConfig is present) - Add networkPolicyAllowProxyEgressAll: Managed to 5 ESM test expected blocks where proxy is configured (field defaulted by kubebuilder marker) Fixes 11 CI unit test failures in PR openshift#144. Ref: openshift/enhancements#1998 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… response The networkPolicyAllowProxyEgressAll invalid-value test triggers two simultaneous validation errors (enum rejection + CEL null check), causing the API server to wrap them in brackets: `is invalid: [err1, err2]`. The previous expectedError included the `is invalid: ` prefix which no longer matches the bracketed form `is invalid: [`. Drop the object-level prefix and match only the field-level error substring, which is present in both single and multi-error responses. Ref: openshift/enhancements#1998 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

openshift-ci · 2026-05-08T11:32:52Z

@siddhibhor-56: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

…ale and proxy egress NPs The EP-1998 controller implementation calls r.Delete on NetworkPolicies in two places: cleanupMigratedNetworkPolicies (pruning stale entries) and createOrApplyProxyEgressNetworkPolicy (removing the policy when proxy is not configured). Without delete permission, these calls fail with 403, causing reconcile to abort before reaching createOrApplyDeployments. This blocked the env var test from seeing LOG_LEVEL applied to the deployment. Ref: openshift/enhancements#1998 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ESO:418 updated the enhancement proposal to include the network polic…

0295e83

…y for proxy

openshift-ci Bot requested review from TrilokGeer and mytreya-rh May 5, 2026 12:26

bharath-b-rh reviewed May 7, 2026

View reviewed changes

siddhibhor-56 mentioned this pull request May 7, 2026

[WIP]: Feature Implementation of Egress proxy network policy. openshift/external-secrets-operator#144

Open

Addressed review comments

4d597fa

	- As a cluster administrator upgrading from an older release, I want the operator to clean up legacy unprefixed NetworkPolicy objects after it has applied the new prefixed ones, so there are no stale or duplicate policies left behind.
	- As a developer after upgrading from an older release, I want the operator to clean up legacy unprefixed NetworkPolicy objects after it has applied the new prefixed ones, so there are no stale or redundant policies left behind.

	- Introduce a stable `eso-sys-` / `eso-user-` naming scheme for all operator-managed NetworkPolicy objects to enable unambiguous ownership and safe pruning.
	- Introduce a stable naming scheme (eso-sys- / eso-user-) for all operator-managed NetworkPolicy objects. This ensures unambiguous ownership and prevents naming overlaps with default policies, which currently lead to reconciliation conflicts

	- Migrate legacy unprefixed NetworkPolicy names to the new scheme on upgrade and clean up stale objects without leaving gaps in coverage.
	- Handle the migration of legacy, unprefixed NetworkPolicies to the new naming scheme during upgrades. This includes cleaning up stale objects while ensuring no gaps in traffic coverage during the transition.

	* `getProxyConfiguration()` returns a non-nil result - i.e., an effective proxy is actually configured on the cluster.
	* Proxy is configured in ExternalSecretsManager or ExternalSecretsConfig or in the OpenShift cluster.

Conversation

siddhibhor-56 commented May 5, 2026

Uh oh!

openshift-ci Bot commented May 5, 2026

Uh oh!

bharath-b-rh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants