From f81aff59c03243773bdad692a32c773b1a0d99db Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 19 Sep 2024 07:57:54 -0700 Subject: [PATCH 1/5] Add test for preserving env var during collector choco upgrade --- .github/workflows/win-package-test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/win-package-test.yml b/.github/workflows/win-package-test.yml index 6936a40a09..872bf8d629 100644 --- a/.github/workflows/win-package-test.yml +++ b/.github/workflows/win-package-test.yml @@ -379,6 +379,7 @@ jobs: } $env_vars = Get-ItemPropertyValue -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" $env_vars += "MY VAR WITH SPACES=my value" + $env_vars += "GOOD_ENV_VAR_NAME=good value" Set-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" -Value $env_vars -type MultiString Start-Sleep 30 write-host "Upgrading $choco_file_name ..." @@ -391,6 +392,10 @@ jobs: $env_vars throw "'MY VAR WITH SPACES=my value' not found!" } + if (!($env_vars -contains "GOOD_ENV_VAR_NAME=good value")) { + $env_vars + throw "'GOOD_ENV_VAR_NAME=good value' not found!" + } } Start-Sleep -s 30 & ${{ github.workspace }}\.github\workflows\scripts\win-test-services.ps1 -mode "${{ matrix.MODE }}" -access_token "${{ env.token }}" -realm "${{ env.realm }}" -memory "${{ env.memory }}" -with_fluentd "${{ matrix.WITH_FLUENTD }}" From a55457c1fdd82c5ba13f9589af5c5e38b10e3f68 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 19 Sep 2024 09:57:18 -0700 Subject: [PATCH 2/5] Preserve custom env vars and ensure no error on MSI call --- .github/workflows/win-package-test.yml | 20 +++++++++------- .../tools/chocolateyinstall.ps1 | 20 +++++++++------- .../splunk-otel-collector/tools/common.ps1 | 24 +++++++++++++++++++ 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/.github/workflows/win-package-test.yml b/.github/workflows/win-package-test.yml index 872bf8d629..76a8018464 100644 --- a/.github/workflows/win-package-test.yml +++ b/.github/workflows/win-package-test.yml @@ -378,8 +378,12 @@ jobs: throw "choco install failed!" } $env_vars = Get-ItemPropertyValue -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" - $env_vars += "MY VAR WITH SPACES=my value" - $env_vars += "GOOD_ENV_VAR_NAME=good value" + $test_env_vars = @( + "MY_VAR_WITH_SPACES=my value", + "GOOD_ENV_VAR_NAME=good value", + "GOOD_KEY_NEEDS_ESCAPING=`" that breaks MSI call if not escaped properly due to double-quote" + ) + $env_vars += $test_env_vars Set-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" -Value $env_vars -type MultiString Start-Sleep 30 write-host "Upgrading $choco_file_name ..." @@ -388,13 +392,11 @@ jobs: throw "choco upgrade failed!" } $env_vars = Get-ItemPropertyValue -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" - if (!($env_vars -contains "MY VAR WITH SPACES=my value")) { - $env_vars - throw "'MY VAR WITH SPACES=my value' not found!" - } - if (!($env_vars -contains "GOOD_ENV_VAR_NAME=good value")) { - $env_vars - throw "'GOOD_ENV_VAR_NAME=good value' not found!" + foreach ($test_env_var in $test_env_vars) { + if (!($env_vars -contains $test_env_var)) { + $env_vars + throw "'$test_env_var' not found!" + } } } Start-Sleep -s 30 diff --git a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 index 5f28c6bfd4..263c8c42a4 100644 --- a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 +++ b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 @@ -47,7 +47,7 @@ $env_var_names = @( "SPLUNK_BUNDLE_DIR", "SPLUNK_LISTEN_INTERFACE" ) -$custom_entries = @() + $upgraded_from_version_with_machine_wide_env_vars = $false Write-Host "Checking for previous installation..." @@ -79,10 +79,10 @@ if (Test-Path $reg_path) { Write-Host "Found previous environment variables for the $service_name service." foreach ($entry in $previous_environment) { $k, $v = $entry.Split("=", 2) - if ($k -Match "^[0-9A-Za-z_]+$") { - $env_vars[$k] = $v - } else { - $custom_entries += $entry + if ($k -and $k -Match "^[0-9A-Za-z_]+$" -and $v) { + # If the name is like a bash variable name, it is safe to pass them to the MSI even if + # they are not in the list of MSI properties. They will be just ignored by the MSI. + $env_vars[$k] = $v.Replace('"', '""') # Escape double quotes for MSI properties. } } } @@ -168,10 +168,12 @@ try { Install-ChocolateyInstallPackage @packageArgs } finally { # Add any custom entries back to the reg key - if ($custom_entries) { - $custom_entries += (Get-ItemPropertyValue -Path $reg_path -Name "Environment") - $custom_entries = $custom_entries | Sort-Object -Unique - Set-ItemProperty -Path $reg_path -Name "Environment" -Value $custom_entries -Type MultiString + if ($previous_environment) { + # Preserve any environment variables that were set by the MSI installer, but add any other custom entries. + $svc_env_after_install = Get-ItemPropertyValue -Path $reg_path -Name "Environment" + $merged_environment = merge_multistring_env $svc_env_after_install $previous_environment + $merged_environment = $merged_environment | Sort-Object -Unique + Set-ItemProperty -Path $reg_path -Name "Environment" -Value $merged_environment -Type MultiString } } diff --git a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 index eb660ccc0b..1f288a9576 100644 --- a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 +++ b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 @@ -105,6 +105,30 @@ function set_env_var_value_from_package_params([hashtable] $env_vars, [hashtable Write-Host "The $name package parameter was not set, using the default value: '$value'" } +# merge array of strings, used as environment variables, given priority to the ones defined in the left array +function merge_multistring_env([string[]]$l, [string[]]$r) { + $keys = @{} + [string[]]$merged = @() + foreach ($lentry in $l) { + if (-not $lentry) { + continue + } + $keys[$lentry.Split('=',2)[0]] = $true + $merged += $lentry + } + foreach ($rentry in $r) { + if (-not $rentry) { + continue + } + $key = $rentry.Split('=',2)[0] + if (-not $keys.ContainsKey($key)) { + $merged += $rentry + } + } + + return $merged +} + # check that we're not running with a restricted execution policy function check_policy() { $executionPolicy = (Get-ExecutionPolicy) From 8408bed739402f708d471a2ac427b8d36deccd81 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 19 Sep 2024 10:45:31 -0700 Subject: [PATCH 3/5] Old cpde to validate the test --- .../tools/chocolateyinstall.ps1 | 20 +++++++--------- .../splunk-otel-collector/tools/common.ps1 | 24 ------------------- 2 files changed, 9 insertions(+), 35 deletions(-) diff --git a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 index 263c8c42a4..5f28c6bfd4 100644 --- a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 +++ b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 @@ -47,7 +47,7 @@ $env_var_names = @( "SPLUNK_BUNDLE_DIR", "SPLUNK_LISTEN_INTERFACE" ) - +$custom_entries = @() $upgraded_from_version_with_machine_wide_env_vars = $false Write-Host "Checking for previous installation..." @@ -79,10 +79,10 @@ if (Test-Path $reg_path) { Write-Host "Found previous environment variables for the $service_name service." foreach ($entry in $previous_environment) { $k, $v = $entry.Split("=", 2) - if ($k -and $k -Match "^[0-9A-Za-z_]+$" -and $v) { - # If the name is like a bash variable name, it is safe to pass them to the MSI even if - # they are not in the list of MSI properties. They will be just ignored by the MSI. - $env_vars[$k] = $v.Replace('"', '""') # Escape double quotes for MSI properties. + if ($k -Match "^[0-9A-Za-z_]+$") { + $env_vars[$k] = $v + } else { + $custom_entries += $entry } } } @@ -168,12 +168,10 @@ try { Install-ChocolateyInstallPackage @packageArgs } finally { # Add any custom entries back to the reg key - if ($previous_environment) { - # Preserve any environment variables that were set by the MSI installer, but add any other custom entries. - $svc_env_after_install = Get-ItemPropertyValue -Path $reg_path -Name "Environment" - $merged_environment = merge_multistring_env $svc_env_after_install $previous_environment - $merged_environment = $merged_environment | Sort-Object -Unique - Set-ItemProperty -Path $reg_path -Name "Environment" -Value $merged_environment -Type MultiString + if ($custom_entries) { + $custom_entries += (Get-ItemPropertyValue -Path $reg_path -Name "Environment") + $custom_entries = $custom_entries | Sort-Object -Unique + Set-ItemProperty -Path $reg_path -Name "Environment" -Value $custom_entries -Type MultiString } } diff --git a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 index 1f288a9576..eb660ccc0b 100644 --- a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 +++ b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/common.ps1 @@ -105,30 +105,6 @@ function set_env_var_value_from_package_params([hashtable] $env_vars, [hashtable Write-Host "The $name package parameter was not set, using the default value: '$value'" } -# merge array of strings, used as environment variables, given priority to the ones defined in the left array -function merge_multistring_env([string[]]$l, [string[]]$r) { - $keys = @{} - [string[]]$merged = @() - foreach ($lentry in $l) { - if (-not $lentry) { - continue - } - $keys[$lentry.Split('=',2)[0]] = $true - $merged += $lentry - } - foreach ($rentry in $r) { - if (-not $rentry) { - continue - } - $key = $rentry.Split('=',2)[0] - if (-not $keys.ContainsKey($key)) { - $merged += $rentry - } - } - - return $merged -} - # check that we're not running with a restricted execution policy function check_policy() { $executionPolicy = (Get-ExecutionPolicy) From d58fce863aa8e8662b963aabe233d9e950107d67 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 19 Sep 2024 11:46:01 -0700 Subject: [PATCH 4/5] Ensure that the MSI runs correctly so the test can be run --- .../choco/splunk-otel-collector/tools/chocolateyinstall.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 index 5f28c6bfd4..9a4b7b25a3 100644 --- a/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 +++ b/internal/buildscripts/packaging/choco/splunk-otel-collector/tools/chocolateyinstall.ps1 @@ -80,7 +80,7 @@ if (Test-Path $reg_path) { foreach ($entry in $previous_environment) { $k, $v = $entry.Split("=", 2) if ($k -Match "^[0-9A-Za-z_]+$") { - $env_vars[$k] = $v + $env_vars[$k] = $v.Replace('"', '""') # Ensure that the MSI runs correctly by escaping double quotes. } else { $custom_entries += $entry } From 15f4e5b89afaad3ef201a79ef0e008f25f873444 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 19 Sep 2024 12:03:58 -0700 Subject: [PATCH 5/5] Fix test of env var with spaces in the name Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com> --- .github/workflows/win-package-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/win-package-test.yml b/.github/workflows/win-package-test.yml index 76a8018464..a186484163 100644 --- a/.github/workflows/win-package-test.yml +++ b/.github/workflows/win-package-test.yml @@ -379,7 +379,7 @@ jobs: } $env_vars = Get-ItemPropertyValue -Path "HKLM:\SYSTEM\CurrentControlSet\Services\splunk-otel-collector" -Name "Environment" $test_env_vars = @( - "MY_VAR_WITH_SPACES=my value", + "MY VAR WITH SPACES=my value", "GOOD_ENV_VAR_NAME=good value", "GOOD_KEY_NEEDS_ESCAPING=`" that breaks MSI call if not escaped properly due to double-quote" )