Re: Review Request 49387: Support password type for custom properties

[Alexandr Antonenko]

> On July 25, 2016, 9:08 p.m., Alexandr Antonenko wrote:
> > Ship It!
> 
> Keta Patel wrote:
> Thank you Alexandr!
> Could you please help with pushing in the changes?

done


- Alexandr


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143428
---


On July 25, 2016, 8:47 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 25, 2016, 8:47 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  aba45bf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js 24c9cfa 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 9bb977a 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the patch "AMBARI-17041-trunk-July08.patch" the following is result of 
> ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 tests pending
>   
> 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 25, 2016, 9:08 p.m., Alexandr Antonenko wrote:
> > Ship It!

Thank you Alexandr!
Could you please help with pushing in the changes?


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143428
---


On July 25, 2016, 8:47 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 25, 2016, 8:47 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  aba45bf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js 24c9cfa 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 9bb977a 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the patch "AMBARI-17041-trunk-July08.patch" the following is result of 
> ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 tests pending
>   
> With the patch 

Re: Review Request 49387: Support password type for custom properties

[Alexandr Antonenko]

> On July 20, 2016, 11:17 p.m., Alexandr Antonenko wrote:
> > ambari-web/app/utils/config.js, line 264
> > 
> >
> > I do not see FINAL populated anywhere. I think this code will trigger 
> > fatal error. Have you tested this part manually ? 
> > 
> > Also, there is no need to check attribute[0], you just created it, and 
> > anyway there will be one and more values in that array
> > 
> > In code : attributes[0][FINAL][index] && attributes[0][FINAL][index] 
> > === "true"
> > 
> > You can skip cheking attributes[0][FINAL][index], just check 
> > attributes[0][FINAL][index] === "true".
> > 
> > If property "attributes[0][FINAL][index]" does not exist 
> > attributes[0][FINAL][index] === "true" will not trigger any error
> 
> Keta Patel wrote:
> The updated patch "AMBARI-17041-July21-updated.patch" conatins this 
> correction. I have kept the original statement with the name of the variable 
> updated to "attributes[0][FINAL]".
> 
> Thank you!
> 
> Alexandr Antonenko wrote:
> You are using your newly created variable FINAL only one time in 
> attributes[0][FINAL][index]. Why not simply use 
> attributes[0]["FINAL"][index], passing needed string directly, without 
> creating variable FINAL. especially when you have file with tons of variables 
> and functions which names have FINAL in it.
> 
> Keta Patel wrote:
> Hello Alexandr,
> I have updated the latest patch "AMBARI-17041-July22.patch" by removing 
> the "FINAL" variable. I have still kept the variable "PASSWORD" as it is used 
> in 2 functions (just once in both functions though).
> 
> Thank you!

Good job


- Alexandr


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143024
---


On July 25, 2016, 8:47 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 25, 2016, 8:47 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information 

Re: Review Request 49387: Support password type for custom properties

