Skip to content

Conversation

@symious
Copy link
Contributor

@symious symious commented Jan 13, 2021

…er setup

What changes were proposed in this pull request?

  • Add the option of "--security" to the tool of "genconf".

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2212

How was this patch tested?

Manually tested. The usage of the tool is now as follows.

Usage: ozone genconf [-hV] [--security] [--verbose] [-conf=<configurationPath>]
                     [-D=<String=String>]... <path>
Tool to generate template ozone-site.xml
      <path>       Directory path where ozone-site file should be generated.
      --security   Generate security config.
      --verbose    More verbose output. Show the stack trace of the errors.
      -conf=<configurationPath>

  -D, --set=<String=String>

  -h, --help       Show this help message and exit.
  -V, --version    Print version information and exit.

@amaliujia
Copy link
Contributor

amaliujia commented Jan 13, 2021

@symious is it possible to attach a screenshot for the output of this command flag?

@symious
Copy link
Contributor Author

symious commented Jan 13, 2021

@amaliujia Sure. The screen shot is as follows.
Screenshot 2021-01-13 at 2 00 19 PM

@elek
Copy link
Member

elek commented Feb 8, 2021

Thanks the patch @symious. It looks a simple change for me.

My only fear is that I am not sure if the generated config file is enough to start Ozone in secure environment.

For unsecure environment all the required attributes are filled with some reasonable default (like using /tmp for metadata).

I don't know what is a good approach, but the overall goal is to make the configuration easier for the users.

@symious
Copy link
Contributor Author

symious commented Feb 8, 2021

Thanks the patch @symious. It looks a simple change for me.

My only fear is that I am not sure if the generated config file is enough to start Ozone in secure environment.

For unsecure environment all the required attributes are filled with some reasonable default (like using /tmp for metadata).

I don't know what is a good approach, but the overall goal is to make the configuration easier for the users.

@elek Thanks for the reply. IMHO, secure config is for advanced users, that they do have the ability and requirement to build a secure cluster. The default config may also mislead the user somehow.

@adoroszlai
Copy link
Contributor

My only fear is that I am not sure if the generated config file is enough to start Ozone in secure environment.

Thanks @elek for sharing your thoughts. I also had the same doubt: most of the security-related configs are empty by default. That's why I pinged @dineshchitlangia, the reporter of this task, for review, in the hope that we can clarify the intended scope.

@symious
Copy link
Contributor Author

symious commented Feb 10, 2021

@elek @adoroszlai Thanks for the reply. I think it will be good to wait for @dineshchitlangia 's reply. Meanwhile, I can try to build a secure cluster and fill in the required configs.

Copy link
Contributor

@dineshchitlangia dineshchitlangia left a comment

Choose a reason for hiding this comment

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

@symious Thanks for working on this.

I also reviewed thoughts from @adoroszlai and @elek .

My thoughts on this are:

  1. We certainly need to make it easier for users as @elek mentioned.
  2. I think we can start by generating a security config template and that is why I suggested the inline change.
  3. We can file another jira to generate conf with predefined security config values(if at all possible).
  4. Add test for security template in TestGenerateOzoneRequiredConfigurations

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Option(names = "--security", description = "Generate security config.")
@Option(names = "--security", description = "Generates security config template, update template with proper values before use.")

@symious
Copy link
Contributor Author

symious commented Feb 13, 2021

@dineshchitlangia Thanks for the review. Added a new commit with the following update:

  1. Added the unit test for security config generation, and update the command option description.
  2. After checking the configurations with the tag of "SECURITY", I found a lot of unrelated or fundamental configs that users shouldn't use usually, such ashdds.x509.signature.algorithm. And then I walk through the document of Ozone Security, I found that all configs are related to kerberos, so my idea is to add a new tag named as "KERBEROS" to track the common used security configs.
  3. After checking the docker file of ozonesecure, I found that the principal and keytab are coordinated with the KDC server, which means the KDC server and the keytab file has to be generated and prepared before we start ozone cluster, so the configs must be updated manually unless we set up a KDC server and update the keytab files for the users, which I think is quite strange.

In docker, flokkr/issuer helps to initial KDC and update the keytab files, so for testing, it would be quite convenient for the user to just use docker. And for genconf --security, I think it's mainly for the users who has Kerberos set up already and want to fill the required security configs template for a productive secure cluster.

Copy link
Contributor

@dineshchitlangia dineshchitlangia left a comment

Choose a reason for hiding this comment

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

@symious Thank you for updating the PR.
Minor changes requested inline.

@elek what are your thoughts on the new KERBEROS tag? For me, it is fine to add the new tag as it is easy to identify which configs to deal with when you want to implement kerberos but I would like to hear your opinion too.

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1 let's merge it after a green build

I asked others on the community meeting and the agreement was that it's better to have this even if it doesn't generate all the required configs (keytabs should be provided, etc...)

Comments from @dineshchitlangia are adressed as far as I see, the only thing what we need is a green build...

@symious
Copy link
Contributor Author

symious commented Apr 26, 2021

@elek Thanks for the review, I just found the other comment @dineshchitlangia mentioned above, let me add the "SECURITY" tag, too.

Copy link
Contributor

@dineshchitlangia dineshchitlangia left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes. +1

@elek
Copy link
Member

elek commented May 3, 2021

Merging it now. Thanks the update @symious and the review @dineshchitlangia and @adoroszlai

@elek elek merged commit 48b84c5 into apache:master May 3, 2021
errose28 added a commit to errose28/ozone that referenced this pull request May 4, 2021
…ing-upgrade-master-merge2

* upstream/master: (56 commits)
  HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788)
  HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195)
  Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)"
  HDDS-4983. Display key offset for each block in command key info (apache#2051)
  HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)
  HDDS-4585. Support bucket acl operation in S3g (apache#1701)
  HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190)
  HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188)
  HDDS-5152. Fix Suggested leader in Client. (apache#2189)
  HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184)
  HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181)
  HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187)
  HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141)
  HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173)
  HDDS-4889. Add simple CI check for docs (apache#2156)
  HDDS-5131. Use timeout in github actions (apache#2176)
  HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155)
  HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166)
  HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096)
  HDDS-5083. Bump version of common-compress (apache#2139)
  ...

Conflicts:
	hadoop-hdds/common/pom.xml
	hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
	hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
	hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStorageConfig.java
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
	hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMStorage.java
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
	hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java
errose28 added a commit to errose28/ozone that referenced this pull request May 13, 2021
…k-in-auth

* HDDS-3698-nonrolling-upgrade: (57 commits)
  Fix compilation errors afte merge Update javassist in recon pom Fix changes introduced in merge that failed TestSCMNodeManager upgrade tests Fix checkstyle Fix intermittent test failure TestSCMNodeManager#testSetNodeOpStateAndCommandFired after merge Skip scm init default layout version in TestOzoneConfigurationFields
  HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788)
  HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195)
  Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)"
  HDDS-4983. Display key offset for each block in command key info (apache#2051)
  HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)
  HDDS-4585. Support bucket acl operation in S3g (apache#1701)
  HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190)
  HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188)
  HDDS-5152. Fix Suggested leader in Client. (apache#2189)
  HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184)
  HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181)
  HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187)
  HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141)
  HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173)
  HDDS-4889. Add simple CI check for docs (apache#2156)
  HDDS-5131. Use timeout in github actions (apache#2176)
  HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155)
  HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166)
  HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096)
  ...
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.

5 participants