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

feat: Auto-provision domain for TLS Edges #386

Merged
merged 2 commits into from
May 29, 2024

Conversation

jonstacks
Copy link
Collaborator

contributes to #375. Pulling this change out for easier review.

What

The hostports for a given TLS Edge need to be reserved as domains in ngrok before they can be used. Because of this, it makes sense that the TLSEdge handles reserving the domains if they aren't already reserved in the same way that the TCPEdge controller will take care of reserved addrs and the same way that the ingress controller builds the list of domains to reserve in its store across namespaces.

How

Parse out the hostports defined in the TLSEdge and automatically find or reserve the necessary domains. Also, populate a new field cnameTargets map in the TLSEdge's Status that contains the CNAME target from the domains. This will allow other controllers which may create TLSEdges to use the CNAMETargets map. For example, Services can use this to update the .status.loadBalancer.ingress fields.

Also handles dangling CNAME records more gracefully in the domain controller by retrying them.

Breaking Changes

No

@jonstacks jonstacks self-assigned this May 29, 2024
@jonstacks jonstacks requested a review from a team as a code owner May 29, 2024 19:34
@github-actions github-actions bot added area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart labels May 29, 2024
Copy link
Contributor

@bobzilladev bobzilladev left a comment

Choose a reason for hiding this comment

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

Couple comments, nothing blocking.

internal/controller/ingress/tlsedge_controller.go Outdated Show resolved Hide resolved
internal/controller/ingress/tlsedge_controller.go Outdated Show resolved Hide resolved
The hostports for a given TLS Edge need to be reserved in ngrok
before they can be used. Because of this, it makes sense that
the TLSEdge handles reserving the domains if they aren't already
reserved in the same way that the TCPEdge controller will take care
of reserved addrs.
@jonstacks jonstacks force-pushed the tls-edge-controller-reserves-domain branch from b47fb97 to 87a7333 Compare May 29, 2024 21:48
@jonstacks jonstacks merged commit aeb9e92 into ngrok:main May 29, 2024
7 checks passed
@jonstacks jonstacks deleted the tls-edge-controller-reserves-domain branch May 29, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants