Feature dna 363 by LindnerBrewery · Pull Request #15 · LindnerBrewery/OctopusDeploy

LindnerBrewery · 2025-12-19T14:34:22Z

Description
Added a new PowerShell function Set-CommonTenantVariable to manage common tenant variables in Octopus Deploy. This function provides support for setting or resetting tenant variables with optional environment scoping.

Key features:

Set single or multiple variables using hashtables
Scope variables to specific environments or leave unscoped
Intelligent scope conflict handling (disjoint, overlapping, equal, contained)
Reset variables to default with empty string values
Preserves non-modified tenant variables
Atomic operations - all changes succeed or none are applied

…function

…nvironment filtering and optional variable set retrieval

…nction

…ons for improved variable handling and add new Get-VariableTemplate and New-TenantCommonVariablePayload functions

…CommonTenantVariable functions; add IsSensitive parameter and improve payload creation logic

…-CommonTenantVariable function

… function and fix variable name typo in Set-CommonTenantVariable function

…improved clarity and performance

…TenantVariable function

… and error handling for scoped variables

…rent version details

…by explicitly passing parameters for temporary value assignment

Copilot

Pull request overview

This PR enhances the Set-CommonTenantVariable function to support environment-scoped tenant variables with intelligent scope conflict resolution. It also improves Get-CommonTenantVariable to optionally filter by environment and retrieve variables across all variable sets.

Enhanced tenant variable management with support for environment scoping and automatic scope conflict handling (disjoint, overlapping, equal, contained scenarios)
Added new helper functions for scope comparison and payload creation
Improved existing functions with better error handling and additional filtering capabilities

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
OctopusDeploy/Public/Set-CommonTenantVariable.ps1	Major enhancement adding environment scoping support, intelligent scope conflict handling, comprehensive validation, and detailed documentation
OctopusDeploy/Public/Get-CommonTenantVariable.ps1	Made VariableSet parameter optional, added Environment filter parameter, improved output format with scope information
OctopusDeploy/Public/GetCommonTenantVariable.ps1	New internal function that implements the core variable retrieval logic with environment filtering support
OctopusDeploy/Public/Compare-EnvironmentScope.ps1	New utility function to compare environment scopes and determine relationship (Equal, Disjoint, Overlap, Contained)
OctopusDeploy/Private/New-TenantCommonVariablePayload.ps1	New helper function to create TenantCommonVariablePayload objects with proper type handling
OctopusDeploy/Private/Get-VariableTemplate.ps1	New utility function to retrieve variable templates from variable sets
OctopusDeploy/Public/Invoke-RunbookRun.ps1	Removed early return statements to allow proper error handling flow
OctopusDeploy/Public/Get-Task.ps1	Added support for DeploymentResource objects, improved code formatting
OctopusDeploy/Public/Add-RoleToMachine.ps1	Enhanced with return value checking and verbose logging
CHANGELOG.md	Restructured and updated with new changes under Unreleased section
GettingStarted.md	Added examples for new Environment filtering capabilities
.github/workflows/release.yaml	Enhanced with automatic changelog extraction and commit functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T14:39:24Z

+
+                        $payloads += $payload
+
+                        # there seems to be and issue with setting sensitive variables to empty string in overlapping scope scenario


The comment "there seems to be and issue" contains a grammatical error. It should be "there seems to be an issue".

Suggested change

# there seems to be and issue with setting sensitive variables to empty string in overlapping scope scenario

# there seems to be an issue with setting sensitive variables to empty string in overlapping scope scenario

Copilot · 2025-12-19T14:39:24Z

