Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
88 changes: 84 additions & 4 deletions ci/check-environment-variable-names.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ Describe 'Find-EnvironmentNameProblem' {
@{ Line = 'if "resource_key" in os.environ:' }
@{ Line = '$env:resource_key = "key"' }
@{ Line = '[Environment]::SetEnvironmentVariable("license_key", $k)' }
@{ Line = 'RESOURCE = env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY")' }
@{ Line = 'LICENCE = env("_51DEGREES_LICENSE_KEY", "license_key")' }
@{ Line = 'LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key"' }
@{ Line = 'legacy_license_key_env_var = "x"' }
@{ Line = 'self.baseURL = os.environ.get("FOD_CLOUD_API_URL")' }
@{ Line = 'FOD_CLOUD_API_URL = "FOD_CLOUD_API_URL"' }
@{ Line = 'saved = os.environ.pop("FOD_CLOUD_API_URL", None)' }
@{ Line = '$env:FOD_CLOUD_API_URL = "https://cloud.51degrees.com/api/v4/"' }
@{ Line = 'key = env("_51DEGREES_RESOURCE_KEY", "resource_key") or os.environ.get("license_key")' }
@{ Line = 'RESOURCE_KEY_ENV_VAR = "resource_key"' }
@{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"' }
@{ Line = 'ORIGIN_ENV_VAR = "cloud_request_origin"' }
Expand All @@ -49,7 +49,6 @@ Describe 'Find-EnvironmentNameProblem' {
@{ Line = 'env = os.environ.get("IPINTELLIGENCEDATAFILE")' }
@{ Line = 'os.environ["ipintelligencetestkey"] = "value"' }
@{ Line = 'if "run_performance_tests" in os.environ:' }
@{ Line = 'saved = os.environ.pop("FOD_CLOUD_API_URL", None)' }
) {
Get-Kind $Line | Should -Contain 'fallback'
}
Expand Down Expand Up @@ -82,4 +81,85 @@ Describe 'Find-EnvironmentNameProblem' {
) {
Get-Kind $Line | Should -BeNullOrEmpty
}

# A variable whose name changed is still read under its old name when
# the new one is not set. That read gives the old name as the argument
# straight after the new one, and it is left alone.
It 'leaves the deprecated read <Line> alone' -ForEach @(
@{ Line = 'RESOURCE = env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY")' }
@{ Line = 'LICENCE = env("_51DEGREES_LICENSE_KEY", "license_key")' }
@{ Line = 'API = env("_51DEGREES_CLOUD_ENDPOINT", "FOD_CLOUD_API_URL")' }
@{ Line = 'read_environment("_51DEGREES_IPI_PATH", "IPINTELLIGENCEDATAFILE")' }
@{ Line = ' "_51DEGREES_RUN_PERFORMANCE_TESTS", "run_performance_tests")' }
@{ Line = "read_environment('_51DEGREES_DD_PATH', '51DEGREES_DD_PATH')" }
) {
Get-Kind $Line | Should -BeNullOrEmpty
}
}

Describe 'Find-EnvironmentNameProblem with the old names a file may hold' {

It 'leaves <Line> alone when the file may hold <Allowed>' -ForEach @(
@{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"'; Allowed = @('cloud_endpoint') }
@{ Line = 'os.environ["resource_key"] = "old-key"'; Allowed = @('resource_key', 'license_key') }
@{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_DD_PATH"'; Allowed = @('51DEGREES_DD_PATH') }
@{ Line = ' FOD_CLOUD_API_URL'; Allowed = @('FOD_CLOUD_API_URL') }
@{ Line = 'os.environ["RESOURCE_KEY"] = "old-resource-key"'; Allowed = @('RESOURCE_KEY') }
) {
@(Find-EnvironmentNameProblem $Line $Allowed) | Should -BeNullOrEmpty
}

It 'finds <Kind> in <Line> when the file may hold only <Allowed>' -ForEach @(
@{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"'; Allowed = @('resource_key'); Kind = 'fallback' }
@{ Line = 'os.environ["license_key"] = "old-key"'; Allowed = @('resource_key'); Kind = 'fallback' }
@{ Line = 'os.environ["RESOURCE_KEY"] = "old-key"'; Allowed = @('resource_key'); Kind = 'fallback' }
@{ Line = ' IPINTELLIGENCEDATAFILE'; Allowed = @('FOD_CLOUD_API_URL'); Kind = 'fallback' }
@{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_IPI_PATH"'; Allowed = @('51DEGREES_DD_PATH'); Kind = 'digit' }
@{ Line = ' 51DEGREES_DD_PATH_OTHER'; Allowed = @('51DEGREES_DD_PATH'); Kind = 'digit' }
@{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_DD_PATH"'; Allowed = @(); Kind = 'digit' }
) {
@(Find-EnvironmentNameProblem $Line $Allowed | ForEach-Object { $_.Kind }) |
Should -Contain $Kind
}
}

Describe 'Get-FallbackName' {

It 'lets <Path> hold <Name>' -ForEach @(
@{ Path = 'pipeline/fiftyone_pipeline_did/tox.ini'; Name = 'FOD_CLOUD_API_URL' }
@{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = '51DEGREES_DD_PATH' }
@{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = 'cloud_endpoint' }
@{ Path = 'ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py'; Name = 'IPINTELLIGENCEDATAFILE' }
@{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py'; Name = 'FOD_CLOUD_API_URL' }
@{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/readme.md'; Name = 'FOD_CLOUD_API_URL' }
@{ Path = 'pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py'; Name = 'FOD_CLOUD_API_URL' }
@{ Path = 'pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py'; Name = 'resource_key' }
) {
@(Get-FallbackName $Path) | Should -Contain $Name
}

It 'does not let <Path> hold <Name>' -ForEach @(
@{ Path = 'pipeline/fiftyone_pipeline_did/tox.ini'; Name = '51DEGREES_DD_PATH' }
@{ Path = 'pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py'; Name = 'resource_key' }
@{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py'; Name = '51DEGREES_DD_PATH' }
@{ Path = 'ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py'; Name = '51DEGREES_DD_PATH' }
@{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = 'IPINTELLIGENCEDATAFILE' }
) {
@(Get-FallbackName $Path) | Should -Not -Contain $Name
}

It 'lets <Path> hold no old name' -ForEach @(
@{ Path = 'ci/run-unit-tests.ps1' }
@{ Path = 'readme.md' }
@{ Path = 'pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipeline.py' }
@{ Path = 'pipeline/fiftyone_pipeline_core/readme.md' }
@{ Path = 'device-detection/fiftyone_devicedetection_examples/tests/test_cloudexamples.py' }
@{ Path = 'device-detection/ci/run-integration-tests.ps1' }
@{ Path = 'pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py' }
@{ Path = 'location/examples/cloud/gettingstarted.py' }
@{ Path = 'some/new/package/tox.ini' }
@{ Path = 'some/new/example_utils.py' }
) {
@(Get-FallbackName $Path) | Should -BeNullOrEmpty
}
}
149 changes: 132 additions & 17 deletions ci/check-environment-variable-names.ps1
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
# Every 51Degrees environment variable name in this repository starts with an
# underscore, because a POSIX shell cannot set a name that starts with a digit,
# and each variable has one name with nothing read in its place. This reads
# every file git tracks, which leaves out build output and the submodules, and
# fails on any name that starts with the digit and on any fallback name.
# and everything documents that one name. This reads every file git tracks,
# which leaves out build output and the submodules, and fails on any name
# that starts with the digit and on any old name.
#
# A variable whose name changed is still read under its old name when the new
# one is not set, which is deprecated. So an old name is left alone in the two
# places that read needs it. One is a line where the old name is the argument
# straight after its _51DEGREES_ name, which is how each deprecated read is
# written. The other is the files $FallbackNames lists, each of which may hold
# the old names it is listed with and no others.
#
# Names are matched in any case, because Windows reads an environment
# variable name in any case, so a lower case old name is still the old name
Expand Down Expand Up @@ -31,9 +38,8 @@ $script:DigitInitialPattern = [regex]::new(
"(?<![A-Za-z0-9_])$digitInitial[A-Za-z0-9_]*",
[Text.RegularExpressions.RegexOptions]::IgnoreCase)

# An old name read or set as an environment variable, or given as a second
# name after a _51DEGREES_ one, or a constant holding a legacy name to fall
# back to. Only those are matched, so the builder setting resource_key or
# An old name read or set as an environment variable, or a constant holding
# one. Only those are matched, so the builder setting resource_key or
# cloud_endpoint, placeholder text such as YOUR_RESOURCE_KEY, and longer names
# that end in the same words, are left alone. cloud_endpoint in particular is
# a pipeline setting as well as an old environment variable name, and only the
Expand All @@ -48,8 +54,7 @@ $script:DigitInitialPattern = [regex]::new(
# Those four, the ones $plainOld holds, keep their case, unlike every other
# pattern here, because the IP Intelligence data file's old name differs from
# the class name IpIntelligenceDataFile by case alone, and a class is not a
# variable. They are gone from the repository, so only an exact return of one
# matters.
# variable.
#
# RESOURCE_KEY, LICENSE_KEY, cloud_endpoint and cloud_request_origin also name
# things that are not environment variables, being pipeline settings, builder
Expand All @@ -71,19 +76,127 @@ $script:FallbackPattern = [regex]::new(
# expression braces one too.
"env[:.]\\?$sharedOld(?![A-Za-z0-9_])|" +
"['`"]$sharedOld['`"]\s+in\s+os\.environ|" +
"['`"]_51DEGREES_[A-Za-z0-9_]*['`"]\s*,\s*['`"]$sharedOld['`"]|" +
# A constant whose own name says it holds a variable name, which is how
# the cloud request engine and the example helpers name the one they then
# read. A fixture value such as resourceKey = "resource_key" is left
# alone, because that name goes on the wire and not in the environment.
"[A-Za-z_][A-Za-z0-9_]*(?:ENV_VAR|ENVIRONMENT_VARIABLE|_VARIABLE)" +
"[A-Za-z0-9_]*\s*=\s*['`"]$sharedOld['`"]|" +
'\bLEGACY_[A-Z_]*ENV_VARS?\b',
"[A-Za-z0-9_]*\s*=\s*['`"]$sharedOld['`"]",
[Text.RegularExpressions.RegexOptions]::IgnoreCase)

# A name given as the argument straight after a _51DEGREES_ name, which is how
# a deprecated read of an old name is written. The old name is taken out of
# the line before the line is checked.
$script:DeprecatedReadPattern = [regex]::new(
"(['`"]_51DEGREES_[A-Za-z0-9_]*['`"]\s*,\s*)['`"][A-Za-z0-9_]+['`"]")

# The old names each file may hold. The files are the ones that hold the
# deprecated constants, the documentation that says an old name is still
# read, the tests that set an old name to check it is read, and the lists of
# names tox passes to the tests. A file is given by its path from the root of
# the repository, and may hold only the names it is listed with, so any other
# old name still fails there, and so does any other name that starts with a
# digit. The names that are matched wherever they appear are built from
# pieces, so that this file does not name one itself.
$endpointOld = 'FOD_CLOUD' + '_API_URL'
$ipiFileOld = 'IPINTELLIGENCE' + 'DATAFILE'
$performanceOld = 'run_performance' + '_tests'
$ddPathOld = '5' + '1DEGREES_DD_PATH'
$script:FallbackNames = @(
@{
Names = @($ddPathOld, 'LICENSE_KEY')
Paths = @(
'device-detection/fiftyone_devicedetection_examples/tests/test_environment_names.py'
)
}
@{
Names = @($ddPathOld, 'cloud_endpoint', 'license_key', 'resource_key')
Paths = @(
'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'
)
}
@{
Names = @($endpointOld)
Paths = @(
'pipeline/fiftyone_pipeline_cloudrequestengine/examples/tox.ini'
'pipeline/fiftyone_pipeline_cloudrequestengine/readme.md'
'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py'
'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py'
'pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py'
'pipeline/fiftyone_pipeline_did/examples/creator_context_web/server.py'
'pipeline/fiftyone_pipeline_did/readme.md'
'pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py'
'pipeline/fiftyone_pipeline_did/tests/test_did_client.py'
'pipeline/fiftyone_pipeline_did/tox.ini'
)
}
@{
Names = @($endpointOld, 'LICENSE_KEY', 'RESOURCE_KEY')
Paths = @(
'pipeline/fiftyone_pipeline_did/tests/test_creator_context_server.py'
)
}
@{
Names = @($ipiFileOld)
Paths = @(
'ip-intelligence/fiftyone_ipintelligence_examples/tox.ini'
'ip-intelligence/fiftyone_ipintelligence_onpremise/tests/test_environment_names.py'
'ip-intelligence/fiftyone_ipintelligence_onpremise/tox.ini'
)
}
@{
Names = @($ipiFileOld, 'LICENSE_KEY')
Paths = @(
'ip-intelligence/fiftyone_ipintelligence_examples/tests/test_environment_names.py'
)
}
@{
Names = @($ipiFileOld, 'cloud_endpoint', 'license_key', 'resource_key')
Paths = @(
'ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py'
)
}
@{
Names = @('cloud_endpoint', 'cloud_request_origin', 'license_key', 'resource_key')
Paths = @(
'pipeline/fiftyone_pipeline_cloudrequestengine/examples/src/fiftyone_pipeline_examples/example_utils.py'
)
}
@{
Names = @('license_key', 'resource_key')
Paths = @(
'pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py'
)
}
@{
Names = @($performanceOld)
Paths = @(
'device-detection/fiftyone_devicedetection_examples/tox.ini'
)
}
)

# The old names a file may hold, given its path from the root of the
# repository written with forward slashes. Nothing for any other file.
function Get-FallbackName([string]$Relative) {
foreach ($entry in $script:FallbackNames) {
if ($entry.Paths -ccontains $Relative) {
$entry.Names
}
}
}

# The problems on one line, each as its kind, 'digit' or 'fallback', and the
# text matched.
function Find-EnvironmentNameProblem([string]$Line) {
# text matched. Allowed holds the old names the line's file may hold, which
# are taken out of the line, in the case they are written in, before the
# line is checked.
function Find-EnvironmentNameProblem([string]$Line, [string[]]$Allowed = @()) {
$Line = $script:DeprecatedReadPattern.Replace($Line, '$1""')
if ($Allowed.Count -gt 0) {
$names = ($Allowed | ForEach-Object { [regex]::Escape($_) }) -join '|'
$Line = [regex]::Replace(
$Line, "(?<![A-Za-z0-9_])(?:$names)(?![A-Za-z0-9_])", '')
}
foreach ($match in $script:DigitInitialPattern.Matches($Line)) {
[pscustomobject]@{ Kind = 'digit'; Value = $match.Value }
}
Expand Down Expand Up @@ -115,6 +228,7 @@ foreach ($relative in $tracked) {
if ($relative -eq $ownTests) {
continue
}
$allowedNames = @(Get-FallbackName $relative)
$path = Join-Path $root $relative
# A submodule is tracked as a directory, and its files belong to another
# repository.
Expand All @@ -126,7 +240,7 @@ foreach ($relative in $tracked) {
$lineNumber = 0
foreach ($line in [IO.File]::ReadLines($path)) {
$lineNumber++
foreach ($problem in Find-EnvironmentNameProblem $line) {
foreach ($problem in Find-EnvironmentNameProblem $line $allowedNames) {
$text = "${relative}:${lineNumber}: $($problem.Value)"
if ($problem.Kind -eq 'digit') {
$found += $text
Expand All @@ -145,8 +259,9 @@ if ($found.Count -gt 0 -or $fallbacks.Count -gt 0) {
Write-Output $fallbacks
throw ("$($found.Count) names start with a digit, which a POSIX shell " +
'cannot set, so each must start with an underscore, and ' +
"$($fallbacks.Count) fallback names are read, set or kept, where " +
'each variable has one name.')
"$($fallbacks.Count) old names are read, set or kept outside the " +
'deprecated reads and the files listed with them.')
}

Write-Output 'No tracked file names a variable starting with a digit or a fallback name.'
Write-Output ('No tracked file names a variable starting with a digit, or ' +
'an old name outside the deprecated reads.')
Loading
Loading