Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PUP-12046) Send facts for catalog download #9424

Conversation

tvpartytonight
Copy link
Contributor

No description provided.

@tvpartytonight tvpartytonight force-pushed the PUP-12046_send_facts_catalog_download branch 2 times, most recently from 3209b3d to d4d9216 Compare July 25, 2024 19:40
@tvpartytonight tvpartytonight force-pushed the PUP-12046_send_facts_catalog_download branch from d4d9216 to 0a68b60 Compare July 29, 2024 21:10
@tvpartytonight tvpartytonight changed the title (PUP-12046) Send facts for catalog download WIP (PUP-12046) Send facts for catalog download Jul 29, 2024
@tvpartytonight tvpartytonight marked this pull request as ready for review July 29, 2024 21:27
@tvpartytonight tvpartytonight requested a review from a team as a code owner July 29, 2024 21:27
stub_request(:post, 'https://puppet.server.test:8140/puppet/v3/catalog/puppet.node.test?environment=*root*')
.with(
headers: { 'Content-Type' => 'application/x-www-form-urlencoded' },
body: /catalog_face_request_test_value/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up using a regex here because I couldn't figure out how to use hash_including when the facts value is a url double-encoded.

Copy link
Contributor

@joshcooper joshcooper Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the double encoded facts are added as the value of the facts query parameter, so you can do:

.with(body: hash_including("facts" => /#{test_fact[:encoded]}/))

That said I think what you have is fine.

headers: { 'Content-Type' => 'application/x-www-form-urlencoded' },
body: /catalog_face_request_test_value/
).to_return(:status => 200, :body => catalog.render(:json), :headers => {'Content-Type' => 'application/json'})
subject.download
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, stub_request doesn't verify the request was made, only that if a request is made that it matches at least one stub, which could be in this file or in spec_helper. Might be good to do something like https://github.com/bblimke/webmock?tab=readme-ov-file#setting-expectations-in-rspec-on-the-stub

Facts were not always sent with the catalog request when
using the catalog face. This change allows the face to send
facts for the request so that the catalog returned is
compiled with the correct set of facts.
@tvpartytonight tvpartytonight force-pushed the PUP-12046_send_facts_catalog_download branch from 0a68b60 to c47de03 Compare July 29, 2024 23:03
@joshcooper joshcooper merged commit 7080b6a into puppetlabs:main Jul 29, 2024
9 checks passed
@tvpartytonight tvpartytonight added the backport 7.x Generate a backport PR to 7.x label Jul 31, 2024
Copy link

Successfully created backport PR for 7.x:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 7.x Generate a backport PR to 7.x bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants