Skip to content
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

Added functionality to remove passwords from clear text and support auditdb runtime credentials #8

Closed
wants to merge 2 commits into from

Conversation

adeelmalik78
Copy link
Contributor

Modified a number of files in this pull request. These mods tackle two main use cases:

  1. Remove database runtime credentials from showing up in clear text - Issue Password stored in XLD appear in clear text in deploy log #6
  2. Creates new properties so that user can also specify credentials for AUDITDB - Issue Support database credentials for AuditDB #7

Regarding #1...
For Windows, sensitive output is removed using "@".
For Linux, sensitive output is removed using "/dev/null"

Here is a list of new files added
src/main/resources/datical/datical_credentials.bat.ftl
src/main/resources/datical/datical_credentials.sh.ftl
src/main/resources/datical/daticalweb_status_refresh.bat.ftl
src/main/resources/datical/daticalweb_status_refresh.sh.ftl

Here is a list of existing files that were modified:
modified: src/main/resources/datical/datical_deploy.bat.ftl
modified: src/main/resources/datical/datical_deploy.sh.ftl
modified: src/main/resources/datical/datical_forecast.bat.ftl
modified: src/main/resources/datical/datical_forecast.sh.ftl
modified: src/main/resources/datical/datical_generic.ftl
modified: src/main/resources/datical/datical_status_detail.bat.ftl
modified: src/main/resources/datical/datical_status_detail.sh.ftl
modified: src/main/resources/datical/datical_upload.bat.ftl
modified: src/main/resources/datical/datical_undeploy.bat.ftl
modified: src/main/resources/datical/datical_undeploy.sh.ftl
modified: src/main/resources/synthetic.xml

Copy link
Contributor

@jdewinne jdewinne left a comment

Choose a reason for hiding this comment

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

The reporting option, since which Datical version is that available? Should that be mentioned in the README?

${ddb_audit_pass}
${ddb_user}
${ddb_pass}
<#--
Copy link
Contributor

Choose a reason for hiding this comment

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

Can those lines be removed? Why are they commented out? Looks like a leftover from testing the changes.

@@ -10,4 +10,18 @@

-->
<#include "/datical/datical_generic.ftl">
<#include "/datical/datical_credentials.bat.ftl">
${ddb_audit_user}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these 4 lines needed? Shouldn't the include be enough? So taking out the assign from the include statement?

@@ -13,10 +13,12 @@

<#assign login>${deployed.container.home} <#if deployed.container.username?has_content>-un ${environment}:::${deployed.container.username} -pw ${environment}:::${deployed.container.password}</#if></#assign>

<#assign login_simple>${deployed.container.home} </#assign>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is login still used? If not, why not rename login_simple into login?

<#include "/datical/datical_generic_undeploy.ftl">
<#if previousDeployed.changeids?size gt 0>
<#list previousDeployed.changeids as changeid>
<#--
${login} -p ${previousDeployed.targetPath} rollback ${environment} changeid:id=${changeid}
-->
${login} -p ${previousDeployed.targetPath} rollback ${environment} changeid:id=${changeid}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be login_simple?

<operation>MODIFY</operation>
</conditions>
<steps>
<os-script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this using a Jython step (maybe using requests). That would remove the dependency on curl.

@@ -78,7 +78,7 @@
</steps>
</rule>


<!--
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this rule commented out?

@adeelmalik78
Copy link
Contributor Author

Updating files to remove commented code and adopt suggested from jdewinne.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants