feat: runtime NetworkPolicies for AWX operand workloads - #2132
Conversation
7c1ea6a to
8f0fe5a
Compare
Code reviewFound 2 issues:
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
64bab6e to
bbb914b
Compare
|
@TheRealHaoLiu resolved. please check again |
rooftopcellist
left a comment
There was a problem hiding this comment.
ANSTRAT-2322 NetworkPolicy compliance review: four findings — one critical (RBAC), three major. See inline comments.
|
I had to apply the same fix of removing the explicit ingress/egress empty arrays from default-deny NetworkPolicy, as observed in ansible/galaxy-operator#278, to avoid an infinite reconciliation loop. |
rooftopcellist
left a comment
There was a problem hiding this comment.
NetworkPolicy implementation meets all ANSTRAT-2322 requirements. The reconcile-loop concern with empty ingress/egress arrays is fully resolved — the molecule test correctly validates the Kubernetes normalization behavior. The namespace-selector approach for OCP router ingress is consistent with the fleet-wide pattern and the OCP-recommended implementation. Approved.
| - from: | ||
| - podSelector: | ||
| matchLabels: | ||
| app.kubernetes.io/managed-by: '{{ deployment_type }}-operator' |
There was a problem hiding this comment.
The FROM selector allows any AWX-managed pod to reach postgres, regardless of which AWX instance it belongs to. In a namespace with two AWX instances, the postgres NP for example-awx-1 permits ingress from example-awx-2's web and task pods.
Add instance scoping to both FROM entries here:
- podSelector:
matchLabels:
app.kubernetes.io/managed-by: '{{ deployment_type }}-operator'
app.kubernetes.io/part-of: '{{ ansible_operator_meta.name }}'Apply the same fix to the control-plane: controller-manager podSelector below (line 31).
There was a problem hiding this comment.
Fixed — added app.kubernetes.io/part-of: '{{ ansible_operator_meta.name }}' to both from podSelector entries, scoping ingress to pods belonging to this specific AWX instance.
| {% endif %} | ||
| # Unrestricted egress for LDAP/SAML/webhooks and other user-configured | ||
| # external services whose addresses cannot be enumerated. | ||
| - {} |
There was a problem hiding this comment.
This - {} rule supersedes the scoped postgres egress block above it. Kubernetes evaluates NP egress rules as a union — if any rule matches, the packet is allowed. - {} permits egress to every destination and port, so the {% if managed_database | bool %} postgres block above it (lines 73–80) has no runtime effect.
Remove the conditional postgres egress block. The catch-all already covers it, and keeping the scoped block implies it provides a restriction it doesn't actually enforce.
There was a problem hiding this comment.
Fixed — removed the redundant conditional postgres egress block. The catch-all - {} already permits all egress.
| {% endif %} | ||
| # Unrestricted egress for receptor mesh, SCM polling, execution | ||
| # environments, Kubernetes API, and other external services. | ||
| - {} |
There was a problem hiding this comment.
Same issue as web.networkpolicy.yaml.j2: this - {} supersedes the {% if managed_database | bool %} postgres egress block above it (lines 29–36). Remove the conditional block — the catch-all already permits this traffic.
There was a problem hiding this comment.
Fixed — same as web, removed the redundant conditional block.
rooftopcellist
left a comment
There was a problem hiding this comment.
All open findings addressed. Postgres NP now has instance scoping via part-of label on the operand pod selector. Dead conditional managed_database egress blocks removed from web and task NPs — catch-all egress with descriptive comments is the correct pattern. Approved.
c0bf516 to
df05c2a
Compare
Implements runtime NetworkPolicy creation for all AWX operator-managed workloads per OCPSTRAT-819 (OCP 5.0, Q4 2026). Policies created: - default-deny: baseline deny-all for pods with managed-by label - web: ingress from router + AAP components on 8052; unrestricted egress - task: egress-only; unrestricted for receptor mesh, SCM, EEs - postgres: ingress from AWX pods on 5432; DNS-only egress (managed DB) - db-management: egress to postgres + DNS for backup/restore/migration - mesh-ingress: ingress from router on 27199; unrestricted egress Also adds managed-by and component labels to the mesh-ingress deployment template so the default-deny catches receptor pods. Closes: AAP-73974 Co-authored-by: Claude <noreply@anthropic.com>
Extract the selectattr chain into a vars block to keep the fail_msg under the 170-character line-length rule.
Address two issues raised in review: 1. Add port-5432 egress so the new postgres pod can pg_dump from the old postgres service during major-version upgrades (upgrade_postgres.yml). 2. Add conditional cross-namespace ingress rule when backup_pvc_namespace differs from the CR namespace, so backup/restore management pods can still reach postgres. Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
- Split networkpolicies into standalone RBAC rule separate from ingresses - Add explicit ingress: [] and egress: [] to default-deny template - Narrow db-management NP podSelector to component: db-management and add matching label to backup/restore management-pod templates - Tighten postgres NP controller-manager selector with managed-by constraint to prevent OR-semantics granting unintended DB access - Move mesh-ingress NP include_tasks before Deployment creation - Update molecule assertions for explicit empty ingress/egress Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
The Kubernetes API server normalizes empty arrays (ingress: [] and egress: []) by dropping them entirely from the stored object. This causes apply-based reconciliation to detect a perpetual diff between the desired state (with empty arrays) and the live object (without them), triggering an infinite reconciliation loop. policyTypes: [Ingress, Egress] without ingress/egress fields already enforces default-deny semantics. Ref: ansible/galaxy-operator#278 Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
…serving ports Kind v0.24.0+ enforces NetworkPolicies via kube-network-policies bundled in kindnetd. The previous ingress rules used OCP-specific namespace selectors (policy-group.network.openshift.io/ingress) that don't exist in vanilla K8s/Kind, breaking molecule tests that access services via NodePort. Since ports 8052/8053 (web) and 27199 (mesh-ingress) are publicly exposed via ingress/route/NodePort, restricting ingress by source on these ports is unnecessary—the default-deny NP still protects all other ports, and internal-only services (postgres, task, rsyslog) retain strict pod-level source restrictions. Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
- Postgres NP: add instance scoping (app.kubernetes.io/part-of) to the
FROM selector so pods from a different AWX instance in the same namespace
cannot reach this instance's database.
- Web and Task NPs: remove redundant conditional postgres egress blocks
that are superseded by the unrestricted egress catch-all (- {}).
The catch-all already permits all egress including postgres traffic,
so the conditional blocks had no runtime effect and implied a
restriction they did not enforce.
Jira: AAP-73976
Ref: ANSTRAT-2322
Co-authored-by: Claude <noreply@anthropic.com>
The migration job pod inherits common labels which set app.kubernetes.io/component to 'awx'. Under the default-deny NetworkPolicy, the pod needs to match the db-management NP to reach postgres. Without this label, the migration job is blocked by default- deny and never completes, causing the operator to time out. Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
The db-management and postgres NPs used OCP-specific DNS egress rules targeting namespace openshift-dns on port 5353. In Kind and vanilla Kubernetes, CoreDNS runs in kube-system on port 53, so these rules blocked DNS resolution entirely. Replace the destination-locked rules with port-only rules that allow DNS on both port 53 and 5353 from any namespace, ensuring compatibility with all Kubernetes distributions. Jira: AAP-73976 Ref: ANSTRAT-2322 Co-authored-by: Claude <noreply@anthropic.com>
df05c2a to
3c89a33
Compare
|
The read the docs and nox-sessions changes are unrelated to the changes here. |
Summary
Implements runtime NetworkPolicy creation for all AWX operator-managed workloads per OCPSTRAT-819 (OCP 5.0, Q4 2026).
managed-bylabel (layered product pattern)managed_database)Also adds
managed-byandcomponentlabels to the mesh-ingress deployment template so the default-deny catches receptor pods, following the same approach as PR #191 which added labels to job templates.References
Test plan
Co-Authored-By: Claude noreply@anthropic.com