+            # Check that all the variables are defined in Variable Set
            foreach ($h in $VariableHash.GetEnumerator()) {
-                if ($VariableSet.Templates.name -notcontains $h.Name) {
+                if ($currentVariables.Name -notcontains $h.Name) {
                    $message = "Couldn't find {0} in variable set {1}" -f $h.Name, $VariableSet.Name
-                    throw $message
-                } else {
+                    $err = [System.Management.Automation.ErrorRecord]::new(
+                        [System.Management.Automation.ItemNotFoundException]::new("$message"),
+                        'NotSpecified',
+                        'InvalidData',
+                        "$($Variableset.name) / $($h.name)"
+                    )
+                    $errorDetails = [System.Management.Automation.ErrorDetails]::new("$message")
+                    $errorDetails.RecommendedAction = "Check variable exists in variable set"
+                    $err.ErrorDetails = $errorDetails
+                    $PSCmdlet.ThrowTerminatingError($err)
+                }
+                else {
                    $message = "Found variable {0} in variable set {1}" -f $h.Name, $VariableSet.Name
                    Write-Verbose $message
                }
            }


There's a duplicate validation check here. Lines 152-171 already check that all variables exist in the variable set. This second check at lines 174-192 performs the same validation but with slightly different error handling. One of these checks should be removed to avoid redundant validation.

Copilot · 2025-12-19T14:39:24Z

+                        IsSensitive     = $_.DefaultValue.IsSensitive
+                        VariableId      = $null
+                        LibraryVariableSetId = $vSet.Id
+                        origObject     = $_


The property name "origObject" uses inconsistent casing compared to other property names in the same object. All other properties use PascalCase (VariableSetName, Name, Value, etc.), but this one uses camelCase. Consider renaming to "OrigObject" for consistency.

Copilot · 2025-12-19T14:39:25Z

@@ -0,0 +1,101 @@
+function GetCommonTenantVariable {


This function is missing comment-based help documentation. Internal/private functions should include at minimum a SYNOPSIS and DESCRIPTION to explain the function's purpose and behavior, especially when they implement complex logic.

Suggested change

function GetCommonTenantVariable {

<#

.SYNOPSIS

Retrieves common tenant variables for the specified tenant, library variable set, and environment.

.DESCRIPTION

GetCommonTenantVariable validates the Octopus Deploy connection and then retrieves tenant-scoped

variables from the server. It can optionally scope the returned variables to a specific library

variable set and environment when they are provided.

.PARAMETER Tenant

The Octopus Deploy tenant resource for which common variables will be retrieved. This parameter is mandatory.

.PARAMETER VariableSet

An optional Octopus Deploy library variable set resource used to filter the tenant variables.

.PARAMETER Environment

An optional Octopus Deploy environment resource used to filter the tenant variables by environment.

#>

function GetCommonTenantVariable {

Copilot · 2025-12-19T14:39:25Z

+            $variableToPreserve = $currentVariables | Where-Object { ($_.name.ToLower() -notin $VariableHash.Keys.ToLower() -and -not $_.IsDefaultValue -and $_.LibraryVariableSetId -eq $VariableSet.id ) -or 
+                (-not $_.IsDefaultValue -and $_.LibraryVariableSetId -ne $VariableSet.id ) }


The case-insensitive comparison on line 206 is calling ToLower() on a collection (VariableHash.Keys), which won't work as intended. The ToLower() method should be called on individual strings within a comparison operation, not on the collection itself. Consider using the -notin operator with proper case handling, such as: Where-Object { $VariableHash.Keys -notcontains $_.name } with case-insensitive comparison logic, or restructure to compare individual elements.

Suggested change

$variableToPreserve = $currentVariables | Where-Object { ($_.name.ToLower() -notin $VariableHash.Keys.ToLower() -and -not $_.IsDefaultValue -and $_.LibraryVariableSetId -eq $VariableSet.id ) -or

(-not $_.IsDefaultValue -and $_.LibraryVariableSetId -ne $VariableSet.id ) }

$variableToPreserve = $currentVariables | Where-Object { ( $_.Name -notin $VariableHash.Keys -and -not $_.IsDefaultValue -and $_.LibraryVariableSetId -eq $VariableSet.Id ) -or

( -not $_.IsDefaultValue -and $_.LibraryVariableSetId -ne $VariableSet.Id ) }

Copilot · 2025-12-19T14:39:25Z

+                            TemplateId           = $var.TemplateId
+                            Value                = $VariableHash[$var.Name]
+                            IsSensitive          = $var.IsSensitive
+                            Scope                = if (-not $null -eq $comparison.ExistingScope) { $($comparison.ExistingScope) }else { $($comparison.NewScope) }


The null comparison is using the wrong order. In PowerShell, the recommended pattern is to place the value being tested on the left side, not the null check. The condition should be "$comparison.ExistingScope -ne $null" or "($null -ne $comparison.ExistingScope)" for better readability and correctness.

Suggested change

Scope = if (-not $null -eq $comparison.ExistingScope) { $($comparison.ExistingScope) }else { $($comparison.NewScope) }

Scope = if ($comparison.ExistingScope -ne $null) { $comparison.ExistingScope } else { $comparison.NewScope }

Copilot · 2025-12-19T14:39:26Z

+
+                }
+            }
+            # if an environment was specified, return all scoped variables for that environment and unscoped variables a far the variable has no environment scope


The comment "a far the variable" contains a spelling error. It should be "as far as the variable".

Suggested change

# if an environment was specified, return all scoped variables for that environment and unscoped variables a far the variable has no environment scope

# if an environment was specified, return all scoped variables for that environment and unscoped variables as far as the variable has no environment scope

Copilot · 2025-12-19T14:39:26Z

+                    IsSensitive     = $_.Value.IsSensitive
+                    VariableId      = $_.Id
+                    LibraryVariableSetId = $_.LibraryVariableSetId
+                    origObject     = $_


The property name "origObject" uses inconsistent casing compared to other property names in the same object. All other properties use PascalCase (VariableSetName, Name, Value, etc.), but this one uses camelCase. Consider renaming to "OrigObject" for consistency.

Copilot · 2025-12-19T14:39:26Z

@@ -0,0 +1,35 @@
+function Compare-EnvironmentScope {


This function is missing comment-based help documentation. Public functions should include a SYNOPSIS, DESCRIPTION, parameter descriptions, and usage examples to help users understand how to use the function.

Suggested change

function Compare-EnvironmentScope {

<#

.SYNOPSIS

Compares an existing environment scope with a new environment scope and describes their relationship.

.DESCRIPTION

Compare-EnvironmentScope normalizes and compares two sets of environment identifiers to determine how

the new scope relates to the existing scope. It returns an object with a Status property that indicates

the relationship, and properties that show which environments are considered part of each side.

The Status property can have one of the following values:

- Equal : Both scopes contain the same environments (after sorting and removing duplicates).

- Disjoint : The scopes share no environments in common.

- Overlap : The scopes share some environments, but the existing scope includes environments that are

not present in the new scope.

- Contained : All environments in the existing scope are present in the new scope.

Both ExistingScope and NewScope may be null or empty to represent an unscoped value.

.PARAMETER ExistingScope

The current set of environment IDs or names that represent the existing scope. This parameter

can be null or an empty collection to indicate that the value is currently unscoped.

.PARAMETER NewScope

The proposed set of environment IDs or names that represent the new scope. This parameter

can be null or an empty collection to indicate that the value should be unscoped.

.EXAMPLE

PS C:\> Compare-EnvironmentScope -ExistingScope @('Dev','Test') -NewScope @('Dev','Test')

Returns:

Status ExistingScope NewScope

------ ------------- -------

Equal {Dev, Test} null

This indicates that the new scope is identical to the existing scope.

.EXAMPLE

PS C:\> Compare-EnvironmentScope -ExistingScope @('Dev','Test','Prod') -NewScope @('Test','Prod')

Returns:

Status ExistingScope NewScope

------ ------------- -------

Overlap {Dev} {Test, Prod}

This indicates that the scopes partially overlap and that the existing scope includes environments

that are not present in the new scope.

#>

function Compare-EnvironmentScope {

LindnerBrewery and others added 14 commits November 28, 2025 11:13

Add support for retrieving tasks from DeploymentResource in Get-Task …

60b1a8c

…function

Update CHANGELOG and improve Get-CommonTenantVariable function with e…

45a4533

…nvironment filtering and optional variable set retrieval

Fix type declaration for results array in Get-CommonTenantVariable fu…

ee1001c

…nction

+semver:minor

8d79553

Refactor Get-CommonTenantVariable and Set-CommonTenantVariable functi…

c057f8a

…ons for improved variable handling and add new Get-VariableTemplate and New-TenantCommonVariablePayload functions

Enhance variable handling in New-TenantCommonVariablePayload and Set-…

c6ccb6f

…CommonTenantVariable functions; add IsSensitive parameter and improve payload creation logic

Remove commented-out code for tenant variable payload creation in Set…

c2f7ab4

…-CommonTenantVariable function

Add null check for Value parameter in New-TenantCommonVariablePayload…

4bc8ddc

… function and fix variable name typo in Set-CommonTenantVariable function

better verbose output in Add-RoleToMachine function

fa00979

Refactor variable retrieval in Set-CommonTenantVariable function for …

2017e74

…improved clarity and performance

Remove commented-out code for variable payload creation in Set-Common…

2ee9c3d

…TenantVariable function

Enhance Set-CommonTenantVariable function with improved documentation…

49fdf4b

… and error handling for scoped variables

Update release workflow to automate CHANGELOG updates and extract cur…

fd3bdc3

…rent version details

Fix sensitive variable handling in Set-CommonTenantVariable function …

3163b12

…by explicitly passing parameters for temporary value assignment

Copilot AI review requested due to automatic review settings December 19, 2025 14:34

Copilot started reviewing on behalf of LindnerBrewery December 19, 2025 14:34 View session

LindnerBrewery merged commit a8571a7 into main Dec 19, 2025
12 checks passed

LindnerBrewery deleted the feature_DNA-363 branch December 19, 2025 14:39

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Navigation Menu

Feature dna 363 - #15

Feature dna 363#15
LindnerBrewery merged 14 commits into
mainfrom
feature_DNA-363

LindnerBrewery commented Dec 19, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		$payloads += $payload

		# there seems to be and issue with setting sensitive variables to empty string in overlapping scope scenario

	# there seems to be and issue with setting sensitive variables to empty string in overlapping scope scenario
	# there seems to be an issue with setting sensitive variables to empty string in overlapping scope scenario

-function GetCommonTenantVariable {
+<#
+.SYNOPSIS
+Retrieves common tenant variables for the specified tenant, library variable set, and environment.
+.DESCRIPTION
+GetCommonTenantVariable validates the Octopus Deploy connection and then retrieves tenant-scoped
+variables from the server. It can optionally scope the returned variables to a specific library
+variable set and environment when they are provided.
+.PARAMETER Tenant
+The Octopus Deploy tenant resource for which common variables will be retrieved. This parameter is mandatory.
+.PARAMETER VariableSet
+An optional Octopus Deploy library variable set resource used to filter the tenant variables.
+.PARAMETER Environment
+An optional Octopus Deploy environment resource used to filter the tenant variables by environment.
+#>
+function GetCommonTenantVariable {

		$variableToPreserve = $currentVariables \| Where-Object { ($_.name.ToLower() -notin $VariableHash.Keys.ToLower() -and -not $_.IsDefaultValue -and $_.LibraryVariableSetId -eq $VariableSet.id ) -or
		(-not $_.IsDefaultValue -and $_.LibraryVariableSetId -ne $VariableSet.id ) }

	Scope = if (-not $null -eq $comparison.ExistingScope) { $($comparison.ExistingScope) }else { $($comparison.NewScope) }
	Scope = if ($comparison.ExistingScope -ne $null) { $comparison.ExistingScope } else { $comparison.NewScope }

	# if an environment was specified, return all scoped variables for that environment and unscoped variables a far the variable has no environment scope
	# if an environment was specified, return all scoped variables for that environment and unscoped variables as far as the variable has no environment scope

-function Compare-EnvironmentScope {
+<#
+.SYNOPSIS
+Compares an existing environment scope with a new environment scope and describes their relationship.
+.DESCRIPTION
+Compare-EnvironmentScope normalizes and compares two sets of environment identifiers to determine how
+the new scope relates to the existing scope. It returns an object with a Status property that indicates
+the relationship, and properties that show which environments are considered part of each side.
+The Status property can have one of the following values:
+- Equal     : Both scopes contain the same environments (after sorting and removing duplicates).
+- Disjoint  : The scopes share no environments in common.
+- Overlap   : The scopes share some environments, but the existing scope includes environments that are
+              not present in the new scope.
+- Contained : All environments in the existing scope are present in the new scope.
+Both ExistingScope and NewScope may be null or empty to represent an unscoped value.
+.PARAMETER ExistingScope
+The current set of environment IDs or names that represent the existing scope. This parameter
+can be null or an empty collection to indicate that the value is currently unscoped.
+.PARAMETER NewScope
+The proposed set of environment IDs or names that represent the new scope. This parameter
+can be null or an empty collection to indicate that the value should be unscoped.
+.EXAMPLE
+PS C:\> Compare-EnvironmentScope -ExistingScope @('Dev','Test') -NewScope @('Dev','Test')
+Returns:
+Status         ExistingScope NewScope
+------         ------------- -------
+Equal          {Dev, Test}  null
+This indicates that the new scope is identical to the existing scope.
+.EXAMPLE
+PS C:\> Compare-EnvironmentScope -ExistingScope @('Dev','Test','Prod') -NewScope @('Test','Prod')
+Returns:
+Status         ExistingScope NewScope
+------         ------------- -------
+Overlap        {Dev}         {Test, Prod}
+This indicates that the scopes partially overlap and that the existing scope includes environments
+that are not present in the new scope.
+#>
+function Compare-EnvironmentScope {

Conversation

LindnerBrewery commented Dec 19, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants