Skip to content

feat(gatekeeper): add PluginDefinition for OPA Gatekeeper#1572

Open
mikolajkucinski wants to merge 1 commit intomainfrom
feat/gatekeeper
Open

feat(gatekeeper): add PluginDefinition for OPA Gatekeeper#1572
mikolajkucinski wants to merge 1 commit intomainfrom
feat/gatekeeper

Conversation

@mikolajkucinski
Copy link
Copy Markdown
Contributor

Pull Request Details

Adds a new gatekeeper PluginDefinition to the catalog, wrapping the upstream OPA Gatekeeper Helm chart.
Five user-facing options are exposed: replicas, auditInterval, validatingWebhookFailurePolicy, constraintViolationsLimit, logDenies. Defaults match the upstream chart.

Breaking Changes

None. This is a new PluginDefinition and does not modify any existing ones.

Issues Fixed

  1. Closes [gatekeeper] First release #1420

Other Relevant Information

See the README for details. A follow-up ticket will track the gatekeeper-config PluginDefinition that ships ConstraintTemplates and Constraints originating from the legacy sapcc/helm-charts/system/gatekeeper chart.

Wraps the upstream OPA Gatekeeper Helm chart as a Greenhouse
PluginDefinition. Only the operator is deployed - constraint templates
and constraints are not bundled and will be shipped in a separate
PluginDefinition.

On-behalf-of: @SAP <mikolaj.kucinski@sap.com>
@mikolajkucinski mikolajkucinski requested review from a team as code owners April 28, 2026 08:56
@majewsky
Copy link
Copy Markdown

majewsky commented May 5, 2026

Comparing against the existing implementation in https://github.com/sapcc/helm-charts/tree/master/system/gatekeeper:

  1. The current chart engages in vendoring shenanigans to ensure that linkerd.io/inject annotations are added on all pods. Has this been considered? I'm not saying that the existing mechanism needs to be copied over verbatim, but since there is no mention of linkerd in the PR as of right now, I want to flag this as a concern. The existing chart explicitly disables linkerd injection because the only relevant network traffic goes through k8s apiserver or ingress and thus is already encrypted, so it is possible that the answer is just "no linkerd annotations needed here".

  2. Since DOOP (Decentralized Observer Of Policies, see docu for doop-analyzer and doop-api) is an integral part of a Gatekeeper multi-cluster setup, I feel like it ought to be part of this plugin. I'm not sure how this connects to the DOOP UI, which is a different Greenhouse plugin, so I'm only flagging this for consideration.

  3. I suggest reviewing https://github.com/sapcc/helm-charts/blob/master/system/gatekeeper/values.yaml for additional values that need to be set. For example, disabledBuiltins = [] will be necessary because policies using helm-manifest-parser or doop-image-checker need http.send().

@mikolajkucinski
Copy link
Copy Markdown
Contributor Author

Comparing against the existing implementation in https://github.com/sapcc/helm-charts/tree/master/system/gatekeeper:

  1. The current chart engages in vendoring shenanigans to ensure that linkerd.io/inject annotations are added on all pods. Has this been considered? I'm not saying that the existing mechanism needs to be copied over verbatim, but since there is no mention of linkerd in the PR as of right now, I want to flag this as a concern. The existing chart explicitly disables linkerd injection because the only relevant network traffic goes through k8s apiserver or ingress and thus is already encrypted, so it is possible that the answer is just "no linkerd annotations needed here".
  2. Since DOOP (Decentralized Observer Of Policies, see docu for doop-analyzer and doop-api) is an integral part of a Gatekeeper multi-cluster setup, I feel like it ought to be part of this plugin. I'm not sure how this connects to the DOOP UI, which is a different Greenhouse plugin, so I'm only flagging this for consideration.
  3. I suggest reviewing https://github.com/sapcc/helm-charts/blob/master/system/gatekeeper/values.yaml for additional values that need to be set. For example, disabledBuiltins = [] will be necessary because policies using helm-manifest-parser or doop-image-checker need http.send().

Thanks Stefan, addressing each:

  1. No linkerd annotations are set by default. The upstream chart 3.22.2 exposes podAnnotations and auditPodAnnotations so deployments running Linkerd can disable injection via PluginPreset:
optionValues:
  - name: podAnnotations
    value:
      linkerd.io/inject: disabled
  - name: auditPodAnnotations
    value:
      linkerd.io/inject: disabled
  1. Per discussion with @IvoGoman and @uwe-mayer, I propose to:
  • Keep the gatekeeper operator decoupled from DOOP
  • Ship DOOP across separate PDs (mirroring the legacy system/gatekeeper + system/doop-central split, but with per-cluster DOOP components extracted into their own PD instead of bundled into the operator):
    • gatekeeper-doop - per-cluster components (doop-analyzer, doop-image-checker, helm-manifest-parser)
    • doop-central - centralized doop (api + landing-page + ingress)
    • doop - existing UI plugin
  1. Good catch, you're right about http.send(). Upstream 3.22.2 has disabledBuiltins: ["{http.send}"] by default (http.send disabled), so policies using helm-manifest-parser or doop-image-checker would fail without an override. I'll add disabledBuiltins as a configurable option and also sweep for others worth surfacing.

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.

[gatekeeper] First release

2 participants