diff --git a/.ci/unit-tests/puppet-chef.sh b/.ci/unit-tests/puppet-chef.sh index 8e5a0d4002fa..0999932e847b 100755 --- a/.ci/unit-tests/puppet-chef.sh +++ b/.ci/unit-tests/puppet-chef.sh @@ -10,11 +10,20 @@ if [ $PROVIDER = "chef" ]; then # Re-enable chef tests by deleting this if block once the tests are fixed. echo "Skipping tests... See issue #236" elif [ -z "$EXCLUDE_PATTERN" ]; then - DISABLE_COVERAGE=true bundle exec parallel_rspec spec/ + # The unit tests are all under e.g. spec/gcompute* - the integration tests + # are better run using `bundle exec rake spec` instead of + # `bundle exec parallel_rspec`, but if you just let it run the default + # set of specs, parallel_rspec will try to run the integration tests + # in addition. This is just running all the tests that aren't excluded - + # or, in the event of an empty exclude list, all the tests with the right + # names. + if ls spec/g$PRODUCT* > /dev/null 2&>1; then + DISABLE_COVERAGE=true bundle exec parallel_rspec spec/g$PRODUCT* + fi else # parallel_rspec doesn't support --exclude_pattern IFS="," read -ra excluded <<< "$EXCLUDE_PATTERN" - filtered=$(find spec -name '*_spec.rb' $(printf "! -wholename %s " ${excluded[@]})) + filtered=$(find spec -name "g$PRODUCT*_spec.rb" $(printf "! -wholename %s " ${excluded[@]})) DISABLE_COVERAGE=true bundle exec parallel_rspec ${filtered[@]} fi diff --git a/provider/puppet.rb b/provider/puppet.rb index 828f1ed7a4cd..471812a05e82 100644 --- a/provider/puppet.rb +++ b/provider/puppet.rb @@ -190,6 +190,22 @@ def compile_examples(output_folder) "products/#{@api.prefix[1..-1]}/examples/puppet/#{file}"] end ) + + # Compile the spec tests which depend on the examples. + # If a create and delete example both exist, generate + # the spec file. Select all the objects which have + # both create and delete examples, named the standard way. + @api.objects.each do |obj| + ex = @config.examples[obj.name] + next if ex.nil? + o = Google::StringUtils.underscore(obj.name) + # We can only generate the spec file if both examples exist. + next unless ex.include?("#{o}.pp") && ex.include?("delete_#{o}.pp") + compile_file_list(output_folder, + [["spec/integration/#{o}/integration_spec.rb", + 'templates/puppet/integration_spec.rb']], + obj: obj) + end end def compile_end2end_tests(output_folder) diff --git a/provider/puppet/common~compile~before.yaml b/provider/puppet/common~compile~before.yaml index af0eff7edb59..71efbbe9c639 100644 --- a/provider/puppet/common~compile~before.yaml +++ b/provider/puppet/common~compile~before.yaml @@ -17,6 +17,7 @@ '.gitignore': 'templates/dot~gitignore' '.rubocop.yml': 'templates/dot~rubocop~root.yml' 'Gemfile': 'templates/puppet/Gemfile' +'.fixtures.yml': 'templates/puppet/fixtures.yml.erb' 'README.md': 'templates/puppet/README.md.erb' 'metadata.json': 'templates/puppet/metadata.json.erb' 'spec/.rubocop.yml': 'templates/dot~rubocop~spec.yml' @@ -25,3 +26,4 @@ 'spec/puppetlint_spec.rb': 'templates/puppet/puppetlint_spec.rb.erb' 'spec/spec_helper.rb': 'templates/puppet/spec_helper.rb.erb' 'spec/test_constants.rb': 'templates/test_constants.rb.erb' +'spec/fixtures/mock_hc_fn.pp': 'templates/puppet/mock_healthcheck_fn.pp' diff --git a/provider/puppet/common~copy.yaml b/provider/puppet/common~copy.yaml index ad4facb9b511..7b2b534ae6ce 100644 --- a/provider/puppet/common~copy.yaml +++ b/provider/puppet/common~copy.yaml @@ -26,6 +26,7 @@ end <% end # unless config.nil? -%> 'LICENSE': 'templates/LICENSE' 'Gemfile.lock': 'templates/puppet/Gemfile.lock' +'Rakefile' : 'templates/puppet/Rakefile' 'lib/google/hash_utils.rb': 'google/hash_utils.rb' 'lib/google/string_utils.rb': 'google/string_utils.rb' 'spec/copyright.rb': 'spec/copyright.rb' diff --git a/templates/puppet/Gemfile b/templates/puppet/Gemfile index 1dfcafa9df48..db58282a05c9 100644 --- a/templates/puppet/Gemfile +++ b/templates/puppet/Gemfile @@ -18,7 +18,10 @@ source 'https://rubygems.org' group :test do + gem 'google-api-client' + gem 'googleauth' gem 'metadata-json-lint' + gem 'parallel_tests' gem 'puppet', ENV['PUPPET_GEM_VERSION'] || '>= 4.2.0' gem 'puppet-lint' gem 'puppet-lint-unquoted_string-check' @@ -28,7 +31,9 @@ group :test do gem 'rspec' gem 'rspec-mocks' gem 'rspec-puppet' - gem 'parallel_tests' gem 'rubocop' + gem 'semantic_puppet' gem 'simplecov' + gem 'vcr' + gem 'webmock' end diff --git a/templates/puppet/Gemfile.lock b/templates/puppet/Gemfile.lock index eb424232b060..f64cad47fb36 100644 --- a/templates/puppet/Gemfile.lock +++ b/templates/puppet/Gemfile.lock @@ -4,9 +4,15 @@ GEM addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) ast (2.4.0) + crack (0.4.3) + safe_yaml (~> 1.0.0) + declarative (0.0.10) + declarative-option (0.1.0) diff-lcs (1.3) docile (1.1.5) facter (2.5.1) + faraday (0.15.2) + multipart-post (>= 1.2, < 3) fast_gettext (1.1.2) gettext (3.2.6) locale (>= 2.0.5) @@ -15,17 +21,42 @@ GEM fast_gettext (~> 1.1.0) gettext (>= 3.0.2) locale + google-api-client (0.23.8) + addressable (~> 2.5, >= 2.5.1) + googleauth (>= 0.5, < 0.7.0) + httpclient (>= 2.8.1, < 3.0) + mime-types (~> 3.0) + representable (~> 3.0) + retriable (>= 2.0, < 4.0) + signet (~> 0.9) + googleauth (0.6.6) + faraday (~> 0.12) + jwt (>= 1.4, < 3.0) + memoist (~> 0.12) + multi_json (~> 1.11) + os (>= 0.9, < 2.0) + signet (~> 0.7) + hashdiff (0.3.7) hiera (3.4.2) + httpclient (2.8.3) json (2.1.0) json-schema (2.8.0) addressable (>= 2.4) + jwt (2.1.0) locale (2.1.2) + memoist (0.16.0) metaclass (0.0.4) metadata-json-lint (2.0.2) json-schema (~> 2.8) spdx-licenses (~> 1.0) + mime-types (3.2.2) + mime-types-data (~> 3.2015) + mime-types-data (3.2018.0812) mocha (1.3.0) metaclass (~> 0.0.1) + multi_json (1.13.1) + multipart-post (2.0.0) + os (1.0.0) parallel (1.12.1) parallel_tests (2.21.3) parallel @@ -50,6 +81,11 @@ GEM rspec-puppet (~> 2.0) rainbow (3.0.0) rake (10.5.0) + representable (3.0.4) + declarative (< 0.1.0) + declarative-option (< 0.2.0) + uber (< 0.2.0) + retriable (3.1.2) rspec (3.7.0) rspec-core (~> 3.7.0) rspec-expectations (~> 3.7.0) @@ -73,6 +109,13 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.9.0) + safe_yaml (1.0.4) + semantic_puppet (1.0.2) + signet (0.9.1) + addressable (~> 2.3) + faraday (~> 0.9) + jwt (>= 1.5, < 3.0) + multi_json (~> 1.10) simplecov (0.15.1) docile (~> 1.1.0) json (>= 1.8, < 3) @@ -80,12 +123,20 @@ GEM simplecov-html (0.10.2) spdx-licenses (1.1.0) text (1.3.1) + uber (0.1.0) unicode-display_width (1.3.0) + vcr (4.0.0) + webmock (3.4.2) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff PLATFORMS ruby DEPENDENCIES + google-api-client + googleauth metadata-json-lint parallel_tests puppet (>= 4.2.0) @@ -98,7 +149,10 @@ DEPENDENCIES rspec-mocks rspec-puppet rubocop + semantic_puppet simplecov + vcr + webmock BUNDLED WITH 1.16.1 diff --git a/templates/puppet/Rakefile b/templates/puppet/Rakefile new file mode 100644 index 000000000000..cd3d37995894 --- /dev/null +++ b/templates/puppet/Rakefile @@ -0,0 +1 @@ +require 'puppetlabs_spec_helper/rake_tasks' diff --git a/templates/puppet/fixtures.yml.erb b/templates/puppet/fixtures.yml.erb new file mode 100644 index 000000000000..afeb935d6df7 --- /dev/null +++ b/templates/puppet/fixtures.yml.erb @@ -0,0 +1,24 @@ +--- +<% if false # the license inside this if block pertains to this file -%> +# Copyright 2017 Google Inc. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +<% end -%> +<%= compile 'templates/license.erb' -%> + +<%= lines(autogen_notice :ruby) -%> + +fixtures: + forge_modules: + gauth: google/gauth + symlinks: + <%= @api.prefix -%>: "#{source_dir}" diff --git a/templates/puppet/integration_spec.rb b/templates/puppet/integration_spec.rb new file mode 100644 index 000000000000..c3b95b4a587d --- /dev/null +++ b/templates/puppet/integration_spec.rb @@ -0,0 +1,130 @@ +<% if false # the license inside this if block pertains to this file -%> +# Copyright 2017 Google Inc. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +<% end -%> +<%# +This file is complicated, and deserves some documentation here, so that +you, brave adventurer, can understand what's happened. This file's +goal is to run the examples required by puppet compilation. Essentially, +you can think of it as a shell script that runs 'puppet apply examples/foo.pp', +then 'puppet apply examples/delete_foo.pp', and checks for errors. + +It has to be more complicated than that. :) +"Fiddling with puppet internal APIs is a lonely and painful path to take." +- David Schmitt, Puppet SWE, September 2018 + +First, let's talk about the reason that it *isn't* a shell script. +We need unit tests that really, really validate our puppet modules - we +can't have them breaking on us, because the community isn't active enough +that we can trust that we'll find out through regular channels. We +absolutely must have tests that we can count on to tell us if the module +is working as expected. And those tests need to run on every PR, because +these modules are complicated enough that they can break in subtle ways. +Consequently, we need to be able to run tests that validate use against the +real GCP APIs, but without spending incredible amounts of time (or money) +or actually creating hordes of resources. That means a VCR test. + +A VCR test is a kind of test that runs once, making real HTTP requests along +the way. It records the requests and responses. On later runs, it mocks +those same HTTP requests with those same responses, making your tests +repeatable (desirable), fast (desirable), and in our case, also free! + +You can't write a test like that using the puppet command line tool, so +instead we plumb into the guts of puppet. Our main hook is a function called +apply_compiled_manifest (and its sibling, apply_with_error_check). + +These functions are not really very similar to `puppet apply`. Instead of +taking a filename, they take the entire manifest as a string. This causes +subtle problems deep within Puppet - Puppet depends on the filename of the +manifest in a handful of ways, not least of which is to find out what +module it is part of (to automagically load dependencies and functions). +Since that isn't possible, we tack on the Puppet override of our own +environment and loader. Since these functions don't call out to Facter +the way that `puppet apply` does, we need to substitute in our own project +name and credential path (which all our examples use). Hence the +get_example() function - it does more than just read the file. + +You can see all this stuff in spec_helper.rb.erb. + +There's also the issue of the begin/rescue pattern you'll see here. It's +ugly! It's also necessary. compile_to_ral() calls out to our mocked up +loader, but fails to successfully compile the manifest due to some +initialization which is mandatory, but isn't possible to complete from +outside puppet. HOWEVER! We're in luck! In calling some cleanup code, +in reporting on the failure, compile_to_ral actually *performs* the +initialization we need. So, if we try it twice, it works the second +time. Lucky. + +So! We *can* successfully apply a manifest from +under webmock, as long as we: +- try twice, eating the first failure. +- don't need to use any functions that are loaded from modules +- ensure all our dependent modules have their 'lib' dir in $LOAD_PATH +- do all our own variable substitution +- rewrite any necessary functions in puppet and stick them in + the manifest. +- import all the magic from puppetlabs_spec_helper/module_spec_helper, + which I have done without fully understanding it. The comments in + the code suggest that the authors also do not fully understand it. + +Hopefully that explains the complexity here in a way that will continue +to be useful as the tests develop further. + +Let me also explain the order in which these commands are run. First, +there is a pre-destroy - in the event that an older test failed to clean +up after itself, the next run should take care of it. Second, we create +the resources in the manifest. Third, we run the create again, mocking +out any requests during 'flush' and making them crash the test - this is +to check idempotency of the manifests. Fourth, we delete the resources +in the manifest, and fifth, we check the idempotency of that delete by +confirming that no resources make web requests during 'flush'. + +VCR recordings are called 'cassettes', and they are stored in +'spec/cassettes'. If present, they are used - if absent, created. +#-%> +<%= compile 'templates/license.erb' -%> + +<%= lines(autogen_notice :puppet) -%> +require 'spec_helper' +require 'vcr' + +VCR.configure do |c| + c.cassette_library_dir = 'spec/cassettes' + c.hook_into :webmock + c.configure_rspec_metadata! +end + +describe '<%= Google::StringUtils.underscore(obj.name) -%>.create', vcr: true do + it 'creates and destroys non-existent <%= Google::StringUtils.underscore(obj.name) -%>' do + puts 'pre-destroying <%= Google::StringUtils.underscore(obj.name) -%>' + VCR.use_cassette('pre_destroy_<%= Google::StringUtils.underscore(obj.name) -%>') do + run_example('delete_<%= Google::StringUtils.underscore(obj.name) -%>') + end + puts 'creating <%= Google::StringUtils.underscore(obj.name) -%>' + VCR.use_cassette('create_<%= Google::StringUtils.underscore(obj.name) -%>') do + run_example('<%= Google::StringUtils.underscore(obj.name) -%>') + end + puts 'checking that <%= Google::StringUtils.underscore(obj.name) -%> is created' + VCR.use_cassette('check_<%= Google::StringUtils.underscore(obj.name) -%>') do + validate_no_flush_calls('<%= Google::StringUtils.underscore(obj.name) -%>') + end + puts 'destroying <%= Google::StringUtils.underscore(obj.name) -%>' + VCR.use_cassette('destroy_<%= Google::StringUtils.underscore(obj.name) -%>') do + run_example('delete_<%= Google::StringUtils.underscore(obj.name) -%>') + end + puts 'confirming <%= Google::StringUtils.underscore(obj.name) -%> destroyed' + VCR.use_cassette('check_destroy_<%= Google::StringUtils.underscore(obj.name) -%>') do + validate_no_flush_calls('delete_<%= Google::StringUtils.underscore(obj.name) -%>') + end + end +end diff --git a/templates/puppet/mock_healthcheck_fn.pp b/templates/puppet/mock_healthcheck_fn.pp new file mode 100644 index 000000000000..11567b140539 --- /dev/null +++ b/templates/puppet/mock_healthcheck_fn.pp @@ -0,0 +1,21 @@ +<% if false # the license inside this if block assertains to this file -%> +# Copyright 2017 Google Inc. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +<% end -%> +<%= compile 'templates/license.erb' -%> + +<%= lines(autogen_notice :ruby) -%> + +function gcompute_health_check_ref(String $name, String $project_name) >> String { + "https://www.googleapis.com/compute/v1/projects/$project_name/global/healthChecks/$name" +} diff --git a/templates/puppet/provider_spec.erb b/templates/puppet/provider_spec.erb index 9cdd14af73f2..0e636fd165cd 100644 --- a/templates/puppet/provider_spec.erb +++ b/templates/puppet/provider_spec.erb @@ -17,6 +17,11 @@ <%= lines(autogen_notice :puppet) -%> require 'spec_helper' +require 'network_blocker' +require 'spec/bundle.rb' +require 'spec/copyright.rb' +require 'spec/fake_auth.rb' +require 'spec/test_constants.rb' <% if object.readonly diff --git a/templates/puppet/spec_helper.rb.erb b/templates/puppet/spec_helper.rb.erb index 555ea74096ac..8be3a68474a7 100644 --- a/templates/puppet/spec_helper.rb.erb +++ b/templates/puppet/spec_helper.rb.erb @@ -24,42 +24,21 @@ ENV['TZ'] = 'UTC' -#---------------------------------------------------------- -# Setup code coverage - -require 'simplecov' -SimpleCov.start unless ENV['DISABLE_COVERAGE'] - #---------------------------------------------------------- # Add test path to the search libs $LOAD_PATH.unshift(File.expand_path('.')) <% if spec_stubs? -%> $LOAD_PATH.unshift(File.expand_path('./spec/stubs')) +# Add fixtures so that they can be loaded by integration tests. +$LOAD_PATH.unshift(File.expand_path('./spec/fixtures/modules/gauth/lib')) +$LOAD_PATH.unshift(File.expand_path('./spec/fixtures/modules/<%= @api.prefix -%>/lib')) <% end # spec_stubs? -%> #---------------------------------------------------------- # Block all network traffic - -require 'network_blocker' - -#---------------------------------------------------------- -# Auto require files - -files = [] -files << 'spec/bundle.rb' -files << 'spec/copyright.rb' -files << 'spec/fake_auth.rb' -files << 'spec/test_constants.rb' -files << File.join('lib', '**', '*.rb') - -# Require all files so we can track them via code coverage -Dir[*files].reject { |p| File.directory? p } - .each do |f| - puts "Auto requiring #{f}" \ - if ENV['RSPEC_DEBUG'] - require f - end +require 'puppetlabs_spec_helper/module_spec_helper' +require 'webmock/rspec' #---------------------------------------------------------- # Setup PuppetSpec to allow executing the Puppet manifests from within tests @@ -90,6 +69,10 @@ RSpec.configure do |c| Puppet::Test::TestHelper.after_all_tests end + c.mock_with :rspec do |conf| + conf.syntax = [:should, :expect] + end + c.before :each do base = PuppetSpec::Files.tmpdir('tmp_settings') Puppet[:vardir] = File.join(base, 'var') @@ -106,11 +89,79 @@ RSpec.configure do |c| c.after :each do Puppet::Test::TestHelper.after_each_test - Dir.stub(:entries) PuppetSpec::Files.cleanup end end +def get_example(example_name) + # This is a little nutty, but we actually put a mocked up + # version of the ruby function for healthchecks at the top + # of the manifest. Any other function which gets used in an + # example is going to need the same treatment. This *IS* + # limiting, not everything that can be expressed in a ruby + # function can be expressed in a puppet function - but see comment + # in integration_spec.rb. We haven't figured out a better way yet. + ex = read_example(example_name) + healthcheck_fn = File.open('spec/fixtures/mock_hc_fn.pp').read + healthcheck_fn + "\n\n" + ex +end + +def read_example(example_name) + # It's not ideal, but puppet unit tests don't support + # these facts-as-variables, so we have to do these + # substitutions ourselves. + File.open("examples/#{example_name}.pp", 'rb').read\ + .gsub('$project', "\"#{ENV['GOOGLE_PROJECT']}\"")\ + .gsub('$cred_path', "\"#{ENV['GOOGLE_CREDENTIALS_PATH']}\"") +end + +def run_example(example_name) + # This function runs an example specified by name (not + # including '.pp'. It generates a puppet environment, + # tricks puppet into feeling semi-initialized by + # compiling the example, catching the failure, then + # compiling it again within the context of the failure, + # and then applies it. + mods = [File.expand_path('.'), File.expand_path('./spec/fixtures/modules/')] + example = get_example(example_name) + test = Puppet::Node::Environment.create(:test, mods) + loaders = Puppet::Pops::Loaders.new(test, true) + Puppet.override(current_environment: test, loaders: loaders) do + begin + compile_to_ral(example) + rescue + apply_with_error_check(example) + end + end +end + +def validate_no_flush_calls(example_name) + # This function is very much like run_example, above, except + # that it also validates that no POST calls are made to a + # google API during the 'flush' step (i.e. 'update if + # necessary' step). + mods = [File.expand_path('.'), File.expand_path('./spec/fixtures/modules/')] + example = get_example(example_name) + test = Puppet::Node::Environment.create(:test, mods) + loaders = Puppet::Pops::Loaders.new(test, true) + Puppet.override(current_environment: test, loaders: loaders) do + begin + compile_to_ral(example) + rescue + apply_compiled_manifest(example) do |res| + if res.provider&.respond_to? 'flush' + # Any request to Google APIs during a flush is not + # acceptable - that means that a diff was detected. + omnistub = stub_request(:any, /google/) + .to_raise("Shouldn't have made network call.") + res.provider.flush + remove_request_stub(omnistub) + end + end + end + end +end + require 'mocha/test_unit' #----------------------------------------------------------