Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute matching for mutation pathTests #3450

Open
skaven81 opened this issue Jul 16, 2024 · 5 comments
Open

Attribute matching for mutation pathTests #3450

skaven81 opened this issue Jul 16, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@skaven81
Copy link

skaven81 commented Jul 16, 2024

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 skaven81 added the enhancement New feature or request label Jul 16, 2024
@pikehuang
Copy link

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.

@spandan541
Copy link

spandan541 commented Jul 20, 2024

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.

Copy link

stale bot commented Sep 19, 2024

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 stale bot added the stale label Sep 19, 2024
@skaven81
Copy link
Author

not stale

@stale stale bot removed the stale label Sep 23, 2024
@JaydipGabani
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants