Merge pull request #151 from da-ar/env_up

(FM-7690) Update transports cache to be grouped by environment

Author: DavidS

lib/puppet/resource_api/transport.rb

@@ -1,7 +1,4 @@
-# rubocop:disable Style/Documentation
-module Puppet; end
-module Puppet::ResourceApi; end
-# rubocop:enable Style/Documentation
+module Puppet::ResourceApi; end # rubocop:disable Style/Documentation
 
 # Remote target transport API
 module Puppet::ResourceApi::Transport
@@ -12,19 +9,21 @@ def register(schema)
     raise Puppet::DevError, 'requires `:connection_info`' unless schema.key? :connection_info
     raise Puppet::DevError, '`:connection_info` must be a hash, not `%{other_type}`' % { other_type: schema[:connection_info].class } unless schema[:connection_info].is_a?(Hash)
 
-    @transports ||= {}
-    raise Puppet::DevError, 'Transport `%{name}` is already registered.' % { name: schema[:name] } unless @transports[schema[:name]].nil?
-    @transports[schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
+    init_transports
+    unless @transports[@environment][schema[:name]].nil?
+      raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
+        name: schema[:name],
+        environment: @environment,
+      }
+    end
+    @transports[@environment][schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
   end
   module_function :register # rubocop:disable Style/AccessModifierDeclarations
 
   # retrieve a Hash of transport schemas, keyed by their name.
   def list
-    if @transports
-      Marshal.load(Marshal.dump(@transports))
-    else
-      {}
-    end
+    init_transports
+    Marshal.load(Marshal.dump(@transports[@environment]))
   end
   module_function :list # rubocop:disable Style/AccessModifierDeclarations
 
@@ -37,17 +36,36 @@ def connect(name, connection_info)
   module_function :connect # rubocop:disable Style/AccessModifierDeclarations
 
   def self.validate(name, connection_info)
-    @transports ||= {}
+    init_transports
     require "puppet/transport/schema/#{name}" unless @transports.key? name
-    transport_schema = @transports[name]
-    raise Puppet::DevError, 'Transport for `%{target}` not registered' % { target: name } if transport_schema.nil?
+    transport_schema = @transports[@environment][name]
+    if transport_schema.nil?
+      raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % {
+        target: name,
+        environment: @environment,
+      }
+    end
 
     transport_schema.check_schema(connection_info)
     transport_schema.validate(connection_info)
   end
+  private_class_method :validate
 
   def self.get_context(name)
     require 'puppet/resource_api/puppet_context'
-    Puppet::ResourceApi::PuppetContext.new(@transports[name])
+    Puppet::ResourceApi::PuppetContext.new(@transports[@environment][name])
+  end
+  private_class_method :get_context
+
+  def self.init_transports
+    lookup = Puppet.lookup(:current_environment) if Puppet.respond_to? :lookup
+    @environment =  if lookup.nil?
+                      :transports_default
+                    else
+                      lookup.name
+                    end
+    @transports ||= {}
+    @transports[@environment] ||= {}
   end
+  private_class_method :init_transports
 end

spec/puppet/resource_api/transport_spec.rb

@@ -1,6 +1,24 @@
 require 'spec_helper'
 
 RSpec.describe Puppet::ResourceApi::Transport do
+  def change_environment(name = nil)
+    environment = class_double(Puppet::Node::Environment)
+
+    if name.nil?
+      allow(Puppet).to receive(:respond_to?).and_return(false)
+    else
+      allow(Puppet).to receive(:respond_to?).and_return(true)
+    end
+
+    allow(Puppet).to receive(:lookup).with(:current_environment).and_return(environment)
+
+    # allow clean up scripts to run unhindered
+    allow(Puppet).to receive(:lookup).with(:root_environment).and_call_original
+    allow(Puppet).to receive(:lookup).with(:environments).and_call_original
+
+    allow(environment).to receive(:name).and_return(name)
+  end
+
   let(:strict_level) { :error }
 
   before(:each) do
@@ -60,10 +78,64 @@
           },
         }
       end
+      let(:schema2) do
+        {
+          name: 'schema2',
+          desc: 'basic transport',
+          connection_info: {
+            host: {
+              type: 'String',
+              desc: 'the host ip address or hostname',
+            },
+          },
+        }
+      end
+      let(:schema3) do
+        {
+          name: 'schema3',
+          desc: 'basic transport',
+          connection_info: {
+            user: {
+              type: 'String',
+              desc: 'the user to connect as',
+            },
+            password: {
+              type: 'String',
+              sensitive: true,
+              desc: 'the password to make the connection',
+            },
+          },
+        }
+      end
 
       it 'adds to the transports register' do
         expect { described_class.register(schema) }.not_to raise_error
       end
+
+      context 'when a transports are added to multiple environments' do
+        it 'will record the schemas in the correct structure' do
+          change_environment(:wibble)
+          described_class.register(schema)
+          expect(described_class.instance_variable_get(:@transports)).to be_key(:wibble)
+          expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
+          expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]].definition).to eq(schema)
+
+          change_environment(:foo)
+          described_class.register(schema)
+          described_class.register(schema2)
+          expect(described_class.instance_variable_get(:@transports)).to be_key(:foo)
+          expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
+          expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]].definition).to eq(schema)
+          expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
+          expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]].definition).to eq(schema2)
+
+          change_environment(:bar)
+          described_class.register(schema3)
+          expect(described_class.instance_variable_get(:@transports)).to be_key(:bar)
+          expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
+          expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]].definition).to eq(schema3)
+        end
+      end
     end
 
     context 'when registering a transport with a bad type' do
@@ -176,8 +248,15 @@
   end
 
   describe '#validate(name, connection_info)', agent_test: true do
+    context 'when the transport does not exist' do
+      it { expect { described_class.send(:validate, 'wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} }
+    end
+
     context 'when the transport being validated has not be registered' do
-      it { expect { described_class.validate('wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} }
+      it 'will throw an unregistered error message' do
+        expect(described_class).to receive(:require).with('puppet/transport/schema/wibble')
+        expect { described_class.send(:validate, 'wibble', {}) }.to raise_error Puppet::DevError, %r{ not registered with }
+      end
     end
 
     context 'when the transport being validated has been registered' do
@@ -189,10 +268,33 @@
 
         described_class.register(schema)
 
+        expect(described_class).to receive(:require).with('puppet/transport/schema/validate')
         expect(schema_def).to receive(:check_schema).with('connection_info').and_return(nil)
         expect(schema_def).to receive(:validate).with('connection_info').and_return(nil)
 
-        described_class.validate('validate', 'connection_info')
+        described_class.send :validate, 'validate', 'connection_info'
+      end
+    end
+  end
+
+  describe '#init_transports' do
+    context 'when there is not a current_environment' do
+      it 'will return the default transport environment name' do
+        change_environment
+
+        described_class.send :init_transports
+
+        expect(described_class.instance_variable_get(:@environment)).to eq(:transports_default)
+      end
+    end
+
+    context 'when there is a current_environment' do
+      it 'will return the set environment name' do
+        change_environment(:something)
+
+        described_class.send :init_transports
+
+        expect(described_class.instance_variable_get(:@environment)).to eq(:something)
       end
     end
   end