From c6e87d9655617d7d4b334b3e565cdac3abe443b0 Mon Sep 17 00:00:00 2001 From: nimisha Date: Mon, 30 Jan 2017 18:13:02 +0530 Subject: Fixing review comments Signed-off-by: nimisha --- lib/chef/mixin/user_identity.rb | 90 ----------------------------------------- lib/chef/provider/execute.rb | 12 ------ lib/chef/provider/script.rb | 8 ++-- lib/chef/resource/execute.rb | 90 ++++++++++++++++++++++++++++------------- 4 files changed, 65 insertions(+), 135 deletions(-) delete mode 100644 lib/chef/mixin/user_identity.rb (limited to 'lib') diff --git a/lib/chef/mixin/user_identity.rb b/lib/chef/mixin/user_identity.rb deleted file mode 100644 index bc8626ac65..0000000000 --- a/lib/chef/mixin/user_identity.rb +++ /dev/null @@ -1,90 +0,0 @@ -# -# Author:: Adam Edwards () -# Copyright:: Copyright (c) 2016 Chef Software, Inc. -# License:: Apache License, Version 2.0 -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -class Chef - module Mixin - module UserIdentity - - def validate_identity(specified_user, password = nil, specified_domain = nil) - validate_identity_platform(specified_user, password, specified_domain) - validate_identity_syntax(specified_user, password, specified_domain) - end - - def validate_identity_platform(specified_user, password = nil, specified_domain = nil) - if ! Chef::Platform.windows? - if password || specified_domain - raise Exceptions::UnsupportedPlatform, "Values for `domain` and `password` are only supported on the Windows platform" - end - else - if specified_user && password.nil? - raise ArgumentError, "A value for `password` must be specified when a value for `user` is specified on the Windows platform" - end - end - end - - def validate_identity_syntax(specified_user, password = nil, specified_domain = nil) - identity = qualify_user(specified_user, specified_domain) - - if ( password || identity[:domain] ) && identity[:user].nil? - raise ArgumentError, "A value for `password` or `domain` was specified without specification of a value for `user`" - end - end - - def qualify_user(specified_user, specified_domain = nil) - domain = specified_domain - user = specified_user - - if specified_user.nil? && ! specified_domain.nil? - raise ArgumentError, "The domain `#{specified_domain}` was specified, but no user name was given" - end - - # if domain is provided in both username and domain - if specified_user && ((specified_user.include? '\\') || (specified_user.include? "@")) && specified_domain - raise ArgumentError, "The domain is provided twice. Username: `#{specified_user}`, Domain: `#{specified_domain}`. Please specify domain only once." - end - - if ! specified_user.nil? && specified_domain.nil? - # Splitting username of format: Domain\Username - domain_and_user = user.split('\\') - - if domain_and_user.length == 2 - domain = domain_and_user[0] - user = domain_and_user[1] - elsif domain_and_user.length == 1 - # Splitting username of format: Username@Domain - domain_and_user = user.split("@") - if domain_and_user.length == 2 - domain = domain_and_user[1] - user = domain_and_user[0] - elsif domain_and_user.length != 1 - raise ArgumentError, "The specified user name `#{user}` is not a syntactically valid user name" - end - end - end - - { domain: domain, user: user } - end - - private(:validate_identity) - private(:validate_identity_platform) - private(:validate_identity_syntax) - private(:qualify_user) - - end - end -end diff --git a/lib/chef/provider/execute.rb b/lib/chef/provider/execute.rb index 5494405a02..a605d9f7ec 100644 --- a/lib/chef/provider/execute.rb +++ b/lib/chef/provider/execute.rb @@ -19,15 +19,12 @@ require "chef/log" require "chef/provider" require "forwardable" -require "chef/mixin/user_identity" class Chef class Provider class Execute < Chef::Provider extend Forwardable - include Chef::Mixin::UserIdentity - provides :execute def_delegators :@new_resource, :command, :returns, :environment, :user, :domain, :password, :group, :cwd, :umask, :creates @@ -43,10 +40,6 @@ class Chef def define_resource_requirements # @todo: this should change to raise in some appropriate major version bump. - requirements.assert(:all_actions) do |a| - a.assertion { validate_identity(new_resource.user, new_resource.password, new_resource.domain) } - end - if creates && creates_relative? && !cwd Chef::Log.warn "Providing a relative path for the creates attribute without the cwd is deprecated and will be changed to fail in the future (CHEF-3819)" end @@ -59,11 +52,6 @@ class Chef end def action_run - # parse username if it's in the following format: domain/username or username@domain - identity = qualify_user(new_resource.user, new_resource.domain) - new_resource.user identity[:user] - new_resource.domain identity[:domain] - if creates && sentinel_file.exist? Chef::Log.debug("#{new_resource} sentinel file #{sentinel_file} exists - nothing to do") return false diff --git a/lib/chef/provider/script.rb b/lib/chef/provider/script.rb index 5df31786bd..fca9cea467 100644 --- a/lib/chef/provider/script.rb +++ b/lib/chef/provider/script.rb @@ -67,14 +67,14 @@ class Chef end def set_owner_and_group - if ! Chef::Platform.windows? + if Chef::Platform.windows? + # And on Windows also this is a no-op if there is no user specified. + grant_alternate_user_read_access + else # FileUtils itself implements a no-op if +user+ or +group+ are nil # You can prove this by running FileUtils.chown(nil,nil,'/tmp/file') # as an unprivileged user. FileUtils.chown(new_resource.user, new_resource.group, script_file.path) - else - # And on Windows also this is a no-op if there is no user specified. - grant_alternate_user_read_access end end diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb index d0e4d958f6..cf81a24cec 100644 --- a/lib/chef/resource/execute.rb +++ b/lib/chef/resource/execute.rb @@ -135,37 +135,11 @@ class Chef ) end - def user(arg = nil) - set_or_return( - :user, - arg, - :kind_of => [ String, Integer ] - ) - end - - def domain(arg = nil) - set_or_return( - :domain, - arg, - :kind_of => [ String ] - ) - end + property :user, [ String, Integer ] - def password(arg = nil) - set_or_return( - :password, - arg, - :kind_of => [ String ] - ) - end + property :domain, String - def sensitive(args = nil) - if ! password.nil? - true - else - super - end - end + property :password, String, sensitive: true def self.set_guard_inherited_attributes(*inherited_attributes) @class_inherited_attributes = inherited_attributes @@ -183,6 +157,64 @@ class Chef ancestor_attributes.concat(@class_inherited_attributes ? @class_inherited_attributes : []).uniq end + def after_created + validate_identity_platform(user, password, domain) + identity = qualify_user(user, password, domain) + domain(identity[:domain]) + user(identity[:user]) + end + + def validate_identity_platform(specified_user, password = nil, specified_domain = nil) + if Chef::Platform.windows? + if specified_user && password.nil? + raise ArgumentError, "A value for `password` must be specified when a value for `user` is specified on the Windows platform" + end + else + if password || specified_domain + raise Exceptions::UnsupportedPlatform, "Values for `domain` and `password` are only supported on the Windows platform" + end + end + end + + def qualify_user(specified_user, password = nil, specified_domain = nil) + domain = specified_domain + user = specified_user + + if specified_user.nil? && ! specified_domain.nil? + raise ArgumentError, "The domain `#{specified_domain}` was specified, but no user name was given" + end + + # if domain is provided in both username and domain + if specified_user && ((specified_user.include? '\\') || (specified_user.include? "@")) && specified_domain + raise ArgumentError, "The domain is provided twice. Username: `#{specified_user}`, Domain: `#{specified_domain}`. Please specify domain only once." + end + + if ! specified_user.nil? && specified_domain.nil? + # Splitting username of format: Domain\Username + domain_and_user = user.split('\\') + + if domain_and_user.length == 2 + domain = domain_and_user[0] + user = domain_and_user[1] + elsif domain_and_user.length == 1 + # Splitting username of format: Username@Domain + domain_and_user = user.split("@") + if domain_and_user.length == 2 + domain = domain_and_user[1] + user = domain_and_user[0] + elsif domain_and_user.length != 1 + raise ArgumentError, "The specified user name `#{user}` is not a syntactically valid user name" + end + end + end + + if ( password || domain ) && user.nil? + raise ArgumentError, "A value for `password` or `domain` was specified without specification of a value for `user`" + end + + { domain: domain, user: user } + end + set_guard_inherited_attributes( :cwd, :environment, -- cgit v1.2.1