Hi John,

> On Jun 29, 2018, at 7:37 PM, John Kennedy <[email protected]> wrote:
> 
> Hi Igor,
> 
> On Fri, Jun 29, 2018 at 8:55 AM Igor Kozhukhov <[email protected]> wrote:
>> right now, by step in zfstest - we do symbolic links to tempory directory 
>> and update PATH to use it.
>> 
>> but - we are using 'su user -c "<cmd line>" - where we use another PATH - it 
>> is not the same as we defined.
>> 
>> as example: zfs-tests/tests/functional/acl/cifs/cifs_attr_001_pos
>> 
>> it use 'su' in some places where we use 'chmod' from another location - it 
>> is not the same what we linked to temporary PATH.
>> 
>> it is one example what we can’t trust to linked tools to temporary directory 
>> with PATH.
>> 
>> we have a lot of another scenarios where we can use another tools from 
>> another locations and tests can produce incorrect resuts.
> 
> The su problem is definitely a bug; thanks for finding it. I would
> like to know what the other scenarios are. If you file an issue in the
> illumos tracker, I'll help you fix them.

i have no filed this issue with 'su' yet - i try to investigate zfs tests 
issues on my env and try to report about findings by email.

easy way to test it by:
- copy original binaries to /var/tempdir/* with structure what you are linking 
by 'ln -s' to temprary dir for PATH - check your links to tools at <temp 
path>/<tool> - it should be linked from  /var/tempdir/*
- rename 'chmod'  to /usr/bin/chmod.bin - prepare wrapper /usr/bin/chown where 
you can print $PATH - the same for others tools - echo >> /var/tmp/<tool>.txt
but you need to do rename to every tool - to be sure you can right to think 
about correct PATH.

and produce tests - you can find how often you try to use tools from original 
PATH instead of from your predefined one.

and again: with 'sudo' you should do not forgot disable options in 
'/etc/sudoers'
# Defaults      env_reset
# Defaults      
secure_path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"

because 'sudo -E <cmd>' will not work with your predefined PATH.

>> based on these issues i’d like to start activity again to use full static 
>> path to tools like CHMOD=/usr/bin/chmod - where we can be 100% sure we are 
>> using what we want to use.
>> 
>> current zfs tests define tools by symbolic links to temporary directory and 
>> try to use it by providing PATH, but we can’t trust to this PATH and time to 
>> time we are using tools from another location - it is not good and we can’t 
>> trust to results.
> 
> I respectfully disagree. The constrained path has other benefits which
> we have talked about before. I think the bug above can be solved
> without throwing out what's been done here.

yes - it was discussed and right now it can produce a lot of issues on another 
env with this scheme - where you need to use illumos versions of tools.
but - by another env - these issues are detected and i can point to it where 
they are incorrectly are using.

you just try to trust your env and try to think - you are using all tools what 
you want to use - but - it is not correct - and have to spend more time try to 
hack env to be sure are you using correct versions of tools.

i think - more easy will be back to previous scheme - where you defind tool 
with full path and you exectly know - you can using tool, what you defined: 
ZPOOL=/sbin/zpool - and nothing else can change it with PATH.

but - it is my opinion.

i try to go to back to this idea because i try to play with zfs tests on DilOS 
and Debian env - and i see a lot of issues with PATH instead of full path to 
tools.

-Igor

> --
> John Wren Kennedy
> http://www.delphix.com/

------------------------------------------
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T73c62864791e915c-M1c3de25e461a7ff9c78e5afd
Delivery options: https://openzfs.topicbox.com/groups

Reply via email to