-
Notifications
You must be signed in to change notification settings - Fork 70
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
Installer updates #656
Installer updates #656
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't review in detail, but looks good. Please consider adding documentation updates to the README and Wiki, where appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this PR @day1player! 😄
In case you haven't noticed it, the linter is not happy 😬
# Check if the app is provisioned | ||
$provisionedPackage = Get-AppxProvisionedPackage -Online | Where-Object { $_.DisplayName -eq $appName } -ErrorAction SilentlyContinue | ||
if ($provisionedPackage) { | ||
$result = Remove-AppxProvisionedPackage -PackageName $provisionedPackage.PackageName -Online |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did, and I think I fixed what I could! The remaining errors are that the installer and debloater dont have a category (they shouldnt) and that the erroractionpreference in the installer isnt set to stop (I copied from the other installer) |
For things that doesn't make sense to fix, you need to exclude the package in the linter as we do for example here: This way we know in future PR what things need to be tested for what package 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. I think we just need to fix the linter (and ensure the rest of the CI succeeds) and we are good to go! 👍
Can you please squash the commits (to several small commits with related changes removing the half-way work or just a big one) in oder to keep a clean commit history without fix/updates commits? Alternatively, you can use squash and merge when merging that will combine the PR int a single commit.
Done! I think we just need the PR approved and I can merge it. |
The tests fail. I think we want to exclude the installer from the tests as we do with the current installer: https://github.com/mandiant/VM-Packages/blob/main/scripts/test/test_install.ps1#L42 It is failing because it doesn't find the config file. But the real issue with testing the installer is that it installs other packages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes plz, then should be gtg
} | ||
} | ||
|
||
# Process the services |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Please remember to use squash and merge 😄
`TOOL_LIST_SHORTCUT` has been removed in: mandiant/VM-Packages#656 Install the tools shortcuts directly into `%UserProfile%\Desktop\Tools`.
`TOOL_LIST_SHORTCUT` has been removed in: mandiant/VM-Packages#656 Install the tools shortcuts directly into `%UserProfile%\Desktop\Tools`.
`TOOL_LIST_SHORTCUT` has been removed in: mandiant/VM-Packages#656. We removed it from the config in: mandiant#492 But we didn't remove it from the GUI.
CommandoVM refactored the functions and configuration files to a separate repository for improved modularity and reusability, i.e. https://github.com/mandiant/VM-Packages/. See Also - mandiant/VM-Packages#656 - https://github.com/mandiant/VM-Packages/tree/main/packages/common.vm/ - https://github.com/mandiant/VM-Packages/tree/main/packages/debloat.vm/
Created
debloater.vm
packagecommon.vm
from recent Commando release:VM-Get-WindowsVersion
function tocommon.vm
to identify Win10, Win11, or Win11ARM installationsCreated
installer.vm
packagefailed_packages.txt
to$Env:VM_COMMON_DIR
readme.md
and the background settingconfig.xml
(previously holding package and variables) topackages.xml
to avoid confusion with the new custom configs that can be supportedpackages.xml
file AND aconfig.xml
file into$Env:VM_COMMON_DIR
installer.vm
now expects packages to be listed in thepackages.xml
fileinstaller.vm
now expects OS configurations to be listed in theconfig.xml
file.ico
file (expected in$Env:VM_COMMON_DIR\vm.ico
)$Env:MandiantVM
to exist (explained below)Tools
folder (using the custom.ico
file above)log.txt
for cleaner exit of installationGet-InstalledPackages
function tocommon.vm
Changes/additions to
common.vm
VM-Configure-Prompts
function to add "Mandiant VM" and timestamps to PowerShell and cmd$Env:MandiantVM
to existVM-Configure-PS-Logging
functionTools
folder to actually be on the desktopRAW_TOOLS_DIR
as that is different.. This folder is just the folder with all the shortcuts.ico
file for the folder for ultimate flexingTOOL_LIST_SHORTCUT
because this variable is not referenced by any of our packages or install scripts. This function is also what used to create the shortcut on the desktop, and now that the Tools folder lives on the desktop (Not theRAW_TOOLS_DIR
, just the folder that hosts all the other shortcuts) we dont need the shortcut