From f2866d1efeba7fc5a47af553bbcb0a3bed00a22f Mon Sep 17 00:00:00 2001
From: Justin Stoller <justin.stoller@gmail.com>
Date: Tue, 24 Oct 2023 13:53:51 -0700
Subject: [PATCH] (maint) Compare Environment#name in compiler

During catalog compilation, after loading the requested environment we
load the node and server specified environment. Previously, We would
then compare the Environment objects to determine if the requested
environment matched the server specified environment. However, if the
environment cache is flushed between those two environments being loaded
they could be different object instances representing the same desired
environment. Environment objects define a `==` method but require the
name and modulepath to be the same. If we are running with versioned
environment dirs those modulepaths may be different (ie different
versions of the same environment).

To resolve this race condition we now check the environment names
(Environment#name canoncalizes to symbols to avoid symbol/string
mismatch). The catalog already uses the server specified environment
which, if we are running with versioned environment dirs and have
different versions of the same environment, will be the most up to date
version of the environment.
---
 lib/puppet/indirector/catalog/compiler.rb     | 16 ++++++++++++----
 spec/unit/indirector/catalog/compiler_spec.rb | 17 +++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
index 9a6db303d47..f12fc53ca9c 100644
--- a/lib/puppet/indirector/catalog/compiler.rb
+++ b/lib/puppet/indirector/catalog/compiler.rb
@@ -53,12 +53,20 @@ def find(request)
     node.trusted_data = Puppet.lookup(:trusted_information) { Puppet::Context::TrustedInformation.local(node) }.to_h
 
     if node.environment
-      # If the requested environment doesn't match the server specified environment,
-      # as determined by the node terminus, and the request wants us to check for an
+      # If the requested environment name doesn't match the server specified environment
+      # name, as determined by the node terminus, and the request wants us to check for an
       # environment mismatch, then return an empty catalog with the server-specified
       # enviroment.
-      if request.remote? && request.options[:check_environment] && node.environment != request.environment
-        return Puppet::Resource::Catalog.new(node.name, node.environment)
+      if request.remote? && request.options[:check_environment]
+        # The "environment" may be same while environment objects differ. This
+        # is most likely because the environment cache was flushed between the request
+        # processing and node lookup. Environment overrides `==` but requires the
+        # name and modulepath to be the same. When using versioned environment dirs the
+        # same "environment" can have different modulepaths so simply compare names here.
+        if node.environment.name != request.environment.name
+          Puppet.warning _("Requested environment '%{request_env}' did not match server specified environment '%{server_env}'") % {request_env: request.environment.name, server_env: node.environment.name}
+          return Puppet::Resource::Catalog.new(node.name, node.environment)
+        end
       end
 
       node.environment.with_text_domain do
diff --git a/spec/unit/indirector/catalog/compiler_spec.rb b/spec/unit/indirector/catalog/compiler_spec.rb
index 4e883b43b08..4e1a3b07344 100644
--- a/spec/unit/indirector/catalog/compiler_spec.rb
+++ b/spec/unit/indirector/catalog/compiler_spec.rb
@@ -271,6 +271,23 @@ def set_facts(fact_hash)
         expect(catalog).to have_resource('Stage[main]')
       end
 
+      # versioned environment directories can cause this
+      it 'allows environments with the same name but mismatched modulepaths' do
+        envs = Puppet.lookup(:environments)
+        env_server = envs.get!(:env_server)
+        v1_env = env_server.override_with({ modulepath: ['/code-v1/env-v1/'] })
+        v2_env = env_server.override_with({ modulepath: ['/code-v2/env-v2/'] })
+
+        @request.options[:check_environment] = "true"
+        @request.environment = v1_env
+        node.environment = v2_env
+
+        catalog = compiler.find(@request)
+
+        expect(catalog.environment).to eq('env_server')
+        expect(catalog).to have_resource('Stage[main]')
+      end
+
       it 'returns an empty catalog if asked to check the environment and they are mismatched' do
         @request.options[:check_environment] = "true"
         catalog = compiler.find(@request)