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

Allow Sensitive data type for secrets #331

Merged
merged 1 commit into from
May 9, 2024

Conversation

cocker-cc
Copy link
Contributor

No description provided.

@cocker-cc cocker-cc requested a review from a team as a code owner June 29, 2021 19:25
@puppet-community-rangefinder
Copy link

puppetdb::database::postgresql is a class

that may have no external impact to Forge modules.

puppetdb is a class

Breaking changes to this file WILL impact these 6 modules (exact match):
Breaking changes to this file MAY impact these 2 modules (near match):

puppetdb::server is a class

that may have no external impact to Forge modules.

puppetdb::server::database is a class

that may have no external impact to Forge modules.

puppetdb::server::read_database is a class

that may have no external impact to Forge modules.

puppetdb::server::validate_db is a class

that may have no external impact to Forge modules.

puppetdb::server::validate_read_db is a class

that may have no external impact to Forge modules.

This module is declared in 33 of 576 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
Copy link

CLAassistant commented Sep 1, 2021

CLA assistant check
All committers have signed the CLA.

metadata.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

A few more extra Optional[...] seems to be present.

Also, while we are improving this, what about making the default Sensitive too in params.pp?

$foo = Sensitive('bar')

manifests/init.pp Outdated Show resolved Hide resolved
manifests/server.pp Outdated Show resolved Hide resolved
manifests/server/read_database.pp Outdated Show resolved Hide resolved
@cocker-cc
Copy link
Contributor Author

A few more extra Optional[...] seems to be present.

I removed Optional. Indeed it was unnecessary.

Also, while we are improving this, what about making the default Sensitive too in params.pp?
$foo = Sensitive('bar')

I do not see this necessary, as the Default-Value is publicly visible anyway.

@smortex
Copy link
Collaborator

smortex commented Sep 22, 2023

I do not see this necessary, as the Default-Value is publicly visible anyway.

My understanding is that it helps to have Puppet automatically redact secrets in diff, and that at some point only a Sensitive would be accepted in a future major version. That being said, we have default passwords in this module which is not a best practice so maybe this will not happen before a loooong time 😄.

I am fine with the PR as it is, so will let other reviewers tell what they think!

Thank you!

@smortex smortex changed the title Use Datatype Sensitive for Secrets Allow Sensitive data type for secrets Sep 30, 2023
@cocker-cc cocker-cc requested a review from h0tw1r3 as a code owner May 8, 2024 16:41
@h0tw1r3 h0tw1r3 merged commit 17c83e2 into puppetlabs:main May 9, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants