Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step towards experimental group of settings in ADMX Policies #789

Merged
merged 12 commits into from
Jan 24, 2025

Conversation

AndrewDemski-ad-gmail-com
Copy link
Contributor

Proposed Changes

Given the ever-increasing number of WAU users, it would be beneficial to separate experimental settings from those already in production. The proposed pull request brings such a possibility.
I hope that this will be the foundation for future settings that will not conflict with those already tested in this way.
image

Related Issues

issue link
In the attached example, a text field has been added indicating the name of the non-default repository name that WAU could use when calling winget.exe sub-commands. The logic in PS1 scripts will be provided in the next commits.

Setting Enabled Setting disabled
image image

BR
AD

Updated ADML for new experimental group and one experimental setting.
Updated ADMX for new experimental group and one experimental setting.
@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

@KnifMelti
May I ask yo to take a look at the structure I think that it is following the schema for ADMX/ADML files, but I'd love to get a 2nd opinion. It is my first time with those files.

Thanks in advance.
AD

@KnifMelti
Copy link
Contributor

Looks like it's working, I'm no expert.
The thing I know is that revision="4.8" must be increased in both files for every new version to get the domain to import and overwrite the previous

Added $Script:WingetSourceCustom (with fallback to 'winget')
Type aware refactor
fixed void return from function
#region Log running context
#region Run Scope Machine function if run as System
last commit to that file today
removed duplicates
I lied.. now it is complete
Get-WingetOutdatedApps expects winget repo name and wont continue without it.
@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

AndrewDemski-ad-gmail-com commented Dec 8, 2024

Very well.
I'd like to leave that 4.8 for now.. Maybe something new pops up in the meantime.
.. E.g. separate winget repo for system and user contexts.. etc.. etc..

Trailing commits carry that custom winget repo name logic which relies on our new GP setting.

BR
AD
note for the future, do not set the name trollolo on your own computer or forget to change it.. :)

@AndrewDemski-ad-gmail-com AndrewDemski-ad-gmail-com changed the title Step towards experimental group in of settings in ADMX Policies Step towards experimental group of settings in ADMX Policies Dec 8, 2024
@Romanitho
Copy link
Owner

What about setting 'Winget' as the default source, which could be changed via settings or GPO as suggested here?

Second question: what happens when an experimental feature (and the corresponding GPO settings) is approved and moves to the root?

@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

Q1

What about setting 'Winget' as the default source, which could be changed via settings or GPO as suggested here?

A1

It is already set in that PR.
Entire logic is in expanded Winget-Upgrade.ps1 file

#region Winget Source Custom
    # Default name of winget repository used within this script
    [string]$DefaultWingetRepoName = 'winget';

    # Defining custom repository for winget tool (only if GPO management is active)
    if($Script:WAUConfig.WAU_ActivateGPOManagement) {
        if($null -eq $Script:WAUConfig.WAU_WingetSourceCustom) {
            [string]$Script:WingetSourceCustom = $DefaultWingetRepoName;
        } 
        else {
            [string]$Script:WingetSourceCustom = $Script:WAUConfig.WAU_WingetSourceCustom.Trim();
        }
        Write-ToLog "Selecting winget repository named '$($Script:WingetSourceCustom)'";
    }
#endregion Winget Source Custom

it calls Get-WingetOutdatedApps function which received a new param

       #Get outdated Winget packages
       Write-ToLog "Checking application updates on Winget Repository named '$($Script:WingetSourceCustom)' .." "yellow"
       $outdated = Get-WingetOutdatedApps -src $Script:WingetSourceCustom;

That function checks if repo name is not empty

#Function to get the outdated app list, in formatted array

