From b935261c2ee1b6897a47c9a96514e67f91c6b71e Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Mon, 28 Sep 2026 10:26:50 -0500 Subject: [PATCH] fix: restore activity entries for JSON Patch changes --- .../policies/backendtlspolicy-policy.yaml | 2 +- .../activity/policies/connector-policy.yaml | 2 +- .../connectoradvertisement-policy.yaml | 2 +- .../milo/activity/policies/domain-policy.yaml | 2 +- .../activity/policies/gateway-policy.yaml | 2 +- .../activity/policies/httpproxy-policy.yaml | 33 ++-- .../activity/policies/httproute-policy.yaml | 2 +- .../policies/securitypolicy-policy.yaml | 6 +- .../trafficprotectionpolicy-policy.yaml | 17 +- .../activitypolicy/jsonpatch_policy_test.go | 174 ++++++++++++++++++ 10 files changed, 210 insertions(+), 32 deletions(-) create mode 100644 internal/activitypolicy/jsonpatch_policy_test.go diff --git a/config/milo/activity/policies/backendtlspolicy-policy.yaml b/config/milo/activity/policies/backendtlspolicy-policy.yaml index 7b5258c3..9bf50c5b 100644 --- a/config/milo/activity/policies/backendtlspolicy-policy.yaml +++ b/config/milo/activity/policies/backendtlspolicy-policy.yaml @@ -36,7 +36,7 @@ spec: # BackendTLSPolicy update with spec available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated backend TLS policy {{ link(audit.objectRef.name, audit.objectRef) }}" # BackendTLSPolicy update fallback (no spec) diff --git a/config/milo/activity/policies/connector-policy.yaml b/config/milo/activity/policies/connector-policy.yaml index c084c2a1..b5ea1e40 100644 --- a/config/milo/activity/policies/connector-policy.yaml +++ b/config/milo/activity/policies/connector-policy.yaml @@ -36,7 +36,7 @@ spec: # Connector update with spec available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated connector {{ link(audit.objectRef.name, audit.objectRef) }}" # Connector update fallback (no spec) diff --git a/config/milo/activity/policies/connectoradvertisement-policy.yaml b/config/milo/activity/policies/connectoradvertisement-policy.yaml index af3c0b3e..8c236228 100644 --- a/config/milo/activity/policies/connectoradvertisement-policy.yaml +++ b/config/milo/activity/policies/connectoradvertisement-policy.yaml @@ -37,7 +37,7 @@ spec: # ConnectorAdvertisement update with spec available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated connector advertisement {{ link(audit.objectRef.name, audit.objectRef) }}" # ConnectorAdvertisement update fallback (no spec) diff --git a/config/milo/activity/policies/domain-policy.yaml b/config/milo/activity/policies/domain-policy.yaml index b4527c9c..3603e110 100644 --- a/config/milo/activity/policies/domain-policy.yaml +++ b/config/milo/activity/policies/domain-policy.yaml @@ -37,7 +37,7 @@ spec: # Domain update with spec available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated domain {{ link(audit.responseObject.spec.domainName, audit.objectRef) }}" # Domain update fallback (no spec) diff --git a/config/milo/activity/policies/gateway-policy.yaml b/config/milo/activity/policies/gateway-policy.yaml index a25aeeea..e94099f1 100644 --- a/config/milo/activity/policies/gateway-policy.yaml +++ b/config/milo/activity/policies/gateway-policy.yaml @@ -36,7 +36,7 @@ spec: # Gateway update with spec available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated gateway {{ link(audit.objectRef.name, audit.objectRef) }}" # Gateway update fallback (no spec) diff --git a/config/milo/activity/policies/httpproxy-policy.yaml b/config/milo/activity/policies/httpproxy-policy.yaml index f9474b91..b7c2a653 100644 --- a/config/milo/activity/policies/httpproxy-policy.yaml +++ b/config/milo/activity/policies/httpproxy-policy.yaml @@ -4,7 +4,8 @@ # Prefer display-* / activity-* annotations stamped at admission. Fall back to # metadata.name / objectRef.name and spec.rules[0].backends[0].endpoint. # Create/update rules require a 2xx response. Update rules require -# requestObject.spec so metadata-only patches are silent. +# a spec-bearing object or a JSON Patch that changes spec; metadata-only patches +# and JSON Patch test operations are silent (except explicit rename activities). apiVersion: activity.miloapis.com/v1alpha1 kind: ActivityPolicy metadata: @@ -48,23 +49,23 @@ spec: summary: "{{ actor }} deleted a load balancer" - name: update-hostname-added - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'hostname' && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'hostname' && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} added a custom hostname {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-name'] }} to {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-hostname-removed - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'hostname' && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'hostname' && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} removed hostname {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-name'] }} from {{ audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'] }}" - name: update-backend - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'backend' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'backend' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} changed the origin of {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }} to {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'] }}" - name: update-force-https-enabled - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'force-https' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'force-https' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} enabled Force HTTPS on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-force-https-disabled - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'force-https' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'force-https' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} disabled Force HTTPS on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-display-name @@ -72,43 +73,43 @@ spec: summary: "{{ actor }} renamed {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-name'] }} to {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'], audit.objectRef) }}" - name: update-host-header-added - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} added a host header {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'] }} to {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-host-header-removed - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} removed the host header {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-name'] }} from {{ audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'] }}" - name: update-host-header - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'host-header' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} changed the host header on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }} to {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'] }}" - name: update-rule - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'rule' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'rule' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated routing on load balancer {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-health-check-enabled - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'added' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} enabled passive health checks on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-health-check-disabled - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-change' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-change'] == 'removed' && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} disabled passive health checks on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-health-check - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'health-check' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated passive health checks on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-annotated - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated load balancer {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-name - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.objectRef.name) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.objectRef.name) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated load balancer {{ link(audit.objectRef.name, audit.objectRef) }}" - name: update-fallback - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated {{ link('a load balancer', audit.objectRef) }}" eventRules: diff --git a/config/milo/activity/policies/httproute-policy.yaml b/config/milo/activity/policies/httproute-policy.yaml index 23703226..bf902131 100644 --- a/config/milo/activity/policies/httproute-policy.yaml +++ b/config/milo/activity/policies/httproute-policy.yaml @@ -36,7 +36,7 @@ spec: # HTTPRoute update with spec.hostnames available - excludes status subresource - name: update - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.requestObject.spec.hostnames) && size(audit.requestObject.spec.hostnames) > 0 && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && has(audit.requestObject.spec.hostnames) && size(audit.requestObject.spec.hostnames) > 0 && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated HTTP route {{ link(string(audit.requestObject.spec.hostnames[0]), audit.objectRef) }}" # HTTPRoute update fallback (no spec or no hostnames) diff --git a/config/milo/activity/policies/securitypolicy-policy.yaml b/config/milo/activity/policies/securitypolicy-policy.yaml index bcc255ee..829c8fbd 100644 --- a/config/milo/activity/policies/securitypolicy-policy.yaml +++ b/config/milo/activity/policies/securitypolicy-policy.yaml @@ -1,6 +1,8 @@ # ActivityPolicy for SecurityPolicy resources (gateway.envoyproxy.io). # Product noun is "basic authentication". The portal names the policy after # the load balancer. Prefer display-name when the client stamps it. +# Updates accept spec-bearing objects and JSON Patch operations that change spec; +# metadata-only patches and test operations are silent. apiVersion: activity.miloapis.com/v1alpha1 kind: ActivityPolicy metadata: @@ -32,9 +34,9 @@ spec: summary: "{{ actor }} removed basic authentication" - name: update-annotated - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated basic authentication on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-fallback - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated basic authentication on {{ link(audit.objectRef.name, audit.objectRef) }}" diff --git a/config/milo/activity/policies/trafficprotectionpolicy-policy.yaml b/config/milo/activity/policies/trafficprotectionpolicy-policy.yaml index 4aa096ad..036e7b75 100644 --- a/config/milo/activity/policies/trafficprotectionpolicy-policy.yaml +++ b/config/milo/activity/policies/trafficprotectionpolicy-policy.yaml @@ -3,8 +3,9 @@ # # Prefer display-* / activity-* annotations stamped at admission. Fall back to # spec.mode and spec.targetRefs[0].name. Create/update rules require a 2xx -# response. Update rules require requestObject.spec so metadata-only patches -# are silent. Event links prefer event.related (the HTTPProxy) when present. +# response. Updates accept spec-bearing objects and JSON Patch operations that +# change spec; metadata-only patches and test operations are silent. +# Event links prefer event.related (the HTTPProxy) when present. apiVersion: activity.miloapis.com/v1alpha1 kind: ActivityPolicy metadata: @@ -36,27 +37,27 @@ spec: summary: "{{ actor }} removed traffic protection" - name: update-mode - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'mode' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'mode' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && 'networking.datumapis.com/activity-value' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} changed traffic protection on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }} from {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'] }}" - name: update-sampling - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'sampling' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'sampling' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} changed traffic protection sampling on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }} to {{ audit.responseObject.metadata.annotations['networking.datumapis.com/activity-value'] }}" - name: update-exclusions - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'exclusions' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/activity-field' in audit.responseObject.metadata.annotations && audit.responseObject.metadata.annotations['networking.datumapis.com/activity-field'] == 'exclusions' && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated traffic protection exclusions on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-annotated - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && has(audit.responseObject.metadata.annotations) && 'networking.datumapis.com/display-name' in audit.responseObject.metadata.annotations && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated traffic protection on {{ link(audit.responseObject.metadata.annotations['networking.datumapis.com/display-name'], audit.objectRef) }}" - name: update-mode-spec - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && has(audit.requestObject.spec.mode) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map && has(audit.requestObject.spec)) && has(audit.requestObject.spec.mode) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated traffic protection {{ link(audit.objectRef.name, audit.objectRef) }} to {{ audit.requestObject.spec.mode }} mode" - name: update-fallback - match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && has(audit.requestObject.spec) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" + match: "!audit.user.username.startsWith('system:') && audit.verb in ['update', 'patch'] && !has(audit.objectRef.subresource) && (type(audit.requestObject) == map ? has(audit.requestObject.spec) : type(audit.requestObject) == list && audit.requestObject.exists(patch, patch.op in ['add', 'remove', 'replace', 'copy', 'move'] && (patch.path == '' || patch.path == '/spec' || patch.path.startsWith('/spec/') || (patch.op == 'move' && (patch.from == '/spec' || patch.from.startsWith('/spec/')))))) && audit.responseStatus.code >= 200 && audit.responseStatus.code < 300" summary: "{{ actor }} updated {{ link('traffic protection', audit.objectRef) }}" eventRules: diff --git a/internal/activitypolicy/jsonpatch_policy_test.go b/internal/activitypolicy/jsonpatch_policy_test.go new file mode 100644 index 00000000..550f80ff --- /dev/null +++ b/internal/activitypolicy/jsonpatch_policy_test.go @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package activitypolicy_test + +import ( + "encoding/json" + "regexp" + "strings" + "testing" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types/ref" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + activityv1alpha1 "go.miloapis.com/activity/pkg/apis/activity/v1alpha1" +) + +// These fixtures use the array-shaped requestObject recorded for JSON Patch +// audit events, including the hostname-removal request that failed in production. +func TestPolicies_JSONPatch(t *testing.T) { + t.Parallel() + policies := []struct { + file string + silentMetadata bool + wantRule string + }{ + {"httpproxy", true, "update-name"}, + {"trafficprotectionpolicy", true, "update-fallback"}, + {"securitypolicy", true, "update-fallback"}, + {"gateway", false, "update-fallback"}, + {"httproute", false, "update-fallback"}, + {"backendtlspolicy", false, "update-fallback"}, + {"connector", false, "update-fallback"}, + {"connectoradvertisement", false, "update-fallback"}, + {"domain", false, "update-fallback"}, + } + tests := []struct { + name string + request string + changesSpec bool + }{ + {"remove all hostnames", `[{"op":"replace","path":"/spec/hostnames","value":[]}]`, true}, + {"add hostname", `[{"op":"add","path":"/spec/hostnames/-","value":"app.example.com"}]`, true}, + {"remove hostname", `[{"op":"remove","path":"/spec/hostnames/0"}]`, true}, + {"replace spec", `[{"op":"replace","path":"/spec","value":{}}]`, true}, + {"replace whole resource", `[{"op":"replace","path":"","value":{"spec":{}}}]`, true}, + {"copy into spec", `[{"op":"copy","from":"/metadata/annotations/example.com~1hostname","path":"/spec/hostnames/0"}]`, true}, + {"move out of spec", `[{"op":"move","from":"/spec/hostnames/0","path":"/metadata/annotations/example.com~1hostname"}]`, true}, + {"test then modify spec", `[{"op":"test","path":"/metadata/resourceVersion","value":"42"},{"op":"replace","path":"/spec/hostnames","value":[]}]`, true}, + {"metadata only", `[{"op":"add","path":"/metadata/labels/example.com~1team","value":"dev"}]`, false}, + {"test spec only", `[{"op":"test","path":"/spec/hostnames","value":[]}]`, false}, + {"test spec and modify metadata", `[{"op":"test","path":"/spec/hostnames","value":[]},{"op":"add","path":"/metadata/labels/example.com~1team","value":"dev"}]`, false}, + {"copy out of spec", `[{"op":"copy","from":"/spec/hostnames/0","path":"/metadata/annotations/example.com~1hostname"}]`, false}, + {"spec prefix is not spec", `[{"op":"add","path":"/specification","value":{}}]`, false}, + {"empty patch", `[]`, false}, + } + for _, p := range policies { + t.Run(p.file, func(t *testing.T) { + t.Parallel() + pol := loadPolicy(t, "config/milo/activity/policies/"+p.file+"-policy.yaml") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var request any + require.NoError(t, json.Unmarshal([]byte(tt.request), &request)) + audit := patchAudit(request, map[string]any{}) + rule := firstMatchingAuditRule(t, pol, audit) + if tt.changesSpec || !p.silentMetadata { + assert.Equal(t, p.wantRule, rule, "successful patch must produce an activity") + assert.NotEmpty(t, auditSummary(t, pol, rule, audit)) + } else { + assert.Empty(t, rule, "non-mutating and metadata-only patches must stay silent") + } + }) + } + for _, exclusion := range []string{"system user", "failed request", "status subresource"} { + t.Run(exclusion, func(t *testing.T) { + audit := patchAudit([]any{map[string]any{"op": "replace", "path": "/spec/hostnames", "value": []any{}}}, map[string]any{}) + switch exclusion { + case "system user": + audit["user"] = map[string]any{"username": "system:serviceaccount:default:controller"} + case "failed request": + audit["responseStatus"] = map[string]any{"code": 422} + case "status subresource": + audit["objectRef"] = map[string]any{"name": "alb", "subresource": "status"} + } + assert.Empty(t, firstMatchingAuditRule(t, pol, audit)) + }) + } + }) + } +} + +func patchAudit(request any, annotations map[string]any) map[string]any { + return map[string]any{ + "user": map[string]any{"username": "alice@example.com"}, + "verb": "patch", + "objectRef": map[string]any{"name": "alb"}, + "requestObject": request, + "responseObject": map[string]any{"metadata": map[string]any{"name": "alb", "annotations": annotations}}, + "responseStatus": map[string]any{"code": 200}, + } +} + +func TestHTTPProxyPolicy_JSONPatchAnnotations(t *testing.T) { + t.Parallel() + pol := loadPolicy(t, "config/milo/activity/policies/httpproxy-policy.yaml") + for _, tt := range []struct { + name string + patch string + field string + change string + want string + wantSummary string + }{ + {"hostname removal", `[{"op":"replace","path":"/spec/hostnames","value":[]}]`, "hostname", "removed", "update-hostname-removed", "Alice removed hostname api.example.com from alb"}, + {"hostname addition", `[{"op":"add","path":"/spec/hostnames/-","value":"api.example.com"}]`, "hostname", "added", "update-hostname-added", "Alice added a custom hostname api.example.com to alb"}, + {"rename", `[{"op":"replace","path":"/metadata/annotations/app.kubernetes.io~1name","value":"New name"}]`, "display-name", "updated", "update-display-name", "Alice renamed api.example.com to New name"}, + {"metadata patch with stale hostname annotations", `[{"op":"add","path":"/metadata/labels/example.com~1team","value":"dev"}]`, "hostname", "removed", "", ""}, + } { + t.Run(tt.name, func(t *testing.T) { + var request any + require.NoError(t, json.Unmarshal([]byte(tt.patch), &request)) + audit := patchAudit(request, map[string]any{ + "networking.datumapis.com/display-name": "alb", + "networking.datumapis.com/activity-field": tt.field, + "networking.datumapis.com/activity-change": tt.change, + "networking.datumapis.com/activity-name": "api.example.com", + "networking.datumapis.com/activity-value": "New name", + }) + rule := firstMatchingAuditRule(t, pol, audit) + assert.Equal(t, tt.want, rule) + if tt.want != "" { + assert.Equal(t, tt.wantSummary, auditSummary(t, pol, rule, audit)) + } + }) + } +} + +// Evaluate the selected policy's summary with the processor's string/dynamic +// link signature. The stub returns display text; link collection is outside +// these policy tests. This catches missing fields and invalid summary types. +func auditSummary(t *testing.T, pol activityv1alpha1.ActivityPolicy, ruleName string, audit map[string]any) string { + t.Helper() + env, err := cel.NewEnv( + cel.Variable("audit", cel.MapType(cel.StringType, cel.DynType)), + cel.Variable("actor", cel.StringType), + cel.Function("link", cel.Overload("link_string_dyn", + []*cel.Type{cel.StringType, cel.DynType}, cel.StringType, + cel.BinaryBinding(func(displayText, resourceRef ref.Val) ref.Val { return displayText }))), + ) + require.NoError(t, err) + var summary string + for _, rule := range pol.Spec.AuditRules { + if rule.Name == ruleName { + summary = rule.Summary + break + } + } + require.NotEmpty(t, summary) + rendered := regexp.MustCompile(`\{\{\s*(.+?)\s*\}\}`).ReplaceAllStringFunc(summary, func(part string) string { + expr := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(part, "{{"), "}}")) + ast, issues := env.Compile(expr) + require.NoError(t, issues.Err()) + program, err := env.Program(ast) + require.NoError(t, err) + value, _, err := program.Eval(map[string]any{"audit": audit, "actor": "Alice"}) + require.NoError(t, err, "rule %s summary expression %s", ruleName, expr) + text, ok := value.Value().(string) + require.True(t, ok, "summary expressions must produce strings") + return text + }) + require.NotContains(t, rendered, "{{", "summary must not retain unevaluated expressions") + return rendered +}