[Alexandr Antonenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143428
---


Ship it!




Ship It!

- Alexandr Antonenko


On July 25, 2016, 8:47 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 25, 2016, 8:47 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  aba45bf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js 24c9cfa 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 9bb977a 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the patch "AMBARI-17041-trunk-July08.patch" the following is result of 
> ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 tests pending
>   
> With the patch "AMBARI-17041-trunk-July13.patch" the following is the result 
> of ambari-web tests:
> 
>   29021 tests complete (26 seconds)
>   154 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 25, 2016, 8:47 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 aba45bf 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js 24c9cfa 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 844806f 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 9bb977a 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
964e193 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing (updated)
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending
  
  
With the patch "AMBARI-17041-trunk-July08.patch" the following is result of 
ambari-web tests:

  29017 tests complete (48 seconds)
  154 tests pending
  
With the patch "AMBARI-17041-trunk-July13.patch" the following is the result of 
ambari-web tests:

  29021 tests complete (26 seconds)
  154 tests pending
  
With the patch "AMBARI-17041-trunk-July20.patch" the following is the result of 
ambari-web tests:
  29292 tests complete (42 seconds)
  154 tests pending
  
With the patch "AMBARI-17041-July21-updated.patch" the following is the result 
of ambari-web tests:
  29343 tests complete (55 seconds)
  154 tests pending
  
**With the patch "AMBARI-17041-July22.patch" the following is the result of 
ambari-web tests:
  29371 tests complete (43 seconds)
  154 tests pending


File Attachments


AMBARI-17041-July21-ES6.patch
  

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 20, 2016, 11:17 p.m., Alexandr Antonenko wrote:
> > ambari-web/app/utils/config.js, line 264
> > 
> >
> > I do not see FINAL populated anywhere. I think this code will trigger 
> > fatal error. Have you tested this part manually ? 
> > 
> > Also, there is no need to check attribute[0], you just created it, and 
> > anyway there will be one and more values in that array
> > 
> > In code : attributes[0][FINAL][index] && attributes[0][FINAL][index] 
> > === "true"
> > 
> > You can skip cheking attributes[0][FINAL][index], just check 
> > attributes[0][FINAL][index] === "true".
> > 
> > If property "attributes[0][FINAL][index]" does not exist 
> > attributes[0][FINAL][index] === "true" will not trigger any error
> 
> Keta Patel wrote:
> The updated patch "AMBARI-17041-July21-updated.patch" conatins this 
> correction. I have kept the original statement with the name of the variable 
> updated to "attributes[0][FINAL]".
> 
> Thank you!
> 
> Alexandr Antonenko wrote:
> You are using your newly created variable FINAL only one time in 
> attributes[0][FINAL][index]. Why not simply use 
> attributes[0]["FINAL"][index], passing needed string directly, without 
> creating variable FINAL. especially when you have file with tons of variables 
> and functions which names have FINAL in it.

Hello Alexandr,
I have updated the latest patch "AMBARI-17041-July22.patch" by removing the 
"FINAL" variable. I have still kept the variable "PASSWORD" as it is used in 2 
functions (just once in both functions though).

Thank you!


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143024
---


On July 21, 2016, 11:11 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 21, 2016, 11:11 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 21, 2016, 11:11 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 066acab 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js 24c9cfa 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 844806f 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 9bb977a 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
964e193 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing (updated)
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending
  
  
With the patch "AMBARI-17041-trunk-July08.patch" the following is result of 
ambari-web tests:

  29017 tests complete (48 seconds)
  154 tests pending
  
With the patch "AMBARI-17041-trunk-July13.patch" the following is the result of 
ambari-web tests:

  29021 tests complete (26 seconds)
  154 tests pending
  
With the patch "AMBARI-17041-trunk-July20.patch" the following is the result of 
ambari-web tests:
  29292 tests complete (42 seconds)
  154 tests pending
  
**With the latest patch "AMBARI-17041-July21-updated.patch" the following is 
the result of ambari-web tests:
  29343 tests complete (55 seconds)
  154 tests pending


File Attachments


AMBARI-17041-July21-ES6.patch
  
https://reviews.apache.org/media/uploaded/files/2016/07/21/866014d0-310d-4e05-8290-2f82de990824__AMBARI-17041-July21-ES6.patch


Thanks,

Keta Patel

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 20, 2016, 11:17 p.m., Alexandr Antonenko wrote:
> > ambari-web/app/utils/config.js, line 264
> > 
> >
> > I do not see FINAL populated anywhere. I think this code will trigger 
> > fatal error. Have you tested this part manually ? 
> > 
> > Also, there is no need to check attribute[0], you just created it, and 
> > anyway there will be one and more values in that array
> > 
> > In code : attributes[0][FINAL][index] && attributes[0][FINAL][index] 
> > === "true"
> > 
> > You can skip cheking attributes[0][FINAL][index], just check 
> > attributes[0][FINAL][index] === "true".
> > 
> > If property "attributes[0][FINAL][index]" does not exist 
> > attributes[0][FINAL][index] === "true" will not trigger any error

The updated patch "AMBARI-17041-July21-updated.patch" conatins this correction. 
I have kept the original statement with the name of the variable updated to 
"attributes[0][FINAL]".

Thank you!


> On July 20, 2016, 11:17 p.m., Alexandr Antonenko wrote:
> > ambari-web/app/utils/config.js, line 248
> > 
> >
> > FYI: this can be also simplified 
> > 
> > var attributes = [];
> > ['FINAL', 'PASSWORD', 'USER', 'GROUP', 'TEXT', 
> > 'ADDITIONAL_USER_PROPERTY', 'NOT_MANAGED_HDFS_PATH', 
> > 'VALUE_FROM_PROPERTY_FILE'].forEach(function (propertyName) {
> >attributes.push(
> >{
> > [propertyName] : Em.get(configJSON, 
> > 'properties_attributes.'+propertyName) || {}})
> > })

Hello Alexandr,
I tried using the optimized statements you've mentioned here. The file attached 
"AMBARI-17041-July21-ES6.patch" contains the patch using this optimized code. 
The build is successful and even the UI behaves as intended after deploying the 
build. However, the ambari-web tests fail for this patch. I get a Syntax Error 
as seen in attachment "ambari_web_failed_to_execute_test" on the Jira. The 
tests didn't give further information on where the error occurred, but I 
figured out that the square brackets used for "propertyName" in the code above, 
belongs to ECMAScript 6 standard. Somehow, the test framework is not able to 
accept this syntax as valid. On removing the square brackets (only for 
testing), the ambari-web tests ran successfully.

I have used a workaround for this issue by using another variable to store the 
json object and then pushing that json variable to "attributes". This code is 
present in "AMBARI-17041-July21-updated.patch" (updated diff). The ambari-web 
tests are successful. Please let me know if I was making any mistake earlier or 
if I could update the patch in some other way.

Thank you!


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143024
---


On July 20, 2016, 9:05 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 20, 2016, 9:05 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 21, 2016, 11:05 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 066acab 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js 24c9cfa 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 844806f 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 9bb977a 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
964e193 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending
  
  
With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
result of ambari-web tests:

  29017 tests complete (48 seconds)
  154 tests pending
  
With the new patch "AMBARI-17041-trunk-July13.patch" the following is the 
result of ambari-web tests:

  29021 tests complete (26 seconds)
  154 tests pending
  
**With the new patch "AMBARI-17041-trunk-July20.patch" the following is the 
result of ambari-web tests:
  29292 tests complete (42 seconds)
  154 tests pending


File Attachments (updated)


AMBARI-17041-July21-ES6.patch
  
https://reviews.apache.org/media/uploaded/files/2016/07/21/866014d0-310d-4e05-8290-2f82de990824__AMBARI-17041-July21-ES6.patch


Thanks,

Keta Patel

Re: Review Request 49387: Support password type for custom properties

[Alexandr Antonenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review143024
---




ambari-web/app/utils/config.js (line 248)


FYI: this can be also simplified 

var attributes = [];
['FINAL', 'PASSWORD', 'USER', 'GROUP', 'TEXT', 'ADDITIONAL_USER_PROPERTY', 
'NOT_MANAGED_HDFS_PATH', 'VALUE_FROM_PROPERTY_FILE'].forEach(function 
(propertyName) {
   attributes.push(
   {
[propertyName] : Em.get(configJSON, 
'properties_attributes.'+propertyName) || {}})
})



ambari-web/app/utils/config.js (line 264)


I do not see FINAL populated anywhere. I think this code will trigger fatal 
error. Have you tested this part manually ? 

Also, there is no need to check attribute[0], you just created it, and 
anyway there will be one and more values in that array

In code : attributes[0][FINAL][index] && attributes[0][FINAL][index] === 
"true"

You can skip cheking attributes[0][FINAL][index], just check 
attributes[0][FINAL][index] === "true".

If property "attributes[0][FINAL][index]" does not exist 
attributes[0][FINAL][index] === "true" will not trigger any error


- Alexandr Antonenko


On July 20, 2016, 9:05 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 20, 2016, 9:05 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  066acab 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js 24c9cfa 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 20, 2016, 9:05 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 066acab 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js 24c9cfa 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 844806f 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 9bb977a 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
964e193 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing (updated)
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending
  
  
With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
result of ambari-web tests:

  29017 tests complete (48 seconds)
  154 tests pending
  
With the new patch "AMBARI-17041-trunk-July13.patch" the following is the 
result of ambari-web tests:

  29021 tests complete (26 seconds)
  154 tests pending
  
**With the new patch "AMBARI-17041-trunk-July20.patch" the following is the 
result of ambari-web tests:
  29292 tests complete (42 seconds)
  154 tests pending


Thanks,

Keta Patel

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 20, 2016, 2:04 p.m., Alexandr Antonenko wrote:
> > ambari-web/app/utils/config.js, line 256
> > 
> >
> > When you see so many if statements, the first thing that comes to mind 
> > that this needs to be optimized )))
> > 
> > Please put "final", "password", "user" and other to array and use this 
> > array in cycle

