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(kuma-cp) Prometheus metrics over mTLS #793

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz commented Jun 2, 2020

Summary

This PR introduces Prometheus metrics over mTLS. There are new properties in Prometheus backend configuration:

conf:
  tags:
    service: dataplane-metrics
  skipMTLS: false

Since metrics will be exposed with mTLS, there has to be a service that TrafficPermission will match on. You can define this service name in tags. skipMTLS lets you skip mTLS defined on the mesh. If the value is nil or true, Prometheus endpoint will be exposed without mTLS.

kumactl install metrics now installs prometheus as a part of the mesh with direct access of "*".
I needed to remove probes for now because health checks are not accessible when mTLS is on.
Node Exporter for some reason was crashing the whole K8S cluster when Kuma DP was deployed there, therefore for now sidecar injection for it is disabled.

I introduced the concept of additional networking in the code, so policies can be matched on extra listeners that we are generating automatically (like Prometheus)

Documentation

Documentation kumahq/kuma-website#214

@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team June 2, 2020 12:57
Copy link
Contributor

@nickolaev nickolaev left a comment

Choose a reason for hiding this comment

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

Few small remarks.

// We assume that skipMTLS = nil is the same as false, so we don't break deployments between 0.5 and 0.5.1
skipMTLS := prometheusEndpoint.SkipMTLS == nil || prometheusEndpoint.SkipMTLS.Value
var listener *envoy_api.Listener
if skipMTLS || !ctx.Mesh.Resource.MTLSEnabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to handle skipMTLS in a separate function?

Copy link
Contributor Author

@jakubdyszkiewicz jakubdyszkiewicz Jun 2, 2020

Choose a reason for hiding this comment

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

Refactored

pkg/xds/generator/prometheus_endpoint_generator_test.go Outdated Show resolved Hide resolved
Base automatically changed from feat/direct-access-v2 to master June 2, 2020 15:21
@jakubdyszkiewicz jakubdyszkiewicz force-pushed the feat/envoy-metrics-mtls-v3 branch from fc5b7ea to 174fed1 Compare June 2, 2020 15:58
Copy link
Contributor

@nickolaev nickolaev left a comment

Choose a reason for hiding this comment

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

Approving.
Please make sure we record the issue with he sidecar-container.

@jakubdyszkiewicz
Copy link
Contributor Author

Issue created. Docs are here kumahq/kuma-website#214

@jakubdyszkiewicz jakubdyszkiewicz merged commit df12dc3 into master Jun 3, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the feat/envoy-metrics-mtls-v3 branch June 3, 2020 09:09
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.

2 participants