Skip to content

etcd: Added ssl options #9414

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.

Sign up for GitHub

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

Jump to bottom
Merged
alstolten merged 5 commits into from
May 28, 2024
Merged

etcd: Added ssl options #9414

alstolten merged 5 commits into from
May 28, 2024

Conversation

@alstolten
Copy link
Member

@alstolten alstolten commented Mar 21, 2024

Proposed commit message

Add ssl options to the etcd integration.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • some of the formatting changes seemed to have been introduced by elastic-package build, they were not made by me

How to test this PR locally

  • This is a little tricky to test. I tested locally with the minikube etcd server and the elastic-package stack. The agent deployed with the elastic-package stack needs to be added to the minikube docker network (docker network connect minikube elastic-package-stack-elastic-agent-1)
  • the certs to the minikube etcd are in /var/lib/docker/volumes/minikube/_data/lib/minikube/certs/etcd by default
  • I tested v3 metrics
  • data was successfully ingested and the shipped Dashboard did contain data
  • agent policy did contain the correct configuration
  • integration configuration UI showed the field

Related issues

This is very related to:

Screenshots

Screenshot from 2024-03-21 19-01-20

Screenshot from 2024-03-21 18-40-33

Screenshot from 2024-03-21 18-43-29

@alstolten alstolten added the enhancement New feature or request label Mar 21, 2024
@alstolten alstolten requested a review from a team as a code owner March 21, 2024 18:03
@alstolten
Copy link
Member Author

alstolten commented Mar 21, 2024

Now also based on the right version...

ali786XI
ali786XI suggested changes Mar 26, 2024
View reviewed changes
@lalit-satapathy lalit-satapathy requested a review from gpop63 March 26, 2024 09:47
alstolten and others added 2 commits March 27, 2024 09:06
@alstolten @ali786XI
Update packages/etcd/changelog.yml
4ebd485 
Co-authored-by: Aliabbas Attarwala <124054599+aliabbas-elastic@users.noreply.github.com>
@alstolten @ali786XI
Update packages/etcd/changelog.yml
9e7fa51 
Co-authored-by: Aliabbas Attarwala <124054599+aliabbas-elastic@users.noreply.github.com>
ali786XI
ali786XI approved these changes Mar 27, 2024
View reviewed changes
Copy link
Contributor

@ali786XI ali786XI left a comment

Choose a reason for hiding this comment

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

LGTM for rest changes

@alstolten
Copy link
Member Author

alstolten commented Apr 2, 2024

Hi @aliabbas-elastic, could you let me know how I can re-trigger the build?

@ali786XI
Copy link
Contributor

ali786XI commented Apr 2, 2024

/test

@alstolten alstolten enabled auto-merge (squash) April 9, 2024 14:48
@elasticmachine
Copy link

elasticmachine commented Apr 22, 2024

💚 Build Succeeded

History

@elastic-sonarqube
Copy link

elastic-sonarqube bot commented Apr 22, 2024

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

ishleenk17
ishleenk17 reviewed Apr 25, 2024
View reviewed changes
Copy link
Member

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

In general, this PR has many files with irrelevant changes.
Maybe, related to formatting. Please revert those.

packages/etcd/data_stream/leader/fields/fields.yml
type: group
description: >
Contains follower statistics.
Copy link
Member

@ishleenk17 ishleenk17 Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This diff is irrelevant

Copy link
Contributor

@gpop63 gpop63 May 4, 2024

Choose a reason for hiding this comment

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

The changes are from the elastic-package format command, do you think we should revert them?

Copy link
Member

@ishleenk17 ishleenk17 May 28, 2024

Choose a reason for hiding this comment

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

Should be good then.

packages/etcd/data_stream/leader/manifest.yml
title: etcd leader metrics
description: Collect etcd leader metrics
enabled: false
enabled: false
Copy link
Member

@ishleenk17 ishleenk17 Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Same here, irrelevant diff

packages/etcd/manifest.yml
- name: ssl
type: yaml
title: SSL Configuration
description: i.e. certificate, certificate_authorities, verification_mode etc. See [SSL](https://www.elastic.co/guide/en/beats/metricbeat/current/configuration-ssl.html#ssl-client-config) for details.
Copy link
Member

@ishleenk17 ishleenk17 Apr 25, 2024

Choose a reason for hiding this comment

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

Lets use link to the Agent instead of beats here

Copy link
Contributor

@gpop63 gpop63 May 4, 2024

Choose a reason for hiding this comment

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

You mean this one: https://www.elastic.co/guide/en/fleet/current/elastic-agent-ssl-configuration.html#client-ssl-options?

Copy link
Member

@ishleenk17 ishleenk17 May 28, 2024

Choose a reason for hiding this comment

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

yes

@Alphayeeeet Alphayeeeet mentioned this pull request May 17, 2024
Closed
@Alphayeeeet
Copy link
Contributor

Alphayeeeet commented May 17, 2024

Is there also the possibility to specify file system paths instead of certificate content inline? WIth this option, you could use the respective Cert and Key files available on Kubernetes Master Nodes.

@gpop63
Copy link
Contributor

gpop63 commented May 17, 2024 •
edited
Loading

Is there also the possibility to specify file system paths instead of certificate content inline? WIth this option, you could use the respective Cert and Key files available on Kubernetes Master Nodes.

@Alphayeeeet yes, you can specify path to certs or certs content.

ishleenk17
ishleenk17 approved these changes May 28, 2024
View reviewed changes
Copy link
Member

@ishleenk17 ishleenk17 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!

@alstolten alstolten merged commit 97f2474 into elastic:main May 28, 2024
@elasticmachine
Copy link

elasticmachine commented May 28, 2024

Package etcd - 1.1.0 containing this change is available at https://epr.elastic.co/search?package=etcd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpop63 gpop63 gpop63 approved these changes

@ishleenk17 ishleenk17 ishleenk17 approved these changes

+1 more reviewer

@ali786XI ali786XI ali786XI approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request Integration:etcd etcd

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ETCD] Enable configuration of CA for https connection to etcd host

6 participants

@alstolten @ali786XI @elasticmachine @Alphayeeeet @gpop63 @ishleenk17