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

Warp Support #78

Open
KAllan357 opened this issue Mar 28, 2023 · 8 comments
Open

Warp Support #78

KAllan357 opened this issue Mar 28, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@KAllan357
Copy link
Contributor

I was wondering if you'd consider adding support for Warp.

Using this operator, we could allow the tunnel to be created and configured in K8s. Maybe it could function as data a ClusterTunnel's TunnelSpec? Adding a string type for a private CIDR would also allow the operator to use cloudflare-go's CreateTunnelRoute API - which is also a part of routing traffic to a warp-enabled tunnel.

Lastly, I see it as part of the struct in cloudflare_configuration.go but I think it would need to be written into the ConfigMap for the tunnel pods to use.

@adyanth
Copy link
Owner

adyanth commented Mar 28, 2023

Hey @KAllan357
Do you mean for the pod to allow access to the cluster's (or a network accessible by the pod's) CIDR to be broadcasted to WARP? Or as an access for pods to connect to IP ranges?

@adyanth adyanth added the enhancement New feature or request label Mar 28, 2023
@KAllan357
Copy link
Contributor Author

Specifically the former here.

@adyanth
Copy link
Owner

adyanth commented Mar 28, 2023

That can be done, yes. As you note, the tunnel spec can contain the warp enable flag. But for adding the actual routes; tunnel spec could be extended to include the routes, but checking the diff with the routes already added to the tunnel might be a bit of a pain. Any idea to make that easier?

@KAllan357
Copy link
Contributor Author

KAllan357 commented Apr 29, 2023

I was thinking about this some more. We can likely manage diffs / what's owned by the operator via the Comments section of the Tunnel Route. https://pkg.go.dev/github.com/cloudflare/cloudflare-go#TunnelRoutesCreateParams. Similar to some of the code which reconciles using data in the TXT record.

In the past I've enabled Warp on a tunnel using the following steps:

  1. Create a tunnel
  2. Add warp-routing.enabled = true to the tunnel's configuration file
  3. Call cloudflared tunnel route ip add <IP/CIDR> <Tunnel NAME or UUID>

Warp Instructions

As for the design, I had some thoughts. The Warp feature is a facet of the tunnel. It aligns with the TunnelBinding CR.

The operator could support an additional Kind in the TunnelBindingSubject part of a TunnelBinding resource:

apiVersion: networking.cfargotunnel.com/v1alpha1
kind: TunnelBinding
metadata:
  name: example
tunnelRef:
  kind: ClusterTunnel
  name: my-tunnel
subjects:
  - name: my-service
    spec:
      fqdn: example.org
      target: http://my-service.default.svc.cluster.local:8080
  # New!
  - name: new-example
    kind: CIDR # a new type of kind
    spec:
      # required
      network: "10.0.0.0/8"
      # optional
      virtualNetworkID: ""

The values inside the new Kind could represent the two things we don't know about which make up the CreateTunnelRoute function of cloudflare-go.

Just like when a TunnelBinding adds a Subject to the tunnel (which modifies the ConfigMap of the tunnel's config), this new Kind could do the same. It adds warp-routing.enabled = true to the ConfigMap and calls CreateTunnelRoute in the CF API.

What do you think?

Also, there are instructions and an API for updating the Split Tunnels list of your account, but I think these are better left to the user to manipulate. Last I checked, the two PUT routes (https://developers.cloudflare.com/api/operations/devices-set-split-tunnel-exclude-list and https://developers.cloudflare.com/api/operations/devices-set-split-tunnel-include-list) needed the whole list (ie submitting just a list with 1 entry would result in the dropping of any other entries). Seems like an weird race condition to me.

@KAllan357
Copy link
Contributor Author

@adyanth wondering if you saw my update here.

@adyanth
Copy link
Owner

adyanth commented May 4, 2023

The Comments section seems like a nice place to keep track of the stuff from the operator (as long as the user does not have any of their own comments to add). I had forgotten that the Kind exists in the TunnelBinding! That will work well for CIDR with the format you show.

Since the warp-routing.enabled is a tunnel level parameter, it makes sense for it to be enabled by the Tunnel, rather than the TunnelBinding. Only CreateTunnelRoute can be called by the Binding reconciler.

Yeah, as you note, I agree messing with the Split Tunnels is not a good idea.

@KAllan357
Copy link
Contributor Author

Awesome! When I get some time next week, I’ll take a stab at implementing and see how it goes.

@sanicheev
Copy link

Will it work? Isn't WARP mode mutually exclusive with ingress mode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants