ESO-437: Implement NetworkPolicy auto-creation for configured proxy#146
ESO-437: Implement NetworkPolicy auto-creation for configured proxy#146siddhibhor-56 wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR prefixes operator-managed static NetworkPolicies with ChangesNetwork Policy Naming and Migration System
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels: Suggested reviewers:
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pkg/controller/external_secrets/networkpolicy_test.go (2)
691-699: ⚡ Quick winStrengthen cleanup test assertions for side effects.
The
"delete unprefixed policies and set annotation"case capturesdeletedNamesbut does not assert what was deleted or that patching occurred, so false positives are possible.Also applies to: 738-747
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/external_secrets/networkpolicy_test.go` around lines 691 - 699, The test case "delete unprefixed policies and set annotation" registers m.DeleteCalls and m.PatchCalls but only captures deletedNames without asserting results; update the test to assert that deletedNames contains the expected policy names (e.g., the unprefixed policy) and that a patch was performed by counting PatchCalls or setting a flag in the PatchCalls handler (similarly add assertions for the other case around the existing handlers at lines ~738-747). Specifically, modify the m.DeleteCalls handler to record names (already done) and add an assertion that deletedNames equals the expected slice, and enhance m.PatchCalls to increment a patchCount or set patched=true and assert patchCount>0 (or patched==true) to verify patch side effects for both test cases.
779-781: ⚡ Quick winAssert the created proxy egress port value, not only list sizes.
The create-path test should verify the actual port (e.g.,
3128) to fully validateextractProxyPort+ policy construction integration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/external_secrets/networkpolicy_test.go` around lines 779 - 781, The test currently only checks lengths of np.Spec.Egress and its Ports; instead assert the actual proxy port value produced by extractProxyPort and used in the policy. After the existing length checks, access the port via np.Spec.Egress[0].Ports[0].Port and compare its int value to the expected port (e.g., 3128) — for Kubernetes intstr.IntOrString values check Port.IntVal (or Port.StrVal if using string form) and fail the test if it does not equal the expected port. This ensures extractProxyPort + policy construction are validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/controller/external_secrets/networkpolicy.go`:
- Around line 355-362: The switch on parsed.Scheme currently defaults to
returning 443 for unknown schemes, which hides invalid proxy URLs; update the
logic that handles parsed.Scheme (the switch block) to return an explicit error
for unsupported schemes instead of silently returning 443, including the invalid
scheme in the error message so callers can fail fast and surface misconfigured
proxy URLs. Ensure the function signature is adjusted to propagate the error (or
wrap/return an error alongside the port) and update call sites accordingly to
handle the error return.
- Around line 205-207: Validate the final Kubernetes resource name length before
concatenating esoUserPrefix with npConfig.Name in the NetworkPolicy ObjectMeta
construction (where Name: esoUserPrefix + npConfig.Name is set). Specifically,
ensure len(esoUserPrefix)+len(npConfig.Name) meets Kubernetes DNS-1123 label
limits (max 63 chars) and contains valid characters; if it would exceed the
limit or be invalid, either truncate npConfig.Name safely to fit or return an
actionable error so reconcile fails fast. Implement this check in the function
that builds the NetworkPolicy (reference esoUserPrefix and npConfig.Name) and
centralize name validation using existing DNS-1123 helpers or a small validator
helper to avoid creating invalid resource names at runtime.
---
Nitpick comments:
In `@pkg/controller/external_secrets/networkpolicy_test.go`:
- Around line 691-699: The test case "delete unprefixed policies and set
annotation" registers m.DeleteCalls and m.PatchCalls but only captures
deletedNames without asserting results; update the test to assert that
deletedNames contains the expected policy names (e.g., the unprefixed policy)
and that a patch was performed by counting PatchCalls or setting a flag in the
PatchCalls handler (similarly add assertions for the other case around the
existing handlers at lines ~738-747). Specifically, modify the m.DeleteCalls
handler to record names (already done) and add an assertion that deletedNames
equals the expected slice, and enhance m.PatchCalls to increment a patchCount or
set patched=true and assert patchCount>0 (or patched==true) to verify patch side
effects for both test cases.
- Around line 779-781: The test currently only checks lengths of np.Spec.Egress
and its Ports; instead assert the actual proxy port value produced by
extractProxyPort and used in the policy. After the existing length checks,
access the port via np.Spec.Egress[0].Ports[0].Port and compare its int value to
the expected port (e.g., 3128) — for Kubernetes intstr.IntOrString values check
Port.IntVal (or Port.StrVal if using string form) and fail the test if it does
not equal the expected port. This ensures extractProxyPort + policy construction
are validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fc416023-3158-4aa1-bd18-fe93b6d5dc75
📒 Files selected for processing (10)
bindata/external-secrets/networkpolicy_allow-api-server-and-webhook-traffic.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-main-controller-traffic.yamlbindata/external-secrets/networkpolicy_allow-dns.yamlbindata/external-secrets/networkpolicy_deny-all.yamlpkg/controller/external_secrets/controller.gopkg/controller/external_secrets/networkpolicy.gopkg/controller/external_secrets/networkpolicy_test.gopkg/operator/assets/bindata.go
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: npConfig.Name, | ||
| Name: esoUserPrefix + npConfig.Name, | ||
| Namespace: namespace, |
There was a problem hiding this comment.
Validate custom policy name length before adding eso-user- prefix.
At Line 206, a long user-provided name can become invalid after prefixing, causing reconcile-time create/update failures.
💡 Proposed fix
func (r *Reconciler) buildNetworkPolicyFromConfig(esc *operatorv1alpha1.ExternalSecretsConfig, npConfig operatorv1alpha1.NetworkPolicy, resourceMetadata common.ResourceMetadata) (*networkingv1.NetworkPolicy, error) {
+ maxUserPolicyNameLen := 253 - len(esoUserPrefix)
+ if len(npConfig.Name) > maxUserPolicyNameLen {
+ return nil, fmt.Errorf("network policy name %q is too long; max allowed is %d characters", npConfig.Name, maxUserPolicyNameLen)
+ }
+
namespace := getNamespace(esc)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/external_secrets/networkpolicy.go` around lines 205 - 207,
Validate the final Kubernetes resource name length before concatenating
esoUserPrefix with npConfig.Name in the NetworkPolicy ObjectMeta construction
(where Name: esoUserPrefix + npConfig.Name is set). Specifically, ensure
len(esoUserPrefix)+len(npConfig.Name) meets Kubernetes DNS-1123 label limits
(max 63 chars) and contains valid characters; if it would exceed the limit or be
invalid, either truncate npConfig.Name safely to fit or return an actionable
error so reconcile fails fast. Implement this check in the function that builds
the NetworkPolicy (reference esoUserPrefix and npConfig.Name) and centralize
name validation using existing DNS-1123 helpers or a small validator helper to
avoid creating invalid resource names at runtime.
There was a problem hiding this comment.
@siddhibhor-56 This is a valid point. And I think we should instead change in the API to update kubebuilder:validation:MaxLength from 253 to 243 and update the godoc there as well.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
c3a5469 to
0da61f2
Compare
openshift-ci
Bot
added
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/controller/external_secrets/networkpolicy_test.go (1)
551-621: ⚡ Quick winAdd explicit malformed-proxy URL coverage in
TestExtractProxyPort.The table tests happy/default paths only, but
extractProxyPorthas a parse-error branch that isn’t exercised. Add at least one invalid URL case (e.g., bad escape/invalid host:port) and assertwantErr=trueso this path is protected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/external_secrets/networkpolicy_test.go` around lines 551 - 621, TestExtractProxyPort currently covers only valid/default proxy URLs; add a table case that triggers the parse-error branch in extractProxyPort. Insert a test case in TestExtractProxyPort with a malformed URL (for example HTTPSProxy: "http://%41:bad" or similar invalid host:port) and set wantErr: true, then assert the returned error is non-nil (and ignore port). This will exercise the parse-error path in extractProxyPort and ensure the test suite protects that branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/govulncheck.sh`:
- Line 27: KNOWN_VULNS_PATTERN was extended to include GO-2026-4971 and
GO-2026-4918 but the rationale/comments do not explain why these IDs are being
suppressed; update the justification block that documents ignored
vulnerabilities to add concise, auditable reasons for each new ID (e.g., why
they are not exploitable in our context, mitigated by configuration X, or
scheduled for a library upgrade) and include references (ticket/PR numbers or
CVE details) so future reviewers can trace the exception; ensure the text format
and location match the existing rationale block that accompanies
KNOWN_VULNS_PATTERN so it stays consistent and discoverable.
In `@pkg/controller/external_secrets/networkpolicy_test.go`:
- Around line 738-747: Test lacks an assertion for the negative delete case:
inside the table-driven test after checking the positive delete case
(wantDeleted && mock.DeleteCallCount() == 0), add the inverse assertion for when
wantDeleted is false by asserting mock.DeleteCallCount() == 0 and failing the
test if it is > 0; update the same test block that checks Create calls (uses
tt.wantDeleted, mock.DeleteCallCount(), mock.CreateCallCount()) so both create
and delete behaviors are asserted for true and false scenarios.
---
Nitpick comments:
In `@pkg/controller/external_secrets/networkpolicy_test.go`:
- Around line 551-621: TestExtractProxyPort currently covers only valid/default
proxy URLs; add a table case that triggers the parse-error branch in
extractProxyPort. Insert a test case in TestExtractProxyPort with a malformed
URL (for example HTTPSProxy: "http://%41:bad" or similar invalid host:port) and
set wantErr: true, then assert the returned error is non-nil (and ignore port).
This will exercise the parse-error path in extractProxyPort and ensure the test
suite protects that branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b88fb920-63ae-4a72-9b3e-74006d1a495e
📒 Files selected for processing (23)
api/v1alpha1/external_secrets_config_types.goapi/v1alpha1/meta.goapi/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yamlapi/v1alpha1/tests/externalsecretsmanager.operator.openshift.io/externalsecretsmanager.testsuite.yamlbindata/external-secrets/networkpolicy_allow-api-server-and-webhook-traffic.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yamlbindata/external-secrets/networkpolicy_allow-api-server-egress-for-main-controller-traffic.yamlbindata/external-secrets/networkpolicy_allow-dns.yamlbindata/external-secrets/networkpolicy_deny-all.yamlbundle/manifests/openshift-external-secrets-operator.clusterserviceversion.yamlbundle/manifests/operator.openshift.io_externalsecretsconfigs.yamlbundle/manifests/operator.openshift.io_externalsecretsmanagers.yamlconfig/crd/bases/operator.openshift.io_externalsecretsconfigs.yamlconfig/crd/bases/operator.openshift.io_externalsecretsmanagers.yamlconfig/rbac/role.yamldocs/api_reference.mdhack/govulncheck.shpkg/controller/external_secrets/constants.gopkg/controller/external_secrets/controller.gopkg/controller/external_secrets/networkpolicy.gopkg/controller/external_secrets/networkpolicy_test.gopkg/operator/assets/bindata.go
✅ Files skipped from review due to trivial changes (8)
- bindata/external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yaml
- api/v1alpha1/external_secrets_config_types.go
- bindata/external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yaml
- bindata/external-secrets/networkpolicy_allow-dns.yaml
- bindata/external-secrets/networkpolicy_deny-all.yaml
- bindata/external-secrets/networkpolicy_allow-api-server-egress-for-main-controller-traffic.yaml
- config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml
- pkg/operator/assets/bindata.go
🚧 Files skipped from review as they are similar to previous changes (2)
- bindata/external-secrets/networkpolicy_allow-api-server-and-webhook-traffic.yaml
- pkg/controller/external_secrets/networkpolicy.go
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
0da61f2 to
0829f28
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
Actionable comments posted: 0 |
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
0829f28 to
86de1be
Compare
|
Actionable comments posted: 0 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
- Coverage 47.94% 46.58% -1.37%
==========================================
Files 29 29
Lines 4207 4416 +209
==========================================
+ Hits 2017 2057 +40
- Misses 1902 2056 +154
- Partials 288 303 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
openshift-ci
Bot
added
the
needs-rebase
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
86de1be to
6bc1338
Compare
openshift-ci
Bot
removed
the
needs-rebase
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
60bd91a to
6bc1338
Compare
swghosh
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
6bc1338 to
43195d2
Compare
ambient-code
Bot
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
43195d2 to
83866c1
Compare
and add NetworkPolicyProvisioning field to ProxyConfig. When set to Managed (default), the operator will automatically create and maintain a NetworkPolicy allowing ESO pods to reach the configured proxy server. When set to Unmanaged, administrators manage proxy egress NetworkPolicies themselves. The field is defined at spec.appConfig.proxy.networkPolicyProvisioning on ExternalSecretsConfig. CRD YAMLs for both ExternalSecretsConfig and ExternalSecretsManager are updated. Six API test cases cover valid values, default injection, invalid value rejection, and mutability. Ref: openshift/enhancements#1998 Co-Authored-By: Ambient Code Bot <bot@ambient-code.local> Co-Authored-By: openshift-app-platform-shift-bot <267347085+openshift-app-platform-shift-bot@users.noreply.github.com>
ambient-code
Bot
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
83866c1 to
d24b528
Compare
|
/retitle ESO-437: Implement NetworkPolicy auto-creation for configured proxy |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@siddhibhor-56: This pull request references ESO-437 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 task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
| // userNetworkPolicyPrefix is prepended to user-defined network policy names from the CR spec. | ||
| userNetworkPolicyPrefix = "eso-user-" | ||
|
|
||
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove after 3 releases once the migration from |
There was a problem hiding this comment.
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove after 3 releases once the migration from | |
| // TODO Remove after 3 releases(in v1.5.0) once the migration from |
| skipNPCleanupAnnotation = "externalsecretsconfig.operator.openshift.io/skip-np-cleanup-check" | ||
|
|
||
| // proxyEgressPolicyName is the Kubernetes object name for the automatic proxy egress policy. | ||
| proxyEgressPolicyName = systemNetworkPolicyPrefix + "proxy-egress-core" |
There was a problem hiding this comment.
Should we follow the same naming pattern as other operator created NPs.
| proxyEgressPolicyName = systemNetworkPolicyPrefix + "proxy-egress-core" | |
| proxyEgressPolicyName = systemNetworkPolicyPrefix + "allow-proxy-egress" |
| allowCertControllerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yaml" | ||
| allowBitwardenServerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yaml" | ||
| allowDnsTrafficAsserName = "external-secrets/networkpolicy_allow-dns.yaml" | ||
| allowDnsTrafficAssetName = "external-secrets/networkpolicy_allow-dns.yaml" |
There was a problem hiding this comment.
| allowDnsTrafficAssetName = "external-secrets/networkpolicy_allow-dns.yaml" | |
| allowDNSTrafficAssetName = "external-secrets/networkpolicy_allow-dns.yaml" |
| return err | ||
| } | ||
|
|
||
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove after 3 releases once the migration from |
There was a problem hiding this comment.
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove after 3 releases once the migration from | |
| // TODO: Remove after 3 releases(in v1.5.0) once the migration from |
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: npConfig.Name, | ||
| Name: esoUserPrefix + npConfig.Name, | ||
| Namespace: namespace, |
There was a problem hiding this comment.
@siddhibhor-56 This is a valid point. And I think we should instead change in the API to update kubebuilder:validation:MaxLength from 253 to 243 and update the godoc there as well.
| }, | ||
| { | ||
| name: "valid BitwardenSDKServer network policy", | ||
| name: "valid BitwardenSDKServer network policy with eso-user prefix", |
There was a problem hiding this comment.
Don't we need any new test cases for proxy?
There was a problem hiding this comment.
Added a test case for proxy
| Context("Static Network Policy Naming", func() { | ||
| listManagedNetworkPolicies := func(ctx context.Context, namespace string) ([]networkingv1.NetworkPolicy, error) { | ||
| npList, err := clientset.NetworkingV1().NetworkPolicies(namespace).List(ctx, metav1.ListOptions{ | ||
| LabelSelector: fmt.Sprintf("%s=%s", managedByLabel, managedByValue), |
There was a problem hiding this comment.
nit: Should we keep it aligned with the implementation where two labels are used for filtering
| }, 2*time.Minute, 5*time.Second).Should(Succeed()) | ||
| }) | ||
|
|
||
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove this test case after 3 releases once the migration from |
There was a problem hiding this comment.
| // TODO(siddhibhor-56,ESO-v1.4.0): Remove this test case after 3 releases once the migration from | |
| // TODO: Remove this test case after 3 releases(in v1.5.0) once the migration from |
|
|
||
| for i := range npList.Items { | ||
| np := &npList.Items[i] | ||
| if _, ok := desired[np.GetName()]; ok { |
There was a problem hiding this comment.
IIUC, are we deleting everyother NP not in the desired slice? What if user has created few NPs by himself without using ExternalSecretsConfig and could contain the same labels added by operator?
There was a problem hiding this comment.
`If the annotation is absent, list all NetworkPolicy resources in the namespace by label (`app.kubernetes.io/managed-by: external-secrets-operator` and `app.kubernetes.io/part-of: external-secrets-operator`) and by name, and delete any whose name is not in the desired set. The unprefixed policies from operator versions prior to 1.2.0 - `deny-all-traffic`, `allow-to-dns`, `allow-api-server-egress-for-main-controller`, etc. - carry the operator labels but are no longer in the desired set, so they get cleaned up. Stale user NPs are also removed by the same diff to avoid the duplicate NPs.
`
referring to the EP openshift/enhancements#1998, any network policy that is not contained in the desired set of Network Policies and contain the necessary labels, should be removed right? It removes any stale network policies that could cause network traffic issues.
If that approach looks wrong, then we need to remove only by name and drop the label based listing altogether. WDYT
There was a problem hiding this comment.
My query is not related to how we create the desired list, but more on negated deletion logic. A manually created NP with the exact same two labels and a name not in the desired set will be deleted here is it not?
There was a problem hiding this comment.
yes, it will get deleted.
There was a problem hiding this comment.
My understanding of what's mentioned in the EP is, only those managed by the operator will be deleted using the mentioned filtering.
We should only delete what's created and managed by operator.
| }) | ||
|
|
||
| // Cluster-wide proxy configuration consumed via OLM env vars. | ||
| It("should create proxy egress policy when configured with Managed provisioning", Label("Proxy:HTTP"), func() { |
There was a problem hiding this comment.
Please assert the created policy’s egress port matches the cluster proxy URL (explicit or scheme default), not only that the policy exists.
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
992c811 to
0ee2c82
Compare
| if p := u.Port(); p != "" { | ||
| if _, err := fmt.Sscanf(p, "%d", &port); err != nil || port <= 0 { | ||
| port = 0 | ||
| } | ||
| } |
There was a problem hiding this comment.
Sscanf is not stringent parser for ints, even "8080abc" is considered as valid.
| if p := u.Port(); p != "" { | |
| if _, err := fmt.Sscanf(p, "%d", &port); err != nil || port <= 0 { | |
| port = 0 | |
| } | |
| } | |
| if p := u.Port(); p != "" { | |
| port, err := strconv.Atoi(p) | |
| if err != nil { | |
| return nil, fmt.Errorf("invalid port %q in proxy URL %q: %w", p, raw, err) | |
| } | |
| if port < 1 || port > 65535 { | |
| return nil, fmt.Errorf("port %d out of range in proxy URL %q", port, raw) | |
| } | |
| } |
| if len(ports) == 0 { | ||
| return nil, fmt.Errorf("unable to determine proxy port: no valid proxy URL with a recognized scheme (http/https) found in proxyConfig") | ||
| } |
There was a problem hiding this comment.
I don't think we should be returning an error, considering when just no_proxy is configured. We should just skip creating the policy.
siddhibhor-56
force-pushed
the
feature/ep-1834-controller-networkpolicy
branch
from
a9f0a86 to
81026de
Compare
|
@siddhibhor-56: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/lgtm Please refer comment for pre-merge results. /label qe-approved cc; @snarayan-redhat for docs approval. Please help update the release notes with this new feature. /hold for @mytreya-rh's review |
openshift-ci
Bot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bharath-b-rh, siddhibhor-56 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
4 participants
Implement the controller-side logic for EP-1834 (Network Policies for External Secrets Operator and Operands):
Ref: openshift/enhancements#1834
Summary by CodeRabbit
New Features
Chores