You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Emil Anca <ea...@hortonworks.com> on 2015/05/05 17:27:38 UTC

Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

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

Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.


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


Repository: ambari


Description
-------

The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
Typically, the configurations structure should look like:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
}
},
{ "mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
}
},
{ "yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
}
]"
The snippet above is just an example to illustrate the expected format.
Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
An example of an incorrect format would be:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
},
"mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
},
"yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
]
"
Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.


Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 

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


Testing
-------

mvn clean test


Thanks,

Emil Anca


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by John Speidel <js...@hortonworks.com>.

> On May 12, 2015, 3:03 p.m., Robert Nettleton wrote:
> > The change looks fine to me, although I think it would be better to have more unit tests for this change.
> > 
> > Could the submitter please detail the testing done for this patch, other than "mvn clean test"?  Once some functional testing is done on this (if it hasn't been already), we can consider moving towards a merge.  
> > 
> > Thanks.

+1 on confirmation of functional testing.


- John


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


On May 5, 2015, 3:27 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:27 p.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Emil Anca <ea...@hortonworks.com>.

> On May 12, 2015, 3:03 p.m., Robert Nettleton wrote:
> > The change looks fine to me, although I think it would be better to have more unit tests for this change.
> > 
> > Could the submitter please detail the testing done for this patch, other than "mvn clean test"?  Once some functional testing is done on this (if it hasn't been already), we can consider moving towards a merge.  
> > 
> > Thanks.
> 
> John Speidel wrote:
>     +1 on confirmation of functional testing.
> 
> Emil Anca wrote:
>     Thanks for the feedback. Will ask Costel to handle your concerns and have this updated.
> 
> Costel Radulescu wrote:
>     Hi, I performed the following functional tests:
>     
>     1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
>     2) tested that the server saves several correctly structured blueprints:
>         a) blueprint having an empty "configurations" element
>         b) blueprint having "configurations" with a single "configuration-type" element
>         c) blueprint having "configurations" with multiple "configuration-type" elements properly structured
>     
>     I have also added new unit tests to account for success scenarions (a) and (b).
>     
>     Thanks for the feedback, I hope this addresses your concerns.

@Robert Nettleton
Will the testing provided by Costel suffice, shall we move on onto merging this?
Thanks.


- Emil


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


On May 14, 2015, 8:53 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 14, 2015, 8:53 a.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Performed the following functional tests:
> 
> 1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
> 2) tested that the server saves several correctly structured blueprints:
>     a) blueprint having an empty "configurations" element
>     b) blueprint having "configurations" with a single "configuration-type" element
>     c) blueprint having "configurations" with multiple "configuration-type" elements properly structured
> 
> Added new unit tests to account for success scenarions (a) and (b).
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Emil Anca <ea...@hortonworks.com>.

> On May 12, 2015, 3:03 p.m., Robert Nettleton wrote:
> > The change looks fine to me, although I think it would be better to have more unit tests for this change.
> > 
> > Could the submitter please detail the testing done for this patch, other than "mvn clean test"?  Once some functional testing is done on this (if it hasn't been already), we can consider moving towards a merge.  
> > 
> > Thanks.
> 
> John Speidel wrote:
>     +1 on confirmation of functional testing.

Thanks for the feedback. Will ask Costel to handle your concerns and have this updated.


- Emil


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


On May 5, 2015, 3:27 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:27 p.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Costel Radulescu <cr...@hortonworks.com>.

> On May 12, 2015, 3:03 p.m., Robert Nettleton wrote:
> > The change looks fine to me, although I think it would be better to have more unit tests for this change.
> > 
> > Could the submitter please detail the testing done for this patch, other than "mvn clean test"?  Once some functional testing is done on this (if it hasn't been already), we can consider moving towards a merge.  
> > 
> > Thanks.
> 
> John Speidel wrote:
>     +1 on confirmation of functional testing.
> 
> Emil Anca wrote:
>     Thanks for the feedback. Will ask Costel to handle your concerns and have this updated.

Hi, I performed the following functional tests:

1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
2) tested that the server saves several correctly structured blueprints:
    a) blueprint having an empty "configurations" element
    b) blueprint having "configurations" with a single "configuration-type" element
    c) blueprint having "configurations" with multiple "configuration-type" elements properly structured

I have also added new unit tests to account for success scenarions (a) and (b).

Thanks for the feedback, I hope this addresses your concerns.


- Costel


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


On May 5, 2015, 3:27 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:27 p.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33848/#review83402
-----------------------------------------------------------


The change looks fine to me, although I think it would be better to have more unit tests for this change.

Could the submitter please detail the testing done for this patch, other than "mvn clean test"?  Once some functional testing is done on this (if it hasn't been already), we can consider moving towards a merge.  

Thanks.

- Robert Nettleton


On May 5, 2015, 3:27 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:27 p.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Costel Radulescu <cr...@hortonworks.com>.

> On May 12, 2015, 2:45 p.m., John Speidel wrote:
> > Looks good although I would prefer that you moved this validation logic to BlueprintFactory which is where all of the current validation logic exists.

Thanks John, I chose not to put the structure validation logic into the BlueprintFactory because it would require that the Factory know about either the requestInfoProperties or the Blueprint in JSON format before hand.


- Costel


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


On May 14, 2015, 8:53 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 14, 2015, 8:53 a.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Performed the following functional tests:
> 
> 1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
> 2) tested that the server saves several correctly structured blueprints:
>     a) blueprint having an empty "configurations" element
>     b) blueprint having "configurations" with a single "configuration-type" element
>     c) blueprint having "configurations" with multiple "configuration-type" elements properly structured
> 
> Added new unit tests to account for success scenarions (a) and (b).
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by John Speidel <js...@hortonworks.com>.

