Skip to content

Commit

Permalink
Secrets refactoring part II (#681)
Browse files Browse the repository at this point in the history
This PR includes:
- Fix for BUG: The DetermineDeploymentEnvironments doesn't work in
private repositories (needs the GITHUB_TOKEN)
- Only transfer Secrets to the Actions which actually needs them. Do not
have ENV:Secrets available in all steps.
- Move CheckAppDependencyProbingPaths out of AnalyzeRepo and call where
needed (remove doNotCheckAppDependencyProbingPaths switch)
- Remove BcContainerHelper usage from ReadSecrets
- Remove BcContainerHelper usage from ReadSettings
- Move functionality for getting a token to use for commits into
ReadSecrets (remove PS code from workflows)
  - Set UseGhTokenWorkflowForCommits as ENV variable based on the input
- Add TokenForCommits in the getSecrets and you will not have an output
variable to use as token for actions needing that

---------

Co-authored-by: freddydk <freddydk@users.noreply.github.com>
Co-authored-by: Maria Zhelezova <43066499+mazhelez@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 1, 2023
1 parent a6169c2 commit 7d7c20d
Show file tree
Hide file tree
Showing 64 changed files with 496 additions and 761 deletions.
288 changes: 146 additions & 142 deletions Actions/AL-Go-Helper.ps1

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Actions/AddExistingApp/AddExistingApp.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
2 changes: 1 addition & 1 deletion Actions/AnalyzeTests/AnalyzeTests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}

