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

Allow adding config.toml to Selenium-Hub #1757

Merged
merged 2 commits into from
Dec 29, 2022

Conversation

ghost
Copy link

@ghost ghost commented Dec 27, 2022

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

We now read the config file as well. The base Dockerfile creates the config.toml every time & leaves it empty. Hence, there will be no issues, if a user decides to not use the config.toml.

Motivation and Context

It is not possible to load a personalized config.toml for the Selenium-Hub, hence adding things like basic auth is not easily possible from the outside (as asked for in #1754 ).

Screenshots

locked

unlocked

This PR enables users to insert their own config.toml as they wish.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jamesmortensen
Copy link
Member

Hi @xcalizorz thanks for working on this. The screenshots demonstrate that it seems to be working.

I have observed that all of the user-editable files (sample docker-compose.yml and other sample files), are generally kept in the root. I think one advantage is that images won't get accidentally built with the modified example file. Because it's in the Hub folder, I believe the example-config.toml would still get copied into the image even if one weren't explicitly using the custom config.

What do you think about creating a generate_config file to generate the config.toml, similar to what we have in the Standalone folder? Before I saw your pull request, I was playing around with something like this as a derivative image:

append-basic-auth:

echo "[router]" >> "$FILENAME"
echo "username = \"$SEL_AUTH_USERNAME\"" >> "$FILENAME"
echo "password = \"$SEL_AUTH_PASSWORD\"" >> "$FILENAME"

Dockerfile:

FROM seleniarm/hub:latest

USER root

COPY append-basic-auth /opt/bin/
COPY start-selenium-grid-hub.sh /opt/bin/

RUN cat /opt/bin/append-basic-auth >> /opt/bin/generate_config && chmod 755 /opt/bin/generate_config

USER seluser

Then inside of start-selenium-grid-hub.sh I added:

FILENAME=/opt/selenium/config.toml /opt/bin/generate_config

and I then added the --config flag to the java command so the Hub would read it.

The only thing I didn't figure out was how to make the auth variables optional yet, since I was making a derivative image.

Will any of this help? What do you think?

@ghost
Copy link
Author

ghost commented Dec 29, 2022

@jamesmortensen sounds interesting, I'll give it a try when I have time later this week.

If we want to ignore files in from the build, I could just add a simple .dockerignore file to the ./Hub that ignores the example config file. The config.toml will be added to the image anyway (via ./Base/Dockerfile) - the default is empty and can be changed by overriding via Volumes/Mounting.

What do you think about solving this via .dockerignore?

@robinmatz
Copy link

Hey @xcalizorz and @jamesmortensen ,

I have been following this conversation. I was wondering if the changes you are about to make will also support adding config other than basic auth, like relaying to an appium server (https://www.selenium.dev/documentation/grid/configuration/toml_options/#relaying-commands-to-a-service-endpoint-that-supports-webdriver), which would be the use case I happen to be interested in?

@ghost
Copy link
Author

ghost commented Dec 29, 2022

@robinmatz you can add any config you want. My example config is using basic auth, but as long as you are able to add it into the config.toml you can do whatever you want.

Before this PR the Selenium hub would practically ignore the config.toml, now it's reading the config and acting upon it. As a user, you just have to override the default empty config as shown in the example here: https://github.com/SeleniumHQ/docker-selenium/pull/1757/files#diff-a627c055a568a3c52aef1a4363aefcc87d8f5b0595ad5d1714693d902bc23142R44

@robinmatz
Copy link

Hey @xcalizorz ,
thanks for clarifying! I might have misunderstood James' comment

The only thing I didn't figure out was how to make the auth variables optional yet, since I was making a derivative image.

which made me unsure if allowing basic auth was the only objective of this PR.

@jamesmortensen
Copy link
Member

Hi @xcalizorz based on @robinmatz feedback and me doing a little more digging on my part, I can see that example-config.toml is not passed to the image during build time as I had mistakenly thought. (I was confusing docker context with the actual image being built).

I also validated that the grid works both with the config mapped via the volume property, with the username and password, and that it works without the config.toml and with no authentication and is backwards compatible. I also confirmed that when the config.toml is included, the username and password is required in order to access the Grid.

Thus, I'll go ahead and merge these changes so that they'll be available in the next release.

@jamesmortensen jamesmortensen merged commit 23a351a into SeleniumHQ:trunk Dec 29, 2022
@ErcinDedeoglu
Copy link

Fantastic timing; I was investigating this issue :D
When will it be released?

@omdhimar28
Copy link

@jamesmortensen Looking forward for the release :)
When it will be released?

@ErcinDedeoglu
Copy link

cc @diemol

@jamesmortensen
Copy link
Member

FYI: 4.8.0 was just released yesterday. So the ability to add the config.toml to the Hub should be available now. @omdhimar28 @ErcinDedeoglu :)

https://github.com/SeleniumHQ/docker-selenium/releases/tag/4.8.0-20230123

@omdhimar28
Copy link

Thanks for the update @jamesmortensen :)

prashanth-volvocars pushed a commit to prashanth-volvocars/docker-selenium that referenced this pull request Jun 29, 2023
* Use config.toml in hub as well

Fixes SeleniumHQ#1754

* Add docker-compose demonstrating usage of config.toml

Co-authored-by: Xcalizorz <[email protected]>
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.

4 participants