Skip to content

New option: puppetdb_ssl_crl #249

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

Merged
merged 2 commits into from
Aug 11, 2022
Merged

New option: puppetdb_ssl_crl #249

merged 2 commits into from
Aug 11, 2022

Conversation

Lavaburn
Copy link
Contributor

Puppet 6 support: The CRL file is required by newer versions of Puppet.

Overview

On Puppet 6.21.1, the following issue is thrown when using PuppetDB with SSL verification: "The CRL is missing from '/tmp/.../var/ssl/crl.pem'

With this option, the CRL file can be copied into the builddir.

Checklist

  • REQUIRED: Add new file in lib/octocatalog-diff/cli/options
  • REQUIRED: Add corresponding test in spec/octocatalog-diff/tests/cli/options
  • OPTIONAL: Add default value in lib/octocatalog-diff/cli.rb
  • OPTIONAL: Add configuration example in examples/octocatalog-diff.cfg.rb
  • REQUIRED: Add code to implement your option in lib
  • REQUIRED: Add corresponding tests for code to implement your option in spec/octocatalog-diff/tests
  • OPTIONAL: Add an integration test to test your option in spec/octocatalog-diff/integration

The CRL file is required by newer version of Puppet(Server)(DB).
@ahayworth
Copy link
Contributor

Thanks @Lavaburn! Two questions:

  1. Do you know which version started requiring a CRL? Could we put that in the docs perhaps?
  2. Is there a way we could detect when passing a CRL is required and notify users that they've not passed a needed option?

@Lavaburn
Copy link
Contributor Author

@ahayworth

1a. I am not sure, as I can't test with older versions, but from what I can see in Puppet code, the error message was introduced in 6.4.0. I have confirmed the issue is present with 6.5.0 - 6.21.1
1b. We could definitely add in the documentation that when using Puppet >= 6 and receiving the following error: "The CRL is missing", this option would be required.
2a. I doubt, the error is thrown by the puppet agent. There is probably an obscure option to disable this requirement, so just relying on the Puppet version would be "overkill".
2b. You could issue a debug notice when the option is not used and the agent is version >= 6.4.0.

@ahayworth
Copy link
Contributor

That all makes sense! If you want to just update the PR with a documentation note, I think that will suffice. :)

@DLeich
Copy link

DLeich commented Feb 14, 2022

Just wanted to note that I see this issue still present in Puppet 7 (7.13.1 to be exact). What's needed to push this pull request along? Without this, octocatalog-diff is not usable for modern versions of Puppet.

@claviola
Copy link

just stumbled upon this as well. @ahayworth is there something still pending?

@ahayworth
Copy link
Contributor

@claviola the PR was previously stalled on documentation:

That all makes sense! If you want to just update the PR with a documentation note, I think that will suffice. :)

However, we could probably move forward with this regardless. Unfortunately, I am no longer an admin on this repo since I left GitHub. I know @claytono was recently moving some of my previous responsibilities (around RubyGems) - perhaps he would be kind enough to pass this along to the current maintainer of octocatalog-diff?

@claytono
Copy link

@ahayworth I pinged the folks I believe are the current owners.

@ahayworth
Copy link
Contributor

@claytono Thank you kindly, I appreciate it!

@claviola
Copy link

@claviola the PR was previously stalled on documentation:

Thanks! I noticed that you had asked for more in regards to docs, but I was wondering where, because I'd also gladly jump in on that. I don't have a lot of experience with Ruby but I'd love to get this in shape for current versions of Puppet at least.

@anth1y
Copy link

anth1y commented Apr 20, 2022

this is awesome and thank you all for your work here... @claytono & @ahayworth thank you for the heads up. I'll get this merged shortly...

@anth1y anth1y self-assigned this Apr 20, 2022
@claviola
Copy link

hi @anth1y! is there anything one can help you with to get this through?

@anth1y
Copy link

anth1y commented Jul 12, 2022

Apologies @claviola I let this slip through I'll get this on my todo for this week

Copy link

@anth1y anth1y left a comment

Choose a reason for hiding this comment

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

apologies for getting back to this soo late. It looks like I dont have write access

@iheartski
Copy link

Any updates on the merge??

@ahayworth
Copy link
Contributor

@anth1y were you able to find anyone with access, or to obtain access somehow?

@anth1y anth1y merged commit fb924b8 into github:master Aug 11, 2022
@anth1y
Copy link

anth1y commented Aug 11, 2022

@anth1y were you able to find anyone with access, or to obtain access somehow?

i somehow got write access

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.

7 participants