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

home folder and ssl sert #21

Closed
wants to merge 1 commit into from
Closed

Conversation

EvgenIn
Copy link

@EvgenIn EvgenIn commented Nov 6, 2014

Added selenium home folder managment and no SSL cert check.
puppet-lint alingment

Added selenium home folder managment and no SSL cert check.
@@ -24,7 +24,8 @@
include wget

user { $user:
gid => $group,
gid => $group,
managehome => true,
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this needed? I'd rather that the role user optionally be defiend external to the module rather than adding a bunch user management features.

@jhoblitt
Copy link
Owner

jhoblitt commented Nov 6, 2014

Just FYI - it's considered bad form to have to unrelated changes as part of the same commit.

timeout => $download_timeout,
execuser => $user,
require => File[$jar_path],
nocheckcertificate => true,
Copy link
Owner

Choose a reason for hiding this comment

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

What problem is this trying to resolve? https://selenium-release.storage.googleapis.com/ is returning a public CA signed cert.

@jhoblitt
Copy link
Owner

@EvgenIn ping?

@EvgenIn
Copy link
Author

EvgenIn commented Nov 25, 2014

I apologize for the delayed answer. Didn't notice the previous msg.
The issue I was trying to resolve with the "nocheckcertificate => true" happened on an official CentOS 6.5 AMI on EC2. I can try to reproduce the error if you are interested.

@jhoblitt
Copy link
Owner

That would be great.

@EvgenIn
Copy link
Author

EvgenIn commented Nov 25, 2014

It seems that all is working now, without any errors. I now think that the certificate issue was on the server side, not the client. I guess this change can be ignored now.

@jhoblitt
Copy link
Owner

That or a MITM attempt (seems unlikely on AWS) would be my guess.

What about the change to the user resource? Do we need to be more explicit about how that role account is setup? Setting the user/group as system => true probably won't hurt.

@EvgenIn
Copy link
Author

EvgenIn commented Nov 25, 2014

To my knowledge, selenium user must have a home folder for the server to function.
Taking into consideration that this module should build a fully working selenium setup, I thought that creating a home folder (not just the user) would be a desirable feature.

PS: Creating the 'selenium' user as a system user sounds like a good idea.

@zivan
Copy link
Contributor

zivan commented Jan 30, 2015

@jhoblitt i agree with @EvgenIn

maybe create system user 'selenium' was best decision because into defaults params use user/group 'selenium' but this user/group not exist into system.

i using the vagrant&puppet (vagrant use virtualbox centos6.5-64) and i set
https://github.com/jhoblitt/puppet-selenium#user & https://github.com/jhoblitt/puppet-selenium#group the 'vagrant'

@EvgenIn if your use Amazon EC2 should set default ec2 user/group 'ec2-user' it work well for me.

anyway create system user 'selenium' or not it decision for @jhoblitt

to be, or not to be, creating the system user 'selenium' into this module

@jhoblitt
Copy link
Owner

@EvgenIn / @zivan user/group management was made optional by 2911fa6 allowing management of the user and/or group resource(s) external to the module. This is is vastly more flexible than trying to expose parameters to those resources via the selenium class.

I'm going to go ahead and close this PR out (yikes has it been open for a long time). I would be happy to merge a PR to set the system attribute on the default user/group resources.

@jhoblitt jhoblitt closed this Apr 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants