(PDK-773) Implementation of PDK::Module::Update

lib/pdk/cli/update.rb

@@ -23,7 +23,15 @@ module PDK::CLI
         raise PDK::CLI::ExitWithError, _('You can not specify --noop and --force when updating a module')
       end
 
-      PDK::Module::Update.invoke(opts)
+      updater = PDK::Module::Update.new(opts)
+
+      if updater.current_version == updater.new_version
+        PDK.logger.info _('This module is already up to date with version %{version} of the template.') % {
+          version: updater.new_version,
+        }
+      else
+        updater.run
+      end
     end
   end
 end

lib/pdk/module/convert.rb

@@ -62,6 +62,10 @@ def force?
         options[:force]
       end
 
+      def needs_bundle_update?
+        update_manager.changed?('Gemfile')
+      end
+
       def stage_changes!
         PDK::Module::TemplateDir.new(template_url, nil, false) do |templates|
           new_metadata = update_metadata('metadata.json', templates.metadata)

lib/pdk/module/update.rb

@@ -1,8 +1,98 @@
+require 'pdk/module/convert'
+
 module PDK
   module Module
-    class Update
-      def self.invoke(_opts = {})
-        # TODO: do some updatey things
+    class Update < Convert
+      GIT_DESCRIBE_PATTERN = %r{\A(?<base>.+?)-(?<additional_commits>\d+)-g(?<sha>.+)\Z}
+
+      def run
+        stage_changes!
+
+        PDK.logger.info(update_message)
+
+        print_summary
+        full_report('update_report.txt') unless update_manager.changes[:modified].empty?
+
+        return if noop?
+
+        unless force?
+          message = _('Do you want to continue and make these changes to your module?')
+          return unless PDK::CLI::Util.prompt_for_yes(message)
+        end
+
+        if needs_bundle_update?
+          update_manager.remove_file('Gemfile.lock')
+          update_manager.remove_file(File.join('.bundle', 'config'))
+        end
+
+        update_manager.sync_changes!
+
+        PDK::Util::Bundler.ensure_bundle! if needs_bundle_update?
+
+        print_result 'Update completed'
+      end
+
+      def module_metadata
+        @module_metadata ||= PDK::Module::Metadata.from_file('metadata.json')
+      rescue ArgumentError => e
+        raise PDK::CLI::ExitWithError, e.message
+      end
+
+      def template_url
+        @template_url ||= module_metadata.data['template-url']
+      end
+
+      def current_version
+        @current_version ||= describe_ref_to_s(current_template_version)
+      end
+
+      def new_version
+        @new_version ||= fetch_remote_version(new_template_version)
+      end
+
+      private
+
+      def current_template_version
+        @current_template_version ||= module_metadata.data['template-ref']
+      end
+
+      def describe_ref_to_s(describe_ref)
+        data = GIT_DESCRIBE_PATTERN.match(describe_ref)
+
+        return data if data.nil?
+
+        if data[:base].start_with?('heads/')
+          "#{data[:base].gsub(%r{^heads/}, '')}@#{data[:sha]}"
+        else
+          data[:base]
+        end
+      end
+
+      def new_template_version
+        PDK::Util.default_template_ref
+      end
+
+      def fetch_remote_version(version)
+        return version unless version.include?('/')
+
+        branch = version.partition('/').last
+        sha_length = GIT_DESCRIBE_PATTERN.match(current_template_version)[:sha].length - 1
+        "#{branch}@#{PDK::Util::Git.ls_remote(template_url, "refs/heads/#{branch}")[0..sha_length]}"
+      end
+
+      def update_message
+        format_string = if template_url == PDK::Util.puppetlabs_template_url
+                          _('Updating %{module_name} using the default template, from %{current_version} to %{new_version}')
+                        else
+                          _('Updating %{module_name} using the template at %{template_url}, from %{current_version} to %{new_version}')
+                        end
+
+        format_string % {
+          module_name:     module_metadata.data['name'],
+          template_url:    template_url,
+          current_version: current_version,
+          new_version:     new_version,
+        }
       end
     end
   end

lib/pdk/util/git.rb

@@ -37,6 +37,21 @@ def self.repo_exists?(repo, ref = nil)
 
         git(*args)[:exit_code].zero?
       end
+
+      def self.ls_remote(repo, ref)
+        output = git('ls-remote', '--refs', repo, ref)
+
+        unless output[:exit_code].zero?
+          PDK.logger.error output[:stdout]
+          PDK.logger.error output[:stderr]
+          raise PDK::CLI::ExitWithError, _('Unable to access the template repository "%{repository}"') % {
+            repository: repo,
+          }
+        end
+
+        matching_refs = output[:stdout].split("\n").map { |r| r.split("\t") }
+        matching_refs.find { |_sha, remote_ref| remote_ref == ref }.first
+      end
     end
   end
 end

spec/unit/pdk/cli/update_spec.rb

@@ -2,6 +2,11 @@
 
 describe 'PDK::CLI update' do
   let(:help_text) { a_string_matching(%r{^USAGE\s+pdk update}m) }
+  let(:updater) do
+    instance_double(PDK::Module::Update, run: true, current_version: current_version, new_version: new_version)
+  end
+  let(:current_version) { '1.2.3' }
+  let(:new_version) { '1.2.4' }
 
   context 'when not run from inside a module' do
     include_context 'run outside module'
@@ -24,23 +29,26 @@
 
     context 'and provided no flags' do
       it 'invokes the updater with no options' do
-        expect(PDK::Module::Update).to receive(:invoke).with({})
+        expect(PDK::Module::Update).to receive(:new).with({}).and_return(updater)
+        expect(updater).to receive(:run)
 
         PDK::CLI.run(%w[update])
       end
     end
 
     context 'and the --noop flag has been passed' do
       it 'passes the noop option through to the updater' do
-        expect(PDK::Module::Update).to receive(:invoke).with(noop: true)
+        expect(PDK::Module::Update).to receive(:new).with(noop: true).and_return(updater)
+        expect(updater).to receive(:run)
 
         PDK::CLI.run(%w[update --noop])
       end
     end
 
     context 'and the --force flag has been passed' do
       it 'passes the force option through to the updater' do
-        expect(PDK::Module::Update).to receive(:invoke).with(force: true)
+        expect(PDK::Module::Update).to receive(:new).with(force: true).and_return(updater)
+        expect(updater).to receive(:run)
 
         PDK::CLI.run(%w[update --force])
       end
@@ -57,5 +65,17 @@
         }
       end
     end
