search

puppetlabs / puppetserver-ca-cli

A simple Ruby CLI tool to interact with the Puppet Server's included Certificate Authority
Apache License 2.0
3 stars 24 forks source link

(maint) Fix bad memoization in default certname #74

Closed Magisus closed 3 years ago

Magisus commented 3 years ago

The default_certname method would do the right thing the first time it was called, and then afterwards fail to get the hostname. This commit fixes the bad short circuiting logic.

Magisus commented 3 years ago

The travis failures are caused by this commit and are not fixed when I roll forward to latest (what this PR is currently pinned to).

@puppetlabs/night-s-watch do you see what might be going on here?

The tests are failing because fqdn is nil here. Ignore the weird code in that function, that's what this PR is trying to fix (but doesn't seem to avoid the issue).

Magisus commented 3 years ago

These tests all pass for me locally, I don't feel like we need to block merge on getting an updated Facter (given we need a release in order to pull it in here).

mwaggett commented 3 years ago

@Magisus will our other pipelines pass if we merge this?

Magisus commented 3 years ago

The tests on this repo are failing before this change. Our pipelines get Facter in a totally different way than these unit tests, and are passing now. My guess is in that context they have ffi available and are therefore fine.

Magisus commented 3 years ago

We have a floating dependency on facter so these tests will fix themselves the next time they release.