From 0742b2035b1cc83575097893a94afd7512355c23 Mon Sep 17 00:00:00 2001 From: Andrea Spadaccini Date: Fri, 12 Jun 2020 13:39:17 +0100 Subject: [PATCH] =?UTF-8?q?Blacklist=20=E2=86=92=20denylist,=20whitelist?= =?UTF-8?q?=20=E2=86=92=20allowlist=20(#20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this is an incompatible change, also bump up version to 2.0.0 (major version change, to signal incompatible API). --- FeatureFlags.PowerShell.nuspec | 4 +- FeatureFlags.psd1 | Bin 5702 -> 5702 bytes FeatureFlags.psm1 | 30 +++++------ README.md | 46 ++++++++--------- examples/features.json | 8 +-- featureflags.schema.json | 19 +++---- test/FeatureFlags.Tests.ps1 | 77 +++++++++++++++++------------ test/multiple-stages-features.json | 10 ++-- test/multiple-stages.json | 8 +-- test/single-stage.json | 2 +- 10 files changed, 109 insertions(+), 95 deletions(-) diff --git a/FeatureFlags.PowerShell.nuspec b/FeatureFlags.PowerShell.nuspec index f90cdd0..5bc5046 100644 --- a/FeatureFlags.PowerShell.nuspec +++ b/FeatureFlags.PowerShell.nuspec @@ -2,7 +2,7 @@ FeatureFlags.PowerShell - 1.0.1 + 2.0.0 PowerShell Feature Flags Andrea Spadaccini,Nick Hara Andrea Spadaccini @@ -10,7 +10,7 @@ https://github.com/microsoft/PowerShell-FeatureFlags/ false PowerShell module containing a Feature Flags implementation based on a local config file. - Fixed bug with the "probability" condition (issue #18). + Renamed blacklist to denylist, whitelist to allowlist. Incompatible with 1.0.x. Copyright 2020 Microsoft diff --git a/FeatureFlags.psd1 b/FeatureFlags.psd1 index f05dfb0930286908effb8aed6b96d70511e65b0d..0673c2897cf27890139cb098e64b32b92928698c 100644 GIT binary patch delta 22 bcmX@6b4+K06%(fsgC2tc2yb>~Vio}aMp6XV delta 22 ccmX@6b4+K06%(f+gC2tc5F2iGW?~ir07gm#*8l(j diff --git a/FeatureFlags.psm1 b/FeatureFlags.psm1 index adb62d5..d31bb8d 100644 --- a/FeatureFlags.psm1 +++ b/FeatureFlags.psm1 @@ -219,24 +219,24 @@ function Test-FeatureConditions ) # Conditions are evaluated in the order they are presented in the configuration file. foreach ($condition in $conditions) { - # Each condition object can have only one of the whitelist, blacklist or probability + # Each condition object can have only one of the allowlist, denylist or probability # attributes set. This invariant is enforced by the JSON schema, which uses the "oneof" - # strategy to choose between whitelist, blacklist or probability and, for each of these + # strategy to choose between allowlist, denylist or probability and, for each of these # condition types, only allows the homonym attribute to be set. - if ($null -ne $condition.whitelist) { - Write-Verbose "Checking the whitelist condition" - # The predicate must match any of the regexes in the whitelist in order to - # consider the whitelist condition satisfied. - $matchesWhitelist = Test-RegexList $predicate @($condition.whitelist) - if (-not $matchesWhitelist) { + if ($null -ne $condition.allowlist) { + Write-Verbose "Checking the allowlist condition" + # The predicate must match any of the regexes in the allowlist in order to + # consider the allowlist condition satisfied. + $matchesallowlist = Test-RegexList $predicate @($condition.allowlist) + if (-not $matchesallowlist) { return $false } - } elseif ($null -ne $condition.blacklist) { - Write-Verbose "Checking the blacklist condition" - # The predicate must not match all of the regexes in the blacklist in order to - # consider the blacklist condition satisfied. - $matchesBlacklist = Test-RegexList $predicate @($condition.blacklist) - if ($matchesBlacklist) { + } elseif ($null -ne $condition.denylist) { + Write-Verbose "Checking the denylist condition" + # The predicate must not match all of the regexes in the denylist in order to + # consider the denylist condition satisfied. + $matchesdenylist = Test-RegexList $predicate @($condition.denylist) + if ($matchesdenylist) { return $false } } elseif ($null -ne $condition.probability) { @@ -250,7 +250,7 @@ function Test-FeatureConditions return $false } } else { - throw "${condition} is not a supported condition type (blacklist, whitelist or probability)." + throw "${condition} is not a supported condition type (denylist, allowlist or probability)." } } return $true diff --git a/README.md b/README.md index 74234bb..4ddd59a 100644 --- a/README.md +++ b/README.md @@ -49,14 +49,14 @@ Imagine to have a feature flag configuration file called `features.json`: { "stages": { "test": [ - {"whitelist": ["test.*", "dev.*"]} + {"allowlist": ["test.*", "dev.*"]} ], "canary": [ - {"whitelist": ["prod-canary"]} + {"allowlist": ["prod-canary"]} ], "prod": [ - {"whitelist": ["prod.*"]}, - {"blacklist": ["prod-canary"]} + {"allowlist": ["prod.*"]}, + {"denylist": ["prod-canary"]} ] }, "features": { @@ -122,13 +122,13 @@ An example lifecycle of a feature flag might be the following: Here is how these example stages could be implemented: -* Stage 1 can be implemented with a `blacklist` condition with value `.*`. -* Stages 2 and 3 can be implemented with `whitelist` conditions. +* Stage 1 can be implemented with a `denylist` condition with value `.*`. +* Stages 2 and 3 can be implemented with `allowlist` conditions. * Stages 4 and 5 can be implemented with `probability` conditions. ## Conditions -There are two types of conditions: *deterministic* (whitelist and blacklist, +There are two types of conditions: *deterministic* (allowlist and denylist, regex-based) and *probabilistic* (probability, expressed as a number between 0 and 1). Conditions can be repeated if multiple instances are required. @@ -138,9 +138,9 @@ in the configuration file, for the feature to be considered enabled. If any condition is not met, evaluation of conditions stops and the feature is considered disabled. -### Whitelist +### Allow list -The `whitelist` condition allows to specify a list of regular expressions; if the +The `allowlist` condition allows to specify a list of regular expressions; if the predicate matches any of the expressions, then the condition is met and the evaluation moves to the next condition, if there is any. @@ -150,9 +150,9 @@ unintended matches, it's recommended to always anchor the regex. So, for example, `"^storage$"` will only match `"storage"` and not `"storage1"`. -### Blacklist +### Deny list -The `blacklist` condition is analogous to the whitelist condition, except that if +The `denylist` condition is analogous to the allowlist condition, except that if the predicate matches any of the expressions the condition is considered not met and the evaluation stops. @@ -172,25 +172,25 @@ the following example: ```json { "stages": { - "whitelist-first": [ - {"whitelist": ["storage.*"]}, + "allowlist-first": [ + {"allowlist": ["storage.*"]}, {"probability": 0.1} ], "probability-first": [ {"probability": 0.1} - {"whitelist": ["storage.*"]}, + {"allowlist": ["storage.*"]}, ] } } ``` -The first stage definition, `whitelist-first`, will evaluate the `probability` condition -only if the predicate first passes the whitelist. +The first stage definition, `allowlist-first`, will evaluate the `probability` condition +only if the predicate first passes the allowlist. The second stage definition, `probability-first`, will instead first evaluate -the `probability` condition, and then apply the whitelist. +the `probability` condition, and then apply the allowlist. -Assuming there are predicates that do not match the whitelist, the second stage definition +Assuming there are predicates that do not match the allowlist, the second stage definition is more restrictive than the first one, leading to fewer positive evaluations of the feature flag. @@ -231,19 +231,19 @@ for comments. Don't add comments to your feature flag configuration file. ], // Examples of deterministic stages. "all-storage": [ - {"whitelist": [".*Storage.*"]}, + {"allowlist": [".*Storage.*"]}, ], "storage-except-important": [ - {"whitelist": [".*Storage.*"]}, - {"blacklist": [".*StorageImportant.*"]}, + {"allowlist": [".*Storage.*"]}, + {"denylist": [".*StorageImportant.*"]}, ], // Example of mixed roll-out stage. // This stage will match on predicates containing the word "Storage" // but not the word "StorageImportant", and then will consider the feature // enabled in 50% of the cases. "50-percent-storage-except-StorageImportant": [ - {"whitelist": [".*Storage.*"]}, - {"blacklist": ["StorageImportant"]}, + {"allowlist": [".*Storage.*"]}, + {"denylist": ["StorageImportant"]}, {"probability": 0.5}, ], }, diff --git a/examples/features.json b/examples/features.json index 7f1921b..e60af8a 100644 --- a/examples/features.json +++ b/examples/features.json @@ -1,14 +1,14 @@ { "stages": { "test": [ - {"whitelist": ["test.*", "dev.*"]} + {"allowlist": ["test.*", "dev.*"]} ], "canary": [ - {"whitelist": ["prod-canary"]} + {"allowlist": ["prod-canary"]} ], "prod": [ - {"whitelist": ["prod.*"]}, - {"blacklist": ["prod-canary"]} + {"allowlist": ["prod.*"]}, + {"denylist": ["prod-canary"]} ] }, "features": { diff --git a/featureflags.schema.json b/featureflags.schema.json index 754727f..76d86a3 100644 --- a/featureflags.schema.json +++ b/featureflags.schema.json @@ -29,30 +29,30 @@ "required": ["stages"], "additionalProperties": false, "definitions": { - "whitelist": { + "allowlist": { "type": "object", "properties": { - "whitelist": { + "allowlist": { "type": "array", "items": { "type": "string" } } }, - "required": ["whitelist"], + "required": ["allowlist"], "additionalProperties": false }, - "blacklist": { + "denylist": { "type": "object", "properties": { - "blacklist": { + "denylist": { "type": "array", "items": { "type": "string" } } }, - "required": ["blacklist"], + "required": ["denylist"], "additionalProperties": false }, "probability": { @@ -70,11 +70,12 @@ "conditions": { "type": "array", "items": { - "oneOf": [{ - "$ref": "#/definitions/whitelist" + "oneOf": [ + { + "$ref": "#/definitions/allowlist" }, { - "$ref": "#/definitions/blacklist" + "$ref": "#/definitions/denylist" }, { "$ref": "#/definitions/probability" diff --git a/test/FeatureFlags.Tests.ps1 b/test/FeatureFlags.Tests.ps1 index d4a4fe4..3ec8676 100644 --- a/test/FeatureFlags.Tests.ps1 +++ b/test/FeatureFlags.Tests.ps1 @@ -75,6 +75,19 @@ Describe 'Confirm-FeatureFlagConfig' { $cfg = @" { "stags": { + "storage": [ + {"allowlist": [".*storage.*"]} + ] + } + } +"@ + Confirm-FeatureFlagConfig -EA 0 $cfg | Should -Be $false + } + + It 'Fails if a deprecated condition is used (whitelist)' { + $cfg = @" + { + "stages": { "storage": [ {"whitelist": [".*storage.*"]} ] @@ -84,12 +97,12 @@ Describe 'Confirm-FeatureFlagConfig' { Confirm-FeatureFlagConfig -EA 0 $cfg | Should -Be $false } - It 'Fails if a condition name contains a typo (whtelist)' { + It 'Fails if a condition name contains a typo (allwlist)' { $cfg = @" { "stages": { "storage": [ - {"whtelist": [".*storage.*"]} + {"allwlist": [".*storage.*"]} ] } } @@ -102,7 +115,7 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { "storage": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] }, "featurs": { @@ -118,7 +131,7 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { "": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] } } @@ -131,7 +144,7 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { " ": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] } } @@ -141,12 +154,12 @@ Describe 'Confirm-FeatureFlagConfig' { } Context 'Successful validation of simple stages' { - It 'Succeeds with a simple stage with two whitelists' { + It 'Succeeds with a simple stage with two allowlists' { $cfg = @" { "stages": { "storage": [ - {"whitelist": [".*storage.*", ".*compute.*"]} + {"allowlist": [".*storage.*", ".*compute.*"]} ] } } @@ -154,13 +167,13 @@ Describe 'Confirm-FeatureFlagConfig' { Confirm-FeatureFlagConfig $cfg | Should -Be $true } - It 'Succeeds with a simple stage with a whitelist and a blacklist' { + It 'Succeeds with a simple stage with a allowlist and a denylist' { $cfg = @" { "stages": { "storage": [ - {"whitelist": [".*storage.*"]}, - {"blacklist": ["ImportantStorage"]} + {"allowlist": [".*storage.*"]}, + {"denylist": ["ImportantStorage"]} ] } } @@ -187,7 +200,7 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { "storage": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] }, "features": { @@ -205,10 +218,10 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { "all": [ - {"whitelist": [".*"]} + {"allowlist": [".*"]} ], "storage": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] }, "features": { @@ -231,7 +244,7 @@ Describe 'Confirm-FeatureFlagConfig' { { "stages": { "storage": [ - {"whitelist": [".*storage.*"]} + {"allowlist": [".*storage.*"]} ] }, "features": { @@ -255,13 +268,13 @@ Describe 'Get-FeatureFlagConfigFromFile' { } Describe 'Test-FeatureFlag' { - Context 'Whitelist condition' { - Context 'Simple whitelist configuration' { + Context 'allowlist condition' { + Context 'Simple allowlist configuration' { $serializedConfig = @" { "stages": { "all": [ - {"whitelist": [".*"]} + {"allowlist": [".*"]} ] }, "features": { @@ -283,12 +296,12 @@ Describe 'Test-FeatureFlag' { } } - Context 'Chained whitelist configuration' { + Context 'Chained allowlist configuration' { $serializedConfig = @" { "stages": { "test-repo-and-branch": [ - {"whitelist": [ + {"allowlist": [ "storage1/.*", "storage2/dev-branch" ]} @@ -315,13 +328,13 @@ Describe 'Test-FeatureFlag' { } } - Context 'Blacklist condition' { + Context 'denylist condition' { Context 'Reject-all configuration' { $serializedConfig = @" { "stages": { "none": [ - {"blacklist": [".*"]} + {"denylist": [".*"]} ] }, "features": { @@ -346,7 +359,7 @@ Describe 'Test-FeatureFlag' { { "stages": { "all-except-important": [ - {"blacklist": ["^important$"]} + {"denylist": ["^important$"]} ] }, "features": { @@ -360,7 +373,7 @@ Describe 'Test-FeatureFlag' { Confirm-FeatureFlagConfig $serializedConfig $config = ConvertFrom-Json $serializedConfig - # Given that the regex is ^important$, only the exact string "important" will match the blacklist. + # Given that the regex is ^important$, only the exact string "important" will match the denylist. It 'Allows the flag if the predicate does not match exactly' { Test-FeatureFlag "some-feature" "Storage/master" $config | Should -Be $true Test-FeatureFlag "some-feature" "foo" $config | Should -Be $true @@ -378,7 +391,7 @@ Describe 'Test-FeatureFlag' { { "stages": { "all-except-important": [ - {"blacklist": ["storage-important/master", "storage-important2/master"]} + {"denylist": ["storage-important/master", "storage-important2/master"]} ] }, "features": { @@ -391,7 +404,7 @@ Describe 'Test-FeatureFlag' { Confirm-FeatureFlagConfig $serializedConfig $config = ConvertFrom-Json $serializedConfig - It 'Allows predicates not matching the blacklist' { + It 'Allows predicates not matching the denylist' { Test-FeatureFlag "some-feature" "storage1/master" $config | Should -Be $true Test-FeatureFlag "some-feature" "storage2/master" $config | Should -Be $true Test-FeatureFlag "some-feature" "storage-important/dev" $config | Should -Be $true @@ -404,13 +417,13 @@ Describe 'Test-FeatureFlag' { } } - Context 'Mixed whitelist/blacklist configuration' { + Context 'Mixed allowlist/denylist configuration' { $serializedConfig = @" { "stages": { "all-storage-important": [ - {"whitelist": ["storage.*"]}, - {"blacklist": ["storage-important/master", "storage-important2/master"]} + {"allowlist": ["storage.*"]}, + {"denylist": ["storage-important/master", "storage-important2/master"]} ] }, "features": { @@ -509,13 +522,13 @@ Describe 'Test-FeatureFlag' { } } - Context 'Complex whitelist + blacklist + probability configuration' { + Context 'Complex allowlist + denylist + probability configuration' { $serializedConfig = @" { "stages": { "all-storage-important-50pc": [ - {"whitelist": ["storage.*"]}, - {"blacklist": ["storage-important/master", "storage-important2/master"]}, + {"allowlist": ["storage.*"]}, + {"denylist": ["storage-important/master", "storage-important2/master"]}, {"probability": 0.5} ] }, @@ -589,7 +602,7 @@ Describe 'Out-EvaluatedFeaturesFiles' -Tag Features { Mock -ModuleName $ModuleName Add-Content { ${global:featuresIniContent}.Add($Value) } -ParameterFilter { $Path.EndsWith("features.ini") } Mock -ModuleName $ModuleName Add-Content { ${global:featuresEnvConfigContent}.Add($Value) } -ParameterFilter { $Path.EndsWith("features.env.config") } - It 'Honors blacklist' { + It 'Honors denylist' { $features = Get-EvaluatedFeatureFlags -predicate "important" -config $config $expectedFeaturesIniContent = @("filetracker`tfalse", "newestfeature`tfalse", "testfeature`tfalse") $expectedFeaturesEnvConfigContent = @() diff --git a/test/multiple-stages-features.json b/test/multiple-stages-features.json index b34d8dd..b21a71e 100644 --- a/test/multiple-stages-features.json +++ b/test/multiple-stages-features.json @@ -2,28 +2,28 @@ "stages": { "all-except-important": [ { - "blacklist": [ + "denylist": [ "^important$" ] } ], "poc": [ { - "whitelist": [ + "allowlist": [ "demo/.*demofeature.*" ] } ], "dev": [ { - "whitelist": [ + "allowlist": [ "Dev.*" ] } ], "test": [ { - "whitelist": [ + "allowlist": [ "demo/.*test.*/.*Official.*", "demo2/.*test2.*" ] @@ -31,7 +31,7 @@ ], "prod": [ { - "whitelist": [ + "allowlist": [ "production/.*" ] } diff --git a/test/multiple-stages.json b/test/multiple-stages.json index 2ad4b99..4a86b78 100644 --- a/test/multiple-stages.json +++ b/test/multiple-stages.json @@ -1,14 +1,14 @@ { "stages": { "all-storage": [ - {"whitelist": [".*Storage.*"]} + {"allowlist": [".*Storage.*"]} ], "specific-storage-repos": [ - {"whitelist": ["Storage-repo1", "storage-repo2"]} + {"allowlist": ["Storage-repo1", "storage-repo2"]} ], "all-storage-except-important": [ - {"whitelist": [".*Storage.*"]}, - {"blacklist": ["Storage-important"]} + {"allowlist": [".*Storage.*"]}, + {"denylist": ["Storage-important"]} ] } } diff --git a/test/single-stage.json b/test/single-stage.json index ea585cc..23126c3 100644 --- a/test/single-stage.json +++ b/test/single-stage.json @@ -1,7 +1,7 @@ { "stages": { "all-storage": [ - {"whitelist": [".*Storage.*"]} + {"allowlist": [".*Storage.*"]} ] } }