(FM-8092) Fix caching scope of transport schemas

When running in puppetserver for the [environment_transports](https://github.com/puppetlabs/puppetserver/blob/master/documentation/puppet-api/v3/environment_transports.markdown)
API, the Resource API would cache registered transport schemas forever.
This meant that deploying a new module version and running the usual
environment cache cleaning scripts would not update the transport schema.

With this change, we push the transport cache into an ObjectIdCacheAdapter
around the environment, which will be cleaned up by the internal utilities.

Author: DavidS

lib/puppet/resource_api/transport.rb

@@ -12,7 +12,7 @@ def register(schema)
     unless transports[schema[:name]].nil?
       raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
         name: schema[:name],
-        environment: current_environment,
+        environment: current_environment_name,
       }
     end
     transports[schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
@@ -27,7 +27,7 @@ def list
   module_function :list # rubocop:disable Style/AccessModifierDeclarations
 
   # retrieve a Hash of transport schemas, keyed by their name.
-  # This uses the Puppet autoloader, provide a  environment name as `force_environment`
+  # This uses the Puppet autoloader, provide an environment name as `force_environment`
   # to choose where to load from.
   # @api private
   def list_all_transports(force_environment)
@@ -45,7 +45,7 @@ def self.load_all_schemas
     require 'puppet/settings'
     require 'puppet/util/autoload'
     @autoloader ||= Puppet::Util::Autoload.new(self, 'puppet/transport/schema')
-    @autoloader.loadall(Puppet.lookup(:current_environment))
+    @autoloader.loadall(current_environment)
   end
   private_class_method :load_all_schemas
 
@@ -74,7 +74,7 @@ def self.validate(name, connection_info)
     if transport_schema.nil?
       raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % {
         target: name,
-        environment: current_environment,
+        environment: current_environment_name,
       }
     end
 
@@ -108,21 +108,26 @@ def self.wrap_sensitive(name, connection_info)
   private_class_method :wrap_sensitive
 
   def self.transports
-    @transports ||= {}
-    @transports[current_environment] ||= {}
+    env = current_environment
+    if env
+      ObjectIdCacheAdapter.adapt(env).retrieve(:rsapi_transport_cache)
+    else
+      @transports_default ||= {}
+    end
   end
   private_class_method :transports
 
   def self.current_environment
-    if Puppet.respond_to? :lookup
-      env = Puppet.lookup(:current_environment)
-      env.nil? ? :transports_default : env.name
-    else
-      :transports_default
-    end
+    Puppet.lookup(:current_environment) if Puppet.respond_to? :lookup
   end
   private_class_method :current_environment
 
+  def self.current_environment_name
+    env = current_environment
+    env.nil? ? :transports_default : env.name
+  end
+  private_class_method :current_environment_name
+
   def self.clean_bolt_attributes(transport_schema, connection_info)
     context = get_context(transport_schema.name)
 
@@ -154,4 +159,19 @@ def self.clean_bolt_attributes(transport_schema, connection_info)
     nil
   end
   private_class_method :clean_bolt_attributes
+
+  # copy from https://github.com/puppetlabs/puppet/blob/8cae8a17dbac08d2db0238d5bce2f1e4d1898d65/lib/puppet/pops/adapters.rb#L6-L17
+  # to keep backwards compatibility with puppet4 and 5, which don't have this yet.
+  class ObjectIdCacheAdapter < Puppet::Pops::Adaptable::Adapter
+    attr_accessor :cache
+
+    def initialize
+      @cache = {}
+    end
+
+    # Retrieves a mutable hash with all stored values
+    def retrieve(o)
+      @cache[o.__id__] ||= {}
+    end
+  end
 end

spec/puppet/resource_api/transport_spec.rb

@@ -5,9 +5,9 @@ def change_environment(name = nil)
     environment = class_double(Puppet::Node::Environment)
 
     if name.nil?
-      allow(Puppet).to receive(:respond_to?).and_return(false)
+      allow(Puppet).to receive(:respond_to?).with(:lookup).and_return(false)
     else
-      allow(Puppet).to receive(:respond_to?).and_return(true)
+      allow(Puppet).to receive(:respond_to?).with(:lookup).and_return(true)
     end
 
     allow(Puppet).to receive(:lookup).with(:current_environment).and_return(environment)
@@ -73,64 +73,16 @@ def change_environment(name = nil)
           },
         }
       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
-        let(:transports) { described_class.instance_variable_get(:@transports) }
+      context 'when no environment is available' do
+        before(:each) { change_environment(nil) }
 
-        it 'will record the schemas in the correct structure' do
-          change_environment(nil)
-          described_class.register(schema)
-          expect(transports).to have_key(:transports_default)
-          expect(transports[:transports_default][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports[:transports_default][schema[:name]].definition).to eq(schema)
-
-          change_environment('foo')
-          described_class.register(schema)
-          described_class.register(schema2)
-          expect(transports).to have_key('foo')
-          expect(transports['foo'][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports['foo'][schema[:name]].definition).to eq(schema)
-          expect(transports['foo'][schema2[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports['foo'][schema2[:name]].definition).to eq(schema2)
-
-          change_environment(:bar)
-          described_class.register(schema3)
-          expect(transports).to have_key(:bar)
-          expect(transports[:bar][schema3[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports[:bar][schema3[:name]].definition).to eq(schema3)
+        it 'adds to the transports register' do
+          expect { described_class.register(schema) }.not_to raise_error
         end
       end
     end