Add support for certificate_content and private_key_content parameters #385

hajee · 2022-03-15T10:21:00Z

The current implementation only allows you to pass a file name to the certificate and private_key parameters. When you are fetching certificates from vault or another secure store, you'll first have to save them to a file. This is very innconveniant.

This PR add's the parameters certificate_content and private_key_content. These parameters are mutually exclusive from their file counterparts.

With this change, you can now fetch a certificate and/or a password from vault (through a hiera lookup for example) and use it directly on the type.

Because these values can be sensitive, both of the new parameters support passing the value as a sensitive data type.

puppet-community-rangefinder · 2022-03-15T10:21:07Z

java_ks is a type

Breaking changes to this file MAY impact these 19 modules (near match):

This module is declared in 6 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

CLAassistant · 2022-03-15T10:41:37Z

All committers have signed the CLA.

hajee · 2022-03-22T13:10:22Z

Is there a timeline for review end eventually merge of the PR?

chelnak · 2022-03-22T13:50:00Z

Hey @hajee, thank you for your contribution and sorry for such a long wait.

I'm happy with your PR and would like to get it merged. I'm just going to re-kick CI by closing and re-opening the PR, then pending a green run, we are good to go.

puppet-community-rangefinder · 2022-03-22T13:50:15Z

java_ks is a type

Breaking changes to this file MAY impact these 19 modules (near match):

This module is declared in 6 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

puppet-community-rangefinder · 2022-03-22T13:51:30Z

java_ks is a type

Breaking changes to this file MAY impact these 19 modules (near match):

This module is declared in 6 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

chelnak · 2022-03-23T08:03:31Z

Hey @hajee, please could you rebase so that this PR has the latest from main? That should get CI going again!

hajee · 2022-03-23T15:07:26Z

I don't think I can do something to ensure a running acceptance test, do I?

hajee · 2022-03-29T14:11:19Z

@chelnak What is the progress on this? Anything I can do?

puppet-community-rangefinder · 2022-03-29T14:30:48Z

java_ks is a type

Breaking changes to this file MAY impact these 19 modules (near match):

This module is declared in 6 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

chelnak · 2022-03-29T14:31:25Z

I've just re-kicked the CI jobs. Lets see what happens.

chelnak · 2022-03-29T17:38:07Z

@hajee Could you add an example to the certificates section of the README explaining how a developer might get started with the new feature?

Once that's done I think we will be ready to merge!

hajee · 2022-03-30T07:12:41Z

@chelnak I added some text to the README and force pushed it so there is a clean git history

chelnak · 2022-03-30T07:53:02Z

Fantastic thank you! I'll get this merged!

chelnak · 2022-03-30T08:20:24Z

@hajee I just had a second look prior to merge and noticed a couple of things.

Firstly, we like to keep the scope of PRs limited to their intention. I noticed you'd updated the ToC in this contribution. Please could you revert the change here? I'd be happy to accept it in another PR!

Secondly I've added a comment where I think there may be a small typo in the example

README.md

chelnak

Added a comment regarding the example

The current implementation only allows you to pass a file name to the certificate and private_key parameters. When you are fetching certificates from vault or another secure store, you'll first have to save them to a file. This is very innconveniant. This PR add's the parameters certificate_content and private_key_content. These parameters are mutually exclusive from their file counterparts. With this change, you can now fetch a certificate and/or a password from vault (through a hiera lookup for example) and use it directly on the type. Because these values can be sensitive, both of the new parameters support passing the value as a sensitive data type.

chelnak

Looks great! Thank you 🚀

hajee · 2022-03-31T05:40:56Z

@chelnak Thanks for merging. The next question is of course: When is a release planned?

chelnak · 2022-03-31T11:06:42Z

I'll see if we can get something out soon!

alexjfisher · 2024-10-23T08:42:56Z

lib/puppet/provider/java_ks/keytool.rb

+
+    # When no certificate file is specified, we infer the usage of
+    # certificate content and create a tempfile containing this value.
+    # we leave it to to the tempfile to clean it up after the pupet run exists.


or... anytime after this method exits leading to a race condition. See #425

hajee force-pushed the add_support_for_certificate_content branch from fa6e69e to 3dd4ecc Compare March 15, 2022 10:41

hajee force-pushed the add_support_for_certificate_content branch 2 times, most recently from 7198628 to bdee364 Compare March 15, 2022 14:53

hajee marked this pull request as ready for review March 15, 2022 14:53

hajee requested a review from a team as a code owner March 15, 2022 14:53

chelnak added the community label Mar 21, 2022

chelnak closed this Mar 22, 2022

chelnak reopened this Mar 22, 2022

chelnak closed this Mar 22, 2022

chelnak reopened this Mar 22, 2022

hajee force-pushed the add_support_for_certificate_content branch from bdee364 to cffcf77 Compare March 23, 2022 08:11

chelnak closed this Mar 29, 2022

chelnak reopened this Mar 29, 2022

chelnak self-requested a review March 29, 2022 14:50

chelnak previously approved these changes Mar 29, 2022

View reviewed changes

david22swan added the feature label Mar 29, 2022

hajee dismissed chelnak’s stale review via c153723 March 30, 2022 07:11

hajee force-pushed the add_support_for_certificate_content branch from cffcf77 to c153723 Compare March 30, 2022 07:11

bastelfreak previously approved these changes Mar 30, 2022

View reviewed changes

chelnak reviewed Mar 30, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

chelnak suggested changes Mar 30, 2022

View reviewed changes

hajee dismissed bastelfreak’s stale review via 2875855 March 30, 2022 08:28

hajee force-pushed the add_support_for_certificate_content branch from c153723 to 2875855 Compare March 30, 2022 08:28

chelnak self-requested a review March 30, 2022 20:53

chelnak approved these changes Mar 30, 2022

View reviewed changes

chelnak merged commit 9dfc366 into puppetlabs:main Mar 30, 2022

hajee deleted the add_support_for_certificate_content branch March 31, 2022 06:33

alexjfisher reviewed Oct 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for certificate_content and private_key_content parameters #385

Add support for certificate_content and private_key_content parameters #385

hajee commented Mar 15, 2022 •

edited

Loading

puppet-community-rangefinder bot commented Mar 15, 2022

CLAassistant commented Mar 15, 2022 •

edited

Loading

hajee commented Mar 22, 2022

chelnak commented Mar 22, 2022

puppet-community-rangefinder bot commented Mar 22, 2022

puppet-community-rangefinder bot commented Mar 22, 2022

chelnak commented Mar 23, 2022

hajee commented Mar 23, 2022

hajee commented Mar 29, 2022

puppet-community-rangefinder bot commented Mar 29, 2022

chelnak commented Mar 29, 2022

chelnak commented Mar 29, 2022

hajee commented Mar 30, 2022

chelnak commented Mar 30, 2022

chelnak commented Mar 30, 2022

chelnak left a comment

chelnak left a comment

hajee commented Mar 31, 2022

chelnak commented Mar 31, 2022 •

edited

Loading

alexjfisher Oct 23, 2024

Add support for certificate_content and private_key_content parameters #385

Add support for certificate_content and private_key_content parameters #385

Conversation

hajee commented Mar 15, 2022 • edited Loading

puppet-community-rangefinder bot commented Mar 15, 2022

java_ks is a type

This module is declared in 6 of 579 indexed public Puppetfiles.

CLAassistant commented Mar 15, 2022 • edited Loading

hajee commented Mar 22, 2022

chelnak commented Mar 22, 2022

puppet-community-rangefinder bot commented Mar 22, 2022

java_ks is a type

This module is declared in 6 of 579 indexed public Puppetfiles.

puppet-community-rangefinder bot commented Mar 22, 2022

java_ks is a type

This module is declared in 6 of 579 indexed public Puppetfiles.

chelnak commented Mar 23, 2022

hajee commented Mar 23, 2022

hajee commented Mar 29, 2022

puppet-community-rangefinder bot commented Mar 29, 2022

java_ks is a type

This module is declared in 6 of 579 indexed public Puppetfiles.

chelnak commented Mar 29, 2022

chelnak commented Mar 29, 2022

hajee commented Mar 30, 2022

chelnak commented Mar 30, 2022

chelnak commented Mar 30, 2022

chelnak left a comment

Choose a reason for hiding this comment

chelnak left a comment

Choose a reason for hiding this comment

hajee commented Mar 31, 2022

chelnak commented Mar 31, 2022 • edited Loading

alexjfisher Oct 23, 2024

Choose a reason for hiding this comment

hajee commented Mar 15, 2022 •

edited

Loading

This module is declared in 6 of 579 indexed public `Puppetfiles`.

CLAassistant commented Mar 15, 2022 •

edited

Loading

This module is declared in 6 of 579 indexed public `Puppetfiles`.

This module is declared in 6 of 579 indexed public `Puppetfiles`.

This module is declared in 6 of 579 indexed public `Puppetfiles`.

chelnak commented Mar 31, 2022 •

edited

Loading