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

chore: allow usage of https in URL for distro tree #219

Draft
wants to merge 1 commit into
base: python-3
Choose a base branch
from

Conversation

JohnVillalovos
Copy link
Collaborator

When specifying the URL for the location of the distro for a lab controller, allow usage of https URLs.

Previously you could add an https URL, but then when provisioning the system it would fail with the error:
Command 1234566 aborted: No usable URL found for distro tree 123 in lab lab1.example.com

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/allow_https branch 3 times, most recently from 4115e9c to 035c707 Compare June 14, 2024 19:00
When specifying the URL for the location of the distro for a lab
controller, allow usage of `https` URLs.

Previously you could add an `https` URL, but then when provisioning
the system it would fail with the error:
  Command 1234566 aborted: No usable URL found for distro tree 123 in lab lab1.example.com
@StykMartin
Copy link
Contributor

This is definitely not a chore, and I'm not fully on board with this change in its current form.
Changes to the implementation will favor HTTPs over HTTP. This can create a huge blast radius if HTTPs is present but hidden behind self-signed certificates or even public CAs with recent changes to the certificate chain (e.g. recent changes to DigiCert).

This code explicitly uses all non-TLS variants to make sure we can actually spin up resources.
From my perspective, this looks like a feature flag that needs to be explicitly enabled by the Beaker owner, as they need to make sure HTTPs is usable in their infrastructure.

WDYT @p3ck

@p3ck
Copy link
Contributor

p3ck commented Jun 26, 2024

This is definitely not a chore, and I'm not fully on board with this change in its current form. Changes to the implementation will favor HTTPs over HTTP. This can create a huge blast radius if HTTPs is present but hidden behind self-signed certificates or even public CAs with recent changes to the certificate chain (e.g. recent changes to DigiCert).

This code explicitly uses all non-TLS variants to make sure we can actually spin up resources. From my perspective, this looks like a feature flag that needs to be explicitly enabled by the Beaker owner, as they need to make sure HTTPs is usable in their infrastructure.

WDYT @p3ck

I agree that this should be turned on via a config flag.

Maybe support a config option to specify http_schemes

Default

config.http_schemes = ['http']
#config.http_schemes = ['https','http']

@JohnVillalovos JohnVillalovos marked this pull request as draft December 17, 2024 14:15
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.

3 participants