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

Allow customization of (& provide sensible defaults for) securityContext across each component #3989

Closed
johnharris85 opened this issue Mar 10, 2022 · 8 comments
Labels
kind/feature New feature triage/duplicated already exists

Comments

@johnharris85
Copy link
Contributor

Description

There is another issue (#3925) tracking customization of the sidecar, but we should also provide sensible defaults (and the ability to override) for the other components, cp, webhook deletion jobs, etc...

@johnharris85 johnharris85 added kind/feature New feature triage/pending This issue will be looked at on the next triage meeting labels Mar 10, 2022
@lahabana
Copy link
Contributor

You might be able to bring some light here. What's the point of overriding the security context? Isn't it in the depending on what runs in the pod?

@lahabana lahabana added triage/needs-information Reviewed and some extra information was asked to the reporter and removed triage/pending This issue will be looked at on the next triage meeting labels Mar 15, 2022
@lahabana
Copy link
Contributor

@johnharris85 waiting on you to get a bit info about this.

@johnharris85
Copy link
Contributor Author

So, depends on the components, but generally sure, overriding makes less sense if they are set correctly by us (although there are valid use cases like debugging where a user might want to allow higher privilege for other tools / etc...). So best case scenario is that we set the least privilege necessary for all our different pods / containers. Interim 'good' case scenario is that we allow users to customize them to it themselves in the case of special requirements.

  • Sidecar: Obviously podSecurityContext will be set by the user for their application pod, but we can (and should) set a least privilege securityContext on the container (separate options). This also needs to be override-able but that's issue Ability to set custom configuration on the sidecar container #3925
  • CP / Jobs (crd/webhook/ns) / Ingress / Egress / CNI: We don't currently set these or allow them to be overridden. We should add a section in the helm chart and default to least privilege for each of these components.

Right now we don't set or allow these to be set by the user. Leads to issues with security admission controllers rejecting the deployments because they're out of policy, and / or users 'guessing' at what correct configuration should be and having to work around the helm chart causing either broken deployments (too restrictive) or insecure deployments (too permissive).

@lahabana
Copy link
Contributor

@johnharris85 I believe this was done in #4153 and #3925 covers the remaining gap of customizing the sidecar container notably for security context.
Are you ok for me to close this issue as #3925 covers exactly what's left?

@johnharris85
Copy link
Contributor Author

The only piece that (may?) be missing is our recommended defaults. I see that there are values commented out in the Helm chart additions in #4153 but not sure if we've validated they are actually the least-privilege defaults that still allow a working configuration?

@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label May 14, 2022
@github-actions
Copy link
Contributor

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

@lahabana lahabana removed the triage/stale Inactive for some time. It will be triaged again label May 16, 2022
@lahabana
Copy link
Contributor

Tracking recommended defaults in: #4298

@lahabana lahabana added triage/duplicated already exists and removed triage/needs-information Reviewed and some extra information was asked to the reporter labels May 17, 2022
@github-actions
Copy link
Contributor

Automatically closing the issue due to having one of the "closed state label".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature triage/duplicated already exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants