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 users to configure Solr container's SecurityContext #743

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

gerlowskija
Copy link
Contributor

An alternate approach to #489.

#702 attempts to solve the same underlying issue by giving the Solr container in our STS template a hardcoded SecurityContext. But that effort is, at the time of writing, stalled out: partly due to questions about whether the hardcoded SecurityContext would be too restrictive for some usecases, and partly due to an inability to test in the relevant environments.

This PR takes a different approach by leaving the securityContext unset by default, and instead giving interested users the ability to specify an arbitrary securityContext as a part of their SolrCloud (or as a 'solr' helm chart setting). A securityContext can be provided in the resource YAML at .spec.customSolrKubeOptions.podOptions.containerSecurityContext (or using the podOptions.containerSecurityContext variable in the helm chart).

e.g.

apiVersion: solr.apache.org/v1beta1
kind: SolrCloud
metadata:
    ...
spec:
  solrImage:
    repository: solr
  customSolrKubeOptions:
    podOptions:
      containerSecurityContext:
        capabilities:
          drop:
            - ALL
        allowPrivilegeEscalation: false
  ...
  zookeeperRef:
    provided:
      chroot: "/"
      replicas: 3
      maxUnavailableReplicas: 1

No support yet in helm chart.
Still need to prepare, tidy, e2e-test, etc.
But need to vet approach first.
@gerlowskija gerlowskija marked this pull request as ready for review January 9, 2025 14:27
Copy link
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Looks great! Just needs a changeling entry.

@@ -38,6 +39,11 @@ func newBoolPtr(value bool) *bool {
return &newBool
}

func newIntPtr(value int64) *int64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to use pointer.Int64(...) for this. (The pointer library being "k8s.io/utils/pointer")

Copy link
Contributor

Choose a reason for hiding this comment

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

I see the I probably added the methods above that do the same thing. So we can always go and remove them independently, but let's not add another one.

@gerlowskija gerlowskija merged commit 9f3109e into apache:main Jan 10, 2025
3 checks passed
@gerlowskija gerlowskija deleted the security-context-placeholders branch January 10, 2025 19:08
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