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

Add missing paths to allowed module layout #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seanmil
Copy link

@seanmil seanmil commented Apr 24, 2024

No description provided.

@@ -45,6 +45,12 @@ already explains what a module is and what it can contain.
| `/lib/puppet/type/` | Custom types |
| `/lib/puppet/provider/` | Custom provider for one or multiple types |
| `/lib/puppet/functions/` | Modern functions in Ruby for the new API |
| `/lib/puppet/datatypes/` | Custom Puppet Datatypes |
Copy link
Contributor

Choose a reason for hiding this comment

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

is that the same as /lib/puppet/type/?

Copy link
Author

Choose a reason for hiding this comment

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

No, /lib/puppet/type/ is for Resource Types. /lib/puppet/datatypes/ is for custom data types. "Type" is overused in Puppet, but ah well. You can see an example here: https://github.com/hlindberg/tahu/tree/master/lib/puppet/datatypes/tahu

Copy link
Contributor

Choose a reason for hiding this comment

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

ahhh those types! :D

Copy link
Author

Choose a reason for hiding this comment

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

I pushed an update which clarifies "Resource types" in the type and provider lines.

@bastelfreak
Copy link
Contributor

I'm not sure which schema/changelog Puppet likes to have. the bottom of the page mentions it. should we add a new section with a changelog and increase the schema version?

Also, clarify Resource Types vs. Data types in the
various path descriptions.
| `/lib/puppet/datatypes/` | Custom Puppet Data types |
| `/lib/puppet/face/` | Custom Puppet Faces |
| `/lib/puppet/feature/` | Custom Puppet Features for providers |
| `/lib/puppet/property/` | Custom Puppet Properties for Types/Providers |
Copy link
Contributor

Choose a reason for hiding this comment

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

There's also /lib/puppet/parameters

Copy link
Contributor

Choose a reason for hiding this comment

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

Also /lib/puppet/applications and /lib/puppet/reports

| `/lib/puppet/type/` | Custom types |
| `/lib/puppet/provider/` | Custom provider for one or multiple types |
| `/lib/puppet/type/` | Custom Resource types |
| `/lib/puppet/provider/` | Custom provider for one or multiple Resource types |
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, modules can put whatever they want in the /lib directory and it will be packaged in the module and pluginsynced to agents. So from an allowlist perspective, listing individual /lib/puppet/* directories is misleading. But from a "what are the various extension points" perspective, it's informative.

Copy link
Contributor

@bastelfreak bastelfreak Apr 24, 2024

Choose a reason for hiding this comment

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

Technically, modules can put whatever they want in the /lib directory

is that something that we should allow here? Do you have anything in mind that isn't already covered by the different /lib/puppet/ subdirs?

Copy link
Contributor

Choose a reason for hiding this comment

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

is that something that we should allow here?

I think we should allow anything under /lib since that's how puppet behaves today. For example, there was some early confusion about /lib/puppetx vs /lib/puppet_x. And given /lib/augeas there may be some others?

In the future, I think it would be good to document which extension points are allowed in the /lib directory and add a deprecation warning to if we detect a dir/file that isn't allowed during pluginsync. Then in a major release, only pluginsync files that are in the allow list.

Copy link
Contributor

Choose a reason for hiding this comment

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

add a deprecation warning to if we detect a dir/file that isn't allowed during pluginsync

Do you think it's realistic to introduce the warnings in Puppet 8 or maybe with Puppet 9.0?

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.

3 participants