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

Adding a chocolatey installer #14043

Merged
merged 8 commits into from Oct 30, 2023
Merged

Conversation

johnmccrae
Copy link
Contributor

Description

This PR replaces #13603. That PR branch was borked.

This PR comes from a customer request - https://chefio.atlassian.net/browse/INFC-354
We want to provide a way, consistent with other resources, that allows for the admin to install Choco if it is not present.

The main problem here is that we have a number of resources for chocolatey users but customers attempt to use them without having choco installed already and they get weird errors. We did not correctly set customer expectations with the Choco resources that we did with other package managers. The others will typically offer and option to install that package manager if it's missing. Until now, we have not done that with Choco. This improvement corrects that by adding an installer resource for Chocolatey and improves error output to direct the customer to use the installer. We do NOT install Chocolatey for the user as that would deviate from Chef design philosophies around being absolutely transparent about or explicit about what we're doing - meaning we don't perform some action that the customer has not explicitly chosen to do : We don't install the package manager if it's missing, we inform the customer and offer the new resource to fix that issue.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

Signed-off-by: John <john.mccrae@progress.com>
@johnmccrae johnmccrae requested review from a team as code owners October 20, 2023 19:05
@github-actions github-actions bot added the documentation How do we use this project? label Oct 20, 2023
Signed-off-by: John <john.mccrae@progress.com>
Copy link
Contributor

@tpowell-progress tpowell-progress left a comment

Choose a reason for hiding this comment

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

Verify in chef-oss and all GH Actions green.

lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
description: "The URL to download Chocolatey from. This defaults to the value of $env:ChocolateyDownloadUrl, if it is set, and otherwise falls back to the official Chocolatey community repository to download the Chocolatey package. It can be used for offline installation by providing a path to a Chocolatey.nupkg."

property :chocolatey_version, String,
description: "Specifies a target version of Chocolatey to install. By default, the latest stable version is installed. This will use the value in $env:ChocolateyVersion by default, if that environment variable is present. This parameter is ignored if download_url is set."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

Copy link
Contributor

Choose a reason for hiding this comment

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

Per our discussion, this is actually being used directly by the Chocolatey install.ps1 script and the environment variable can be used to redirect the standard installer to an alternate version.

lib/chef/resource/chocolatey_installer.rb Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
lib/chef/resource/chocolatey_installer.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@tpowell-progress tpowell-progress left a comment

Choose a reason for hiding this comment

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

Logic simplification

Signed-off-by: John <john.mccrae@progress.com>
Signed-off-by: John <john.mccrae@progress.com>
Signed-off-by: John <john.mccrae@progress.com>
Signed-off-by: John <john.mccrae@progress.com>
Copy link
Contributor

@tpowell-progress tpowell-progress left a comment

Choose a reason for hiding this comment

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

Re-approving after talking through the env var concerns on the Community PR review call.

Signed-off-by: John <john.mccrae@progress.com>
Signed-off-by: John <john.mccrae@progress.com>
@sonarcloud
Copy link

sonarcloud bot commented Oct 30, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johnmccrae johnmccrae merged commit 7c3cb46 into main Oct 30, 2023
31 checks passed
@johnmccrae johnmccrae deleted the chef18_choco_ensure_installed_4 branch October 30, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation How do we use this project?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants