Skip to content

Add per-service NetworkPolicies covering every Fission listener#3365

Merged
sanketsudake merged 2 commits into
mainfrom
networkpolicy-coverage
May 7, 2026
Merged

Add per-service NetworkPolicies covering every Fission listener#3365
sanketsudake merged 2 commits into
mainfrom
networkpolicy-coverage

Conversation

@sanketsudake

@sanketsudake sanketsudake commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

Extends NetworkPolicy coverage from storagesvc-only to every Fission service that exposes a non-metrics listener. When networkPolicy.enabled=true (default false, backwards-compat-preserving), six policies render:

Policy Pod selector Ingress allowed
router-allow-ingress svc=router, application=fission-router 8888 from any (LB/Ingress); 8888 from internal publishers (kubewatcher, timer, mqtrigger, mqtrigger-keda, canaryconfig); 8080 from any
executor-allow-internal svc=executor 8888 from application=fission-router only; 8080 from any
storagesvc-allow-internal svc=storagesvc, application=fission-storage 8000 from owner=buildermgr and executorType ∈ {poolmgr,newdeploy,container}; 8080 from any (relocated, rules unchanged)
webhook-server-allow-apiserver svc=webhook-service, application=fission-webhook 9443 (kube-apiserver, unselectable by podSelector); 8080 from any
builderpods-allow-buildermgr owner=buildermgr 8000 fetcher + 8001 builder, both from svc=buildermgr
functionpods-allow-internal executorType ∈ {poolmgr,newdeploy,container} 8000 from svc=executor (specialize); 8888 from application=fission-router (function invocation)

Each policy lives next to the service folder it protects. Controller-created pods (env-builder, function pods) get their policy under the controlling service's folder for discoverability. All policies are ingress-only, gated on the existing global networkPolicy.enabled flag, and pair every podSelector with namespaceSelector: {} so user-namespace pods can still reach Fission install-namespace services. Reuses labels already produced by the chart and controllers (svc, application, owner, executorType) — no Deployment template changes.

Closes the .security-fixes/findings-index promise that the IDOR cluster and http-no-ssl finding are mitigated by NetworkPolicy in lieu of the deferred RFC 0001 application-layer auth.

Test plan

  • helm lint charts/fission-all --set networkPolicy.enabled=true → 0 chart(s) failed
  • helm template ... --set networkPolicy.enabled=false → renders 0 NetworkPolicy resources
  • helm template ... --set networkPolicy.enabled=true → renders exactly 6 NetworkPolicy resources, all parse cleanly with expected ingress-rule counts
  • CI integration tests on the kind-ci skaffold profile (which already sets networkPolicy.enabled=true via a replace patch) pass — any selector regression surfaces here
  • Manual: invoke an HTTP-triggered function on a kind cluster with the flag enabled, verify build + invoke succeed end-to-end

Out of scope (deferred)

  • Egress NetworkPolicies — would need DNS / API-server / external-MQ allowlists; tracked as a follow-up
  • Default-deny baseline in the install namespace
  • Metrics-only services (buildermgr, kubewatcher, timer, mqtrigger, mqtrigger-keda, canaryconfig) — only listener is the metrics port, public-by-design
  • pprof port (6060) ingress — runtime debug flag; operators who turn it on accept the surface
  • RFC 0001 HMAC application-layer auth — separate workstream

🤖 Generated with Claude Code


This change is Reviewable

Extend NetworkPolicy coverage from storagesvc-only to every Fission
service that exposes a non-metrics listener. Six policies render when
networkPolicy.enabled=true (default false, backwards-compat-preserving):

- router            (8888 open + internal-publishers, 8080 metrics)
- executor          (8888 from router only,           8080 metrics)
- storagesvc        (relocated under storagesvc/ folder; rules unchanged)
- webhook-server    (9443 admission + 8080 metrics)
- buildermgr-pods   (env-builder pods: 8000 fetcher, 8001 builder)
- executor-pods     (function pods: 8000 fetcher, 8888 env runtime)

Each policy lives next to the service folder it protects; controller-
created pods (env-builder, function pods) get their policy under the
controlling service's folder for discoverability. All policies are
ingress-only, gated on the existing global networkPolicy.enabled flag,
and pair every podSelector with namespaceSelector: {} so user-namespace
pods can still reach Fission install-namespace services. Reuses the
labels already produced by the chart and controllers (svc, application,
owner, executorType) — no Deployment changes.

Closes the .security-fixes/ findings-index promise that the IDOR
cluster and http-no-ssl finding are mitigated by NetworkPolicy in lieu
of the deferred RFC 0001 application-layer auth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 7, 2026 12:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds Helm-rendered, ingress-only Kubernetes NetworkPolicies to the charts/fission-all chart to better document and restrict network reachability of Fission control-plane listeners when networkPolicy.enabled=true.

