gatekeeper icon indicating copy to clipboard operation
gatekeeper copied to clipboard

Attribute matching for mutation pathTests

Open skaven81 opened this issue 1 year ago • 18 comments

Describe the solution you'd like It is currently not possible to limit the application of a Gatekeeper mutator to only instances of a resource that have certain attributes set. A prime example of this would be creating an Assign mutator that sets spec.allocateLoadBalancerNodePorts=false for any LoadBalancer Services that are created. The match spec can only be as granular as matching v1.Service resources, so it will apply to all of ClusterIP, NodePort, and LoadBalancer type Services. And unfortunately the Kubernetes API refuses to allow spec.allocateLoadBalancerNodePorts to be present in the Service spec if spec.type!=LoadBalancer. Thus, we need a pathTest that can check for spec.type == LoadBalancer to gate the application of the mutation. The existing functionality of subPath with MustExist or MustNotExist does not work, as spec.type is not a prefix of spec.allocateLoadBalancerNodePorts. And even if subPath was possible to use in this case, the two existing operators MustExist and MustNotExist are insufficient to limit application to just LoadBalancer Services.

One way this could be implemented is with an additional type of pathTest that can check for arbitrary values in the review object:

parameters:
  pathTests:
  - subPath: "spec.containers[name: foo]"
    condition: MustExist
  - subPath: "spec.containers[name: foo].securityContext.capabilities"
    condition: MustNotExist
  - pathMatch: spec.type
    operator: In
    values: [ LoadBalancer ]

Adding a new pathMatch behavior to pathTests would allow for gating the activation of the mutator. Unlike subPath behavior, the path designators in pathMatch don't have to be prefixes of the target attribute path. The purpose of pathMatch is to check for specific attribute values, not the presence or absence of a path. They can be structured similarly to matchExpressions to make construction familiar, and would thus implement the same In, NotIn, Exists and DoesNotExist operators that matchExpressions do.

With this implementation in place, it would be possible to create the Assign mutator I need:

apiVersion: mutations.gatekeeper.sh/v1
kind: Assign
metadata:
  name: loadbalancer-nodeports
spec:
  applyTo:
  - groups: [""]
    kinds: [ Service ]
    versions: [ v1 ]
  match:
    kinds:
    - apiGroups: [ "" ]
      kinds: [ Service ]
  location: spec.allocateLoadBalancerNodePorts
  parameters:
    assign:
      value: false
    pathTests:
    - pathMatch: spec.type
      operator: In
      values: [ LoadBalancer ]
    - subPath: spec.allocateLoadBalancerNodePorts
      condition: MustNotExist

Anything else you would like to add:

I can see how implementing this functionality under match might be a better option, as the whole point is to determine whether or not the object under review is even eligible for mutation. I'd be equally happy with an implementation in match or pathTests.

Environment:

  • Gatekeeper version: v3.13.4
  • Kubernetes version: (use kubectl version): Client Version: v1.26.12 Kustomize Version: v4.5.7 Server Version: v1.26.8

skaven81 avatar Jul 16 '24 23:07 skaven81

oh, I met the same question: I use assign mutator to add iptable rules to pod in create phase. however for hostnetwork pod, the iptable rule in pod would take effect in node scope, thus leading to other pod in this node works uncorrectly.

pikehuang avatar Jul 17 '24 10:07 pikehuang

I have faced similar troubles with adding an annotation to LoadBalancer services - https://github.com/orgs/open-policy-agent/discussions/457

I agree with @skaven81, there is a need to support pathMatch to test the values of a field or match resources to narrow down resources eligible for a mutation.

As far as I know, the two existing operators MustExist and MustNotExist are not capable enough to filter out Loadbalancer Services for applying mutations.

From #1548 by @maxsmythe , I understand that testing values of a field (assignIf) was supported earlier but was removed later on due to concerns of circular mutations. But, in certain cases like these it would be a helpful feature to have.

I would love to discuss about how we can add such a feature while continuing to maintaining idempotent & linear mutations. Looking forward to hearing from the OPA Gatekeeper team.

spandan541 avatar Jul 20 '24 10:07 spandan541

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 19 '24 01:09 stale[bot]

not stale

skaven81 avatar Sep 23 '24 14:09 skaven81

For anyone visiting this issue, a workaround could be -

  1. Create Mutation:
    • Use match.labelSelector (or other relevant selectors) to identify the resources for mutation.
    • Exclude any subpath checks.
  2. Validation Policy:
    • Ensure that resources not intended for mutation do not satisfy the "matching criteria" (e.g., labelSelector or other selectors).
    • Confirm that resources targeted for mutation have the appropriate field values as per the defined mutation criteria.

JaydipGabani avatar Oct 08 '24 18:10 JaydipGabani

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 08 '24 08:12 stale[bot]

not stale

skaven81 avatar Dec 09 '24 00:12 skaven81

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Feb 07 '25 03:02 stale[bot]

not stale

skaven81 avatar Feb 07 '25 03:02 skaven81

I have a similar case where I had hoped Gatekeeper could help me. During a migration process, I would like to rewrite all VirtualService objects which has a spec.gateways: [istio-ingress/myoldgateway] and change it to spec.gateways: [istio-ingress/myoldgateway, istio-ingress/newgateway]. But I would not want to touch objects with any other gateways, which co-exist in same NS.

Being able to put a pathTest, or a match with some kind of test that matches if list spec.gateways contains certain values would solve this. Rego matching might be useful?

stromnet avatar Mar 03 '25 10:03 stromnet

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 02 '25 11:05 stale[bot]

Not stale

skaven81 avatar May 02 '25 14:05 skaven81

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 01 '25 14:07 stale[bot]

not stale

skaven81 avatar Jul 01 '25 15:07 skaven81

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 30 '25 15:08 stale[bot]

Not stale

On Sat, Aug 30, 2025, 8:39 AM stale[bot] @.***> wrote:

stale[bot] left a comment (open-policy-agent/gatekeeper#3450) https://github.com/open-policy-agent/gatekeeper/issues/3450#issuecomment-3239353066

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

— Reply to this email directly, view it on GitHub https://github.com/open-policy-agent/gatekeeper/issues/3450#issuecomment-3239353066, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMW7DO5VEC5A2Q76BOASG33QHARTAVCNFSM6AAAAABK7QIQBCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMZZGM2TGMBWGY . You are receiving this because you were mentioned.Message ID: @.***>

skaven81 avatar Aug 30 '25 15:08 skaven81

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 30 '25 01:10 stale[bot]

not stale

skaven81 avatar Oct 30 '25 02:10 skaven81

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Dec 31 '25 00:12 github-actions[bot]

not stale

skaven81 avatar Dec 31 '25 00:12 skaven81