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 security context #238

Merged
merged 1 commit into from
Nov 17, 2022
Merged

Add security context #238

merged 1 commit into from
Nov 17, 2022

Conversation

danielpalstra
Copy link
Contributor

Add securityContext to the WildflyServer spec so that the STS can spawn Pods with limited privileges.

@mchoma
Copy link
Contributor

mchoma commented Nov 4, 2022

I am facing this warning on Kubernetes 1.24

[2022-11-04 09:45:23,379] INFO - Install the operator - quay.io/wildfly/wildfly-operator:latest.
Warning: would violate PodSecurity "restricted:v1.24": allowPrivilegeEscalation != false (container "wildfly-operator" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (container "wildfly-operator" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or container "wildfly-operator" must set securityContext.runAsNonRoot=true), seccompProfile (pod or container "wildfly-operator" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")

Do I assume correctly this MR is fixing it?

@danielpalstra
Copy link
Contributor Author

I am facing this warning on Kubernetes 1.24

[2022-11-04 09:45:23,379] INFO - Install the operator - quay.io/wildfly/wildfly-operator:latest.
Warning: would violate PodSecurity "restricted:v1.24": allowPrivilegeEscalation != false (container "wildfly-operator" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (container "wildfly-operator" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or container "wildfly-operator" must set securityContext.runAsNonRoot=true), seccompProfile (pod or container "wildfly-operator" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")

Do I assume correctly this MR is fixing it?

This PR makes it possible to fix the issue by setting the securityContext in the spec. In this case you would add the securityContext block to the configuration and make sure allowPrivilegeEscalation is set to false and all capabilities are dropped.

Copy link
Collaborator

@yersan yersan left a comment

Choose a reason for hiding this comment

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

Hello @danielpalstra , thanks for your contribution, we are evaluating it, two notes regarding your PR:

  1. Would you mind having a specific commit for the code formatting changes? At the moment, the wildfly-operator needs some adjustments on the build tools for building by using the latest Go version in addition to the required changes in the code formatting.
    Having a separate commit just to address the code formatting helps us backport the changes to other branches.

  2. Could you also add the required changes to the documentation (apis.adoc)?
    For your changes is pretty simple, you have just to follow what has been done for the Resources, see https://github.com/wildfly/wildfly-operator/blame/main/doc/apis.adoc#L47

@danielpalstra danielpalstra changed the title Add security context WIP: Add security context Nov 8, 2022
@danielpalstra
Copy link
Contributor Author

  1. I reverted the formatting changes made by go1.19. I suggest creating a seperate issue/ PR for the move to go1.19
  2. I've added the SecurityContext block to the docs as requested.

@danielpalstra danielpalstra changed the title WIP: Add security context Add security context Nov 8, 2022
Copy link
Collaborator

@yersan yersan left a comment

Choose a reason for hiding this comment

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

I reverted the formatting changes made by go1.19. I suggest creating a seperate issue/ PR for the move to go1.19

Thanks @danielpalstra , yes, we have #239 and #240 to address some details about 1.19

I've added the SecurityContext block to the docs as requested.

Great, I've added a minor point regarding the documentation. Would you mind fixing it and squashing all the commits into a single one?

I'm approving this PR since I don't have any other concerns about it, but we need the above point done to get it ready for merging.

doc/apis.adoc Outdated Show resolved Hide resolved
@yersan
Copy link
Collaborator

yersan commented Nov 9, 2022

Do I assume correctly this MR is fixing it?

@mchoma The warning comes due to the wildfly-operator pod. This PR is about adding capabilities to modify the SecurityContext for the pods created by the StatefulSet, so it is not going to fix the specific warning you are seeing.

My understanding is the warning comes due to the standard pod security policies managed by the cluster or namespace where you are installing the Operator. We have to address them by defining the SecurityContext on the Deployment resource that deploys the Operator, we have to think about how we should do it and whether it will need to be dynamic or exposed to the user. I'm not sure yet.

With this PR the user will be able to configure the SecurityContext of the pods generated by the StatefulSet, so it will help the users to adapt the Operator workload to the current cluster policy if they need it.

@danielpalstra
Copy link
Contributor Author

I've changed the docs and rebased as requested.

@danielpalstra
Copy link
Contributor Author

Hi @yersan do you have any eta on the merge of this PR? I would love to further test drive this in a real environment.

@yersan
Copy link
Collaborator

yersan commented Nov 17, 2022

Hi @danielpalstra , I was waiting a bit more because this PR is going to generate conflicts with others in progress, but let's do it now and I'll resolve the conflicts.

Notice, we don't have an ETA for releasing the Operator on the Operator Hub, but you will get your changes on the Operator image available at quay.io/wildfly/wildfly-operator:latest

Thanks for the contribution, please report any issue if you find something when you are testing on your environment.

@yersan yersan merged commit 6f2e4e9 into wildfly:main Nov 17, 2022
@jmesnil jmesnil added this to the 1.0.0 milestone Mar 13, 2023
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.

4 participants