-
Notifications
You must be signed in to change notification settings - Fork 262
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
Add cleanup for Windows env vars #694
Conversation
recipes/_install-windows.rb
Outdated
windows_env 'DDAGENTUSER_NAME' do | ||
action :delete | ||
end | ||
|
||
windows_env 'DDAGENTUSER_PASSWORD' do | ||
action :delete | ||
end |
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.
I think we can still keep these deletes, so the env vars are cleaned earlier than at run_completed
or run_failed
.
Eg: if a run takes long and you kill it, on_failed
might not get called, so it's better to delete the env vars earlier when we know we don't need them anymore.
libraries/windows_cleanup.rb
Outdated
def do_cleanup(context) | ||
Chef::Log.info 'Cleanup started.' | ||
resource = context.resource_collection.lookup("windows_env[DDAGENTUSER_NAME]") | ||
resource.run_action(:delete) if resource | ||
resource = context.resource_collection.lookup("windows_env[DDAGENTUSER_PASSWORD]") | ||
resource.run_action(:delete) if resource | ||
Chef::Log.info 'Cleanup finished.' | ||
end |
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.
I would go for compact code and keep this helper function in recipes/_install-windows.rb
instead of having several small files, but no strong opinion in case you prefer it this way.
* Revert "Merge pull request #694 from DataDog/julien.lebot/fix_credentials_leak" This reverts commit fa40d3d, reversing changes made to 018ff8b. * Revert "Set windows installer as sensitive resource and use env var to specify win credentials (#691)" This reverts commit 07d3a62. * Update CHANGELOG.md
When the
windows_package[Datadog Agent]
fails, it does not cleanup the environment variables containing the credentials for the installer.This PR aims to fix that by setting a hanlder that runs after the recipe.