You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "devmadhuu (via GitHub)" <gi...@apache.org> on 2023/08/21 13:48:11 UTC

[GitHub] [ozone] devmadhuu opened a new pull request, #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

devmadhuu opened a new pull request, #5203:
URL: https://github.com/apache/ozone/pull/5203

   ## What changes were proposed in this pull request?
   
   Most of the calls to SCM are only done by the namespace layer (OzoneManager), it makes sense to make all the client-facing APIs of SCM to do Admin check. SCM should only get calls from namespace service or from admin, it cannot get direct calls from a client.
   allocateBlock, deleteBlock and sortDatanodes doesn't have Admin check now. If we do Admin check for getContainerWithPipeline, we should also do the same in allocateBlock, deleteBlock and sortDatanodes.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-1796
   
   ## How was this patch tested?
   
   This patch is tested with existing Junit test cases of all the updated client facing APIs of SCM.
   


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1691802918

   > Secure acceptance tests are failing, likely because OM user is not an SCM admin. What is the best way to resolve this?
   
   @errose28 Thanks for pointing out, As per @ChenSammi  suggested to have specified admin user under "ozone.administrators" config based on test case, it can be handled, but I have noticed that few robot acceptance test cases are expecting for permission denied and if I specify admin user , those failed with message "'[ ]' does not contain 'PERMISSION_DENIED'", but since we have only docker-config where "ozone.administrators" config  can be added, so positive test cases passes, but negative test cases fails. Any suggestion how to handle 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@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1696336833

   > As per @ChenSammi suggested to have specified admin user under "ozone.administrators" config based on test case, it can be handled
   
   Would OM admin be considered an SCM admin by default? I'm not sure we want this, but if we do not do it this way it would be a breaking change because all users would need to update their admin configs.
   
   Also it looks like acceptance tests are still failing. @devmadhuu its probably better to convert this to a draft and debug the failures on your fork until CI is green.


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1316811932


##########
hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot:
##########
@@ -219,6 +219,8 @@ Test native authorizer
                     Should contain      ${result}    PERMISSION_DENIED
     ${result} =     Execute And Ignore Error         ozone sh key list ${protocol}${server}/${volume}/bb1
                     Should contain      ${result}    PERMISSION_DENIED
+    Execute         kdestroy
+    Run Keyword     Kinit test user     testuser2    testuser2.keytab

Review Comment:
   Is this still needed now? 



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1697508312

   Should we handle other file integration tests failing in this PR as part of other JIRA/PR ?


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1690931318

   > 
   
   Good question. We have property "ozone.administrators" and "ozone.administrators.groups" for Ozone administrator configuration. There properties cover both OM and SCM.  I didn't check how the unit is failed. In a real Ozone cluster, if OM and SCM are lauched by same user, then it will be fine. If different users are used to lauch OM and SCM, we requires end user to explictily include the user who launch OM in "ozone.administrators", or the user group in  "ozone.administrators.groups", otherwise the request from OM will be rejected by SCM. 
   


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1700838617

   @ChenSammi  Thank you for detailed review, As discussed, have removed checkAdminAccess for following SCM APIs:
   
   1. queryNode
   2. getReplicationManagerReport
   3. getContainerWithPipelineBatch
   4. getExistContainerWithPipelinesInBatch
   5. sortDatanodes
   
   Pls have a re-look


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1316811268


##########
hadoop-ozone/dist/src/main/smoketest/basic/links.robot:
##########
@@ -56,7 +56,7 @@ Verify Bucket ACL
 
 Can follow link with read access
     Execute             kdestroy
-    Run Keyword         Kinit test user             testuser2         testuser2.keytab
+    Run Keyword         Kinit test user             testuser          testuser.keytab