function Get-WingetOutdatedApps {

Param(
    [Parameter(Position=0,Mandatory=$True,HelpMessage="You MUST supply value for winget repo, we need it")]
    [ValidateNotNullorEmpty()]
    [string]$src
)
# etc etc
    #Get list of available upgrades on winget format
    $upgradeResult = & $Winget upgrade --source $src | Where-Object { $_ -notlike "   *" } | Out-String
# etc etc    

Q2

Second question: what happens when an experimental feature (and the corresponding GPO settings) is approved and moves to the root?

A2

If by root you mean master branch, which will trigger all actions and new release will be deployed to world to be consumed by 6M unsuspecting souls?
.. Nothing
That expansion of the logic basically converts a static string into script:variable.
Experimental part of GPO settings is not colliding with anything currently present in latest release of WAU.

Your question proves one point, we need those experimental settings.

There is no way we will come up with a new mechanism which will be able to distinguish execution context and b able to recognize non-PROD environments.

Experimental features are easy to control and can be easily promoted to become a part of main product.

If everything is tested enough, then it is just a matter of readdressing a particular GPO option to show outside of experimental node.

I forgot about the second function missing commit 1of2
I forgot about the second function missing commit 2of2
@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

Last two commits (3584e24 and 050d176) prove that I am prone to writing incomplete code, i missed the winget word when it was used as constant.
Now everything should be parameterized.

.. i think
AD

P.S. I hope that there will be no need to fix anything else.
P.P.S I'll be gone for holidays till the end of 2024. this PR - if not merged - could be parked till 2025 Q1.

@Romanitho
Copy link
Owner

Regarding my Q1, I was thinking of setting the Winget source as a (default) registry value. This way, it would be automatically overwritten by the GPO setting without requiring any extra work or new function. :)

@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

AndrewDemski-ad-gmail-com commented Dec 17, 2024

We are hitting two birds with one stone now:

  1. custom repo name defined in one place and referenced in Winget-Upgrade.ps1 (with possibility of making that global default value in the future OFC)
  2. experimental settings for the future (interesting for more advanced application of WAU in bigger companies with more complex processes and ecosystem)

IMHO it is worth it.

@Romanitho
Copy link
Owner

It's time to merge this part, I guess. A lot of rework has been done, and probably another task is coming: cleaning the whole code. ^^

@Romanitho Romanitho merged commit dbffc74 into Romanitho:main Jan 24, 2025
1 check passed
#If multiple versions, pick most recent one
$WingetCmd = $WingetInfo[-1].FileName
$WingetCmd = $WingetInfo.FileName;
}
catch {
Copy link
Contributor

@KnifMelti KnifMelti Jan 25, 2025

Choose a reason for hiding this comment

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

When the user part is executed it doesn't enter the catch thereby making $WingetCmd empty...
...and 17:52:56 - Checking application updates on Winget Repository named '' .. it looks like it doesn't use a Repo...

image

Copy link
Contributor

Choose a reason for hiding this comment

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

...nope it doesn't use a repo at all; not finding the usually skipped ones:

16:51:49 - Epic Games Launcher : Skipped upgrade because it is in the excluded app list
16:51:49 - Windows Assessment and Deployment Kit - Windows 10 : Skipped upgrade because it is in the excluded app list
16:51:49 - Ubisoft Connect : Skipped upgrade because it is in the excluded app list
16:51:49 - Mozilla Firefox (x64 en-GB) : Skipped upgrade because it is *wildcard* in the excluded app list
16:51:49 - MSTeams : Skipped upgrade because it is *wildcard* in the excluded app list

Copy link
Contributor

Choose a reason for hiding this comment

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

@AndrewDemski-ad-gmail-com @Romanitho
Maybe inserting -ErrorAction Break in the Admin context check is enough for the problem with executing the user part (works in my small test)?:

#Get Admin Context Winget Location
$WingetInfo = (Get-Item -Path $ps -ErrorAction Break).VersionInfo | Sort-Object -Property FileVersionRaw -Descending | Select-Object -First 1;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only thing that comes to my mind is that these two commands behave differently whether 'Path' argument is used or not:

  • (Get-Item -Path $ps)
  • (Get-Item $ps)

..which would be weird, because that was the only change in that section.

But you are right @KnifMelti, The try block should be started above those two strings are defined and -EA should be specified

AndrewDemski-ad-gmail-com added a commit to AndrewDemski-ad-gmail-com/Winget-AutoUpdate-latestTag that referenced this pull request Jan 26, 2025
Missing failsafe when winget does not exist in system context (trigger for fallback to instance in the user context).
Addendum to logic changed in PR Romanitho#789
@AndrewDemski-ad-gmail-com
Copy link
Contributor Author

There.. #817
Now we should be safe.

@KnifMelti
Copy link
Contributor

Hoiw about the missing repo causing it to not find anything?
17:52:56 - Checking application updates on Winget Repository named '' ..

@KnifMelti
Copy link
Contributor

Hoiw about the missing repo causing it to not find anything? 17:52:56 - Checking application updates on Winget Repository named '' ..

#823

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants