Change to fix issue 144, check for presence of specifed ca_file (#1)

benjamin-robertson commented 2 years ago

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

------                                                                                                                                                       
 > [8/9] RUN bundle exec rake release_checks:                                                                                                                
#12 1.020 ruby -c lib/puppet/functions/gitlab_ci_runner/unregister_from_file.rb                                                                              
#12 1.049 Syntax OK                                                                                                                                          
#12 1.050 ruby -c lib/puppet/functions/gitlab_ci_runner/register.rb                                                                                          
#12 1.078 Syntax OK                                                                                                                                          
#12 1.079 ruby -c lib/puppet/functions/gitlab_ci_runner/unregister.rb
#12 1.105 Syntax OK
#12 1.106 ruby -c lib/puppet/functions/gitlab_ci_runner/to_toml.rb
#12 1.131 Syntax OK
#12 1.132 ruby -c lib/puppet/functions/gitlab_ci_runner/register_to_file.rb
#12 1.159 Syntax OK
#12 1.160 ruby -c lib/puppet_x/gitlab/dumper.rb
#12 1.186 Syntax OK
#12 1.187 ruby -c lib/puppet_x/gitlab/runner.rb
#12 1.212 Syntax OK
#12 1.213 ---> syntax:manifests
#12 1.361 ---> syntax:templates
#12 1.388 ---> syntax:hiera:yaml
#12 1.406 I, [2022-03-07T06:11:38.471937 #8]  INFO -- : Creating symlink from spec/fixtures/modules/gitlab_ci_runner to /opt/puppet
#12 1.411 Cloning into 'spec/fixtures/modules/stdlib'...
#12 1.413 Cloning into 'spec/fixtures/modules/ruby_task_helper'...
#12 1.416 Cloning into 'spec/fixtures/modules/apt'...
#12 1.419 Cloning into 'spec/fixtures/modules/concat'...
#12 1.422 Cloning into 'spec/fixtures/modules/docker'...
#12 1.425 Cloning into 'spec/fixtures/modules/translate'...
#12 1.428 Cloning into 'spec/fixtures/modules/yumrepo_core'...
#12 3.003 4 processes for 10 specs, ~ 2 specs per process
#12 4.128 ..
#12 4.141 An error occurred while loading ./spec/tasks/unregister_runner_spec.rb. - Did you mean?
#12 4.141                     rspec ./spec/tasks/register_runner_spec.rb
#12 4.141 
#12 4.141 Failure/Error: require_relative '../../tasks/unregister_runner'
#12 4.141 
#12 4.141 LoadError:
#12 4.141   cannot load such file -- /opt/ruby_task_helper/files/task_helper
#12 4.141 # ./tasks/unregister_runner.rb:5:in `require_relative'
#12 4.141 # ./tasks/unregister_runner.rb:5:in `<top (required)>'
#12 4.141 # ./spec/tasks/unregister_runner_spec.rb:4:in `require_relative'
#12 4.141 # ./spec/tasks/unregister_runner_spec.rb:4:in `<top (required)>'
#12 4.141 
#12 4.141 
#12 4.141 Finished in 0.00002 seconds (files took 0.97614 seconds to load)
#12 4.141 0 examples, 0 failures, 1 error occurred outside of examples
#12 4.141 
#12 4.146 .......Specified CA file doesn't exist for gitlab-ci-runner. Did you forget to create it?
#12 4.212 .............................
#12 4.458 
#12 4.458 Finished in 0.38674 seconds (files took 0.91178 seconds to load)
#12 4.458 38 examples, 0 failures
#12 4.458 
#12 7.580 
#12 7.580 An error occurred while loading ./spec/tasks/register_runner_spec.rb. - Did you mean?
#12 7.580                     rspec ./spec/tasks/unregister_runner_spec.rb
#12 7.580 
#12 7.580 Failure/Error: require_relative '../../tasks/register_runner'
#12 7.580 
#12 7.580 LoadError:
#12 7.580   cannot load such file -- /opt/ruby_task_helper/files/task_helper
#12 7.580 # ./tasks/register_runner.rb:5:in `require_relative'
#12 7.580 # ./tasks/register_runner.rb:5:in `<top (required)>'
#12 7.580 # ./spec/tasks/register_runner_spec.rb:4:in `require_relative'
#12 7.580 # ./spec/tasks/register_runner_spec.rb:4:in `<top (required)>'
#12 7.580 
#12 7.580 
#12 7.580 Finished in 0.00002 seconds (files took 4.42 seconds to load)
#12 7.580 0 examples, 0 failures, 1 error occurred outside of examples
#12 7.580 
#12 7.840 .................................................................................................................................................................................................................................................................................................................................................................................................
#12 21.23 
#12 21.23 Coverage Report:
#12 21.23 
#12 21.23 Total resources:   21
#12 21.23 Touched resources: 16
#12 21.23 Resource coverage: 76.19%
#12 21.23 
#12 21.23 Untouched resources:
#12 21.23   Concat::Fragment[/etc/gitlab-runner/config.toml - runner_with_ensure_present]
#12 21.23   Concat::Fragment[/etc/gitlab-runner/config.toml - test_runner]
#12 21.23   File[/etc/gitlab-runner/auth-token-runner_with_ensure_absent]
#12 21.23   Package[xz-utils]
#12 21.23   Package[xz]
#12 21.23 
#12 21.23 
#12 21.23 Finished in 13.73 seconds (files took 4.34 seconds to load)
#12 21.23 385 examples, 0 failures
#12 21.23 
#12 21.25 Tests Failed
#12 21.26 
#12 21.26 2 errors, 423 examples, 0 failures
#12 21.26 
#12 21.26 Took 18 seconds

alexjfisher commented 2 years ago

Less important, but does the fix also need to be made to the unregister function?

benjamin-robertson commented 2 years ago

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 iOShttps://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.rbhttps://github.com/voxpupuli/puppet-gitlab_ci_runner/pull/145#discussion_r820636204:

@@ -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 GitHubhttps://github.com/voxpupuli/puppet-gitlab_ci_runner/pull/145#pullrequestreview-901592032, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ARJBKQKTLNWBRJBRCTM2FFLU6XVKHANCNFSM5QCKM3EA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://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: @.***>

benjamin-robertson commented 2 years ago

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.

bastelfreak commented 1 year ago

thanks for the PR! I rebased it in https://github.com/voxpupuli/puppet-gitlab_ci_runner/pull/159 and will merge it afterwards.

voxpupuli / puppet-gitlab_ci_runner

Change to fix issue 144, check for presence of specifed ca_file (#1) #145

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Confirm the specified ca file exists