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

cmd/containerboot: add support for setting funnel TCP portforward #7136

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

maisem
Copy link
Collaborator

@maisem maisem commented Feb 1, 2023

WIP

Signed-off-by: Maisem Ali maisem@tailscale.com

WIP

Signed-off-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Copy link
Member

@danderson danderson left a comment

Choose a reason for hiding this comment

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

I don't think I like this as-is, especially the various "if kubernetes" branches in ipnlocal. We already have a bunch of these branches throughout our code and the magically different behavior makes it hard to maintain stuff :(

Maybe it's time to graduate the store API from plain key/value to structured Get/Set for specific data, so that the underlying implementations cna handle all this weird differential storage we're doing.

@@ -90,8 +90,31 @@ func main() {
AuthOnce: defaultBool("TS_AUTH_ONCE", false),
Root: defaultEnv("TS_TEST_ONLY_ROOT", "/"),
}
funnelForwardPorts := strings.Split(defaultEnv("TS_FUNNEL_TCP_PORTFORWARD", ""), ",")
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably call this just "TS_FUNNEL_TCP", to reduce typing. Also add the envvar to the big comment at the top that lists all the available options.

}
sc.AllowFunnel[ipn.HostPort(fmt.Sprintf("%s:%d", cd, f))] = true
}
return client.SetServeConfig(ctx, sc)
Copy link
Member

Choose a reason for hiding this comment

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

How do we know when funnel is working? Ideally the operator would like some signal that pod startup has completed correctly, so it doesn't advertise the ingress hostname until it's actually serving.

@@ -305,6 +324,35 @@ authLoop:
}
}

func configureForwarding(ctx context.Context, client *tailscale.LocalClient, cfg *settings) error {
if cfg.ProxyTo == "" {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is right for all scenarios, it's valid to configure funnel without ProxyTo if you're running the container as a sidecar. I think funnel needs to work in either case?

@DentonGentry
Copy link
Contributor

Recommend the commit message state that it Updates #6468
(or Fixes, as the case may be)

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.

None yet

3 participants