Hello Alexandr,
I have updated the patch "AMBARI-17041-July20.patch" as per your suggestion.

Thank you!


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review142938
---


On July 18, 2016, 4:40 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 18, 2016, 4:40 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  b2a3761 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js ea53262 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js dcdb236 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch 

Re: Review Request 49387: Support password type for custom properties

[Dmitro Lisnichenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review142943
---


Ship it!




Ship It!

- Dmitro Lisnichenko


On July 18, 2016, 7:40 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 18, 2016, 7:40 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  b2a3761 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js ea53262 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js dcdb236 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
> result of ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 tests pending
>   
> With the new patch "AMBARI-17041-trunk-July13.patch" the following is the 
> result of ambari-web tests:
> 
>   29021 tests complete (26 

Re: Review Request 49387: Support password type for custom properties

[Alexandr Antonenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review142938
---




ambari-web/app/utils/config.js (line 256)


When you see so many if statements, the first thing that comes to mind that 
this needs to be optimized )))

Please put "final", "password", "user" and other to array and use this 
array in cycle


- Alexandr Antonenko


On July 18, 2016, 4:40 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 18, 2016, 4:40 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  b2a3761 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js ea53262 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js dcdb236 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the new latest patch 

Re: Review Request 49387: Support password type for custom properties

