Copilot commented on code in PR #649:
URL: https://github.com/apache/solr-operator/pull/649#discussion_r2891035706
##########
controllers/util/solr_util.go:
##########
@@ -757,6 +757,7 @@ func generateSolrSetupInitContainers(solrCloud
*solr.SolrCloud, solrCloudStatus
Requests: volumePrepResources,
Limits: volumePrepResources,
},
+ SecurityContext: solrCloud.Spec.BusyBoxSecurityContext.ToSC(),
}
Review Comment:
Setting a non-root SecurityContext on the `cp-solr-xml` init container can
break the existing backup-repository prep logic in this same container: when
`BackupRepositories` include volume mounts, `setupCommands` may include
`addgroup`/`adduser`/`su` and `chown`, which typically require root (or extra
capabilities) and will fail when running as UID/GID 65534. Consider either (a)
skipping/reworking the user-creation + chown path when `BusyBoxSecurityContext`
enforces non-root (e.g., rely on FSGroup and only do a write-test with a clear
error), or (b) splitting the permission-fixup into a separate, optionally-root
init container so `cp-solr-xml` can remain non-root for `runAsNonRoot: true`
pods.
##########
api/v1beta1/solrcloud_types.go:
##########
@@ -204,6 +210,12 @@ func (spec *SolrCloudSpec) withDefaults(logger
logr.Logger) (changed bool) {
}
changed = spec.BusyBoxImage.withDefaults(DefaultBusyBoxImageRepo,
DefaultBusyBoxImageVersion, DefaultPullPolicy) || changed
+ if spec.BusyBoxSecurityContext == nil {
+ c := ContainerSecurityContext{}
+ spec.BusyBoxSecurityContext = &c
+ }
+ changed =
spec.BusyBoxSecurityContext.withDefaults(DefaultBusyBoxUserId,
DefaultBusyBoxGroupId, DefaultBusyBoxRunAsNonRoot) || changed
Review Comment:
New defaulting for `BusyBoxSecurityContext` introduces user-visible behavior
(init container securityContext defaults to UID/GID 65534 and `runAsNonRoot:
true`), but there doesn’t appear to be controller/unit test coverage asserting
the generated StatefulSet’s `cp-solr-xml` init container gets this
securityContext (and that overrides work). Adding assertions in the existing
controller tests that inspect init containers would help prevent regressions,
especially given the interaction with `PodSecurityContext`/FSGroup and
backup-repo prep.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
