Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
adoroszlai commented on PR #10239: URL: https://github.com/apache/ozone/pull/10239#issuecomment-4438351226 Thanks @dombizita, @priyeshkaratha for the 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: [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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
dombizita merged PR #10239: URL: https://github.com/apache/ozone/pull/10239 -- 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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
dombizita commented on PR #10239: URL: https://github.com/apache/ozone/pull/10239#issuecomment-4438185402 Thanks for working on this @adoroszlai, thank you for the review @priyeshkaratha! -- 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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
adoroszlai commented on code in PR #10239: URL: https://github.com/apache/ozone/pull/10239#discussion_r3231790553 ## hadoop-ozone/dist/src/main/compose/ozonesecure/docker-config: ## @@ -125,15 +124,6 @@ CORE-SITE.XML_hadoop.http.authentication.type=kerberos CORE-SITE.XML_hadoop.http.authentication.kerberos.principal=HTTP/[email protected] CORE-SITE.XML_hadoop.http.authentication.kerberos.keytab=/etc/security/keytabs/HTTP.keytab - -CORE-SITE.XML_hadoop.security.authorization=true -HADOOP-POLICY.XML_ozone.om.security.client.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.datanode.container.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.container.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.block.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.certificate.protocol.acl=* -HADOOP-POLICY.XML_ozone.security.reconfigure.protocol.acl=* Review Comment: > it was unnecessary before? Yes. > via `security.conf` Except it comes from `common/hadoop-security.conf`. The duplication between these two files in `common` is for future tasks to address. -- 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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
adoroszlai commented on code in PR #10239: URL: https://github.com/apache/ozone/pull/10239#discussion_r3231778063 ## hadoop-ozone/dist/src/main/compose/ozonesecure/docker-config: ## @@ -49,8 +49,7 @@ OZONE-SITE.XML_ozone.recon.address=recon:9891 OZONE-SITE.XML_ozone.security.enabled=true OZONE-SITE.XML_ozone.acl.enabled=true OZONE-SITE.XML_ozone.acl.authorizer.class=org.apache.hadoop.ozone.security.acl.OzoneNativeAuthorizer -OZONE-SITE.XML_ozone.administrators="testuser,recon,om" -OZONE-SITE.XML_ozone.s3.administrators="testuser,recon,om" +OZONE-SITE.XML_ozone.administrators="testuser,recon,om,hadoop" Review Comment: - `ozonesecure-mr` had even wider admin, but only `hadoop` needs to be added: https://github.com/apache/ozone/blob/03ceafcb6085189cb4694a31f094226237ed94b6/hadoop-ozone/dist/src/main/compose/ozonesecure-mr/docker-config#L39 - `ozone.s3.administrators` is removed because it is a duplicate (this is not related to the refactoring, just noticed): https://github.com/apache/ozone/blob/03ceafcb6085189cb4694a31f094226237ed94b6/hadoop-ozone/dist/src/main/compose/ozonesecure/docker-config#L53-L55 -- 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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
dombizita commented on code in PR #10239: URL: https://github.com/apache/ozone/pull/10239#discussion_r3230076925 ## hadoop-ozone/dist/src/main/compose/ozonesecure/docker-config: ## @@ -125,15 +124,6 @@ CORE-SITE.XML_hadoop.http.authentication.type=kerberos CORE-SITE.XML_hadoop.http.authentication.kerberos.principal=HTTP/[email protected] CORE-SITE.XML_hadoop.http.authentication.kerberos.keytab=/etc/security/keytabs/HTTP.keytab - -CORE-SITE.XML_hadoop.security.authorization=true -HADOOP-POLICY.XML_ozone.om.security.client.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.datanode.container.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.container.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.block.protocol.acl=* -HADOOP-POLICY.XML_hdds.security.client.scm.certificate.protocol.acl=* -HADOOP-POLICY.XML_ozone.security.reconfigure.protocol.acl=* Review Comment: I believe the hadoop policy changes are the default values, so it's not needed to be here. But why is that the `hadoop.security.authorization` is not needed to be set in the core-site.xml? One idea I had it that the `hadoop-secure.yaml` has it via `security.conf`, which is added in `hadoop-test.sh` that is used by `test-hadoop.sh`, so the moved `ozonesecure-mr` stuff has it via this extra compose file. In this case this is not needed for the `ozonesecure` suite and it was unnecessary before? ## hadoop-ozone/dist/src/main/compose/ozonesecure/docker-config: ## @@ -49,8 +49,7 @@ OZONE-SITE.XML_ozone.recon.address=recon:9891 OZONE-SITE.XML_ozone.security.enabled=true OZONE-SITE.XML_ozone.acl.enabled=true OZONE-SITE.XML_ozone.acl.authorizer.class=org.apache.hadoop.ozone.security.acl.OzoneNativeAuthorizer -OZONE-SITE.XML_ozone.administrators="testuser,recon,om" -OZONE-SITE.XML_ozone.s3.administrators="testuser,recon,om" +OZONE-SITE.XML_ozone.administrators="testuser,recon,om,hadoop" Review Comment: Just to make sure I understand: the only difference in the `ozonesecure-mr` and `ozonesecure` (other than the below change around hadoop configs) was that the hadoop user was added as ozone admin? The `ozone.s3.administrators` can be removed, as it's by default the same as `ozone.administrators`, which is good for this suite? -- 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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
adoroszlai commented on code in PR #10239:
URL: https://github.com/apache/ozone/pull/10239#discussion_r3223905753
##
hadoop-ozone/dist/src/main/compose/ozonesecure/docker-compose.yaml:
##
@@ -14,6 +14,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+# reusable fragments
+x-common-config:
+ &common-config
+ image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+ dns_search: .
+ volumes:
+- ../..:/opt/hadoop
+- ../_keytabs:/etc/security/keytabs
+- ./krb5.conf:/etc/krb5.conf
+ env_file:
+- docker-config
Review Comment:
`./` is required for `volumes` in the current directory ("to avoid
ambiguities with named volumes, relative paths should always begin with . or
.."). There is no such ambiguity with `env_file`, so the form `docker-config`
is recommended for simplicity.
https://docs.docker.com/reference/compose-file/services/
--
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]
Re: [PR] HDDS-15232. Reduce duplication in ozonesecure env [ozone]
priyeshkaratha commented on code in PR #10239:
URL: https://github.com/apache/ozone/pull/10239#discussion_r3223743878
##
hadoop-ozone/dist/src/main/compose/ozonesecure/docker-compose.yaml:
##
@@ -14,6 +14,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+# reusable fragments
+x-common-config:
+ &common-config
+ image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+ dns_search: .
+ volumes:
+- ../..:/opt/hadoop
+- ../_keytabs:/etc/security/keytabs
+- ./krb5.conf:/etc/krb5.conf
+ env_file:
+- docker-config
Review Comment:
Is it recommended ./docker-config or docker-config?
--
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]
