Skip to content
Open
48 changes: 42 additions & 6 deletions Actions/.Modules/ReadSettings.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,16 @@ $CustomTemplateProjectSettingsFile = Join-Path '.github' $CustomTemplateProjectS
function MergeCustomObjectIntoOrderedDictionary {
Param(
[System.Collections.Specialized.OrderedDictionary] $dst,
[PSCustomObject] $src
[PSCustomObject] $src,
[string[]] $importantSettingsFromHigherPriority = @(),
[ref]$importantSettingsAtThisLevel
)

# Initialize importantSettingsAtThisLevel if passed
if ($null -eq $importantSettingsAtThisLevel) {
$importantSettingsAtThisLevel = [ref]@()
}

# If the src object contains property 'overwriteSettings' (list of settings), remove these settings from the dst object, so that they can be re-added with the new value later on
if ($src.PSObject.Properties.Name -contains "overwriteSettings") {
$src.overwriteSettings | ForEach-Object {
Expand All @@ -31,14 +38,22 @@ function MergeCustomObjectIntoOrderedDictionary {
}
}

# Collect important settings from this level
# Only set if importantSettings is explicitly defined; otherwise set to $null to signal no update
if ($src.PSObject.Properties.Name -contains "importantSettings") {
$importantSettingsAtThisLevel.Value = @($src.importantSettings)
} else {
$importantSettingsAtThisLevel.Value = $null
}

# Loop through all properties in the source object
# If the property does not exist in the destination object, add it with the right type, but no value
# Types supported: PSCustomObject, Object[] and simple types
$src.PSObject.Properties.GetEnumerator() | ForEach-Object {
$prop = $_.Name

# Skip overwriteSettings property as it's only used to remove settings from the destination object and is specific to the source object
if ($prop -eq "overwriteSettings") {
# Skip overwriteSettings and importantSettings properties as they're only used for configuration, not actual settings
if ($prop -eq "overwriteSettings" -or $prop -eq "importantSettings") {
return
}

Expand All @@ -62,15 +77,22 @@ function MergeCustomObjectIntoOrderedDictionary {
# If the property exists in the source object, but is of a different type, throw an error
# If the property exists in the source object:
# If the property is an Object, call this function recursively to merge values
# If the property is an Object[], merge the arrays
# If the property is an Object[], merge the arrays (even if important - arrays always merge)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not intuitive, why would arrays always merge even the setting in the source is marked as important?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure about that either, but I couldn't think of a better solution.

I think it's important for arrays that the “important” values are preserved. For example, with the buildModes, the “important” ones should always be executed, but also it should be possible to add others.

If I were to change the behavior here so that they aren’t always merged, that would mean that whenever I want to add buildModes, I’d also have to set overwriteSettings: ["buildModes"] and include the earlier-level values in the overwrite. I think this is a bit clunky, since, in my opinion, “overwrite” should only be used to remove values from earlier levels.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the biggest problem with this is that some settings are dependent of each other - shell, runs-on, useCompilerFolder, etc. and it becomes very hard to understand why a setting, which I explicitly set in my project now suddenly doesn't work because somebody has added a higher level importantSettings.

I would rather have a setup where a higher level could specify importantSettings - but it didn't affect the merge of settings, but instead - it would give an error when the setting was overridden lower down - then people can fix this - meaning a non-intrusive behavior really.

Another thing we could implement was a dump after readsettings, where it would print out specifically which settings where read from which files:

artifact - taken from project setting xxxx
runs-on - taken from .github/AL-Go-settings.json
.....

That would help people setup the right settings.
Adding overwrite and important settings seems to only complicate things, sorry.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Freddy Kristiansen (@freddydk) Your comment isn't about how to handle arrays, but is more of a general point, right?

it would give an error when the setting was overridden lower down

If we look at Use Case 2 from the PR description, this would be disadvantageous. If I have to define the country in the repo again for the build mode, I might as well skip entering it in the organization settings altogether. Furthermore, I think that anyone who is allowed to edit earlier settings (org + repo) should have the foresight and sense of responsibility to be able to override the repo settings.

Another thing we could implement was a dump after readsettings, where it would print out specifically which settings where read from which files:

artifact - taken from project setting xxxx
runs-on - taken from .github/AL-Go-settings.json

i could do that, Maria Zhelezova (@mazhelez) should i?
print always to the logs or just with ::debug:: annotation?

# If the property is a simple type, replace the value in the destination object with the value from the source object
@($dst.Keys) | ForEach-Object {
$prop = $_

if ($src.PSObject.Properties.Name -eq $prop) {
$dstProp = $dst."$prop"
$srcProp = $src."$prop"
$dstPropType = $dstProp.GetType().Name
$srcPropType = $srcProp.GetType().Name

# For non-array properties: skip if this setting is marked as important from higher priority source
if ($importantSettingsFromHigherPriority -contains $prop -and $srcPropType -ne "Object[]") {
OutputDebug "Skipping important setting '$prop' marked from higher priority source (non-array type)"
return
}
Comment thread
ChrisBlankDe marked this conversation as resolved.
if ($srcPropType -eq "PSCustomObject" -and $dstPropType -eq "OrderedDictionary") {
MergeCustomObjectIntoOrderedDictionary -dst $dst."$prop" -src $srcProp
}
Expand Down Expand Up @@ -262,6 +284,7 @@ function GetDefaultSettings
"filesToExclude" = @()
}
"postponeProjectInBuildOrder" = $false
"importantSettings" = @()
}
}

Expand Down Expand Up @@ -482,11 +505,18 @@ function ReadSettings {
}
}

$currentImportantSettings = @()
foreach($settingsObject in $settingsObjects) {
$settingsJson = $settingsObject.Settings
if ($settingsJson) {
OutputDebug "Applying settings from $($settingsObject.Source) ($($settingsObject.Type))"
MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson
$importantAtThisLevel = @()
$importantRef = [ref]$importantAtThisLevel
MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson -importantSettingsFromHigherPriority $currentImportantSettings -importantSettingsAtThisLevel $importantRef
# Only update currentImportantSettings if this level explicitly defined importantSettings (not null)
if ($null -ne $importantAtThisLevel) {
$currentImportantSettings = $importantAtThisLevel
}
Comment thread
ChrisBlankDe marked this conversation as resolved.
Outdated
Comment thread
ChrisBlankDe marked this conversation as resolved.
if ($settingsJson.PSObject.Properties.Name -eq "ConditionalSettings") {
foreach($conditionalSetting in $settingsJson.ConditionalSettings) {
if ("$conditionalSetting" -ne "") {
Expand All @@ -508,7 +538,13 @@ function ReadSettings {
}
if ($conditionMet) {
OutputDebug "Applying conditional settings for $($conditions -join ", ")"
MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings
$importantAtThisLevel = @()
$importantRef = [ref]$importantAtThisLevel
MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings -importantSettingsFromHigherPriority $currentImportantSettings -importantSettingsAtThisLevel $importantRef
# Only update currentImportantSettings if conditional settings explicitly defined importantSettings
if ($null -ne $importantAtThisLevel) {
$currentImportantSettings = $importantAtThisLevel
}
Comment thread
ChrisBlankDe marked this conversation as resolved.
Outdated
Comment thread
ChrisBlankDe marked this conversation as resolved.
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions Actions/.Modules/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,14 @@
},
"description": "An array of settings to be overwritten by the current settings. See https://aka.ms/ALGoSettings#overwriteSettings"
},
"importantSettings": {
"type": "array",
"items": {
"type": "string"
},
"default": [],
"description": "An array of setting names that should take precedence over normal inheritance hierarchy. Settings listed here will override settings from lower priority sources. See https://aka.ms/ALGoSettings#importantSettings"
Comment thread
ChrisBlankDe marked this conversation as resolved.
Outdated
},
"reportSuppressedDiagnostics": {
"type": "boolean",
"description": "Report suppressed diagnostics. See https://aka.ms/ALGoSettings#reportsuppresseddiagnostics"
Expand Down
34 changes: 34 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
### Important settings protection
Comment thread
mazhelez marked this conversation as resolved.
Comment thread
ChrisBlankDe marked this conversation as resolved.

A new `importantSettings` setting allows you to protect specific settings from being overridden by lower-priority sources in the settings hierarchy. When a setting is marked as important at a higher priority level, it cannot be overridden by non-important settings from lower priority sources.
Comment thread
ChrisBlankDe marked this conversation as resolved.

```json
{
"importantSettings": ["country", "keyVaultName"],
"country": "de",
"keyVaultName": "orgVault"
}
```

**Behavior:**
- Settings marked as important in organization or repository settings cannot be overridden by project, workflow, user, or environment settings
- Important settings from higher priority sources always win, even if lower levels mark the same setting as important
- Important arrays are still merged (not replaced) with lower-priority arrays, unless `overwriteSettings` is explicitly used
- The `overwriteSettings` mechanism can override important settings when explicitly specified
- `ConditionalSettings` respect importantSettings markings, allowing you to enforce conditional important settings based on buildMode, branch, trigger, or user

**Example with ConditionalSettings:**
```json
{
"ConditionalSettings": [
{
"buildModes": ["Validate"],
"settings": {
"importantSettings": ["country"],
"country": "us"
}
}
]
}
```

### Resilient Pull Request Status Check for large builds

The Pull Request Status Check action no longer fails on builds with more than one page of jobs (more than 100 jobs). The jobs API call now uses `--slurp` so multi-page responses are parsed as a single JSON array (previously `gh api --paginate | ConvertFrom-Json` failed with "Invalid JSON primitive" when more than one page was returned). The call is also retried, and requests a smaller page size, to tolerate the intermittent HTTP 502 responses that the GitHub jobs endpoint returns for large builds.
Expand Down
77 changes: 77 additions & 0 deletions Scenarios/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,83 @@ then, after merging, the result settings object will contain the following value

> _**Note**_: `overwriteSettings` isn't a setting on its own and it isn't available in the output of `ReadSetting` action, for example. It's merely used to control the settings merging mechanism and allow overwriting complex settings types. The value of `overwriteSettings` should only contain settings of types _array_ or _object_ and all the settings in `overwriteSettings` should be present with the new value.

## Important settings <a id="importantSettings"></a>

By default, AL-Go follows a standard settings hierarchy where settings from higher priority levels (closer to deployment) override settings from lower priority levels. However, you can mark specific settings as **important** to protect them from being overridden by lower priority settings using the `importantSettings` array.

When a setting is marked as important at a higher level in the hierarchy, it cannot be overridden by settings from lower priority levels. This is useful for enforcing organizational or repository-wide policies that should not be changed at the project or workflow level.
Comment thread
ChrisBlankDe marked this conversation as resolved.
Outdated

_Example_:
Say, `ALGoOrgSettings` (organization level) contains the following values:

```json
{
"importantSettings": ["country", "keyVaultName"],
"country": "de",
"keyVaultName": "OrgVault"
}
```

and `.AL-Go\settings.json` (project level, lower priority) contains the following values:

```json
{
"country": "us",
"keyVaultName": "ProjectVault"
}
```

then, after merging, the result settings object will contain the following values:

```json
{
"importantSettings": ["country", "keyVaultName"],
"country": "de",
"keyVaultName": "OrgVault"
}
```

The `country` and `keyVaultName` settings from the organization level are protected and cannot be overridden by the project level settings.

_Example with ConditionalSettings_:
Say, `ALGoOrgSettings` (organization level) contains conditional settings that set country based on buildMode:

```json
{
"ConditionalSettings": [
{
"buildModes": ["ValidateUS"],
"settings": {
"importantSettings": ["country"],
"country": "us"
}
}
]
}
```

and `.AL-Go\settings.json` (project level) contains:

```json
{
"country": "w1",
"buildModes": ["Default", "ValidateUS"]
}
```

When reading settings for buildMode `ValidateUS`, the conditional setting from the organization level will apply. The result will be:

```json
{
"country": "us",
"buildModes": ["Default", "ValidateUS"]
}
```

Even though the project specifies `country: "w1"`, the conditional setting from the organization level marked the country as important for the `ValidateUS` buildMode, so it takes precedence.

> _**Note**_: `importantSettings` is an array of setting names that should be protected from being overridden by lower priority settings. When a setting is marked as important at a higher level, it will not be overridden by normal (non-important) settings from lower levels. If a setting is marked as important at multiple levels, the value from the highest priority level wins (following normal hierarchy rules). Only top-level setting names can be marked as important; nested properties within complex objects cannot be individually marked as important. Array settings marked as important are still merged with lower-priority arrays, unless `overwriteSettings` is used to force replacement.

<a id="customdelivery"></a>

## Custom Delivery
Expand Down
Loading