Change to fix issue 144, check for presence of specifed ca_file (#1) #145
Conversation
This file contains 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
* Added ca file check * Updated message * Fixed broken end line * with puts * Added spec test * Updated messages * Fixes for rubocop * Fixes for rspec * Remove white space * Updated documentation
alexjfisher
reviewed
Mar 7, 2022
|
Less important, but does the fix also need to be made to the unregister function? |
|
Yeah, I was just running a few tests then had to run out.Will complete the merge back to my master branch in a few hours.
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Alexander Fisher ***@***.***>
Sent: Monday, March 7, 2022 10:57:23 PM
To: voxpupuli/puppet-gitlab_ci_runner ***@***.***>
Cc: benjamin-robertson ***@***.***>; Author ***@***.***>
Subject: Re: [voxpupuli/puppet-gitlab_ci_runner] Change to fix issue 144, check for presence of specifed ca_file (#1) (PR #145)
@alexjfisher commented on this pull request.
________________________________
In lib/puppet/functions/gitlab_ci_runner/register_to_file.rb<#145 (comment)>:
@@ -41,6 +41,11 @@ def register_to_file(url, regtoken, runner_name, additional_options = {}, proxy
return 'DUMMY-NOOP-TOKEN' if Puppet.settings[:noop]
begin
+ # Confirm the specified ca file exists
+ if !ca_file.nil? && !File.exist?(ca_file)
+ puts('Specified CA file doesn\'t exist for gitlab-ci-runner. Did you forget to create it?')
Should probably use Puppet.warning here instead of puts?
The message itself might be better hinting at possibly needing to run Puppet again???
Unable to register gitlab runner at this time as the specified `ca_file` does not exist (yet). If puppet is managing this file, the next run should complete the registration process.
???
—
Reply to this email directly, view it on GitHub<#145 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ARJBKQKTLNWBRJBRCTM2FFLU6XVKHANCNFSM5QCKM3EA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
* Added ca file check * Updated message * Fixed broken end line * with puts * Added spec test * Updated messages * Fixes for rubocop * Fixes for rspec * Remove white space * Updated documentation * Changed to Puppet warning * Update warning message * Updates to rspec * Added rspec for unregister function * Updated unregister message
|
I've added the same check to the unregister. It prevents the errors, and unregisters the runner locally. However does not update the gitlab server since the cert wasn't present. User is provided with a warning as such. |
|
thanks for the PR! I rebased it in #159 and will merge it afterwards. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Fixes issue 144 by adding a check to confirm if the ca_file set in the parameters actually exists on the machine.
This problem causes Puppet runs to completely fail when using a Gitlab server with an internally signed CA. Making it impossible to install the ca file with Puppet when the gitlab_ci_runner module is in use.
Added rspec test to check this new condition.
Tests passing in github actions (with the exception of the Ubuntu 18.04 on Puppet 6 which was already failing.).
Docker build . test passing (to the same standard as the existing master branch). Output provided below.
I'm not sure if we should output a message to the user. See register_to_file.rb line 46. Or it should be let to fail silently. Thoughts?
This Pull Request (PR) fixes the following issues
Fixes #144