Hi Nirmal,
I have a few comments:
usr/src/lib/install_common/__init__.py.src:
- line 114: I think it is more accurate for this comment of this function
to be: "exception raised when the user does not have a specified
authorization".
- line 115: Can we pass in the authorization that is checked as an argument
in the __init__() function? That way, the string that is constructed
can include that information.
- line 543-545: This name of this function is check_auth(), and even the
comment
of the function says it will check for whether the user has the
specified authorization.
I am surprised that see that in addition to the authorization, the euid
is also checked.
If it's intentional for the euid to be also checked, I think the name of
the function as well
as the comment should be updated to reflect what exactly it is doing.
Testing:
-----------
The detail manual testing file is very useful for me to understand what
tests are run.
Did you confirm that you can do an AI install successfully using a
service that's created by a user
that's assigned these profiles?
Thanks,
--Karen
On 04/17/12 10:33 AM, Nirmal Agarwal wrote:
Hi Sue, Karen
Thanks for review,
I have fixed comments provided.
Please find the updated webrev :
diff webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev1-diff/webrev.diff/
webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev1/webrev/
Slim-test results :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/slim-test
Detailed Manual tests :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/manual-testing
Pep8 is clean.
Ran pylint and removed "unused imports".
Test cases : I will file a separate CR and write test case for
functions/class introduced.
Let me know if I need to run other tests.
Thanks
Nirmal
On 04/12/12 04:24, Sue Sohn wrote:
On 04/11/12 03:41 AM, Nirmal Agarwal wrote:
Hi all
Can I please get the code review for CR 7108281
7108281 create RBAC execution profiles for installadm
Webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev0/webrev/
<https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev0/webrev/;jsessionid=9662DF16CB545E8304942C4DC3F2DA3A>
Hi Nirmal,
In your manual testing, did you test installadm-convert?? I don't see it
listed.
Same question for validate_profile and export.
Also, did you test the commands with both with a profile with proper
privileges and one that didn't have them?
installadm_common.py
205 I think it would be better to raise some other exception from
check_auth and then have the various subcommands do the try/except and
raise the SystemExit if desired.
208 I'm not seeing where this is done?
205 and/or 226: Is it possible to add a unit test for this in
test_installadm_common.py?
test_create_profile.py
47 alphabetize import
221 typo: "do noting"
503 should this be reverted in the tearDown method?
create_client.py
37 this line should be moved back to where it was as it is an internal
module
delete_client.py
28 Don't think you need to import os anymore
delete_service.py
28 same here
Thanks,
Sue
As a part of this CR, following authorizations for installadm will be
introduced.
1) solaris.installadm.client:RO::Administer Automated Install Clients::
2)solaris.installadm.manifest:RO::Administer Automated Install
Manifests::
3)solaris.installadm.profile:RO::Administer System Configuration
Profiles::
4)solaris.installadm.service:RO::Administer Automated Install
Services::
Testing :
Pep8 clean
Slim test results :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/slim-test
Manual Testing :
Below commands were tested :
--> create-service
--> create-service -n /export/home/test2 -i 192.168.56.4 -c 2 -n test2
--> create-service -d /export/home/test3 -i 192.168.56.7 -c 2 -n
test3 -s
/var/tmp/sol-11_1-12-ai-sparc.iso
--> create-service -t test3 -n test4
--> create-manifest -m test5 -n test4 -f
/usr/share/auto_install/manifest/default.xml
--> set-criteria -m test5 -n test4 -c hostname="test.com"
--> update-manifest
--> delete-manifest
--> set-criteria -p
--> create-profile
--> delete-profile
--> create-client
--> delete-client
--> disable (service)
--> enable (service)
PSARC detail : PSARC/2012/139 FastTrack ( under review)
Ethan : Thanks for all the guidance
Thanks
Nirmal
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