[Jonathan Hurley]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review142933
---


Ship it!




Ship It!

- Jonathan Hurley


On July 18, 2016, 12:40 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 18, 2016, 12:40 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  b2a3761 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js ea53262 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 844806f 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js dcdb236 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> 964e193 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
> result of ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 tests pending
>   
> With the new patch "AMBARI-17041-trunk-July13.patch" the following is the 
> result of ambari-web tests:
> 
>   29021 tests complete (26 

Re: Review Request 49387: Support password type for custom properties

[Jonathan Hurley]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review142375
---




ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
(lines 163 - 164)


Is 'password' declared somewhere as an enum or static string? It would be 
better to re-use it as such so that references can be more easily searched.


- Jonathan Hurley


On July 8, 2016, 5:15 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 8, 2016, 5:15 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js bbac1bd 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 861d599 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 6c6e7f6 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> bdd0530 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
>   
>   
> With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
> result of ambari-web tests:
> 
>   29017 tests complete (48 seconds)
>   154 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 9, 2016, 4:49 a.m., Matt wrote:
> > ambari-web/app/mixins/common/configs/configs_saver.js, line 469
> > 
> >
> > Isn't it possible to simplify all these if conditions (considering your 
> > property types and attribute names are the same, besides the 
> > upper/lowercase)?
> > 
> > ```
> > if (Em.get(property,'propertyName') != null) {
> >   Em.get(property,'propertyName').map(function(propType) { 
> > attributes[propType.toLowerCase()][Em.get(property,'name')] = true;
> >   });
> > }
> > ```

Hello Matt,
Yes, your code looks concise. I will make sure to include this in the final 
patch.

Thank you!


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review141391
---


On July 8, 2016, 9:15 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 8, 2016, 9:15 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js bbac1bd 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 861d599 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 6c6e7f6 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> bdd0530 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure 

Re: Review Request 49387: Support password type for custom properties

[Matt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review141391
---




ambari-web/app/mixins/common/configs/configs_saver.js (line 469)


Isn't it possible to simplify all these if conditions (considering your 
property types and attribute names are the same, besides the upper/lowercase)?

```
if (Em.get(property,'propertyName') != null) {
  Em.get(property,'propertyName').map(function(propType) { 
attributes[propType.toLowerCase()][Em.get(property,'name')] = true;
  });
}
```


- Matt


On July 8, 2016, 2:15 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 8, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js bbac1bd 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 861d599 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 6c6e7f6 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> bdd0530 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 8, 2016, 9:15 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js bbac1bd 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 861d599 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 6c6e7f6 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
bdd0530 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing (updated)
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending
  
  
With the new latest patch "AMBARI-17041-trunk-July08.patch" the following is 
result of ambari-web tests:

  29017 tests complete (48 seconds)
  154 tests pending


Thanks,

Keta Patel

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

> On July 8, 2016, 2:12 p.m., Di Li wrote:
> > please add Jonathan Hurley to the review

Thank you Di. I have added him.


- Keta


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review141280
---


On July 8, 2016, 9:05 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated July 8, 2016, 9:05 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
> Jonathan Hurley.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js bbac1bd 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 861d599 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js 6c6e7f6 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> bdd0530 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 49387: Support password type for custom properties

[Keta Patel]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/
---

(Updated July 8, 2016, 9:05 p.m.)


Review request for Ambari, Alexandr Antonenko, Di Li, Dmitro Lisnichenko, and 
Jonathan Hurley.


Changes
---

Updated the patch to allow multiple selection of 


Bugs: AMBARI-17041
https://issues.apache.org/jira/browse/AMBARI-17041


Repository: ambari


Description
---

Currently, services can define properties in the XML configuration files that 
is flagged as type password:
  
my.special.password

PASSWORD
Password to be masked
 
and it will be masked properly in the UI as well as blueprint.

Custom property should also support this option so that password can be added 
as custom property and treat accordingly.


==
Proposed Design for the fix:
==
At present only the key-value information of the service properties is stored 
in the DB ("clusterconfig" table in the "config_data" column). 
The "config_attributes" column stores only certain attributes like "final" 
indicating the list of properties set with the Final flag = true. 
The information about the property-type (i.e PASSWORD, USER, GROUP, 
ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, etc) 
is extracted from the corresponding service's property file (e.g. 
hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
information of the existing properties only. Custom Properties added by ambari 
user have no provision to store their additional attributes. 

Since, for this Jira we are concerned with only  attribute for 
Custom Properties, we could add an additional field called "Property Type" in 
the "Add Property" pop-up which shows up on clicking "Add Property ..." in the 
Custom property section for a service. For now, only 2 options are shown in the 
drop-down list: NONE and PASSWORD .
A few sample test properties are created using the new "Add Property" pop-up as 
can be seen in the following attachments. 
Attachments: 
"add_property_pop_up.tiff"
"custom_property_password_type.tiff"
"custom_property_regular_type.tiff"
"custom_properties_after_save.tiff"

The  information for these Custom properties is stored in the DB 
in "clusterconfig" table, "config_attributes" column.
The schema for "clusterconfig" table can be seen in the attachment:
"schema_of_clusterconfig_table.tiff"

The content of the "config_attributes" column with the  
information from the new Custom properties can be seen in the attachment:
"cluster_config_with_password_type_in_config_attributes_column.tiff"


Note: The fix so far is performed only for new Custom properties. The 
 information for existing properties is extracted from the 
corresponding property xml files for the service.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
 eef3474 
  
ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java 
84f3109 
  ambari-web/app/messages.js bbac1bd 
  ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
  ambari-web/app/models/configs/objects/service_config_property.js 861d599 
  ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
  ambari-web/app/utils/config.js 6c6e7f6 
  ambari-web/app/views/common/configs/service_configs_by_category_view.js 
bdd0530 
  ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 

Diff: https://reviews.apache.org/r/49387/diff/


Testing
---

No new test cases are written in the patch apart from the fix required in 
"configs_saver_test.js" to avoid test failure of existing tests.
If the proposed solution is acceptable, I will create a new patch with the 
necessary new tests.

