saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

[BUG] shortcut.present not idempotent, icon_location still broken #64190

Open darkpixel opened 1 year ago

darkpixel commented 1 year ago

file.shortcut was broken (see #53706) The bug was closed as fixed with PR #62025

I migrated stuff to use the new shortcut.present and it's failing badly.

Simple test state:

test_user_shortcut:
  shortcut.present:
    - name: 'C:\Users\someuser\Desktop\test_user.url'
    - target: 'https://google.com/'
    - icon_location: 'c:\test.ico'

test_public_shortcut:
  shortcut.present:
    - name: 'C:\Users\public\Desktop\test_public.url'
    - target: 'https://yahoo.com/'
    - icon_location: 'c:\test.ico'

To run it, stick a valid icon at c:\test.ico and adjust the c:\users\someuser path to point to your account. Run the state.

The icons are created, but the icon isn't applied.

Notice the salt output that shows things like the icon target being C:\Program Files\Salt Project\Salt\https:\yahoo.com and yet if you right-click the icon and go to 'properties' it points to https://yahoo.com and works properly. Also notice the salt output strips one of the forward slashes from https://.

If you manually set the icon on the 'user' shortcut, it works just fine. If you try to manually set the icon on the 'public' shortcut, it errors out with: "Cannot apply changes to this Internet Shortcut".

Running the state again produces an error: Failed to create the shortcut: C:\Users\public\Desktop\test_public.url Found existing shortcut

It shouldn't be an error that the shortcut exists, just like...say....file.managed doesn't error out on the state application because it created the file during the first run.

darkpixel commented 1 year ago

Issues with file.shortcut are also mentioned in #61170

darkpixel commented 1 year ago

I think the comment in this line is probably wrong: https://github.com/saltstack/salt/blob/cab551c6977e79dc1c2593f590ff4be86474f482/salt/modules/win_shortcut.py#L206

# A shortcut can have either a .lnk or a .url extension. We only want to
# expand the target if it is a .lnk

The code doesn't set an icon if the link is a URL. Windows totally allows that.

darkpixel commented 1 year ago

Good lord...WSH is terrible.

It's easier to just do file.managed and splat out a template like this:

[{000214A0-0000-0000-C000-000000000046}]
Prop3=19,3
[InternetShortcut]
IDList=
URL={{ target }}
IconIndex={{ icon_index }}
IconFile={{ icon_location }}

No need to mess with a ton of WSH calls that don't appear to match the functionality of what Explorer can do.