Review Comment:
   Is this still needed now? 



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1317347593


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -865,6 +863,7 @@ public void transferLeadership(String newLeaderId)
 
   public List<DeletedBlocksTransactionInfo> getFailedDeletedBlockTxn(int count,
       long startTxId) throws IOException {
+    getScm().checkAdminAccess(getRemoteUser(), true);

Review Comment:
   > This check can be removed too.
   
   Done.



##########
hadoop-ozone/dist/src/main/smoketest/basic/links.robot:
##########
@@ -56,7 +56,7 @@ Verify Bucket ACL
 
 Can follow link with read access
     Execute             kdestroy
-    Run Keyword         Kinit test user             testuser2         testuser2.keytab
+    Run Keyword         Kinit test user             testuser          testuser.keytab

Review Comment:
   Yes removed.



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1317348550


##########
hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot:
##########
@@ -219,6 +219,8 @@ Test native authorizer
                     Should contain      ${result}    PERMISSION_DENIED
     ${result} =     Execute And Ignore Error         ozone sh key list ${protocol}${server}/${volume}/bb1
                     Should contain      ${result}    PERMISSION_DENIED
+    Execute         kdestroy
+    Run Keyword     Kinit test user     testuser2    testuser2.keytab

Review Comment:
   Removed.



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi merged pull request #5203: HDDS-1796. Add admin access check for write opertaions in SCMClientProtocolServer and SCMBlockProtocolServer.

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #5203:
URL: https://github.com/apache/ozone/pull/5203


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1302869790


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java:
##########
@@ -333,6 +338,7 @@ public boolean addSCM(AddSCMRequest request) throws IOException {
   @Override
   public List<DatanodeDetails> sortDatanodes(List<String> nodes,
       String clientMachine) throws IOException {
+    scm.checkAdminAccess(getRemoteUser(), false);

Review Comment:
   Thanks @ChenSammi for review. I understood and handled the comment. Pls re-review.



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1689615756

   @devmadhuu , the overall patch looks good. 


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1316809977


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -865,6 +863,7 @@ public void transferLeadership(String newLeaderId)
 
   public List<DeletedBlocksTransactionInfo> getFailedDeletedBlockTxn(int count,
       long startTxId) throws IOException {
+    getScm().checkAdminAccess(getRemoteUser(), true);

Review Comment:
   This check can be removed too. 



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ashishkumar50 commented on pull request #5203: HDDS-1796. Add admin access check for write opertaions in SCMClientProtocolServer and SCMBlockProtocolServer.

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1738686166

   @ChenSammi @devmadhuu, In real ozone secure cluster, If remoteUser is "om" and scmAdmins is having "scm", because of this allocateBlock fails. Do we need to change any configuration?
   Whether allocateBlock, deleteBlock need this check?


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1302745596


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java:
##########
@@ -333,6 +338,7 @@ public boolean addSCM(AddSCMRequest request) throws IOException {
   @Override
   public List<DatanodeDetails> sortDatanodes(List<String> nodes,
       String clientMachine) throws IOException {
+    scm.checkAdminAccess(getRemoteUser(), false);

Review Comment:
   sortDatanodes is kind of a read operation, it doesn't change the in memory or persist state of pipeline.



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1691809191

   > > Secure acceptance tests are failing, likely because OM user is not an SCM admin. What is the best way to resolve this?
   > 
   > @errose28 Thanks for pointing out, As per @ChenSammi suggested to have specified admin user under "ozone.administrators" config based on test case, it can be handled, but I have noticed that few robot acceptance test cases are expecting for permission denied and if I specify admin user , those failed with message "'[ ]' does not contain 'PERMISSION_DENIED'", but since we have only docker-config where "ozone.administrators" config can be added, so positive test cases passes, but negative test cases fails. Any suggestion how to handle this ?
   
   Is it available to use other users which is not included in "ozone.administrators" to run the test which expects a PERMISSION_DENIED? 


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1698494596

   > Should we handle other file integration tests failing in this PR as part of other JIRA/PR ?
   > 
   file integration test failing can be handled with a new JIRA. 
   


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1697161036

   > > As per @ChenSammi suggested to have specified admin user under "ozone.administrators" config based on test case, it can be handled
   > 
   > Would OM admin be considered an SCM admin by default? I'm not sure we want this, but if we do not do it this way it would be a breaking change because all users would need to update their admin configs.
   
   Currently, the "ozone.administrators" applies to both OM and SCM.  There is no individual property to control OM or SCM admins.  And both OM and SCM will add the user who launches the services as admin. Generally, the user who launches OM, will launches SCM too, so it will not be a general problem. 
   Before this PR, there is already admin check on getContainerWithPipeline.  This PR add the check to other APIs, so the behavior is consistent. 


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1708450247

   @ChenSammi  Handled comments, pls review.


-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5203:
URL: https://github.com/apache/ozone/pull/5203#discussion_r1302869790


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java:
##########
@@ -333,6 +338,7 @@ public boolean addSCM(AddSCMRequest request) throws IOException {
   @Override
   public List<DatanodeDetails> sortDatanodes(List<String> nodes,
       String clientMachine) throws IOException {
+    scm.checkAdminAccess(getRemoteUser(), false);

Review Comment:
   Thanks @ChenSammi for review. I understood and handled the comment.



-- 
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@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1690365355

   Secure acceptance tests are failing, likely because OM user is not an SCM admin. What is the best way to resolve 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@ozone.apache.org

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


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


[GitHub] [ozone] devmadhuu commented on pull request #5203: HDDS-1796. SCMClientProtocolServer#getContainerWithPipeline should check for admin access

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #5203:
URL: https://github.com/apache/ozone/pull/5203#issuecomment-1694305536

   > > > Secure acceptance tests are failing, likely because OM user is not an SCM admin. What is the best way to resolve this?
   > > 
   > > 
   > > @errose28 Thanks for pointing out, As per @ChenSammi suggested to have specified admin user under "ozone.administrators" config based on test case, it can be handled, but I have noticed that few robot acceptance test cases are expecting for permission denied and if I specify admin user , those failed with message "'[ ]' does not contain 'PERMISSION_DENIED'", but since we have only docker-config where "ozone.administrators" config can be added, so positive test cases passes, but negative test cases fails. Any suggestion how to handle this ?
   > 
   > Is it available to use other users which are not included in "ozone.administrators" to run the test which expects a PERMISSION_DENIED?
   
   I have resolved and able to successfully run all secure acceptance test cases now, however below 2 test cases related to aws s3g API is still failing. 
   
   <img width="606" alt="image" src="https://github.com/apache/ozone/assets/20607992/7844df00-c3c7-4d2d-9749-c1143b084a0a">
   


-- 
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@ozone.apache.org

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


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