The existing ambari-web tests after applying the patch:

  28977 tests complete (37 seconds)
  154 tests pending


Thanks,

Keta Patel

Re: Review Request 49387: Support password type for custom properties

[Di Li]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49387/#review141280
---



please add Jonathan Hurley to the review

- Di Li


On June 29, 2016, 9:05 p.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49387/
> ---
> 
> (Updated June 29, 2016, 9:05 p.m.)
> 
> 
> Review request for Ambari, Alexandr Antonenko, Di Li, and Dmitro Lisnichenko.
> 
> 
> Bugs: AMBARI-17041
> https://issues.apache.org/jira/browse/AMBARI-17041
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently, services can define properties in the XML configuration files that 
> is flagged as type password:
>   
> my.special.password
> 
> PASSWORD
> Password to be masked
>  
> and it will be masked properly in the UI as well as blueprint.
> 
> Custom property should also support this option so that password can be added 
> as custom property and treat accordingly.
> 
> 
> ==
> Proposed Design for the fix:
> ==
> At present only the key-value information of the service properties is stored 
> in the DB ("clusterconfig" table in the "config_data" column). 
> The "config_attributes" column stores only certain attributes like "final" 
> indicating the list of properties set with the Final flag = true. 
> The information about the property-type (i.e PASSWORD, USER, GROUP, 
> ADDITIONAL_USER_PROPERTY, VALUE_FROM_PROPERTY_FILE, NOT_MANAGED_HDFS_PATH, 
> etc) is extracted from the corresponding service's property file (e.g. 
> hive-site.xml, core-site.xml, webhcat-env.xml, etc). These files contain 
> information of the existing properties only. Custom Properties added by 
> ambari user have no provision to store their additional attributes. 
> 
> Since, for this Jira we are concerned with only  attribute for 
> Custom Properties, we could add an additional field called "Property Type" in 
> the "Add Property" pop-up which shows up on clicking "Add Property ..." in 
> the Custom property section for a service. For now, only 2 options are shown 
> in the drop-down list: NONE and PASSWORD .
> A few sample test properties are created using the new "Add Property" pop-up 
> as can be seen in the following attachments. 
> Attachments: 
> "add_property_pop_up.tiff"
> "custom_property_password_type.tiff"
> "custom_property_regular_type.tiff"
> "custom_properties_after_save.tiff"
> 
> The  information for these Custom properties is stored in the 
> DB in "clusterconfig" table, "config_attributes" column.
> The schema for "clusterconfig" table can be seen in the attachment:
> "schema_of_clusterconfig_table.tiff"
> 
> The content of the "config_attributes" column with the  
> information from the new Custom properties can be seen in the attachment:
> "cluster_config_with_password_type_in_config_attributes_column.tiff"
> 
> 
> Note: The fix so far is performed only for new Custom properties. The 
>  information for existing properties is extracted from the 
> corresponding property xml files for the service.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ConfigurationResponse.java
>  eef3474 
>   
> ambari-server/src/main/java/org/apache/ambari/server/utils/SecretReference.java
>  84f3109 
>   ambari-web/app/messages.js d40a84a 
>   ambari-web/app/mixins/common/configs/configs_saver.js 43a77ce 
>   ambari-web/app/models/configs/objects/service_config_property.js 73960b3 
>   ambari-web/app/templates/common/configs/addPropertyWindow.hbs 659435c 
>   ambari-web/app/utils/config.js a9cb5e9 
>   ambari-web/app/views/common/configs/service_configs_by_category_view.js 
> bdd0530 
>   ambari-web/test/mixins/common/configs/configs_saver_test.js 4baf303 
> 
> Diff: https://reviews.apache.org/r/49387/diff/
> 
> 
> Testing
> ---
> 
> No new test cases are written in the patch apart from the fix required in 
> "configs_saver_test.js" to avoid test failure of existing tests.
> If the proposed solution is acceptable, I will create a new patch with the 
> necessary new tests.
> 
> The existing ambari-web tests after applying the patch:
> 
>   28977 tests complete (37 seconds)
>   154 tests pending
> 
> 
> Thanks,
> 
> Keta Patel
> 
>