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

Ta update configs to enable mtls #3015

Open
wants to merge 78 commits into
base: main
Choose a base branch
from

Conversation

ItielOlenick
Copy link
Contributor

@ItielOlenick ItielOlenick commented Jun 6, 2024

Description: When CertManager and secrets RBAC permissions are granted, mTLS will be used between the target allocator and the collector so that the latter can retrieve authentication secrets for endpoints that require them.

Link to Tracking Issue(s):

Second PR towards a solution for #1669

Testing: Unit tests added. E2E tests added. Tested in-cluster locally.

Documentation: Added documentation

ItielOlenick and others added 30 commits May 13, 2024 21:44
Bumps [github.com/gin-gonic/gin](https://github.com/gin-gonic/gin) from 1.9.1 to 1.10.0.
- [Release notes](https://github.com/gin-gonic/gin/releases)
- [Changelog](https://github.com/gin-gonic/gin/blob/master/CHANGELOG.md)
- [Commits](gin-gonic/gin@v1.9.1...v1.10.0)

---
updated-dependencies:
- dependency-name: github.com/gin-gonic/gin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…elemetry#2951)

Bumps the prometheus group with 1 update: [github.com/prometheus/prometheus](https://github.com/prometheus/prometheus).

Updates `github.com/prometheus/prometheus` from 0.51.2 to 0.52.0
- [Release notes](https://github.com/prometheus/prometheus/releases)
- [Changelog](https://github.com/prometheus/prometheus/blob/main/CHANGELOG.md)
- [Commits](prometheus/prometheus@v0.51.2...v0.52.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/prometheus
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: prometheus
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* enable readiness Probe for otel operator

Signed-off-by: Janario Oliveira <[email protected]>

* generate CRD and controller changes

Signed-off-by: Janario Oliveira <[email protected]>

* Adjusted code to be similar to Liveness logic

Signed-off-by: Janario Oliveira <[email protected]>

* Generated manifests

Signed-off-by: Janario Oliveira <[email protected]>

* Add changelog

Signed-off-by: Janario Oliveira <[email protected]>

* Fix lint

Signed-off-by: Janario Oliveira <[email protected]>

* Removed readinessProbe from alpha CRD

Signed-off-by: Janario Oliveira <[email protected]>

* Generated manifests

Signed-off-by: Janario Oliveira <[email protected]>

* Fix lint

Signed-off-by: Janario Oliveira <[email protected]>

* Centralized probe validation

Signed-off-by: Janario Oliveira <[email protected]>

---------

Signed-off-by: Janario Oliveira <[email protected]>
Co-authored-by: hesam.hamdarsi <[email protected]>
Bumps [github.com/docker/docker](https://github.com/docker/docker) from 26.0.1+incompatible to 26.0.2+incompatible.
- [Release notes](https://github.com/docker/docker/releases)
- [Commits](moby/moby@v26.0.1...v26.0.2)

---
updated-dependencies:
- dependency-name: github.com/docker/docker
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Log Enconder Config

Signed-off-by: Yuri Sa <[email protected]>

* Added new Debug doc

Signed-off-by: Yuri Sa <[email protected]>

---------

Signed-off-by: Yuri Sa <[email protected]>
* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Add test

Signed-off-by: Pavol Loffay <[email protected]>

---------

Signed-off-by: Pavol Loffay <[email protected]>
…ility check (open-telemetry#2964)

* Verify ServiceMonitor and PodMonitor are installed in prom cr availability check

* Added changelog
…try#2968)

Bumps [kyverno/action-install-chainsaw](https://github.com/kyverno/action-install-chainsaw) from 0.2.0 to 0.2.1.
- [Release notes](https://github.com/kyverno/action-install-chainsaw/releases)
- [Commits](kyverno/action-install-chainsaw@v0.2.0...v0.2.1)

---
updated-dependencies:
- dependency-name: kyverno/action-install-chainsaw
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Create a separate Service Monitor when the Prometheus exporter is present

Signed-off-by: Israel Blancas <[email protected]>

* Improve changelog

Signed-off-by: Israel Blancas <[email protected]>

* Fix prometheus-cr E2E test

Signed-off-by: Israel Blancas <[email protected]>

* Remove unused target

Signed-off-by: Israel Blancas <[email protected]>

* Add docstring

Signed-off-by: Israel Blancas <[email protected]>

* Fix typo

Signed-off-by: Israel Blancas <[email protected]>

* Change the label name

Signed-off-by: Israel Blancas <[email protected]>

* Change changelog description

Signed-off-by: Israel Blancas <[email protected]>

* Recover removed labels

Signed-off-by: Israel Blancas <[email protected]>

* Add missing labels

Signed-off-by: Israel Blancas <[email protected]>

* Remove wrong labels

Signed-off-by: Israel Blancas <[email protected]>

---------

Signed-off-by: Israel Blancas <[email protected]>
* Prepare release 0.100.0

Signed-off-by: Vineeth Pothulapati <[email protected]>

* update the chlog

* update the chlog with open-telemetry#2877 merge

---------

Signed-off-by: Vineeth Pothulapati <[email protected]>
* Refactor consistent-hashing strategy

* Refactor per-node strategy

* Refactor least-weighted strategy

* Minor allocation strategy refactor

* Add some common allocation strategy tests

* Fix collector and target reassignment

* Minor allocator fixes

* Add changelog entry

* Fix an incorrect comment
* add back webhook port

* chlog
* Support for kubernetes 1.30 version

* Update makefile
…or, target allocator, opamp bridge (open-telemetry#2933)

* set things

* fix kustomize shim

* restore, better chlog
Bumps alpine from 3.19 to 3.20.

---
updated-dependencies:
- dependency-name: alpine
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…emetry#2991)

Bumps alpine from 3.19 to 3.20.

---
updated-dependencies:
- dependency-name: alpine
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/go-logr/logr](https://github.com/go-logr/logr) from 1.4.1 to 1.4.2.
- [Release notes](https://github.com/go-logr/logr/releases)
- [Changelog](https://github.com/go-logr/logr/blob/master/CHANGELOG.md)
- [Commits](go-logr/logr@v1.4.1...v1.4.2)

---
updated-dependencies:
- dependency-name: github.com/go-logr/logr
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…try#2989)

Bumps [kyverno/action-install-chainsaw](https://github.com/kyverno/action-install-chainsaw) from 0.2.1 to 0.2.2.
- [Release notes](https://github.com/kyverno/action-install-chainsaw/releases)
- [Commits](kyverno/action-install-chainsaw@v0.2.1...v0.2.2)

---
updated-dependencies:
- dependency-name: kyverno/action-install-chainsaw
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the otel group with 5 updates:

| Package | From | To |
| --- | --- | --- |
| [go.opentelemetry.io/otel](https://github.com/open-telemetry/opentelemetry-go) | `1.26.0` | `1.27.0` |
| [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp](https://github.com/open-telemetry/opentelemetry-go) | `1.26.0` | `1.27.0` |
| [go.opentelemetry.io/otel/metric](https://github.com/open-telemetry/opentelemetry-go) | `1.26.0` | `1.27.0` |
| [go.opentelemetry.io/otel/sdk](https://github.com/open-telemetry/opentelemetry-go) | `1.26.0` | `1.27.0` |
| [go.opentelemetry.io/otel/sdk/metric](https://github.com/open-telemetry/opentelemetry-go) | `1.26.0` | `1.27.0` |

Updates `go.opentelemetry.io/otel` from 1.26.0 to 1.27.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.26.0...v1.27.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` from 1.26.0 to 1.27.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.26.0...v1.27.0)

Updates `go.opentelemetry.io/otel/metric` from 1.26.0 to 1.27.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.26.0...v1.27.0)

Updates `go.opentelemetry.io/otel/sdk` from 1.26.0 to 1.27.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.26.0...v1.27.0)

Updates `go.opentelemetry.io/otel/sdk/metric` from 1.26.0 to 1.27.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.26.0...v1.27.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: otel
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: otel
- dependency-name: go.opentelemetry.io/otel/metric
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: otel
- dependency-name: go.opentelemetry.io/otel/sdk
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: otel
- dependency-name: go.opentelemetry.io/otel/sdk/metric
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: otel
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@ItielOlenick
Copy link
Contributor Author

@swiatekm I've Added initial e2e tests.
I want to add a test that will bring up a simple app with basic auth on its metrics endpoint, to thoroughly test the entire feature. Where should I add the files? I was thinking either as another app to the e2e apps or build and add to kind during the e2e tests.

Comment on lines +9 to +15
- name: step-00
try:
- apply:
template: true
file: 00-install.yaml
- assert:
file: 00-assert.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you need to add your other two steps here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waiting for #3120

@swiatekm
Copy link
Contributor

@swiatekm I've Added initial e2e tests. I want to add a test that will bring up a simple app with basic auth on its metrics endpoint, to thoroughly test the entire feature. Where should I add the files? I was thinking either as another app to the e2e apps or build and add to kind during the e2e tests.

I don't think we need to do that - it would really just be testing the prometheus-operator packages, which is presumably handled by prometheus-operator itself. The only feature we're adding here is mTLS for the connection between the collector and target allocator, and it doesn't necessarily matter what data is sent over that connection.

I'm not strongly opposed to it, but I'd rather it happen in a separate PR, so we can more clearly see the complexity that kind of test would introduce.

@ItielOlenick
Copy link
Contributor Author

ItielOlenick commented Jul 11, 2024

@swiatekm I've Added initial e2e tests. I want to add a test that will bring up a simple app with basic auth on its metrics endpoint, to thoroughly test the entire feature. Where should I add the files? I was thinking either as another app to the e2e apps or build and add to kind during the e2e tests.

I don't think we need to do that - it would really just be testing the prometheus-operator packages, which is presumably handled by prometheus-operator itself. The only feature we're adding here is mTLS for the connection between the collector and target allocator, and it doesn't necessarily matter what data is sent over that connection.

I'm not strongly opposed to it, but I'd rather it happen in a separate PR, so we can more clearly see the complexity that kind of test would introduce.

I think it should be tested as part of the e2e. I believe we should test that an actual secret is being retrieved by the collector from the target allocator. I think of this enhancement as not only adding mTLS, but exposing the sensitive data when mTLS is used as done in #2921.

I agree it should be done in a separate PR.
Added - #3120

@ItielOlenick
Copy link
Contributor Author

@swiatekm anything missing?

@swiatekm
Copy link
Contributor

@swiatekm anything missing?

We're still waiting for the change to prometheus receiver so that e2e tests here can pass, right?

@ItielOlenick
Copy link
Contributor Author

@swiatekm anything missing?

We're still waiting for the change to prometheus receiver so that e2e tests here can pass, right?

Yes, waiting for open-telemetry/opentelemetry-collector-contrib#34035 and also #3120.

@swiatekm
Copy link
Contributor

@swiatekm anything missing?

We're still waiting for the change to prometheus receiver so that e2e tests here can pass, right?

Yes, waiting for open-telemetry/opentelemetry-collector-contrib#34035 and also #3120.

How do you want to go about merging this change, then? Can we split out a part of it that doesn't depend on these changes and merge it? I'm also ok with waiting for them, but it may be onerous for you to keep this PR rebased against the operator main branch.

@ItielOlenick
Copy link
Contributor Author

@swiatekm anything missing?

We're still waiting for the change to prometheus receiver so that e2e tests here can pass, right?

Yes, waiting for open-telemetry/opentelemetry-collector-contrib#34035 and also #3120.

How do you want to go about merging this change, then? Can we split out a part of it that doesn't depend on these changes and merge it? I'm also ok with waiting for them, but it may be onerous for you to keep this PR rebased against the operator main branch.

I'm ok with splitting the test that relies on the e2e test image, but we do need open-telemetry/opentelemetry-collector-contrib#34035 as without it the collector will not be able to connect to the TA successfully over mTLS

@ItielOlenick
Copy link
Contributor Author

@swiatekm Now that open-telemetry/opentelemetry-collector-contrib#34035 has been merged, can we get #3120 approved and merged?

change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: target allocator collector
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be only target allocator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought is that since we are modifying the collector's configuration applied by the operator, it should be mentioned.

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.

None yet