> On May 12, 2015, 2:45 p.m., John Speidel wrote:
> > Looks good although I would prefer that you moved this validation logic to BlueprintFactory which is where all of the current validation logic exists.
> 
> Costel Radulescu wrote:
>     Thanks John, I chose not to put the structure validation logic into the BlueprintFactory because it would require that the Factory know about either the requestInfoProperties or the Blueprint in JSON format before hand.

That's fine.


- John


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


On May 14, 2015, 8:53 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 14, 2015, 8:53 a.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Performed the following functional tests:
> 
> 1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
> 2) tested that the server saves several correctly structured blueprints:
>     a) blueprint having an empty "configurations" element
>     b) blueprint having "configurations" with a single "configuration-type" element
>     c) blueprint having "configurations" with multiple "configuration-type" elements properly structured
> 
> Added new unit tests to account for success scenarions (a) and (b).
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33848/#review83399
-----------------------------------------------------------

Ship it!


Looks good although I would prefer that you moved this validation logic to BlueprintFactory which is where all of the current validation logic exists.

- John Speidel


On May 5, 2015, 3:27 p.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:27 p.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Emil Anca <ea...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33848/
-----------------------------------------------------------

(Updated May 14, 2015, 8:53 a.m.)


Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.


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


Repository: ambari


Description
-------

The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
Typically, the configurations structure should look like:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
}
},
{ "mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
}
},
{ "yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
}
]"
The snippet above is just an example to illustrate the expected format.
Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
An example of an incorrect format would be:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
},
"mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
},
"yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
]
"
Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.


Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 

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


Testing (updated)
-------

mvn clean test


Performed the following functional tests:

1) tested that the server returns an error when saving a blueprint with invalid structure for the "configurations" element
2) tested that the server saves several correctly structured blueprints:
    a) blueprint having an empty "configurations" element
    b) blueprint having "configurations" with a single "configuration-type" element
    c) blueprint having "configurations" with multiple "configuration-type" elements properly structured

Added new unit tests to account for success scenarions (a) and (b).


Thanks,

Emil Anca


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Emil Anca <ea...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33848/
-----------------------------------------------------------

(Updated May 14, 2015, 8:51 a.m.)


Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.


Changes
-------

Added additional unit tests.


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


Repository: ambari


Description
-------

The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
Typically, the configurations structure should look like:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
}
},
{ "mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
}
},
{ "yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
}
]"
The snippet above is just an example to illustrate the expected format.
Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
An example of an incorrect format would be:
""configurations": [
{
"mapred-env": {
"properties":
{ "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
},
"mapred-site": {
"properties":
{ "install-test-mapred-site": "install-test-mapred-site-VALUE" }
},
"yarn-env": {
"properties":
{ "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
}
]
"
Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.


Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 

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


Testing
-------

mvn clean test


Thanks,

Emil Anca


Re: Review Request 33848: Blueprints processor needs stronger validation of Blueprint JSON structure

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33848/#review82590
-----------------------------------------------------------

Ship it!


Ship It!

- Robert Levas


On May 5, 2015, 11:27 a.m., Emil Anca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33848/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 11:27 a.m.)
> 
> 
> Review request for Ambari, Costel Radulescu, John Speidel, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-10931
>     https://issues.apache.org/jira/browse/AMBARI-10931
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The Blueprints processor needs to have stronger validation during the process of handling a POST for a newly-submitted Blueprint from a REST client.
> The Blueprints structure requires that the "configurations" element be an array of Maps, with each Map representing a given configuration type ("core-site", "storm-site", etc).
> Typically, the configurations structure should look like:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> }
> },
> { "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> }
> },
> { "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> }
> ]"
> The snippet above is just an example to illustrate the expected format.
> Recent changes to the Blueprint's structure to handle "properties_attributes" (such as "final") have modified the structure of a Blueprint, so that configuration properties are now contained in a "properties" map, internal to each configuration type. Creating new Blueprints using this newer format (the older format is still acceptable to the processor) require that the configuration types are still enclosed in a map, even though an internal map is also present, which may be a source of confusion.
> An example of an incorrect format would be:
> ""configurations": [
> {
> "mapred-env": {
> "properties":
> { "mapreduce_log_dir_prefix": "/grid/0/log/hadoop-mapreduce" }
> },
> "mapred-site": {
> "properties":
> { "install-test-mapred-site": "install-test-mapred-site-VALUE" }
> },
> "yarn-env": {
> "properties":
> { "yarn_log_dir_prefix": "/grid/0/log/hadoop" }
> }
> ]
> "
> Please note that in the example above a single map is created, with all configuration elements being added to that entry's configuration type. This causes a corrupted Blueprint to be stored and used for deployments, since all configuration elements added in the original Blueprint will be registered under one of the config types used in the original Blueprint. This will cause many types of cluster startup failures, as the configuration elements will be added to incorrect locations.
> When the Blueprint processor receives a POST submission with a new Blueprint that includes this incorrect format for "configurations", the processor must reject this submission, and send back a relevant error code/message, so that the user can resolve this problem prior to attempting a cluster deployment.
> This issue will not affect the older format for Blueprints, and so any backwards-compatible Blueprints used should work fine. This issue will only occur when new Blueprints are submitted that use the newer syntax/format for defining configuration properties (with support for properties_attributes) as well.
> This JIRA is being created to track this issue, as it could potentially be a user experience issue going forward, and should probably be resolved in Ambari 2.1, if possible.
> 
> 
> Problem: Blueprint "configurations" element structure is not validated which leads to cluster creations error
> Solution: The "configuration" element is checked for the correct structure as a List of Maps containing a single configuration type
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java aab5395 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java 118a7be 
> 
> Diff: https://reviews.apache.org/r/33848/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Emil Anca
> 
>