Changes:

  • Expands NetworkPolicy templates from storagesvc-only coverage to multiple Fission services (router, executor, webhook, plus controller-managed pods).
  • Updates values.yaml documentation to describe the new policy set and its intent/limitations (ingress-only; metrics left open).
  • Introduces new per-service NetworkPolicy template files colocated with the services they protect.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
charts/fission-all/values.yaml Updates chart docs for the new NetworkPolicy feature set and behavior.
charts/fission-all/templates/webhook-server/networkpolicy.yaml Adds a webhook NetworkPolicy allowing ports 9443 and 8080 (comment currently overstates source restriction).
charts/fission-all/templates/storagesvc/networkpolicy.yaml Adds storagesvc ingress restrictions for archive API (8000) while leaving metrics (8080) open.
charts/fission-all/templates/router/networkpolicy.yaml Adds router ingress policy documenting/allowing router HTTP + metrics ports.
charts/fission-all/templates/executor/networkpolicy.yaml Adds executor ingress policy restricting executor API to router (but currently over-broad via namespaceSelector).
charts/fission-all/templates/executor/networkpolicy-functionpods.yaml Adds function-pod ingress policy for specialize/invoke ports (currently rendered into the wrong namespace).
charts/fission-all/templates/buildermgr/networkpolicy-builderpods.yaml Adds env-builder-pod ingress policy for fetch/build ports (currently rendered into the wrong namespace).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kind: NetworkPolicy
metadata:
name: builderpods-allow-buildermgr
namespace: {{ .Release.Namespace | quote }}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 7d3cdd8. Rendered the policy into fission-builder-ns (builderNamespace || defaultNamespace) via the existing include helper, so it now actually selects owner=buildermgr pods. Verified rendered namespace via helm template for both default install (→ default) and the kind-ci layout (→ fission-builder). Source selector keeps namespaceSelector: {} because buildermgr itself runs in the release namespace, so the source needs to be cross-namespace.

Comment on lines +15 to +23
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
name: functionpods-allow-internal
namespace: {{ .Release.Namespace | quote }}
labels:
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
svc: executor
spec:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 7d3cdd8. Same root cause — relocated the policy into fission-function-ns (functionNamespace || defaultNamespace) so the executorType selector matches the actual function pods. Both source rules (executor on 8000, router on 8888) keep namespaceSelector: {} because both controllers run in the release namespace.

Comment on lines +2 to +7
# Restrict ingress to the validating/mutating webhook server. The only
# legitimate caller of port 9443 is the kube-apiserver, which is not
# selectable by podSelector on most clusters — leaving `from` unset
# is the documented pattern for admission webhooks. This NetworkPolicy
# documents the listener surface and prevents unrelated cluster traffic
# from reaching ports we don't expect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 7d3cdd8. Reworded the header comment to be honest about what the policy enforces — only that no listener other than 9443 and 8080 is reachable on webhook-server pods (NetworkPolicy default-deny semantics for selected pods). The 9443 source remains unrestricted because the kube-apiserver isn't selectable by podSelector on most clusters.

Comment on lines +24 to +25
- namespaceSelector: {}
podSelector:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 7d3cdd8. Dropped namespaceSelector: {} on the router source rule so it's scoped to the policy namespace (release ns). Both router and executor live in the release namespace, so this tightens the rule without affecting normal routing. Other policies that legitimately need cross-namespace sources (storagesvc, builderpods, functionpods) still keep the wildcard because the controllers and target pods are in different namespaces.

Four fixes from PR #3365 review:

- builderpods-allow-buildermgr: render in `fission-builder-ns`
  (builderNamespace || defaultNamespace), not release namespace.
  podSelector matches only pods in the policy's own namespace, so the
  policy was a no-op when builderNamespace differs from release ns
  (notably: kind-ci sets builderNamespace=fission-builder).
- functionpods-allow-internal: same fix, render in `fission-function-ns`.
- executor-allow-internal: drop `namespaceSelector: {}` on the router
  source rule. Router and executor both live in the release namespace,
  so an unscoped namespaceSelector let any pod in any namespace reach
  the executor API by self-labelling as router.
- webhook-server: rewrite the header comment. The previous wording
  implied source restriction on 9443; the policy actually only
  documents the listener surface (no `from` selector means any source).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sanketsudake sanketsudake merged commit 0aa2478 into main May 7, 2026
12 of 13 checks passed
@sanketsudake sanketsudake deleted the networkpolicy-coverage branch June 3, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants