CONSOLE-5431: Add allow-all NetworkPolicy to openshift-console namespace - #1199
CONSOLE-5431: Add allow-all NetworkPolicy to openshift-console namespace#1199spadgett wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)**/*.{yaml,yml,json}📄 CodeRabbit inference engine (Custom checks)
Files:
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)
Files:
**/*.yaml⚙️ CodeRabbit configuration file
Files:
**/*.{yaml,yml}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-07-24T17:22:48.776ZApplied to files:
🔇 Additional comments (1)
WalkthroughAdds a Kubernetes NetworkPolicy for all pods in the ChangesConsole network policy
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
spadgett
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@spadgett: This pull request references CONSOLE-5431 which is a valid jira issue. 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. |
![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 (3)
pkg/console/controllers/networkpolicy/controller.go (2)
3-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the repository’s commented import groups.
This file groups imports only by blank lines; it omits the required standard-library, third-party, kube, OpenShift, and internal comment sections.
As per coding guidelines, “Group imports with commented sections for standard library, third-party, kube, openshift, and operator/internal packages.”
🤖 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/console/controllers/networkpolicy/controller.go` around lines 3 - 27, Update the import block in the network policy controller to use the repository’s required commented sections: standard library, third-party, Kubernetes, OpenShift, and operator/internal packages. Preserve all existing imports and only reorganize them under the appropriate comments.Sources: Coding guidelines, Path instructions
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd Godoc for the exported constructor.
NewNetworkPolicyControllerneeds an immediately preceding comment describing what it constructs.As per coding guidelines, “Document exported Go functions with a Godoc comment that explains what the function does.”
🤖 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/console/controllers/networkpolicy/controller.go` around lines 36 - 42, Add an immediately preceding Godoc comment for the exported NewNetworkPolicyController function, beginning with its name and briefly describing that it constructs and returns a network policy controller. Do not change the constructor signature or implementation.Sources: Coding guidelines, Path instructions
manifests/03-rbac-role-ns-console.yaml (1)
80-90: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope named mutations to
allow-all.The controller applies the fixed policy and deletes only
api.ConsoleNetworkPolicyName(Lines 88 and 96 inpkg/console/controllers/networkpolicy/controller.go), but this grantsget,update, anddeleteon every NetworkPolicy in the namespace. Split those verbs into aresourceNames: ["allow-all"]rule; retain unrestrictedlist,watch, andcreateonly where Kubernetes RBAC requires it.As per coding guidelines, “RBAC manifests must follow least-privilege design: grant only the permissions needed.”
🤖 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 `@manifests/03-rbac-role-ns-console.yaml` around lines 80 - 90, Update the NetworkPolicy rules in the RBAC manifest to scope get, update, and delete permissions with resourceNames containing only allow-all. Keep list, watch, and create unrestricted in a separate rule as required by Kubernetes RBAC, preserving only the permissions needed by the controller.Sources: Coding guidelines, Path instructions, Linters/SAST tools
🤖 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/console/controllers/networkpolicy/controller.go`:
- Around line 65-67: Wrap the error returned by operatorConfigLister.Get in the
controller reconciliation flow with fmt.Errorf using the context “failed to get
operator config: %w”. Also wrap non-NotFound errors from the network policy
delete path with “failed to delete network policy: %w”, while preserving the
existing NotFound handling.
- Around line 71-92: Move creation of the status handler and the
NetworkPolicySync status update/flush flow before the management-state switch in
the controller reconciliation method. Ensure the Removed path reports and
flushes failures from removeNetworkPolicy, and the unknown-state path records
and flushes its error, while Managed and Unmanaged retain their existing
behavior.
---
Nitpick comments:
In `@manifests/03-rbac-role-ns-console.yaml`:
- Around line 80-90: Update the NetworkPolicy rules in the RBAC manifest to
scope get, update, and delete permissions with resourceNames containing only
allow-all. Keep list, watch, and create unrestricted in a separate rule as
required by Kubernetes RBAC, preserving only the permissions needed by the
controller.
In `@pkg/console/controllers/networkpolicy/controller.go`:
- Around line 3-27: Update the import block in the network policy controller to
use the repository’s required commented sections: standard library, third-party,
Kubernetes, OpenShift, and operator/internal packages. Preserve all existing
imports and only reorganize them under the appropriate comments.
- Around line 36-42: Add an immediately preceding Godoc comment for the exported
NewNetworkPolicyController function, beginning with its name and briefly
describing that it constructs and returns a network policy controller. Do not
change the constructor signature or implementation.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1625d09e-4e42-4790-8b3e-ba3b42a7decf
📒 Files selected for processing (5)
bindata/assets/networkpolicies/allow-all.yamlmanifests/03-rbac-role-ns-console.yamlpkg/api/api.gopkg/console/controllers/networkpolicy/controller.gopkg/console/starter/starter.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/console(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
bindata/assets/**/*.yaml
📄 CodeRabbit inference engine (ARCHITECTURE.md)
Store static YAML assets in
bindata/assets/organized by resource type (configmaps/, deployments/, pdb/, routes/, services/)
Files:
bindata/assets/networkpolicies/allow-all.yaml
**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (Custom checks)
**/*.{yaml,yml,json}: Flag privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capability, running as root without justification, and allowPrivilegeEscalation: true in container/Kubernetes manifests
When deployment manifests, operator code, or controllers are added/modified, ensure they do not introduce scheduling constraints assuming standard HA topology (3+ control-plane nodes, dedicated workers). Flag: required pod anti-affinity with maxUnavailable: 0 (deadlocks on SNO/TNF/TNA), pod topology spread with DoNotSchedule and hostname key (breaks on SNO), replica counts derived from node count without topology awareness, nodeSelector/node affinity targeting control-plane nodes (fails on HyperShift), scheduling to all control-plane nodes equally without excluding arbiter nodes (TNA), assuming dedicated worker nodes exist (SNO/TNF), or PodDisruptionBudgets designed for 3+ nodes (TNF/TNA). Do not flag if change checks ControlPlaneTopology, node counts, or topology labels before applying constraints.
Files:
bindata/assets/networkpolicies/allow-all.yamlmanifests/03-rbac-role-ns-console.yaml
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}
📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}: Kubernetes manifests undermanifests/,bindata/assets/,quickstarts/,examples/, andprofile-patches/must include the appropriate cluster profile annotations when they are CVO-deployed resources (for exampleinclude.release.openshift.io/hypershift,include.release.openshift.io/ibm-cloud-managed,include.release.openshift.io/self-managed-high-availability, andinclude.release.openshift.io/single-node-developer).
Console resources in the manifest trees must include the capability annotationcapability.openshift.io/name: Console.
RBAC manifests must follow least-privilege design: grant only the permissions needed, avoid wildcards unless truly justified, use verbs that match the actual operation set, and set correctapiGroups(""for core APIs and specific groups for CRDs).
Resources must use the correct namespace for their role:openshift-consolefor console workload resources andopenshift-console-operatorfor operator resources; any cross-namespace reference must be explicit and intentional.
YAML manifests should use 2-space indentation, keep fields in a consistent order, and include---separators between multiple resources in the same file.
When binding roles to service accounts, useconsole-operatorin theopenshift-console-operatornamespace for operator cross-namespace access, and useconsolein theopenshift-consolenamespace for console workload-scoped access.
Files:
bindata/assets/networkpolicies/allow-all.yamlmanifests/03-rbac-role-ns-console.yaml
**/*.yaml
⚙️ CodeRabbit configuration file
**/*.yaml: Review YAML manifests based on content and kind.Refer to /manifest-review when YAML contains:
kind: Roleorkind: ClusterRole(RBAC review)kind: RoleBindingorkind: ClusterRoleBindingannotations:section (check for cluster profiles)verbs: ["*"]or wildcard permissionsapiGroups: ["*"]or overly broad permissions- ServiceAccount references in subjects
Check for required annotations in manifests/:
include.release.openshift.io/hypershiftinclude.release.openshift.io/ibm-cloud-managedinclude.release.openshift.io/self-managed-high-availabilityinclude.release.openshift.io/single-node-developercapability.openshift.io/name: ConsoleFor quickstarts/, additionally check:
- QuickStart spec structure
- Task descriptions and prerequisites
- See quickstarts/README.md for guidelines
Files:
bindata/assets/networkpolicies/allow-all.yamlmanifests/03-rbac-role-ns-console.yaml
**/*.{yaml,yml}
⚙️ CodeRabbit configuration file
**/*.{yaml,yml}: If this is a Kubernetes/OpenShift manifest or Helm template:
- securityContext: runAsNonRoot, readOnlyRootFilesystem,
allowPrivilegeEscalation: false- Drop ALL capabilities, add only what is required
- Resource limits (cpu, memory) on every container
- No hostPID, hostNetwork, hostIPC, privileged: true
- NetworkPolicy defined for the namespace
- OpenShift: SCC must be restricted or custom-scoped
- Liveness + readiness probes defined
- automountServiceAccountToken: false unless needed
- RBAC: least privilege; no cluster-admin for workloads
- Helm: no .Values interpolation in shell commands
Files:
bindata/assets/networkpolicies/allow-all.yamlmanifests/03-rbac-role-ns-console.yaml
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Follow Go coding standards and patterns documented in CONVENTIONS.md
Organize imports according to conventions documented in CONVENTIONS.md
Usegofmtto format Go code with standard formatting
Rungo vetchecks on all Go packagesFollow Go coding standards and patterns as documented in CONVENTIONS.md, including proper import organization
Organize Go code following the repository structure: main entry point in
cmd/console/main.go, API constants inpkg/api/, operator command setup inpkg/cmd/operator/, and version command inpkg/cmd/version/
**/*.go: Usegofmtfor formatting Go code
Follow standard Go naming conventions
Group imports in order: standard lib, 3rd party, kube/openshift, internal (marked with comments)
Use meaningful error messages with context in Go code
Set status conditions usingstatus.Handle*functions with type prefixes (*Degraded, *Progressing, *Available, *Upgradeable)
Use typed errors and wrap errors to preserve stack contextFlag MD5, SHA1, DES, RC4, 3DES, Blowfish, and ECB mode cryptographic usage. Also flag custom crypto implementations and non-constant-time comparison of secrets or tokens.
**/*.go: Do not use deprecated Go APIs such asioutil.ReadFile,ioutil.WriteFile,ioutil.ReadAll, ornet.DialinDialcallbacks; useos.ReadFile,os.WriteFile,io.ReadAll, andDialContextinstead.
When returning errors in Go, wrap them with%wand include meaningful context instead of returning the raw error or using%v.
Use specific error checks such asapierrors.IsNotFound(err)instead of matching error strings withstrings.Contains(err.Error(), ...).
Propagate the caller’scontext.Contextthrough operations and avoid replacing it withcontext.Background()inside request/controller code.
Usedeferto release acquired resources so cleanup happens on all return paths.
Avoid god functions: keep Go functions to roughly under 100 lines and split code with too many responsibilities into smaller...
Files:
pkg/api/api.gopkg/console/starter/starter.gopkg/console/controllers/networkpolicy/controller.go
⚙️ CodeRabbit configuration file
**/*.go: Review Go code following OpenShift operator patterns.
See CONVENTIONS.md for coding standards and patterns.Refer to the following skills based on CODE PATTERNS, not just file paths:
Refer to /controller-review when code contains:
- Controller struct types (e.g.,
type *Controller struct)func New*Controller(factory functionsfactory.New().WithFilteredEventsInformers(pattern.ToController(method callsSync(ctx context.Context, controllerContext factory.SyncContext)methodsoperatorConfig.Spec.ManagementStatechecksstatus.NewStatusHandlerorstatus.Handle*functionsRefer to /sync-handler-review when code contains:
- Main operator sync functions (e.g.,
sync_v400.gocontent)- Sequential resource syncing with early returns
- Incremental reconciliation loops
- Multiple
resourceapply.Apply*()calls in sequence- Dependency ordering of ConfigMaps → Secrets → Service Accounts → RBAC → Services → Deployments → Routes
- Feature gate conditional logic
Refer to /go-quality-review for all Go code to check:
- Deprecated imports:
ioutil.ReadFile,ioutil.WriteFile,ioutil.ReadAll- Deprecated patterns:
DialwithoutDialContext- Error handling: missing
%win fmt.Errorf- Code smells: deep nesting (4+ levels), functions >100 lines
- Magic values: unexplained numbers/strings
- Context propagation:
context.Background()instead of passed ctx- Missing godoc on exported functions
Files:
pkg/api/api.gopkg/console/starter/starter.gopkg/console/controllers/networkpolicy/controller.go
{pkg,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use gofmt for code formatting on pkg and cmd directories
{pkg,cmd}/**/*.go: Format code usinggofmt -w ./pkg ./cmd
Rungo vetchecks on all Go packages in ./pkg and ./cmd
Files:
pkg/api/api.gopkg/console/starter/starter.gopkg/console/controllers/networkpolicy/controller.go
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}
⚙️ CodeRabbit configuration file
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):
- SQL: parameterized queries only; no string concatenation
- Command: no shell=True, os.system, or backtick exec with user input
- LDAP/XPath: escape special characters in filters
- Path traversal: canonicalize paths, reject ../
- Deserialization: no pickle/yaml.load()/eval on untrusted data
- Prototype pollution: no recursive merge of untrusted objects
- Validate at trust boundaries with allow-lists, not deny-lists
- Normalize Unicode and anchor regexes (^$); watch for ReDoS
Files:
pkg/api/api.gopkg/console/starter/starter.gopkg/console/controllers/networkpolicy/controller.go
pkg/console/starter/**/*.go
📄 CodeRabbit inference engine (ARCHITECTURE.md)
Access feature gates via
featuregates.FeatureGateAccessinstarter.gofor features likeExternalOIDCandConsolePluginContentSecurityPolicy
Files:
pkg/console/starter/starter.go
pkg/console/controllers/**/*.go
📄 CodeRabbit inference engine (ARCHITECTURE.md)
Place all controller implementations in
pkg/console/controllers/subdirectory, with each controller in its own package (e.g.,clidownloads/,oauthclients/,route/,service/)
Files:
pkg/console/controllers/networkpolicy/controller.go
**/*controller*.go
📄 CodeRabbit inference engine (CONVENTIONS.md)
Use the OpenShift library-go factory pattern for controllers
**/*controller*.go: Use the controller factory pattern withfactory.New().WithFilteredEventsInformers(),util.IncludeNamesFilter()for informer filtering, and a descriptiveToController()call with a recorder.
In controller sync logic, handle alloperatorConfig.Spec.ManagementStatevalues:Managed,Unmanaged,Removed, and return an error for unknown states.
Create and usestatus.NewStatusHandler(c.operatorClient), set the appropriate status condition helpers (HandleProgressingOrDegraded(),HandleDegraded(),HandleProgressing(),HandleAvailable()), and always callFlushAndReturn()at the end of sync.
Group imports with commented sections for standard library, third-party, kube, openshift, and operator/internal packages.
Wrap errors with context usingfmt.Errorf("failed to X: %w", err), handleapierrors.IsNotFound()appropriately for delete operations, optional resources, and get-before-create flows, and return meaningful error messages.
Useresourceapply.Apply*()functions from library-go, pass the controller recorder for events, and handle returned errors properly.
Set owner references withutil.OwnerRefFrom(cr)frompkg/console/subresource/util, ensure only one owner reference hascontroller=true, and clean up owner references when replacing existing resources.
Files:
pkg/console/controllers/networkpolicy/controller.go
**/*controller.go
📄 CodeRabbit inference engine (.claude/skills/sync-handler-review.md)
**/*controller.go: ControllerSyncmethods must reconcile dependent resources in dependency order: ConfigMaps and Secrets first, then Service Accounts, RBAC (Roles and RoleBindings), Services, Deployments, and finally Routes.
Status updates must track reconciliation progress accurately by adding progressing/degraded conditions on failures and marking the resource available only after reconciliation completes successfully.
Return immediately on reconciliation errors instead of logging and continuing, to preserve incremental reconciliation behavior.
Useresourceapply.Apply*()helpers for resource creation and updates instead of writing separate create/update logic.
When a resource is removed from the desired config, the controller must delete the corresponding cluster object and treatNotFoundas a successful no-op.
Check feature gates before syncing gated resources, and only reconcile the gated resources when the feature is enabled.
RespectManagementState, including missing cleanup logic forRemovedstate.
Do not mutate live Kubernetes objects directly; build desired state and apply it through reconciliation helpers.
Always update status conditions during reconciliation; missing status condition updates is an anti-pattern.
Files:
pkg/console/controllers/networkpolicy/controller.go
🪛 Trivy (0.72.0)
manifests/03-rbac-role-ns-console.yaml
[error] 80-90: Manage Kubernetes networking
Role 'console-operator' should not have access to resources ["services", "endpoints", "endpointslices", "networkpolicies", "ingresses"] for verbs ["create", "update", "patch", "delete", "deletecollection", "impersonate", "*"]
Rule: KSV-0056
(IaC/Kubernetes)
🔇 Additional comments (3)
bindata/assets/networkpolicies/allow-all.yaml (1)
1-14: LGTM!pkg/api/api.go (1)
59-59: LGTM!pkg/console/starter/starter.go (1)
35-35: LGTM!Also applies to: 627-634, 688-688
Add a CVO-managed NetworkPolicy that allows all ingress and egress traffic in the openshift-console namespace. This ensures console pods have an explicit allow-all policy so that other default-deny policies do not block console traffic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d6b33f2 to
2881c50
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: spadgett, wking 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 |
|
Marking px- and docs-approved for the same reasoning as #1195 (comment) /label px-approved |
openshift-ci
Bot
added
px-approved
|
@spadgett: The following test failed, say
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. |
3 participants
Add an operator-managed NetworkPolicy that allows all ingress and egress traffic in the openshift-console namespace. This ensures console pods have an explicit allow-all policy so that other default-deny policies do not block console traffic.
/cc @wking @TheRealJon
Summary by CodeRabbit