Skip to content

Commit 4eaac00

Browse files
committed
(PUP-10772) Only accept msgpack if gem is installed
Previously the agent always advertised that it accepted the rich data msgpack format (application/vnd.puppet.rich+msgpack) even if the agent didn't have the msgpack gem installed locally. If the msgpack gem was installed on the server, and the server selected the rich data msgpack format, then the agent would fail to deserialize the catalog. Now we call the superclass so the msgpack feature constraint is taken into account. Note the "raw" msgpack format didn't have this issue because it didn't override the "supported?" method.
1 parent bfd2220 commit 4eaac00

File tree

2 files changed

+51
-1
lines changed

2 files changed

+51
-1
lines changed

lib/puppet/network/formats.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ def render(instance)
255255
end
256256

257257
def supported?(klass)
258-
klass == Puppet::Resource::Catalog &&
258+
super &&
259+
klass == Puppet::Resource::Catalog &&
259260
Puppet.lookup(:current_environment).rich_data?
260261
end
261262
end

spec/unit/http/service/compiler_spec.rb

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,26 @@
131131
subject.post_catalog(certname, environment: 'production', facts: facts, checksum_type: %w[sha256 sha384])
132132
end
133133

134+
it 'does not accept msgpack by default' do
135+
stub_request(:post, uri)
136+
.with(headers: {'Accept' => 'application/vnd.puppet.rich+json, application/json, text/pson'})
137+
.to_return(**catalog_response)
138+
139+
allow(Puppet.features).to receive(:msgpack?).and_return(false)
140+
141+
subject.post_catalog(certname, environment: environment, facts: facts)
142+
end
143+
144+
it 'accepts msgpack & rich_json_msgpack if the gem is present' do
145+
stub_request(:post, uri)
146+
.with(headers: {'Accept' => 'application/vnd.puppet.rich+json, application/json, application/x-msgpack, text/pson'})
147+
.to_return(**catalog_response)
148+
149+
allow(Puppet.features).to receive(:msgpack?).and_return(true)
150+
151+
subject.post_catalog(certname, environment: environment, facts: facts)
152+
end
153+
134154
it 'returns a deserialized catalog' do
135155
stub_request(:post, uri)
136156
.to_return(**catalog_response)
@@ -140,6 +160,35 @@
140160
expect(cat.name).to eq(certname)
141161
end
142162

163+
it 'deserializes the catalog from msgpack', if: Puppet.features.msgpack? do
164+
body = catalog.to_msgpack
165+
formatter = Puppet::Network::FormatHandler.format(:msgpack)
166+
catalog_response = { body: body, headers: {'Content-Type' => formatter.mime }}
167+
168+
stub_request(:post, uri)
169+
.to_return(**catalog_response)
170+
171+
_, cat = subject.post_catalog(certname, environment: 'production', facts: facts)
172+
expect(cat).to be_a(Puppet::Resource::Catalog)
173+
expect(cat.name).to eq(certname)
174+
end
175+
176+
it 'deserializes the catalog from rich msgpack', if: Puppet.features.msgpack? do
177+
body = Puppet.override(rich_data: true) do
178+
catalog.to_msgpack
179+
end
180+
181+
formatter = Puppet::Network::FormatHandler.format(:rich_data_msgpack)
182+
catalog_response = { body: body, headers: {'Content-Type' => formatter.mime }}
183+
184+
stub_request(:post, uri)
185+
.to_return(**catalog_response)
186+
187+
_, cat = subject.post_catalog(certname, environment: 'production', facts: facts)
188+
expect(cat).to be_a(Puppet::Resource::Catalog)
189+
expect(cat.name).to eq(certname)
190+
end
191+
143192
it 'returns the request response' do
144193
stub_request(:post, uri)
145194
.to_return(**catalog_response)

0 commit comments

Comments
 (0)