Skip to content

James/security enhancements CEL policy update#83

Open
JPurcell-Braintrust wants to merge 6 commits into
mainfrom
james/security-enhancements
Open

James/security enhancements CEL policy update#83
JPurcell-Braintrust wants to merge 6 commits into
mainfrom
james/security-enhancements

Conversation

@JPurcell-Braintrust
Copy link
Copy Markdown

Adding in the previous security enhancements to fix CEL based policy breaches, previously based on 1.1.32 to the latest helm updates. The security enhancements were:

  • add securityContext and podSecurityContext to all 3 pod types
  • readOnlyRootFilesystem
  • emptyDir size limits for Brainstore volumes

An example/google-autopilot-cel/ has been created to help the known customers currently needing these CEL enhancements in their production environment.

@soldatchenko
Copy link
Copy Markdown
Contributor

Overall this looks good to me. My only asks are around runtime validation rather than the Helm config itself

  • confirm the read-only root filesystem with representative API/scorer + Brainstore trace paths
  • make sure the Brainstore emptyDir.sizeLimit leaves enough headroom above the configured cache size in the example

Comment thread braintrust/examples/google-autopilot-cel/values.yaml
Comment thread braintrust/examples/google-autopilot-cel/values.yaml
Comment thread braintrust/examples/google-autopilot-cel/values.yaml
- name: cache-volume
{{- if and (eq .Values.cloud "azure") .Values.azure.enableAzureContainerStorageDriver }}
{{- if .Values.brainstore.storage.hostPath }}
hostPath:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some time comparing this against the current AKS/GKE behavior. AKS uses Azure Container Storage via ephemeral PVCs, and GKE Autopilot uses pod-local emptyDir with ephemeral-storage requests and local-SSD placement. In both cases, Brainstore’s local cache is expressed as k8s-managed pod-local storage.

The new brainstore.storage.hostPath path is different: if the EKS module sets it, brainstore depends on a raw node filesystem path prepared outside the chart. I think the EKS path should match the AKS/GKE model if technically feasible, likely via EKS Auto Mode with a brainstore-specific NodePool/NodeClass, Brainstore emptyDir, and explicit ephemeral-storage requests.

I think we should remove the hostPath path if EKS Auto Mode is viable. The EKS direction should match AKS/GKE: Brainstore emptyDir, explicit ephemeral-storage requests, and a brainstore-specific EKS Auto Mode NodePool/NodeClass for NVMe-backed capacity.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for readers/writers

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.

2 participants