Expand Down
2 changes: 1 addition & 1 deletion Actions/CheckForUpdates/CheckForUpdates.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
4 changes: 2 additions & 2 deletions Actions/CreateApp/CreateApp.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ try {
if ($type -eq "Performance Test App") {
try {
$settings = ReadSettings -baseFolder $baseFolder -project $project
$settings = AnalyzeRepo -settings $settings -token $token -baseFolder $baseFolder -project $project -doNotIssueWarnings -doNotCheckAppDependencyProbingPaths
$settings = AnalyzeRepo -settings $settings -token $token -baseFolder $baseFolder -project $project -doNotIssueWarnings
$folders = Download-Artifacts -artifactUrl $settings.artifact -includePlatform
$sampleApp = Join-Path $folders[0] "Applications.*\Microsoft_Performance Toolkit Samples_*.app"
if (Test-Path $sampleApp) {
Expand Down Expand Up @@ -140,7 +140,7 @@ try {

}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
2 changes: 1 addition & 1 deletion Actions/CreateReleaseNotes/CreateReleaseNotes.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
6 changes: 3 additions & 3 deletions Actions/Deliver/Deliver.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ try {
Write-Host "Found custom script $customScript for delivery target $deliveryTarget"

$projectSettings = ReadSettings -baseFolder $baseFolder -project $thisProject
$projectSettings = AnalyzeRepo -settings $projectSettings -baseFolder $baseFolder -project $thisProject -doNotCheckArtifactSetting -doNotCheckAppDependencyProbingPaths -doNotIssueWarnings
$projectSettings = AnalyzeRepo -settings $projectSettings -baseFolder $baseFolder -project $thisProject -doNotCheckArtifactSetting -doNotIssueWarnings
$parameters = @{
"Project" = $thisProject
"ProjectName" = $projectName
Expand Down Expand Up @@ -398,7 +398,7 @@ try {
}
elseif ($deliveryTarget -eq "AppSource") {
$projectSettings = ReadSettings -baseFolder $baseFolder -project $thisProject
$projectSettings = AnalyzeRepo -settings $projectSettings -baseFolder $baseFolder -project $thisProject -doNotCheckArtifactSetting -doNotCheckAppDependencyProbingPaths -doNotIssueWarnings
$projectSettings = AnalyzeRepo -settings $projectSettings -baseFolder $baseFolder -project $thisProject -doNotCheckArtifactSetting -doNotIssueWarnings
# if type is Release, we only get here with the projects that needs to be delivered to AppSource
# if type is CD, we get here for all projects, but should only deliver to AppSource if AppSourceContinuousDelivery is set to true
if ($type -eq 'Release' -or ($projectSettings.Keys -contains 'AppSourceContinuousDelivery' -and $projectSettings.AppSourceContinuousDelivery)) {
Expand Down Expand Up @@ -467,7 +467,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
2 changes: 1 addition & 1 deletion Actions/Deploy/Deploy.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ try {

}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
4 changes: 2 additions & 2 deletions Actions/DetermineArtifactUrl/DetermineArtifactUrl.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ try {
$secrets = $env:Secrets | ConvertFrom-Json | ConvertTo-HashTable
$insiderSasToken = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String($secrets.insiderSasToken))
$settings = $env:Settings | ConvertFrom-Json | ConvertTo-HashTable
$settings = AnalyzeRepo -settings $settings -project $project -doNotCheckArtifactSetting -doNotCheckAppDependencyProbingPaths -doNotIssueWarnings
$settings = AnalyzeRepo -settings $settings -project $project -doNotCheckArtifactSetting -doNotIssueWarnings
$artifactUrl = Determine-ArtifactUrl -projectSettings $settings -insiderSasToken $insiderSasToken
$artifactCacheKey = ''
if ($settings.useCompilerFolder) {
Expand All @@ -38,7 +38,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function IncludeDeliveryTarget([string] $deliveryTarget) {
Write-Host "DeliveryTarget $_ - "
# DeliveryTarget Context Secret needs to be specified for a delivery target to be included
$contextName = "$($_)Context"
$secrets = $env:Secrets | ConvertFrom-Json
if (-not $secrets."$contextName") {
Write-Host "- Secret '$contextName' not found"
return $false
Expand Down Expand Up @@ -51,7 +52,6 @@ Get-Item -Path (Join-Path $ENV:GITHUB_WORKSPACE ".github/$($namePrefix)*.ps1") |
$deliveryTargets = @($deliveryTargets | Select-Object -unique)
if ($checkContextSecrets) {
# Check all delivery targets and include only the ones needed
$secrets = $env:Secrets | ConvertFrom-Json
$deliveryTargets = @($deliveryTargets | Where-Object { IncludeDeliveryTarget -deliveryTarget $_ })
}
$contextSecrets = @($deliveryTargets | ForEach-Object { "$($_)Context" })
Expand Down
2 changes: 1 addition & 1 deletion Actions/DetermineDeliveryTargets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Determines the delivery targets to use for the build
| Name | Description |
| :-- | :-- |
| Settings | env.Settings must be set by a prior call to the ReadSettings Action |
| Secrets | env.Secrets with delivery target context secrets must be read by a prior call to the ReadSecrets Action |
| Secrets | env.Secrets with delivery target context secrets must be read by a prior call to the ReadSecrets Action (if checkContextSecrets is set to Y) |

### Parameters
| Name | Required | Description | Default value |
Expand Down
1 change: 1 addition & 0 deletions Actions/DetermineDeploymentEnvironments/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Determines the environments to be used for a build or a publish
| Name | Description |
| :-- | :-- |
| Settings | env.Settings must be set by a prior call to the ReadSettings Action |
| GITHUB_TOKEN | GITHUB_TOKEN must be set as an environment variable when calling this action |

### Parameters
| Name | Required | Description | Default value |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Param(
function DownloadDependenciesFromProbingPaths($baseFolder, $project, $destinationPath) {
$settings = $env:Settings | ConvertFrom-Json | ConvertTo-HashTable -recurse
$settings = AnalyzeRepo -settings $settings -token $token -baseFolder $baseFolder -project $project -doNotCheckArtifactSetting -doNotIssueWarnings
$settings = CheckAppDependencyProbingPaths -settings $settings -token $token -baseFolder $baseFolder -project $project
if ($settings.ContainsKey('appDependencyProbingPaths') -and $settings.appDependencyProbingPaths) {
return Get-Dependencies -probingPathsJson $settings.appDependencyProbingPaths -saveToPath $destinationPath | Where-Object { $_ }
}
Expand Down
1 change: 1 addition & 0 deletions Actions/DownloadProjectDependencies/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The action constructs arrays of paths to .app files, that are dependencies of th
| Name | Description |
| :-- | :-- |
| Settings | env.Settings must be set by a prior call to the ReadSettings Action |
| Secrets | env.Secrets must be read by a prior call to the ReadSecrets Action with appDependencyProbingPathsSecrets in getSecrets |

### Parameters
| Name | Required | Description | Default value |
Expand Down
2 changes: 1 addition & 1 deletion Actions/IncrementVersionNumber/IncrementVersionNumber.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
2 changes: 1 addition & 1 deletion Actions/PipelineCleanup/PipelineCleanup.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ try {
TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
if (Get-Module BcContainerHelper) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
}
throw
Expand Down
4 changes: 3 additions & 1 deletion Actions/PullRequestStatusCheck/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ Check the status of a pull request build and fail the build if any jobs have fai
## INPUT

### ENV variables
GITHUB_TOKEN
| Name | Description |
| :-- | :-- |
| GITHUB_TOKEN | GITHUB_TOKEN must be set as an environment variable when calling this action |

### Parameters
| Name | Required | Description | Default value |
Expand Down
11 changes: 7 additions & 4 deletions Actions/ReadSecrets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ Additionally, the secrets specified by the authToken secret in AppDependencyProb
| :-- | :-: | :-- | :-- |
| shell | | The shell (powershell or pwsh) in which the PowerShell script in this action should run | powershell |
| parentTelemetryScopeJson | | Specifies the parent telemetry scope for the telemetry signal | {} |
| getSecrets | Yes | Comma separated list of secrets to get (add appDependencyProbingPathsSecrets to request secrets needed for resolving dependencies in AppDependencyProbingPaths) | |
| getSecrets | Yes | Comma separated list of secrets to get (add appDependencyProbingPathsSecrets to request secrets needed for resolving dependencies in AppDependencyProbingPaths, add TokenForPush in order to request a token to use for pull requests and commits) | |
| useGhTokenWorkflowForPush | false | Determines whether you want to use the GhTokenWorkflow secret for TokenForPush |

## OUTPUT

### ENV variables
none

### OUTPUT variables
| Name | Description |
| :-- | :-- |
| Secrets | A compressed json construct with all secrets base64 encoded. The secret value + the base64 value of the secret value are masked in the log |
| Secrets | A compressed json construct with all requested secrets base64 encoded. The secret value + the base64 value of the secret value are masked in the log |
| TokenForPush | The token to use when workflows are pushing changes (either directly, or via pull requests). This is either the GITHUB_TOKEN or the GhTokenWorkflow secret (based on the env variable useGhTokenWorkflowForPush) |

### OUTPUT variables
none
95 changes: 50 additions & 45 deletions Actions/ReadSecrets/ReadSecrets.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ Param(
[string] $gitHubSecrets = "",
[Parameter(HelpMessage = "Comma separated list of Secrets to get", Mandatory = $true)]
[string] $getSecrets = "",
[Parameter(HelpMessage = "Specifies the parent telemetry scope for the telemetry signal", Mandatory = $false)]
[string] $parentTelemetryScopeJson = '7b7d'
[Parameter(HelpMessage = "Determines whether you want to use the GhTokenWorkflow secret for TokenForPush", Mandatory = $false)]
[string] $useGhTokenWorkflowForPush = 'false'
)

$telemetryScope = $null

$buildMutexName = "AL-Go-ReadSecrets"
$buildMutex = New-Object System.Threading.Mutex($false, $buildMutexName)
try {
Expand All @@ -24,11 +22,6 @@ try {
}

. (Join-Path -Path $PSScriptRoot -ChildPath "..\AL-Go-Helper.ps1" -Resolve)
DownloadAndImportBcContainerHelper

Import-Module (Join-Path -path $PSScriptRoot -ChildPath "..\TelemetryHelper.psm1" -Resolve)
$telemetryScope = CreateScope -eventId 'DO0078' -parentTelemetryScopeJson $parentTelemetryScopeJson

Import-Module (Join-Path $PSScriptRoot ".\ReadSecretsHelper.psm1") -ArgumentList $gitHubSecrets

$outSecrets = [ordered]@{}
Expand All @@ -44,9 +37,15 @@ try {
}
}
$getAppDependencyProbingPathsSecrets = $false
$getTokenForPush = $false
[System.Collections.ArrayList]$secretsCollection = @()
$getSecrets.Split(',') | Select-Object -Unique | ForEach-Object {
$secret = $_
foreach($secret in ($getSecrets.Split(',') | Select-Object -Unique)) {
if ($secret -eq 'TokenForPush') {
$getTokenForPush = $true
if ($useGhTokenWorkflowForPush -ne 'true') { continue }
# If we are using the ghTokenWorkflow for commits, we need to get ghTokenWorkflow secret
$secret = 'ghTokenWorkflow'
}
$secretNameProperty = "$($secret)SecretName"
if ($secret -eq 'AppDependencyProbingPathsSecrets') {
$getAppDependencyProbingPathsSecrets = $true
Expand All @@ -63,78 +62,84 @@ try {

# Loop through appDependencyProbingPaths and add secrets to the collection of secrets to get
if ($getAppDependencyProbingPathsSecrets -and $settings.Keys -contains 'appDependencyProbingPaths') {
$settings.appDependencyProbingPaths | ForEach-Object {
if ($_.PsObject.Properties.name -eq "AuthTokenSecret") {
if ($secretsCollection -notcontains $_.authTokenSecret) {
$secretsCollection += $_.authTokenSecret
foreach($appDependencyProbingPath in $settings.appDependencyProbingPaths) {
if ($appDependencyProbingPath.PsObject.Properties.name -eq "AuthTokenSecret") {
if ($secretsCollection -notcontains $appDependencyProbingPath.authTokenSecret) {
$secretsCollection += $appDependencyProbingPath.authTokenSecret
}
}
}
}

@($secretsCollection) | ForEach-Object {
$secretSplit = $_.Split('=')
$envVar = $secretSplit[0]
$secret = $envVar
# Loop through secrets (use @() to allow us to remove items from the collection while looping)
foreach($secret in @($secretsCollection)) {
$secretSplit = $secret.Split('=')
$secretsProperty = $secretSplit[0]
$secretName = $secretsProperty
if ($secretSplit.Count -gt 1) {
$secret = $secretSplit[1]
$secretName = $secretSplit[1]
}

if ($secret) {
$value = GetSecret -secret $secret -keyVaultName $keyVaultName
if ($value) {
if ($secretName) {
$secretValue = GetSecret -secret $secretName -keyVaultName $keyVaultName
if ($secretValue) {
$json = @{}
try {
$json = $value | ConvertFrom-Json | ConvertTo-HashTable
$json = $secretValue | ConvertFrom-Json | ConvertTo-HashTable
}
catch {
}
if ($json.Keys.Count) {
if ($value.contains("`n")) {
throw "JSON Secret $secret contains line breaks. JSON Secrets should be compressed JSON (i.e. NOT contain any line breaks)."
if ($secretValue.contains("`n")) {
throw "JSON Secret $secretName contains line breaks. JSON Secrets should be compressed JSON (i.e. NOT contain any line breaks)."
}
$json.Keys | ForEach-Object {
if (@("Scopes","TenantId","BlobName","ContainerName","StorageAccountName") -notcontains $_) {
foreach($keyName in $json.Keys) {
if (@("Scopes","TenantId","BlobName","ContainerName","StorageAccountName") -notcontains $keyName) {
# Mask individual values (but not Scopes, TenantId, BlobName, ContainerName and StorageAccountName)
MaskValue -key "$($secret).$($_)" -value $json."$_"
MaskValue -key "$($secretName).$($keyName)" -value $json."$keyName"
}
}
}
$base64value = [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($value))
$outSecrets += @{ "$envVar" = $base64value }
Write-Host "$envVar successfully read from secret $secret"
$secretsCollection.Remove($_)
$base64value = [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($secretValue))
$outSecrets += @{ "$secretsProperty" = $base64value }
Write-Host "$secretsProperty successfully read from secret $secretName"
$secretsCollection.Remove($secret)
}
}
}

if ($secretsCollection) {
Write-Host "The following secrets was not found: $(($secretsCollection | ForEach-Object {
$unresolvedSecrets = ($secretsCollection | ForEach-Object {
$secretSplit = @($_.Split('='))
if ($secretSplit.Count -eq 1) {
$secretSplit[0]
}
else {
"$($secretSplit[0]) (Secret $($secretSplit[1]))"
}
$outSecrets += @{ ""$($secretSplit[0])"" = """" }
}) -join ', ')"
$outSecrets += @{ "$($secretSplit[0])" = "" }
}) -join ', '
Write-Host "The following secrets was not found: $unresolvedSecrets"
}

#region Action: Output

$outSecretsJson = $outSecrets | ConvertTo-Json -Compress
Add-Content -Encoding UTF8 -Path $env:GITHUB_ENV -Value "Secrets=$outSecretsJson"

#endregion
Add-Content -Encoding UTF8 -Path $env:GITHUB_OUTPUT -Value "Secrets=$outSecretsJson"

TrackTrace -telemetryScope $telemetryScope
}
catch {
if ($env:BcContainerHelperPath) {
TrackException -telemetryScope $telemetryScope -errorRecord $_
if ($getTokenForPush) {
if ($useGhTokenWorkflowForPush -eq 'true' -and $outSecrets.ghTokenWorkflow) {
Write-Host "Use ghTokenWorkflow for Push"
$ghToken = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String($outSecrets.ghTokenWorkflow))
}
else {
Write-Host "Use github_token for Push"
$ghToken = GetGithubSecret -SecretName 'github_token'
}
Add-Content -Encoding UTF8 -Path $env:GITHUB_OUTPUT -Value "TokenForPush=$ghToken"
}
throw

#endregion
}
finally {
$buildMutex.ReleaseMutex()
Expand Down
2 changes: 1 addition & 1 deletion Actions/ReadSecrets/ReadSecretsHelper.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function GetGithubSecret {
if ($secretSplit.Count -gt 1) {
$secret = $secretSplit[1]
}

if ($script:gitHubSecrets.PSObject.Properties.Name -eq $secret) {
$value = $script:githubSecrets."$secret"
if ($value) {
Expand Down
Loading

0 comments on commit 7d7c20d

Please sign in to comment.