Skip to content

[windows] fixes + blacklist #578

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.

Sign up for GitHub

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

Merged
truthbk merged 4 commits into from
Nov 15, 2019
Merged

[windows] fixes + blacklist #578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a07720f
[windows] apply powershell remote fix before uninstall + idiomatic do…
truthbk Nov 11, 2019
1b2cd02
[windows] weak first attempt to blacklist bad MSIs
truthbk Nov 11, 2019
93e1e7e
[windows] multiple fixes + blacklist + hash validation
truthbk Nov 14, 2019
e1cb79a
addressing cops
truthbk Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
}
'Windows': {
$agent5_default_repo = '<agent 5 is not supported by this module on windows>' # Param in init.pp so needs to be defined, but not used on Windows
$agent6_default_repo = "https://s3.amazonaws.com/ddagent-windows-stable/datadog-agent-6-${agent_version}.amd64.msi"
$agent6_default_repo = 'https://s3.amazonaws.com/ddagent-windows-stable/'
$conf5_dir = 'C:/ProgramData/Datadog/agent5' # Not a real path, but integrations use it to ensure => absent so it needs to be a valid path
$conf6_dir = 'C:/ProgramData/Datadog/conf.d'
$dd_user = 'ddagentuser'
Expand Down
30 changes: 23 additions & 7 deletions manifests/windows/agent6.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,32 @@
) inherits datadog_agent::params {

$msi_full_path = "${msi_location}/datadog-agent-6-${agent_version}.amd64.msi"
$msi_source = "${baseurl}ddagent-cli-${agent_version}.msi"

if $ensure == 'present' {
if ($agent_version in ['6.14.0', '6.14.1']) {
fail('The specified agent version has been blacklisted, please specify a version other than 6.14.0 or 6.14.1')
}

file { 'installer':
Copy link
Copy Markdown
Contributor

@albertvaka albertvaka Nov 14, 2019

Choose a reason for hiding this comment

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

The previous behavior was to only download the MSI if it wasn't already installed. Now we are downloading it regardless. Any reason to prefer this?

Copy link
Copy Markdown
Member Author

@truthbk truthbk Nov 14, 2019

Choose a reason for hiding this comment

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

Maybe I missed something with my testing, but it didn't seem to download regardless. Once the file is in the temp directory I don't think the download was reattempted (ie. this resource doesn't behave like the chef counterpart that always pulls).

That said, we can go back to the original code. I made this change because I wanted to use the validate_cmd, but it didn't work particularly well in my tests, so I eventually did the validation in the exec resource below. We could still do it the way it was before, but I left it like this because the behavior seemed similar. I can try to reconfirm this doesn't always download.

The one thing that sucks, and I might change in a separate PR, is the fact we keep multiple files lying around, using up more and more disk space in temp over time. It might make sense to make msi_full_path a constant where we always overwrite with what we pull from the source (if it's not chatty).

Copy link
Copy Markdown
Contributor

@albertvaka albertvaka Nov 18, 2019

Choose a reason for hiding this comment

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

It doesn't always download, but clearing the temp directory will trigger a download that is not needed if the package is already installed.

Copy link
Copy Markdown
Contributor

@albertvaka albertvaka Nov 18, 2019

Choose a reason for hiding this comment

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

Tentative fix, yet to be tested: #582

path => $msi_full_path,
source => $msi_source,
provider => 'windows',
}

exec { 'downloadmsi': # Using exec instead of file so we can specify an onlyif condition
command => "Invoke-WebRequest ${baseurl} -outfile ${msi_full_path}",
onlyif => "if ((Get-Package \"${datadog_agent::params::package_name}\") -or (test-path ${msi_full_path})) { exit 1 }",
provider => powershell,
notify => Package[$datadog_agent::params::package_name]
exec { 'validate':
command => "\$blacklist = '928b00d2f952219732cda9ae0515351b15f9b9c1ea1d546738f9dc0fda70c336','78b2bb2b231bcc185eb73dd367bfb6cb8a5d45ba93a46a7890fd607dc9188194';\$fileStream = [system.io.file]::openread('${msi_full_path}'); \$hasher = [System.Security.Cryptography.HashAlgorithm]::create('sha256'); \$hash = \$hasher.ComputeHash(\$fileStream); \$fileStream.close(); \$fileStream.dispose();\$hexhash = [system.bitconverter]::tostring(\$hash).ToLower().replace('-','');if (\$hexhash -match \$blacklist) { Exit 1 }",
provider => 'powershell',
logoutput => 'on_failure',
Comment thread
truthbk marked this conversation as resolved.
require => File['installer'],
notify => Package[$datadog_agent::params::package_name]
}

package { $datadog_agent::params::package_name:
ensure => installed,
provider => 'windows',
source => $msi_full_path,
install_options => ['/quiet', {'APIKEY' => $api_key, 'HOSTNAME' => $hostname, 'TAGS' => $tags}]
install_options => ['/norestart', {'APIKEY' => $api_key, 'HOSTNAME' => $hostname, 'TAGS' => $tags}]
Copy link
Copy Markdown
Contributor

@albertvaka albertvaka Nov 14, 2019

Choose a reason for hiding this comment

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

👌

}

service { $service_name:
Expand All @@ -40,11 +51,16 @@
require => Package[$datadog_agent::params::package_name]
}
} else {
exec { 'datadog_6_14_fix':
command => "((New-Object System.Net.WebClient).DownloadFile('https://s3.amazonaws.com/ddagent-windows-stable/scripts/fix_6_14.ps1', \$env:temp + '\\fix_6_14.ps1')); &\$env:temp\\fix_6_14.ps1",
provider => 'powershell',
}

package { $datadog_agent::params::package_name:
ensure => absent,
provider => 'windows',
uninstall_options => ['/quiet']
uninstall_options => ['/quiet'],
subscribe => Exec['datadog_6_14_fix'],
}

}
Expand Down
⚡