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

win_package - fix productId check before getting remote files #480

Merged
merged 5 commits into from
Mar 26, 2023

Conversation

AzadShM
Copy link
Contributor

@AzadShM AzadShM commented Mar 15, 2023

win_package should not download/copy $path before checking the installation status if product_id is set

SUMMARY

Based on the win_package documentation:
"When state=present, product_id is not set and the path is a URL, this file will always be downloaded to a temporary directory for idempotency checks, otherwise the file will only be downloaded if the package has not been installed based on the product_id checks."

But win_package always downloads/copies the file to a temporary location even if product_id is set and the package is installed

The productId is not being correctly checked in win_package.ps1 this commit fixes that. fixes #479

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

win_package

ADDITIONAL INFORMATION

win_package should not download/copy $path before checking the installation status if product_id is set
@jborean93
Copy link
Collaborator

Thanks for the PR, unfortunately with the changes here the code will skip downloading the msi into a temp path and when the package is not installed it will fail as it tries to run it from the URI directly instead of the temp file. The code will have to be updated to ensures this scenario continues working.

@AzadShM
Copy link
Contributor Author

AzadShM commented Mar 20, 2023

That is not correct. The code will check the installation status and use the id for that win_package.ps1#L1453. When Get-InstalledStatus does not find the ID and the code is not in CheckMode the package/msi will be downloaded to a temp path win_package.ps1#L1461 and later installed.

@jborean93
Copy link
Collaborator

The CI tests are failing because of what I've mentioned. I did a brief test locally with the changes in this PR and saw the same error. This problem needs to be fixed and CI passing before we can look at merging the fix here.

@AzadShM
Copy link
Contributor Author

AzadShM commented Mar 20, 2023

I now see the problem you are mentioning and it is not because the file is not being downloaded and the URI is being used directly. The problem is that the wrong provider is being used to install the package if the package is not installed and provider is set to auto.
I will check it out and send another commit to fix that.

do an early installation status check if the package is a remote file and productid is set
and make sure to use the correct provider if provider is auto and the package is not installed
@AzadShM
Copy link
Contributor Author

AzadShM commented Mar 21, 2023

The code now runs Get-InstalledStatus twice if state is present, the package is a remote file, productid is set and the package was found not to be installed. The first run is used to avoid downloading/copying the file to a temp folder if it is already installed and the second run is used to determine the correct provider to use for the installation.

Caveats:

  • If productid is set to a wrong value the package is always downloaded/copied to a temp path.
  • If path is set to a network target, productid is set and state is absent the package is always downloaded/copied to a temp path.
  • The product_id parameter documentation says
    "[...] This value is ignored if path is set to a local accesible file path and the package is not an exe. [...]"
    The product_id is being ignored in most of the cases and not only in the mentioned one.

These caveats are not related to these commits, existed before and are out of scope for this bugfix.

@jborean93 jborean93 merged commit b41b4d7 into ansible-collections:main Mar 26, 2023
@AzadShM AzadShM deleted the patch-1 branch March 28, 2023 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants