Skip to content

Feat: support grid basic auth on helm chart #1894

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

Merged
merged 4 commits into from
Aug 7, 2023
Merged

Feat: support grid basic auth on helm chart #1894

merged 4 commits into from
Aug 7, 2023

Conversation

trungprivate
Copy link
Contributor

Description

Simple change to allow Selenium Grid basic auth to be enabled/disabled through the values.yaml file when deploying via the Helm chart.

Motivation and Context

Currently, it is not straightforward to enable basic auth because:

  1. Selenium grid URLs used for internal communication between components are calculated using helpers (_helpers.tpl) that do not take basic auth into consideration.
  2. Autoscaling HPA URL is determined using (1) - therefore autoscaling will not work when basic auth is enabled unless HPA URL is explicitly set without using helpers. This causes duplication of configuration.
  3. Basic auth settings need to be applied to either the hub or router, depending on isolated components set up

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.

Summary of changes:

  1. Updated values.yaml template file to add new configuration items for basic auth
  2. Updated helpers to correctly set seleniumGrid.url and seleniumGrid.graphqlURL when basic auth is enabled
  3. Updated hub and router deployment templates to incorporate basic auth environment variables if basic auth is enabled

Updated helm chart templates to allow basic auth to be enabled/disabled
through values.yaml
Updated helm chart templates to allow basic auth to be enabled/disabled
through values.yaml
@CLAassistant
Copy link

CLAassistant commented Jul 17, 2023

CLA assistant check
All committers have signed the CLA.

@diemol
Copy link
Member

diemol commented Aug 1, 2023

I don't follow. Basic Auth in Grid is meant for the public endpoints, why do you say it is needed for internal components?

@trungprivate
Copy link
Contributor Author

Hi @diemol - It is because the HPA URL needs to have the basic auth details in the URL. Please refer to this issue for more details: #1890

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Thank you

@diemol diemol merged commit 861383c into SeleniumHQ:trunk Aug 7, 2023
@trungprivate trungprivate deleted the feat--support-grid-basic-auth-on-helm-chart branch September 1, 2023 03:57
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