+
+    context 'and the module is already up to date' do
+      let(:current_version) { new_version }
+
+      it 'does not run the updater' do
+        expect(logger).to receive(:info).with(a_string_matching(%r{already up to date with version #{current_version}}i))
+        expect(PDK::Module::Update).to receive(:new).with({}).and_return(updater)
+        expect(updater).not_to receive(:run)
+
+        PDK::CLI.run(%w[update])
+      end
+    end
   end
 end

spec/unit/pdk/module/update_spec.rb

@@ -0,0 +1,136 @@
+require 'spec_helper'
+require 'pdk/module/update'
+
+describe PDK::Module::Update do
+  let(:options) { {} }
+  let(:mock_metadata) do
+    instance_double(
+      PDK::Module::Metadata,
+      data: {
+        'template-url' => template_url,
+        'template-ref' => template_ref,
+      },
+    )
+  end
+  let(:template_url) { 'https://github.com/puppetlabs/pdk-templates' }
+  let(:template_ref) { nil }
+
+  shared_context 'with mock metadata' do
+    before(:each) do
+      allow(PDK::Module::Metadata).to receive(:from_file).with('metadata.json').and_return(mock_metadata)
+    end
+  end
+
+  describe '#run' do
+    let(:instance) { described_class.new(options) }
+    let(:template_ref) { '1.3.2-0-g1234567' }
+
+    include_context 'with mock metadata'
+
+    before(:each) do
+      allow(instance).to receive(:stage_changes!)
+      allow(instance).to receive(:print_summary)
+      allow(instance).to receive(:new_version).and_return('1.4.0')
+    end
+
+    after(:each) do
+      instance.run
+    end
+
+    context 'when running in noop mode' do
+      let(:options) { { noop: true } }
+
+      it 'does not prompt the user to make the changes' do
+        expect(PDK::CLI::Util).not_to receive(:prompt_for_yes)
+      end
+
+      it 'does not sync the pending changes' do
+        expect(instance.update_manager).not_to receive(:sync_changes!)
+      end
+    end
+  end
+
+  describe '#module_metadata' do
+    subject(:result) { described_class.new(options).module_metadata }
+
+    context 'when the metadata.json can be read' do
+      include_context 'with mock metadata'
+
+      it 'returns the metadata object' do
+        is_expected.to eq(mock_metadata)
+      end
+    end
+
+    context 'when the metadata.json can not be read' do
+      before(:each) do
+        allow(PDK::Module::Metadata).to receive(:from_file).with('metadata.json').and_raise(ArgumentError, 'some error')
+      end
+
+      it 'raises an ExitWithError exception' do
+        expect { -> { result }.call }.to raise_error(PDK::CLI::ExitWithError, %r{some error}i)
+      end
+    end
+  end
+
+  describe '#template_url' do
+    subject { described_class.new(options).template_url }
+
+    include_context 'with mock metadata'
+
+    it 'returns the template-url value from the module metadata' do
+      is_expected.to eq('https://github.com/puppetlabs/pdk-templates')
+    end
+  end
+
+  describe '#current_version' do
+    subject { described_class.new(options).current_version }
+
+    include_context 'with mock metadata'
+
+    context 'when the template-ref describes a git tag' do
+      let(:template_ref) { '1.3.2-0-g07678c8' }
+
+      it 'returns the tag name' do
+        is_expected.to eq('1.3.2')
+      end
+    end
+
+    context 'when the template-ref describes a branch commit' do
+      let(:template_ref) { 'heads/master-4-g1234abc' }
+
+      it 'returns the branch name and the commit SHA' do
+        is_expected.to eq('master@1234abc')
+      end
+    end
+  end
+
+  describe '#new_version' do
+    subject { described_class.new(options).new_version }
+
+    context 'when the default_template_ref specifies a tag' do
+      before(:each) do
+        allow(PDK::Util).to receive(:default_template_ref).and_return('1.4.0')
+      end
+
+      it 'returns the tag name' do
+        is_expected.to eq('1.4.0')
+      end
+    end
+
+    context 'when the default_template_ref specifies a branch head' do
+      before(:each) do
+        allow(PDK::Util).to receive(:default_template_ref).and_return('origin/master')
+        allow(PDK::Util::Git).to receive(:ls_remote)
+          .with(template_url, 'refs/heads/master')
+          .and_return('3cdd84e8f0aae30bf40d15556482fc8752899312')
+      end
+
+      include_context 'with mock metadata'
+      let(:template_ref) { 'heads/master-0-g07678c8' }
+
+      it 'returns the branch name and the commit SHA' do
+        is_expected.to eq('master@3cdd84e')
+      end
+    end
+  end
+end

spec/unit/pdk/util/git_spec.rb

@@ -26,4 +26,50 @@
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '.ls_remote' do
+    subject { described_class.ls_remote(repo, ref) }
+
+    let(:repo) { 'https://github.com/puppetlabs/pdk-templates' }
+    let(:ref) { 'refs/heads/master' }
+
+    before(:each) do
+      allow(described_class).to receive(:git).with('ls-remote', '--refs', repo, ref).and_return(git_result)
+    end
+
+    context 'when the repo is unavailable' do
+      let(:git_result) do
+        {
+          exit_code: 1,
+          stdout:    'some stdout text',
+          stderr:    'some stderr text',
+        }
+      end
+
+      it 'raises an ExitWithError exception' do
+        expect(logger).to receive(:error).with(git_result[:stdout])
+        expect(logger).to receive(:error).with(git_result[:stderr])
+
+        expect {
+          described_class.ls_remote(repo, ref)
+        }.to raise_error(PDK::CLI::ExitWithError, %r{unable to access the template repository}i)
+      end
+    end
+
+    context 'when the repo is available' do
+      let(:git_result) do
+        {
+          exit_code: 0,
+          stdout:    [
+            "master-sha\trefs/heads/master",
+            "masterful-sha\trefs/heads/masterful",
+          ].join("\n"),
+        }
+      end
+
+      it 'returns only the SHA for the exact ref match' do
+        is_expected.to eq('master-sha')
+      end
+    end
+  end
 end