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

(PUP-8577) Migrate SELinux to a seperate module #1

Merged
merged 24 commits into from
Jun 12, 2018

Conversation

melissa
Copy link
Contributor

@melissa melissa commented May 31, 2018

No description provided.

melissa added 22 commits May 24, 2018 17:00
Error: puppet-syntax: UTF-8: Could not parse for environment production:
invalid byte sequence

SELinux uses the `.pp` extension, which stands for "Policy Package".
This is great, except that puppet-syntax expects files that have the
`.pp` extension to be puppet manifests. This commit simply removes the
extension from this file. It's only used as the return value for a stub,
so it is safe for us to remove the extension for the sake of testing.
Mocha is very old, we should be using the mocking abilities that are now
native in rspec
Fix rubocop convention violation RSpec/InstanceVariable

Update the spec tests to mock with rspec and not mocha
We do not run tests on windows for this module, and we only support ruby
>= 2.3 so do not need to test old ruby versions
@@ -0,0 +1,153 @@
# Manage SELinux context of files.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshcooper should I be pulling this file as well, or no? k5login uses it (which will be a dependency we can specify when we pull k5login out), but it's also loaded as a part of the file type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@branan @joshcooper was saying you had thoughts on selinux extraction? Is this a file we can move out to the module, or should we keep this in the puppet repo?

Copy link

@branan branan Jun 11, 2018

Choose a reason for hiding this comment

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

This needs to stay in Puppet - it's related to how the file type handles selinux permissions on files. Some selinux bits for core types will always have to stay in Puppet itself (until we figure out how to modularize the really core types)

I'm also /kind/ of inclined to leave the feature in core, since it's super lightweight and might be useful when adding selinux awareness to other types/providers. But I don't totally have a use-case yet, so as long as bringing it back later won't break anything, I'm fine pulling it out for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 sweet, I'll pull those out

There were a few files that had made their way into this module that
should stay in the puppet code base. This commit removes those files.

I had also missed a few spec files which we do want in the module. So
this commit pulls those in.

We are not using any of the helper methods form
spec/lib/puppet_spec/files.rb, so that file, and references to it, have
also been removed.
.rspec Outdated
@@ -0,0 +1,2 @@
--color
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we been checking the .rspec file into the other extracted modules? Seems like it should be part of the .gitignore so people working on the module can have whatever they prefer in there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is added by PDK, so yes, it's in all the other modules

We don't want to keep these files in the repo
@jhelwig jhelwig merged commit f97be45 into puppetlabs:master Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants