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

Add container level security context for task and web deployments #1728

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

gdasson
Copy link

@gdasson gdasson commented Feb 24, 2024

SUMMARY

The security context settings offered today only provide the option to set pod level security context for web and task deployments. This PR adds the option to allow container level security context for all of the containers under web and task deployments.

fixes: #1413
fixes: #890
fixes: #571
fixes: #383

This change doesn't dictate the values and let the users decide and configure the values on need basis. This makes it a safer approach to implement without breaking any functionality

ISSUE TYPE
  • New or Enhanced Feature
ADDITIONAL INFORMATION

Two of the existing variable settings will become irrelevant after this change:

  • redis_capabilities can be covered under redis_security_context_settings after this change
  • task_privileged can be covered under task_security_context_settings after this change

@gdasson gdasson changed the title Added container level security context for task and web deployments Add container level security context for task and web deployments Feb 24, 2024
@rooftopcellist
Copy link
Member

rooftopcellist commented Feb 28, 2024

@gdasson Looks like this PR follows the plan laid out in this comment:

Changes suggested before merge:

  • I think _settings at the end of each of these is redundant and can be removed, what do you think? Otherwise, I think this approach will work for v1beta1.
  • Unfortunately, we have to provide backwards compatibility for the existing securityContext settings,
    • Example of how to fix for task_privileged: here
    • Example of how to fix for redis_capabilities: here
    • Same for postgres_security_context_settings
  • Can you also mark the task_privileged, postgres_security_context_settings and redis_capabilities parameters as Deprecated? (for example)

Note: We may re-write this to be settings nested under each component when we make a v2 CRD schema, but that is down the line and it would be good to give users a way to configure this now.

@gdasson gdasson force-pushed the devel branch 3 times, most recently from eac2297 to 6a7aace Compare March 1, 2024 00:17
@gdasson
Copy link
Author

gdasson commented Mar 1, 2024

@rooftopcellist : Thanks for the review and feedback. I have now incorporated your review comments. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants