wip#1
Open
CallumBanbery wants to merge 3 commits into
Open
Conversation
CallumBanbery
force-pushed
the
bug/install
branch
19 times, most recently
from
2a7feb9 to
ee93550
Compare
As referenced in voxpupuli#240 the official installation method was changed in December 2020, deprecating the use of the `certbot-auto` script, and recommending `snap` as the primary package source. As `snap` isn't yet a builtin package provider in Puppet and I don't fancy creating a dependency on a third party module, nor do I want to add a load of `exec` commands for managing snap packages I chose to fix the VCS install method, and since it's used in the CI there's less to change. Since we can no longer use the `certbot-auto` script we should follow the setup instructions provided by Letsencrypt's development guide https://certbot.eff.org/docs/contributing.html. Their guide simply states to call a python script which configures the virtual environment, and since the script is gone we no longer need the VENV_PATH environment variable. As the command needs to create a python virtual environment we need to add the required package to the VCS dependencies, and from testing I found there was no need to call the initialisation command when installing from a package source, so I've made it conditional to the VCS installation. I've also taken the opportunity to update the version of `certbot` that's checked out during VCS installation since `0.39.0` is now around 1.5 years old. Version `1.7.0` is the latest version that is supported by Python 3.5 which is the lowest Python3 version that's installed by default via package managers.
CallumBanbery
force-pushed
the
bug/install
branch
from
ee93550 to
a316156
Compare
Since the dependencies array is used to install packages it makes sense to ensure a package name has been provided, an empty string would be redundant as it would mean no package is downloaded.
CallumBanbery
force-pushed
the
bug/install
branch
8 times, most recently
from
7c0eca9 to
8536adc
Compare
CallumBanbery
force-pushed
the
bug/install
branch
10 times, most recently
from
724d903 to
12c8f0f
Compare
Since the letsencrpyt initialisation is only required when installing from VCS it stands to reason to limit the resource declaration based on the value of the parameter.
CallumBanbery
force-pushed
the
bug/install
branch
from
12c8f0f to
dc8999f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request (PR) description
This Pull Request (PR) fixes the following issues