brigadecore / charts

Helm charts for Brigade v1
Apache License 2.0
5 stars 13 forks source link

Allow for annotations and securityContext for the different deployment aspects #89

Open steinheber opened 3 years ago

steinheber commented 3 years ago

To consume brigade and also add exceptions for certain OPA Policies that we apply, it would be beneficial if it was possible to pass on annotations to the deployment.

@vdice let me know what you think

vdice commented 3 years ago

Hi @steinheber, thank you for the feature request.

Definitely in support of adding the ability for chart users to supply securityContext values on a deployment. It looks like this can be at the pod and/or the container level. Are there other spots? I've been reviewing https://kubernetes.io/docs/tasks/configure-pod-container/security-context/.

Otherwise, note that most deployment templates do support annotations via podAnnotations, for example the api server pod here. Will this strategy work for your annotations needs?

Finally, I wanted to mention that all of the charts in this repo correspond to the v1 version of Brigade (and associated components). Most likely you are already aware :) For v2, the Brigade chart is currently stored in the main brigadecore/brigade repo on the v2 branch in the charts directory. I'll post a PR to add a note around this to the README. We may want this same functionality in the v2 chart...

krancour commented 3 years ago

Something I'd be curious about is whether, policy-wise, it's enough to add specific annotations or security context to the deployment. Do the workers and jobs require the same? If so, we're not only talking about modifications to the chart, but also some appreciably complex changes to Brigade itself. (And that's in no way meant to suggest that we shouldn't entertain it. Rather, I want to understand in advance whether clearing this immediate hurdle throws up another one in front of us almost right away.)

steinheber commented 3 years ago

Fair point @krancour . Maybe i am not firm enough yet with the brigade artefact wording.

That being said though some more context on where i am coming from: In our project, we are employing a bunch of security policies which we are going to enforce via Open Policy Agent gatekeeper.

Those policies require e.g. for pods to not execute under root, nor running as privileged etc... When being enforced, these policies will / should also apply for dynamically spawned pods e.g. via API. There is the possibility to define exceptions for those enforcements which will need to happen through annotations - als for the dynamically spawned objects.

@vdice , it should be possible to specify those contexts is a similar manner as you do for podAnnotations fr the API server above.

For the podSecurityContext we strive for

      securityContext:        
          fsGroup: 65532        
          runAsGroup: 65532        
          runAsNonRoot: true        
          runAsUser: 65532        
          seccompProfile:          
              type: RuntimeDefault

For the container security context, we shoot for

          securityContext:            
              capabilities:              
                  drop:                
                      - ALL            
              allowPrivilegeEscalation: false            
              privileged: false            
              readOnlyRootFilesystem: true
steinheber commented 3 years ago

@vdice where in the code is it where pods get dynamically spawned ?

vdice commented 3 years ago

@steinheber it's been a while since I reviewed v1 code, but I believe this is the flow (others can check my work):

  1. Worker config is set via the controller-deployment resource defined in the brigade chart in this repo
  2. That config is pulled in by the controller component, which handles creating worker pods e.g. in handler.go
  3. Job config is defined as they are declared in a brigade.js file, using the brigadier library (Job construct defined in TypeScript in https://github.com/brigadecore/brigadier/blob/master/src/job.ts)
  4. Job pod creation happens in the k8s.ts file of the brigade-worker component

So, for adding securityContext confgurability, I think we'd add support for the Worker via chart values and the controller-deployment resource, to be parsed/added at worker pod creation time in handler.go. For Jobs, I think we'd need to add support in the brigadier library for setting this config on a job, and then parsing/adding at job creation time in the brigade-worker.

We can discuss what it would look like for v2 as well -- not sure which you're hoping to add support to. Maybe both eventually?

steinheber commented 3 years ago

@vdice for V1: probably the steps would also comprise allowing to set the securityContext and podSecurityContext via helm values in the following locations:

https://github.com/brigadecore/charts/blob/master/charts/brigade/templates/controller-deployment.yaml https://github.com/brigadecore/charts/blob/master/charts/brigade/templates/api-deployment.yaml https://github.com/brigadecore/charts/blob/master/charts/brigade/templates/gateway-cr-deployment.yaml https://github.com/brigadecore/charts/blob/master/charts/brigade/templates/gateway-generic-deployment.yaml https://github.com/brigadecore/charts/blob/master/charts/kashti/templates/deployment.yaml

For V2: is there a v2 version of the helm chart or can the existing helm charts be reused ?

krancour commented 3 years ago

@steinheber v2 is a whole new beast. Its chart is kept directly with the v2 code.

https://github.com/brigadecore/brigade/tree/v2/charts/brigade