Skip to content

Conversation

@joshcooper
Copy link
Contributor

@joshcooper joshcooper commented Nov 2, 2020

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 suitable? method in the superclass so the msgpack feature
constrain is taken into account. Calling the super supported? method triggers
a bunch of other logic around whether or not the format implements required
methods, so avoid that.

Note the "raw" msgpack format didn't have this issue because it didn't override
the "supported?" method.

@joshcooper joshcooper requested review from a team November 2, 2020 20:28
@joshcooper joshcooper force-pushed the dont_accept_msgpack_10772 branch from 4eaac00 to 28d3efe Compare November 2, 2020 20:43
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 `suitable?` method in the superclass so the msgpack feature
constrain is taken into account. Calling the super `supported?` method triggers
a bunch of other logic around whether or not the format implements required
methods, so avoid that.

Note the "raw" msgpack format didn't have this issue because it didn't override
the "supported?" method.
@joshcooper joshcooper force-pushed the dont_accept_msgpack_10772 branch from 28d3efe to af00807 Compare November 2, 2020 21:12
@puppetcla
Copy link

CLA signed by all contributors.

@GabrielNagy GabrielNagy merged commit 23968a9 into puppetlabs:master Nov 3, 2020
@joshcooper joshcooper deleted the dont_accept_msgpack_10772 branch November 3, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants