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

Windows settings update #93

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

Gijsreyn
Copy link
Contributor

@Gijsreyn Gijsreyn commented Nov 4, 2024


Addresses the Microsoft.Windows.Setting.WindowsUpdate on #34

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Nov 4, 2024

@denelon Hello, I thought giving a shot on the Windows Update settings. Althought I came pretty far and I left the PR as draft, there are some oddities I need help with. Hopefully you (or others) can help me out:

  1. When setting the delivery optimization settings through the UI, additional registry keys are created:

image

I couldn't find any relevant documentation if these keys are actually required, or if the keys that are being set now, are sufficient.

  1. The delivery optimization settings require elevation to be read
  2. I attempted to mimic the work of Stephen in the test. Unfortunately, Pester only creates a hive in HKCU. All settings that we attempt to set, are done in HKLM or HKU.

@denelon
Copy link
Contributor

denelon commented Nov 4, 2024

@crutkas and I have been working on some of the challenges with Windows update for his setup. There are some new ACL changes associated with some registry keys in newer versions of Windows, and we've been working on the "right" way to do this.

I don't know if you've seen the experimental "configureSelfElevate" feature in WinGet, but if you decorate the configuration file appropriately, you'll get a single UAC when running a configuration with one or more resources requiring elevation. That doesn't really help with the challenges here, but it's worth mentioning.

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Nov 4, 2024

That's pretty cool Demitrius. If you have any insights already what keys require to be targeted, I can include them already if the build version is higher than ...

I'm learning every day, thanks for sharing. If you have any links to the experimental features, I will check it out :)

Little off topic. I like the setup what I'm seeing. I was working with DevHome. While I like the GUI, you can guess by now that I also love to automate as much as possible. My setup is a bit different compared to the one from Clint. Because I opted to use the DevHome Hyper-V option, I tried to automate it with DSC resources. It's a combination between the HyperVDsc module, dsc.exe, winget.exe, and some PowerShell code.

When attempting to set it up, I found myself creating a bunch of PowerShell script already. I really wanted to add it either on the DevHome repository, or here demonstrating a combination between multiple orchestrations tools. That brings me already to the point you've already mentioned already during your PSConfEU 2024 presentation. There are still pain points to fully automate it.

To sum it up some of the notes I took:

  • The Windows 11 elevation image that is provisioned by DevHome doesn't set a user password
    • I'm downloading it simple with Invoke-WebRequest. Then I attempt to run Invoke-Command, which unfortunately cannot be done. The -Credential option doesn't allow for a blank password (pity). I have to login manually, reset the password, and kick off the process
  • PSResourceGet ships with PowerShell 7.2+. This fully removes the dependency on PowerShellGet. The only challenge, it doesn't have any DSC resources available to install PS modules.

@denelon
Copy link
Contributor

denelon commented Nov 4, 2024

Experimental features in WinGet can be found using winget features. If you run winget features --help or winget features -? you'll get the link to the help document at GitHub.

Yeah, there are still plenty of rough edges leading you to use the Script resource. We're trying to find the examples where that's necessary so we can reason through gaps in implementation or missing DSC Resources.

Secrets are always a challenge when using Configuration as Code. Lots of products default with no password or a well-known password, and it's a bit tricky to ensure they aren't present in configuration files. This is definitely an area we will need to explore further to come up with some best-practices and model them.

I'll reach out to the PowerShell team to see what they are thinking in regard to PSResourceGet and installing modules.

@denelon
Copy link
Contributor

denelon commented Nov 4, 2024

Related to:

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Nov 5, 2024

Demitrius, can I put this PR on review to get some assistance on writing unit tests, and get an alpha release available somewhere?

@denelon
Copy link
Contributor

denelon commented Nov 5, 2024

You bet :)

@crutkas
Copy link
Member

crutkas commented Nov 5, 2024

Can’t wait to try this out!

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Nov 5, 2024

@ryfu-msft Hey champ, can you take a look with your magic eyes and hopefully give me some assistance in writing Pester tests?

@Gijsreyn Gijsreyn marked this pull request as ready for review November 5, 2024 06:21
@Gijsreyn Gijsreyn mentioned this pull request Nov 5, 2024
2 tasks

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@denelon
Copy link
Contributor

denelon commented Nov 14, 2024

It looks like there are conflicts in this PR now. If you're still working on it, it might be best to convert it to a draft, but if you're ready for review, keep it open after you fix the conflicts :)

@denelon denelon requested a review from ryfu-msft November 14, 2024 17:52

This comment has been minimized.

This comment has been minimized.

@Gijsreyn
Copy link
Contributor Author

Oops. I made a mistake. It should be fixed now. There are also tests added now and while adding the test, I found two caveats, which were quite interesting. If there are any suggestions on it to do it differently, then please let me know.

Originally I designed the validation checking within a function. This was replaced after the suggestion of Kaleb: 7ed2c35. Here came the challenge: when performing testing, when the validation is not within the range of of zero, it cannot grab the original state. That's why I added an additional check in when that particalur property is set. Second challenge was the fact that when specifying either Pct/Bps, it throws an error. Now I'm simply removing the parameters. I'm going to be honest here, there is some rework in here which I totally miscalculate. If either set is provided as properties, it should remove the others, otherwise both settings don't work.

Because the pull request is already quite large, I hope it can be pulled in. I'll fix this one later and add it as TODO on #104 . If not, please let me know.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Dec 2, 2024

Hey @ryfu-msft . I was wondering if you could take a look at this one after my last remark and either approve or let me make some changes. Please let me know.

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.

4 participants