[PATCH 1/1] Fixed #1852 - Correct behaviour when no SELinux bindings

24 views
Skip to first unread message

James Turnbull

unread,
Jan 26, 2009, 6:03:30 PM1/26/09
to puppe...@googlegroups.com

Signed-off-by: James Turnbull <ja...@lovedthanlost.net>
---
CHANGELOG | 2 ++
lib/puppet/type/file/selcontext.rb | 8 ++++++++
spec/unit/type/file/selinux.rb | 6 ++++++
3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 30fc1c6..55915c2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
0.25.x
+ Fixed #1852 - Correct behaviour when no SELinux bindings
+
Updated Red Hat spec file 0.24.7

0.24.7
diff --git a/lib/puppet/type/file/selcontext.rb b/lib/puppet/type/file/selcontext.rb
index 22e3080..9900350 100644
--- a/lib/puppet/type/file/selcontext.rb
+++ b/lib/puppet/type/file/selcontext.rb
@@ -44,6 +44,14 @@ module Puppet
return property_default
end

+ def insync?(value)
+ if not selinux_support?
+ debug("SELinux bindings not found. Ignoring parameter.")
+ return true
+ end
+ super
+ end
+
def sync
self.set_selinux_context(@resource[:path], @should, name)
return :file_changed
diff --git a/spec/unit/type/file/selinux.rb b/spec/unit/type/file/selinux.rb
index a97f9bc..4c16fe5 100644
--- a/spec/unit/type/file/selinux.rb
+++ b/spec/unit/type/file/selinux.rb
@@ -74,6 +74,12 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
@sel.sync
end

+ it "should do nothing for insync? if no SELinux support" do
+ @sel.should = %{newcontext}
+ @sel.expects(:selinux_support?).returns false
+ @sel.insync?("oldcontext").should == true
+ end
+
after do
Puppet::Type.type(:file).clear
end
--
1.6.0.6

Paul Nasrat

unread,
Jan 26, 2009, 7:29:15 PM1/26/09
to puppe...@googlegroups.com
2009/1/26 James Turnbull <ja...@lovedthanlost.net>:
>
>
> Signed-off-by: James Turnbull <ja...@lovedthanlost.net>

Looks good but untested my end atm.

I'm wondering if this isn't a more general symptom of how handle
turning on and off of conditional types and providers being unclear.
Obviously lets fix this specific bug, but I think we're going to have
similar problems in other areas, and we need to think how to deal with
that.

Paul

Luke Kanies

unread,
Jan 26, 2009, 7:32:55 PM1/26/09
to puppe...@googlegroups.com

There is a more general ability, but it's predicated on provider
features, and files don't use providers. See the 'required features'
in various types in the type reference.

--
You only have to be open minded if you're wrong.
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

Paul Nasrat

unread,
Jan 26, 2009, 7:50:25 PM1/26/09
to puppe...@googlegroups.com
> There is a more general ability, but it's predicated on provider
> features, and files don't use providers. See the 'required features'
> in various types in the type reference.

Sure, I guess the language I'd use to describe it is that it is
conditional capabilities of the type(s). Files in particular are
particularly gnarly like that with xattr, data forks, selinux and all
sorts of things wanting to come to the party.

The ProviderFeatures class does that in one way, I was just really
just throwing the question out there that I didn't know if people
developing types/providers easily discovered how to do that.

Are there conditional things in non-providers I guess is the question,
my gut feeling is that over the life time of a collection of
enterprise systems that's going to be true.

Again this might not be a job to do now, but I think we need to think
about the problems of evolving systems under puppet as vendor upgrades
come in.

Paul

Luke Kanies

unread,
Jan 26, 2009, 10:43:26 PM1/26/09
to puppe...@googlegroups.com
On Jan 26, 2009, at 6:50 PM, Paul Nasrat wrote:

>
>> There is a more general ability, but it's predicated on provider
>> features, and files don't use providers. See the 'required features'
>> in various types in the type reference.
>
> Sure, I guess the language I'd use to describe it is that it is
> conditional capabilities of the type(s). Files in particular are
> particularly gnarly like that with xattr, data forks, selinux and all
> sorts of things wanting to come to the party.
>
> The ProviderFeatures class does that in one way, I was just really
> just throwing the question out there that I didn't know if people
> developing types/providers easily discovered how to do that.
>
> Are there conditional things in non-providers I guess is the question,
> my gut feeling is that over the life time of a collection of
> enterprise systems that's going to be true.

Well, providers essentially provide the differentation in the system -
types theoretically don't vary, only the providers.

That being said, it seems pretty likely that people don't necessarily
know about the provider features and the ability to use them to
contain functionality on types.

And really, the behaviour isn't necessarily entirely right - should
unsupported parameters fail, be ignored, or what?

>
> Again this might not be a job to do now, but I think we need to think
> about the problems of evolving systems under puppet as vendor upgrades
> come in.


How do vendor upgrades relate?

--
Today at work an ethernet switch decided to take the 'N' out of NVRAM
-- Richard Letts

Peter Meier

unread,
Jan 27, 2009, 4:40:49 AM1/27/09
to puppe...@googlegroups.com
Hi

> And really, the behaviour isn't necessarily entirely right - should
> unsupported parameters fail, be ignored, or what?

I think they should be ignored and this action should be logged. I think
there will always be differentiations from distro to distro, os to os.
However what I like on puppet that I don't have to care about that, as
it takes care of that.
However I think we have to differentiate.
For example looking at the package type, we have this nice feature
differentiation and puppet fails if you'd like to purge a package on
system which package-manager can't purge (afair, am I right?). But a
package being installed or purged is something quite fundamental to
define the state of a package resource. However on systems which don't
have SELinux (like BSD once) I don't care if SELinux can't be applied or
not, it's not part of the system hence not important to describe the
state of the resource. I would argue that the SELinux label is only
important to describe the state of a resource on systems which supports
SELinux.
On the other side I see that it will be hard to draw such a line, as
well that you can argue, that if you define SELInux types for a resource
and they can't be set, the resource is somehow not in the proper state.
But what I then see is /me ending up in adding conditional statements on
each file resource testing whether I can set the SELinux types or not.
And this is definitely not something with which I'd like to end up, nor
something I think that should be laid out to the manifests or the user.

So far my 2cents about that. Make sense, not or did I miss the point?

cheers pete

Luke Kanies

unread,
Jan 27, 2009, 10:05:04 AM1/27/09
to puppe...@googlegroups.com
On Jan 27, 2009, at 3:40 AM, Peter Meier wrote:

> Hi
>
>> And really, the behaviour isn't necessarily entirely right - should
>> unsupported parameters fail, be ignored, or what?
>
> I think they should be ignored and this action should be logged. I
> think
> there will always be differentiations from distro to distro, os to os.
> However what I like on puppet that I don't have to care about that, as
> it takes care of that.


This is the current behaviour, and most of the rest of your post was
just a reiteration of this, so I'll leave it alone for now, and we can
cover it in more detail later if we need to.

--
The salesman asked me what size I wore, I told him extra-medium.
-- Stephen Wright

Reply all
Reply to author
Forward
0 new messages