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.

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

feat: add mTLS support to otel exporter #1389

Merged
merged 10 commits into from
Sep 23, 2024
Merged

feat: add mTLS support to otel exporter #1389

merged 10 commits into from
Sep 23, 2024
+544 −63

Conversation

kevinschoonover
Copy link
Contributor

@kevinschoonover kevinschoonover commented Aug 29, 2024 •
edited
Loading

This PR

The OpenTelemetry collectors in my production environment are configured to use TLS for uploading metrics / traces so this PR aims to

  • add the ability to use mTLS + self-signed certificates when exporting to the opentelemetry collector

This is the 'quick and dirty' approach so wanted to make an initial PR to make sure the high level implementation is the approach you're looking for.

Follow-up Tasks

  • update the documentation when this approach is approved

How to test

I am struggling to figure out how to test this with self signed certificates to give a specific set of commands you can run because the TLS connection is never successful (assuming this is because of my commands)

openssl req -x509 -newkey rsa:4096 -keyout ca.key.pem -out ca.cert.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=StateName/L=CityName/O=CompanyName/OU=CompanySectionName/CN=localhost"

openssl req -x509 -newkey rsa:4096 -keyout client.key.pem -out client.cert.pem -CA ca.cert.pem -CAkey ca.key.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=StateName/L=CityName/O=CompanyName/OU=CompanySectionName/CN=localhost" -addext "subjectAltName = IP:127.0.0.1"
openssl req -x509 -newkey rsa:4096 -keyout server.key.pem -out server.cert.pem -CA ca.cert.pem -CAkey ca.key.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=StateName/L=CityName/O=CompanyName/OU=CompanySectionName/CN=localhost" -addext "subjectAltName = IP:127.0.0.1"

; however, when I pull certificates from my production environment to test this works

@kevinschoonover
progress save
ef89ed2 
Signed-off-by: Kevin Schoonover <me@kschoon.me>
@kevinschoonover kevinschoonover requested a review from a team as a code owner August 29, 2024 22:40
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 29, 2024
@netlify Netlify
Copy link

netlify bot commented Aug 29, 2024 •
edited
Loading

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit 9e2e0be
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/66ede92a1bc93a0008f8e7ca

@kevinschoonover kevinschoonover mentioned this pull request Aug 29, 2024
Open
@kevinschoonover
feat: add mTLS support to otel exporter
febbbd3 
Signed-off-by: Kevin Schoonover <me@kschoon.me>
@toddbaert
Copy link
Member

Hey @kevinschoonover this is a great addition, and I'm fine with the approach. There's some unit test failures and warnings you'll need to address (integration tests look good though).

@kevinschoonover
progress save on certreloader + working local tests
1d40ba8 
Signed-off-by: Kevin Schoonover <me@kschoon.me>
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 18, 2024
@kevinschoonover
fix integration tests
d5a78b8 
Signed-off-by: Kevin Schoonover <me@kschoon.me>
@kevinschoonover
fix docs
1674c78 
Signed-off-by: Kevin Schoonover <me@kschoon.me>
@kevinschoonover
Copy link
Contributor Author

@toddbaert - sorry for the delay. I think I have addressed the errors I saw, but don't know if i'm able to trigger the PR to fully test.

@toddbaert
fixup: handle err and lint
71943e7 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Copy link
Member

There were some minor lint errors I fixed with this commit, please let me know if you see anything wrong.

toddbaert
toddbaert approved these changes Sep 19, 2024
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Thank you! See: #1389 (comment)

@toddbaert
fixup: typo
c828eca 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@kevinschoonover
Copy link
Contributor Author

There were some minor lint errors I fixed with this commit, please let me know if you see anything wrong.

Looks good to me! Thanks for fixing those!

@toddbaert toddbaert merged commit 8737f53 into open-feature:main Sep 23, 2024
15 checks passed
@github-actions github-actions bot mentioned this pull request Sep 23, 2024
Merged
@toddbaert toddbaert mentioned this pull request Sep 23, 2024
Merged
toddbaert added a commit that referenced this pull request Sep 23, 2024
@toddbaert
fix: remove dep cycle with certreloader (#1410)
5244f6f 
#1389 works well but caused a
dependency cycle: core -> flagd -> core.

I've moved the `certreloader` pkg to core with the existing telemetry
stuff.

cc @kevinschoonover

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert pushed a commit that referenced this pull request Sep 23, 2024
@github-actions
chore: release main (#1395)
851b9da 
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.11.3</summary>

##
[0.11.3](flagd/v0.11.2...flagd/v0.11.3)
(2024-09-23)


### 🐛 Bug Fixes

* **deps:** update kubernetes package and controller runtime, fix proto
lint ([#1290](#1290))
([94860d6](94860d6))
* **deps:** update module
buf.build/gen/go/open-feature/flagd/connectrpc/go to
v1.16.2-20240906125204-0a6a901b42e8.1
([#1399](#1399))
([18dd4e2](18dd4e2))
* **deps:** update module buf.build/gen/go/open-feature/flagd/grpc/go to
v1.5.1-20240906125204-0a6a901b42e8.1
([#1400](#1400))
([954d972](954d972))
* **deps:** update module connectrpc.com/connect to v1.17.0
([#1408](#1408))
([e7eb691](e7eb691))
* **deps:** update module github.com/open-feature/flagd/core to v0.10.2
([#1385](#1385))
([3b5a818](3b5a818))
* **deps:** update module github.com/prometheus/client_golang to v1.20.3
([#1384](#1384))
([8fd16b2](8fd16b2))
* **deps:** update module github.com/prometheus/client_golang to v1.20.4
([#1406](#1406))
([a0a6426](a0a6426))
* **deps:** update module github.com/rs/cors to v1.11.1
([#1392](#1392))
([8bd549e](8bd549e))
* **deps:** update module github.com/rs/xid to v1.6.0
([#1386](#1386))
([2317013](2317013))
* **deps:** update module golang.org/x/net to v0.29.0
([#1398](#1398))
([0721e02](0721e02))
* **deps:** update module google.golang.org/grpc to v1.66.0
([#1393](#1393))
([c96e9d7](c96e9d7))
* **deps:** update module google.golang.org/grpc to v1.66.1
([#1402](#1402))
([50c9cd3](50c9cd3))
* **deps:** update module google.golang.org/grpc to v1.66.2
([#1405](#1405))
([69ec28f](69ec28f))
* **deps:** update module google.golang.org/grpc to v1.67.0
([#1407](#1407))
([1ad6480](1ad6480))
* **deps:** update opentelemetry-go monorepo
([#1387](#1387))
([22aef5b](22aef5b))
* **deps:** update opentelemetry-go monorepo
([#1403](#1403))
([fc4cd3e](fc4cd3e))
* remove dep cycle with certreloader
([#1410](#1410))
([5244f6f](5244f6f))


### ✨ New Features

* add mTLS support to otel exporter
([#1389](#1389))
([8737f53](8737f53))
</details>

<details><summary>flagd-proxy: 0.6.6</summary>

##
[0.6.6](flagd-proxy/v0.6.5...flagd-proxy/v0.6.6)
(2024-09-23)


### 🐛 Bug Fixes

* **deps:** update kubernetes package and controller runtime, fix proto
lint ([#1290](#1290))
([94860d6](94860d6))
* **deps:** update module buf.build/gen/go/open-feature/flagd/grpc/go to
v1.5.1-20240906125204-0a6a901b42e8.1
([#1400](#1400))
([954d972](954d972))
* **deps:** update module github.com/open-feature/flagd/core to v0.10.2
([#1385](#1385))
([3b5a818](3b5a818))
* **deps:** update module github.com/prometheus/client_golang to v1.20.3
([#1384](#1384))
([8fd16b2](8fd16b2))
* **deps:** update module github.com/prometheus/client_golang to v1.20.4
([#1406](#1406))
([a0a6426](a0a6426))
* **deps:** update module golang.org/x/net to v0.29.0
([#1398](#1398))
([0721e02](0721e02))
* **deps:** update module google.golang.org/grpc to v1.66.0
([#1393](#1393))
([c96e9d7](c96e9d7))
* **deps:** update module google.golang.org/grpc to v1.66.1
([#1402](#1402))
([50c9cd3](50c9cd3))
* **deps:** update module google.golang.org/grpc to v1.66.2
([#1405](#1405))
([69ec28f](69ec28f))
* **deps:** update module google.golang.org/grpc to v1.67.0
([#1407](#1407))
([1ad6480](1ad6480))
* **deps:** update opentelemetry-go monorepo
([#1387](#1387))
([22aef5b](22aef5b))
* **deps:** update opentelemetry-go monorepo
([#1403](#1403))
([fc4cd3e](fc4cd3e))
* remove dep cycle with certreloader
([#1410](#1410))
([5244f6f](5244f6f))
</details>

<details><summary>core: 0.10.3</summary>

##
[0.10.3](core/v0.10.2...core/v0.10.3)
(2024-09-23)


### 🐛 Bug Fixes

* **deps:** update kubernetes package and controller runtime, fix proto
lint ([#1290](#1290))
([94860d6](94860d6))
* **deps:** update module buf.build/gen/go/open-feature/flagd/grpc/go to
v1.5.1-20240906125204-0a6a901b42e8.1
([#1400](#1400))
([954d972](954d972))
* **deps:** update module connectrpc.com/connect to v1.17.0
([#1408](#1408))
([e7eb691](e7eb691))
* **deps:** update module github.com/prometheus/client_golang to v1.20.3
([#1384](#1384))
([8fd16b2](8fd16b2))
* **deps:** update module github.com/prometheus/client_golang to v1.20.4
([#1406](#1406))
([a0a6426](a0a6426))
* **deps:** update module gocloud.dev to v0.39.0
([#1404](#1404))
([a3184d6](a3184d6))
* **deps:** update module golang.org/x/crypto to v0.27.0
([#1396](#1396))
([f9a7d10](f9a7d10))
* **deps:** update module golang.org/x/mod to v0.21.0
([#1397](#1397))
([1507e19](1507e19))
* **deps:** update module google.golang.org/grpc to v1.66.0
([#1393](#1393))
([c96e9d7](c96e9d7))
* **deps:** update module google.golang.org/grpc to v1.66.1
([#1402](#1402))
([50c9cd3](50c9cd3))
* **deps:** update module google.golang.org/grpc to v1.66.2
([#1405](#1405))
([69ec28f](69ec28f))
* **deps:** update module google.golang.org/grpc to v1.67.0
([#1407](#1407))
([1ad6480](1ad6480))
* **deps:** update opentelemetry-go monorepo
([#1387](#1387))
([22aef5b](22aef5b))
* **deps:** update opentelemetry-go monorepo
([#1403](#1403))
([fc4cd3e](fc4cd3e))
* remove dep cycle with certreloader
([#1410](#1410))
([5244f6f](5244f6f))


### ✨ New Features

* add mTLS support to otel exporter
([#1389](#1389))
([8737f53](8737f53))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@toddbaert toddbaert toddbaert approved these changes

@bacherfl bacherfl Awaiting requested review from bacherfl

@craigpastro craigpastro Awaiting requested review from craigpastro

@olunusib olunusib Awaiting requested review from olunusib

@thisthat thisthat Awaiting requested review from thisthat

Assignees
No one assigned
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kevinschoonover @toddbaert @beeme1mr