-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[AMBARI-23130] Persist cluster creation request #559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
34cda14
9902c61
2e8c8bf
386c582
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -533,7 +533,8 @@ private RequestStatusResponse processBlueprintCreate(Map<String, Object> propert | |
|
|
||
| ProvisionClusterRequest createClusterRequest; | ||
| try { | ||
| createClusterRequest = topologyRequestFactory.createProvisionClusterRequest(properties, securityConfiguration); | ||
| createClusterRequest = | ||
| topologyRequestFactory.createProvisionClusterRequest(rawRequestBody, properties, securityConfiguration); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe at this point only the "filtered" document should be passed around, meaning that the passwords have already been removed from the document. |
||
| } catch (InvalidTopologyTemplateException e) { | ||
| throw new IllegalArgumentException("Invalid Cluster Creation Template: " + e, e); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,10 @@ public class TopologyRequestEntity { | |
| @Column(name = "description", length = 1024, nullable = false) | ||
| private String description; | ||
|
|
||
| @Lob | ||
| @Column(name = "raw_request_body", length = 100000, nullable = false) | ||
| private String rawRequestBody; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth considering persisting the document in a slightly different way. The cluster "artifacts" directory (clusters/CLUSTER_NAME/artifacts) REST resource already supports persisting arbitrary documents. Persisting the document in the "artifacts" resource would also make it available via the REST API, which would greatly assist in debugging failed Blueprint deployments, particularly when users generate the document and do not store it. It may still be best to persist the information here (minus the passwords of course), but I think its at least a good idea to consider the "artifacts"-related approach.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks to me that the only usage of the Artifacts table used to be to store the kerberos descriptor, but now Kerberos has its on tables it's kind of abandoned. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just checked with @rlevas, the Ambari Kerberos expert, and he mentioned that the Kerberos descriptor is still persisted to the artifacts table. The Kerberos tables you mentioned are somehow related to the accounting process for Kerberos identities, and do not replace the usage of the artifacts resource. We should probably persist the cluster creation template in "artifacts", since this is in keeping with how things have been handled for the Kerberos descriptor, which is also a user-supplied artifact that is useful to retrieve via the REST APIs. I still think that using the "artifacts" resource is a better approach, since that table is already defined for this purpose, and also since using "artifacts" would provide REST access to this resource after a deployment, without adding any new code. |
||
|
|
||
| @OneToMany(mappedBy = "topologyRequestEntity", cascade = CascadeType.ALL) | ||
| private Collection<TopologyHostGroupEntity> topologyHostGroupEntities; | ||
|
|
||
|
|
@@ -141,6 +145,20 @@ public void setDescription(String description) { | |
| this.description = description; | ||
| } | ||
|
|
||
| /** | ||
| * @return the raw request body in JSON | ||
| */ | ||
| public String getRawRequestBody() { | ||
| return rawRequestBody; | ||
| } | ||
|
|
||
| /** | ||
| * @param rawRequestBody the raw request body in JSON | ||
| */ | ||
| public void setRawRequestBody(String rawRequestBody) { | ||
| this.rawRequestBody = rawRequestBody; | ||
| } | ||
|
|
||
| public Collection<TopologyHostGroupEntity> getTopologyHostGroupEntities() { | ||
| return topologyHostGroupEntities; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Persisting the cluster creation template is a great idea, but we probably need to filter some information out of the template first (the "default_password" field, any configuration items in the cluster config that are of type "PASSWORD") prior to persisting the document.
There may be other types of information that should be filtered out as well, but from a security perspective, we should definitely filter out any password information prior to persisting the data.