You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "a-nych (via GitHub)" <gi...@apache.org> on 2023/01/23 10:55:37 UTC

[GitHub] [solr-operator] a-nych opened a new pull request, #520: Use correct user for chmod command in cp-solr-xml init container

a-nych opened a new pull request, #520:
URL: https://github.com/apache/solr-operator/pull/520

   Resolves #519.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1512092257

   Thanks @a-nych , I'm planning on trying to cut the release by early afternoon tomorrow (EST timezone), so if you could get your review in early that'd be great!


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] a-nych commented on a diff in pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "a-nych (via GitHub)" <gi...@apache.org>.
a-nych commented on code in PR #520:
URL: https://github.com/apache/solr-operator/pull/520#discussion_r1169605511


##########
controllers/controller_utils_test.go:
##########
@@ -906,11 +906,13 @@ var (
 	}
 	one                    = int64(1)
 	two                    = int64(2)
+	three                  = int64(2)

Review Comment:
   A typo, it should be `int64(3)`.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman merged pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman merged PR #520:
URL: https://github.com/apache/solr-operator/pull/520


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] a-nych commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "a-nych (via GitHub)" <gi...@apache.org>.
a-nych commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1512071344

   Hi,
   
   Sorry for not responding - somehow missed these notifications. I will take
   a thorough look at this tomorrow.
   
   pon., 17 kwi 2023, 22:40 użytkownik Houston Putman ***@***.***>
   napisał:
   
   > So I have a general question about this. We only specify the fsGroup by
   > default
   > <https://github.com/apache/solr-operator/blob/main/controllers/util/solr_util.go#L479>.
   > Do we not want to use the fsGroup provided in the custom
   > PodSecurityContext?
   >
   > We do want to use the fsGroup, since that is what is used for volumes. I
   > am also making sure to use 8983 as the default fsGroup when a custom
   > podSecurityContext is provided that doesn't use an fsGroup.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/solr-operator/pull/520#issuecomment-1512057325>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACHEM53E4JCDET5JXQ6UOCDXBWTEJANCNFSM6AAAAAAUDWCU54>
   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1505569853

   @a-nych pinging again so that we can get this in the upcoming release


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1458428046

   @a-nych I'm pushing for a v0.7.0 release soon and it would be great to get this in there. Do you have any more info for my `runAsUser` vs `runAsGroup` question?


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #520:
URL: https://github.com/apache/solr-operator/pull/520#discussion_r1084358734


##########
controllers/util/solr_util.go:
##########
@@ -594,6 +594,21 @@ func generateSolrSetupInitContainers(solrCloud *solr.SolrCloud, solrCloudStatus
 	}
 	setupCommands := []string{"cp /tmp/solr.xml /tmp-config/solr.xml"}
 
+	// Figure out the solrUser and solrGroup to use
+	solrUser := DefaultSolrUser
+	solrGroup := DefaultSolrGroup
+	solrPodSecurityContext := solrCloud.Spec.CustomSolrKubeOptions.PodOptions.PodSecurityContext
+
+	if solrPodSecurityContext.RunAsUser != nil {

Review Comment:
   `solrPodSecurityContext` might be nil, which is why the tests are failing.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1513553585

   Had to include some fixes for https://github.com/apache/solr-operator/pull/548 and https://github.com/apache/solr-operator/issues/537 since the E2E tests wouldn't work without the fixes.
   
   This should be good to go though, it's been thoroughly tested using the E2E testing framework.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #520:
URL: https://github.com/apache/solr-operator/pull/520#issuecomment-1512057325

   > So I have a general question about this. We only specify the `fsGroup` [by default](https://github.com/apache/solr-operator/blob/main/controllers/util/solr_util.go#L479). Do we not want to use the `fsGroup` provided in the custom `PodSecurityContext`?
   
   We do want to use the `fsGroup`, since that is what is used for volumes. I am also making sure to use `8983` as the default `fsGroup` when a custom podSecurityContext is provided that doesn't use an fsGroup.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #520:
URL: https://github.com/apache/solr-operator/pull/520#discussion_r1170106387


##########
controllers/controller_utils_test.go:
##########
@@ -906,11 +906,13 @@ var (
 	}
 	one                    = int64(1)
 	two                    = int64(2)
+	three                  = int64(2)

Review Comment:
   Thanks for catching this!



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #520:
URL: https://github.com/apache/solr-operator/pull/520#discussion_r1169244952


##########
controllers/util/solr_util.go:
##########
@@ -594,6 +594,21 @@ func generateSolrSetupInitContainers(solrCloud *solr.SolrCloud, solrCloudStatus
 	}
 	setupCommands := []string{"cp /tmp/solr.xml /tmp-config/solr.xml"}
 
+	// Figure out the solrUser and solrGroup to use
+	solrUser := DefaultSolrUser
+	solrGroup := DefaultSolrGroup
+	solrPodSecurityContext := solrCloud.Spec.CustomSolrKubeOptions.PodOptions.PodSecurityContext
+
+	if solrPodSecurityContext.RunAsUser != nil {
+		solrUser = int(*solrPodSecurityContext.RunAsUser)
+
+		if solrPodSecurityContext.RunAsGroup != nil {

Review Comment:
   So this should be `fsGroup` not `runAsGroup`, since `fsGroup` is meant to be used for volumes.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #520: Use correct user for chmod command in cp-solr-xml init container

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #520:
URL: https://github.com/apache/solr-operator/pull/520#discussion_r1084366285


##########
controllers/util/solr_util.go:
##########
@@ -594,6 +594,21 @@ func generateSolrSetupInitContainers(solrCloud *solr.SolrCloud, solrCloudStatus
 	}
 	setupCommands := []string{"cp /tmp/solr.xml /tmp-config/solr.xml"}
 
+	// Figure out the solrUser and solrGroup to use
+	solrUser := DefaultSolrUser
+	solrGroup := DefaultSolrGroup
+	solrPodSecurityContext := solrCloud.Spec.CustomSolrKubeOptions.PodOptions.PodSecurityContext
+
+	if solrPodSecurityContext.RunAsUser != nil {
+		solrUser = int(*solrPodSecurityContext.RunAsUser)
+
+		if solrPodSecurityContext.RunAsGroup != nil {

Review Comment:
   The user might specify a `runAsGroup` without a `runAsUser`, so we probably want to keep these two checks separate right? Or am I thinking about this incorrectly?



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org