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

Keda doesn't work with TLS inspection #4046

Closed
Zurielevi opened this issue Dec 27, 2022 · 5 comments · Fixed by #4191
Closed

Keda doesn't work with TLS inspection #4046

Zurielevi opened this issue Dec 27, 2022 · 5 comments · Fixed by #4191
Labels
feature All issues for new features that have been committed to

Comments

@Zurielevi
Copy link

I have noticed that there is no parameter to configure cabundle for TLS inspection support.
(Specific with azure log analytics scaler)

Thanks!

@JorTurFer
Copy link
Member

JorTurFer commented Dec 30, 2022

I have never heard about TLS inspection but after a short investigation, I guess this makes sense totally in some scenarios, but I think that this should be fixed at controller level, not as part of a scaler (I feel this CAs as restricted things, with a limted access to administrators).
We should add a mechanism to include user given certificates on every http request. Maybe we can include in the http creation helper something like

rootCAs, _ := x509.SystemCertPool()
	if rootCAs == nil {
		rootCAs = x509.NewCertPool()
	}

	if certVolume != "" {
		// Read in the cert file from volume
		certs, err := ioutil.ReadFile(certVolume)
		if err != nil {
			log.Fatalf("Failed to append %q to RootCAs: %v", localCertFile, err)
		}
	}

config := &tls.Config{
		InsecureSkipVerify: unsafeSsl,
		RootCAs:            rootCAs,
	}

In case of given certificates. I'm thinking about passing them as volume (or volumes), maybe using a specific path as convention and just including all certificates from there.

WDYT @kedacore/keda-core-contributors ?

We can extend the helm chart to cover this scenario, mounting them to the specific path

@zroubalik
Copy link
Member

Just a brain dump: we can do this approach of mounting this stuff to operator, or we can maybe extend TriggerAuth to allow do this on a scaler level? Would it make sense? Maybe both approaches?

@tomkerkhove
Copy link
Member

I'd say both might make sense where operator can be default unless overridden by TriggerAuth?

@JorTurFer
Copy link
Member

We could do at both levels yes, maybe we can introduce a section in the TriggerAuth for TLS trusted CAs. We can register in they system cert pool all the global trusted CAs and in the http client cert pool the CAs for a single trigger.
WDYT?
Are you willing to contribute with this?

@JorTurFer
Copy link
Member

JorTurFer commented Feb 8, 2023

@Zurielevi ,
I think that this feature could solve your problem of TLS inspection (once it's merged) because it adds support to registering custom CA certificates, which I guess that solved the problem (you could register your CA certificate in KEDA and trust in it).
Am I right?

@github-project-automation github-project-automation bot moved this from Proposed to Ready To Ship in Roadmap - KEDA Core Feb 16, 2023
@JorTurFer JorTurFer moved this from Ready To Ship to Done in Roadmap - KEDA Core Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature All issues for new features that have been committed to
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants