feat(core): Add flag to allow insecure TLS connections #5454
Conversation
MisterMX
force-pushed
the
feat/tls-allow-insecure
branch
from
aaa1f97
to
c04aa86
Compare
MisterMX
changed the title
MisterMX
force-pushed
the
feat/tls-allow-insecure
branch
from
c04aa86
to
5b9e1bb
Compare
We thought about that, but this doesn't make it much easier. We want to run a function locally for debugging purposes and the easiest way to have it work with Crossplane is to have it run next to it on the same machine. Using a proxy doesn't work here because my machine is not reachable from within the cluster. |
|
@MisterMX Could you provide some more background here or in #5444 about the use case for deploying development functions and using them with Crossplane? Right now that issue talks about wanting to bypass the package manager but doesn't explain why. In particular I'm wondering why deploy a development function to a real control plane when you could use |
cmd/crossplane/core/core.go
Outdated
| @@ -103,6 +103,7 @@ type startCommand struct { | |||
| TLSServerCertsDir string `env:"TLS_SERVER_CERTS_DIR" help:"The path of the folder which will store TLS server certificate of Crossplane."` | |||
| TLSClientSecretName string `env:"TLS_CLIENT_SECRET_NAME" help:"The name of the TLS Secret that will be store Crossplane's client certificate."` | |||
| TLSClientCertsDir string `env:"TLS_CLIENT_CERTS_DIR" help:"The path of the folder which will store TLS client certificate of Crossplane."` | |||
| TLSAllowInsecure bool `help:"Allow insecure TLS connections to Crossplane functions"` | |||
There was a problem hiding this comment.
If we add an --insecure flag I think it should affect all TLS, not only functions.
We actually use these same certs to connect to external secret stores. See the if c.EnableExternalSecretStores block. I think it was an oversight that we load the same certs twice. I thought we used them to authenticate validation webhooks as well, but I can't see where that is plumbed up (CC @phisco who might know).
MisterMX
force-pushed
the
feat/tls-allow-insecure
branch
2 times, most recently
from
8cc72f3
to
75ba2f0
Compare
cmd/crossplane/core/core.go
Outdated
| if c.TLSAllowInsecure { | ||
| fnRunnerOpts = append(fnRunnerOpts, xfn.WithTLSConfig(&tls.Config{ | ||
| MinVersion: tls.VersionTLS12, | ||
| InsecureSkipVerify: true, //nolint:gosec // User setting | ||
| })) |
There was a problem hiding this comment.
Does this definitely work as expected?
When a function runs with --insecure it uses gRPC's insecure.NewCredentials. I thought this completely disabled TLS, causing the function to serve plaintext. Is that not the case?
There was a problem hiding this comment.
I just ran it with --insecure and it does indeed, unfortunately, not work:
rpc error: code = Unavailable desc = last connection error: connection error: desc = \"transport: authentication handshake failed: tls: first record does not look like a TLS handshake\""
I added a second option --function-connect-plain-text that disables TLS altogether - which worked.
There was a problem hiding this comment.
Could we keep it simple and add just an --insecure flag (to Crossplane) that causes it to:
- Connect to functions using plaintext.
- Connect to webhooks (and external secret stores?) as plaintext?
I like that:
- This matches the existing
--insecureflag at the function end. - (iirc) this matches the
--insecureflag that various Kubernetes components use
I'd prefer to avoid the complexity of having two flags - one to use plaintext, and another to skip TLS certificate verification - if the goal is just to enable local debugging without dealing with TLS.
Signed-off-by: Maximilian Blatt <maximilian.blatt-extern@deutschebahn.com>
Allow connecting to functions that are running on a local developer machine in insecure mode. Signed-off-by: Maximilian Blatt <maximilian.blatt-extern@deutschebahn.com>
MisterMX
force-pushed
the
feat/tls-allow-insecure
branch
from
75ba2f0
to
92027b8
Compare
Description of your changes
Adds a new flag
--tls-allow-insecureto disable certificate validation and allow unsecured connection to function hooks (i.e. development setups).Related to #5444.
I have:
make reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.