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

Disable audit for constraint #2266

Open
csullivanupgrade opened this issue Sep 15, 2022 · 17 comments · May be fixed by #3321
Open

Disable audit for constraint #2266

csullivanupgrade opened this issue Sep 15, 2022 · 17 comments · May be fixed by #3321
Assignees
Labels
enhancement New feature or request triaged
Milestone

Comments

@csullivanupgrade
Copy link

Describe the solution you'd like

I would like a flag on constraints so that I can control whether audit checks a constraint or not. This would prevent needless consumption of resources by audit.

Anything else you would like to add:

My use case is primarily centred on DELETE operations. I have two constraint templates/constraints that are only checking whether the operation is a DELETE on specific resources. Audit is not useful here - any object that it checks will necessarily not be deleted! If the object is deleted, it's already too late for audit!

Specifically we ban any DELETE operation of any kind within the argocd namespace, and we ban any DELETE operation on any CRD across all namespaces. We allow the operation if a specific annotation has been applied. This helps prevent accidental deletes.

---
apiVersion: templates.gatekeeper.sh/v1beta1
kind: ConstraintTemplate
metadata:
  name: namespacedeleteprotected
spec:
  crd:
    spec:
      names:
        kind: namespacedeleteprotected
  targets:
    - target: admission.k8s.gatekeeper.sh
      rego: |
        package namespacedeleteprotected
        
        violation[{"msg": msg}] {
            operation := input.review.operation
            opt := operation == "DELETE"
            opt
            msg := sprintf("namespace protected operation=%v", [operation])
          }
---
apiVersion: templates.gatekeeper.sh/v1beta1
kind: ConstraintTemplate
metadata:
  name: objectprotectedbyname
spec:
  crd:
    spec:
      names:
        kind: objectprotectedbyname
      validation:
        openAPIV3Schema:
          properties:
            names:
              type: array
              items: 
                type: string
  targets:
    - target: admission.k8s.gatekeeper.sh
      rego: |
        package objectprotectedbyname
        
        violation[{"msg": msg}] {
            operation := input.review.operation
            operation == "DELETE"
            protected := input.parameters.names
            crd := protected[_]
            input.review.object.metadata.name == crd
            msg := sprintf("%v is protected from operation=%v", [crd,operation])
        }
---
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: namespacedeleteprotected
metadata:
  name: argonamespacedeleteprotected
  annotations:
    argocd.argoproj.io/sync-options: SkipDryRunOnMissingResource=true
spec:
  match:
    kinds:
      - apiGroups: ["*"]
        kinds: ["*"]
    scope: Namespaced
    namespaces: [argocd]
---
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: objectprotectedbyname
metadata:
  name: crdsdeleteprotected
  annotations:
    argocd.argoproj.io/sync-options: SkipDryRunOnMissingResource=true
spec:
  match:
    kinds:
      - apiGroups: [apiextensions.k8s.io]
        kinds: [CustomResourceDefinition]
  parameters:
    names: ["applications.argoproj.io", "helmreleases.helm.fluxcd.io", "applicationsets.argoproj.io", "appprojects.argoproj.io"]

Environment:

  • Gatekeeper version: 3.7.1
  • Kubernetes version: (use kubectl version): 1.21
@csullivanupgrade csullivanupgrade added the enhancement New feature or request label Sep 15, 2022
@ritazh
Copy link
Member

ritazh commented Sep 15, 2022

Sounds like we need an explicit audit enforcementAction. Today audit is always enabled by default for each constraint. To ensure backward compatibility, do we need to make it opt-out instead of opt-in like all the other enforcement actions?

@csullivanupgrade
Copy link
Author

Opt out would make the most sense to me. If audit is enabled I would expect it to be enabled for all constraints by default.

@ctrought
Copy link
Contributor

On a related note, can a constraint only be checked during audit? An enforcement action of warn or deny will check in real-time which we may not want to reduce load/response time but having it run in the background may still be desirable to have warnings bubble up. Dryrun also still processes the constraint in real-time but takes no action.

For example, what if my constraint requires resources to be synced because it's referential, it might require a lot of resources to do so.. perhaps I only want to have these audited so we don't need to enable cache or slow down api calls. I assume there would still be some benefit to having an enforcementAction other than dry-run to entirely skip processing of these constraints in the webhook. To cover this (if it isn't already possible) perhaps we also need an option to disable the enforcementAction all together?

Example:
enforcementAction: [warn, deny, dryrun, none]
auditEnforcementAction: [warn, dryrun, none]

If auditEnforcementAction is not specified, default to enforcementAction (deny treated as warn).

@maxsmythe
Copy link
Contributor

We don't currently scope which constraints get evaluated where, but it's def. a good idea.

@jimmyraywv
Copy link

This is similar to my request

With the addition of the external data provider, audit can cause calls to other applications, that could result in external calls to cloud APIs.

@sozercan
Copy link
Member

sozercan commented Nov 8, 2022

I think we need a similar way to scope processes as we do in config resource

    - excludedNamespaces: ["kube-*", "my-namespace"]
      processes: ["webhook", "audit"]

      # this doesn't exist yet
    - namespaceSelector: 
         matchExpression:
          - key: audit-me
       processes: [audit]

@maxsmythe
Copy link
Contributor

IMO this is tied in with enforcementAction and enforcement points, not just per-process.

E.g. there may be different enforcement actions available, and taking different actions may make sense, depending on whether a violation is caught shift-left via gator test vs. at webhook time vs. in audit.

Because we probably don't want to couple infrastructure with policy implementation, we should have some means of letting enforcement points self-identify (e.g. webhook.gatekeeper.sh, gator.gatekeeper.sh, etc. -- though maybe we could be a bit less literal and more intent-based), along with a default enforcement action.

@stale
Copy link

stale bot commented Jan 8, 2023

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 Jan 8, 2023
@ritazh ritazh added triaged and removed stale labels Jan 9, 2023
@jimmyraywv
Copy link

@maxsmythe is there a current enforcement action that would disable Audit or is that what you are proposing?

@sozercan
Copy link
Member

sozercan commented Jan 10, 2023

@jimmyraywv There's no mechanism to disable audit selectively per constraint today (it can only be disabled as a whole or on a namespace level). There's a design doc that we are discussing various options: https://docs.google.com/document/d/1QIABjZN9B8oBZF5mm1hz5IhpTGulm1jTNn20nYIF-Hc/edit

@ikusomr
Copy link

ikusomr commented Jun 8, 2023

Hello!
Are there any updates regarding the subject? Is there any chance to see the discussed functionality in upcoming releases?
Thanks!

@ikusomr
Copy link

ikusomr commented Jun 13, 2023

Up

@ikusomr
Copy link

ikusomr commented Jul 13, 2023

Am I right that there are no plans regarding the topic?

@ritazh
Copy link
Member

ritazh commented Jul 13, 2023

@ikusomr This is the design doc that we are discussing various options: https://docs.google.com/document/d/1QIABjZN9B8oBZF5mm1hz5IhpTGulm1jTNn20nYIF-Hc/edit
Feel free to review and give feedback.

@ikusomr
Copy link

ikusomr commented Jul 13, 2023

@ritazh yep, I've seen it, thank you. I mean, no decision has been made on this, right?

@ritazh
Copy link
Member

ritazh commented Jul 14, 2023

no decision yet

@ritazh
Copy link
Member

ritazh commented Jul 18, 2023

@salaxander this might be a good one for you to drive

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

Successfully merging a pull request may close this issue.

9 participants