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

Fix Stack Monitoring with custom certificate without CA #5310

Merged
merged 11 commits into from Feb 2, 2022

Conversation

thbkrkr
Copy link
Contributor

@thbkrkr thbkrkr commented Jan 31, 2022

Resolves #5309.

@thbkrkr thbkrkr added >bug Something isn't working v2.0.0 labels Jan 31, 2022
pkg/controller/common/certificates/ca_test.go Outdated Show resolved Hide resolved
pkg/controller/common/stackmon/config.go Outdated Show resolved Hide resolved
pkg/controller/kibana/stackmon/metricbeat.tpl.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

I did a quick test with Elasticsearch and Kibana pub. certs signed by a well known CA. Metricbeat is failing with the following error:

2022-02-01T08:26:57.702Z	ERROR	module/wrapper.go:259	Error fetching data for metricset elasticsearch.index: error determining if connected Elasticsearch node is master: error making http request: Get "https://localhost:9200/_nodes/_local/nodes": x509: certificate is valid for my-public-hostname.com, not localhost

I think it is related to Peter's comment here.

@thbkrkr
Copy link
Contributor Author

thbkrkr commented Feb 1, 2022

Thanks for the feedback. All have been addressed.

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

Looks good to me now. I did a test with a Let's encrypt certificate and Elasticsearch and one with the self-signed certs ECK uses by default.

pkg/controller/elasticsearch/stackmon/metricbeat.tpl.yml Outdated Show resolved Hide resolved
@thbkrkr
Copy link
Contributor Author

thbkrkr commented Feb 1, 2022

I found a little bug. When disabling TLS, we generate certs for ES but not for KB. This means we shouldn't get the kb-http-public-certs secret to resolve hasCA for the Kibana Metricbeat config without first checking if TLS is enabled.

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

I found a little bug

Good catch. I did not test with Kibana initially, but it looks good now.

@thbkrkr thbkrkr merged commit b02717b into elastic:main Feb 2, 2022
@thbkrkr thbkrkr deleted the fix-stack-monitoring-no-ca branch February 2, 2022 11:06
thbkrkr added a commit to thbkrkr/cloud-on-k8s that referenced this pull request Feb 2, 2022
Stack Monitoring incorrectly assumed that if the monitored Elastic resource has TLS enabled, there is a CA to configure in the input section of the Metricbeat config. This overlooked that you can have a certificate issued by a well-known CA, so you don't always provide a CA when TLS is enabled. This is fixed by differentiating between isSSL/isTLS and HasCA.
pebrc pushed a commit that referenced this pull request Feb 2, 2022
Stack Monitoring incorrectly assumed that if the monitored Elastic resource has TLS enabled, there is a CA to configure in the input section of the Metricbeat config. This overlooked that you can have a certificate issued by a well-known CA, so you don't always provide a CA when TLS is enabled. This is fixed by differentiating between isSSL/isTLS and HasCA.
fantapsody pushed a commit to fantapsody/cloud-on-k8s that referenced this pull request Feb 7, 2023
Stack Monitoring incorrectly assumed that if the monitored Elastic resource has TLS enabled, there is a CA to configure in the input section of the Metricbeat config. This overlooked that you can have a certificate issued by a well-known CA, so you don't always provide a CA when TLS is enabled. This is fixed by differentiating between isSSL/isTLS and HasCA.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack monitoring doesn't work with a resource monitored with a custom certificate without a CA
3 participants