From 8b4e1e64a6662e0b2d5f9b6269050bb1a798804c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 08:41:35 +0200 Subject: [PATCH 01/10] Bump kyverno/action-install-chainsaw from 0.1.9 to 0.2.0 (#2924) Bumps [kyverno/action-install-chainsaw](https://github.com/kyverno/action-install-chainsaw) from 0.1.9 to 0.2.0. - [Release notes](https://github.com/kyverno/action-install-chainsaw/releases) - [Commits](https://github.com/kyverno/action-install-chainsaw/compare/v0.1.9...v0.2.0) --- updated-dependencies: - dependency-name: kyverno/action-install-chainsaw dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/e2e.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index d8bf4d629a..fd876dbb98 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -55,7 +55,7 @@ jobs: path: bin key: ${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('Makefile') }}-${{ steps.setup-go.outputs.go-version }} - name: Install chainsaw - uses: kyverno/action-install-chainsaw@v0.1.9 + uses: kyverno/action-install-chainsaw@v0.2.0 - name: Install tools run: make install-tools - name: Prepare e2e tests From d2046091cfa4320b796c5078b5b0c311632e93b1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 08:48:46 +0200 Subject: [PATCH 02/10] Bump github.com/operator-framework/operator-lib from 0.12.0 to 0.13.0 (#2925) Bumps [github.com/operator-framework/operator-lib](https://github.com/operator-framework/operator-lib) from 0.12.0 to 0.13.0. - [Release notes](https://github.com/operator-framework/operator-lib/releases) - [Commits](https://github.com/operator-framework/operator-lib/compare/v0.12.0...v0.13.0) --- updated-dependencies: - dependency-name: github.com/operator-framework/operator-lib dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 8 ++++---- go.sum | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 123edf529d..20aad7b197 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/oklog/ulid/v2 v2.1.0 github.com/open-telemetry/opamp-go v0.14.0 github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e - github.com/operator-framework/operator-lib v0.12.0 + github.com/operator-framework/operator-lib v0.13.0 github.com/prometheus-operator/prometheus-operator v0.71.2 github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.72.0 github.com/prometheus-operator/prometheus-operator/pkg/client v0.72.0 @@ -176,7 +176,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus-community/prom-label-proxy v0.8.0 // indirect github.com/prometheus/alertmanager v0.27.0 // indirect - github.com/prometheus/client_model v0.6.0 // indirect + github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common/sigv4 v0.1.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect github.com/scaleway/scaleway-sdk-go v1.0.0-beta.25 // indirect @@ -197,7 +197,7 @@ require ( go.uber.org/zap v1.26.0 // indirect golang.org/x/arch v0.3.0 // indirect golang.org/x/crypto v0.21.0 // indirect - golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect + golang.org/x/exp v0.0.0-20240213143201-ec583247a57a // indirect golang.org/x/mod v0.16.0 // indirect golang.org/x/net v0.23.0 // indirect golang.org/x/oauth2 v0.18.0 // indirect @@ -216,7 +216,7 @@ require ( google.golang.org/protobuf v1.33.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect - k8s.io/kube-openapi v0.0.0-20240209001042-7a0d5b415232 // indirect + k8s.io/kube-openapi v0.0.0-20240221221325-2ac9dc51f3f1 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect ) diff --git a/go.sum b/go.sum index 2eb8e9ecf7..dd71ff5e6f 100644 --- a/go.sum +++ b/go.sum @@ -488,10 +488,10 @@ github.com/oklog/ulid v1.3.1 h1:EGfNDEx6MqHz8B3uNV6QAib1UR2Lm97sHi3ocA6ESJ4= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/oklog/ulid/v2 v2.1.0 h1:+9lhoxAP56we25tyYETBBY1YLA2SaoLvUFgrP2miPJU= github.com/oklog/ulid/v2 v2.1.0/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= -github.com/onsi/ginkgo/v2 v2.14.0 h1:vSmGj2Z5YPb9JwCWT6z6ihcUvDhuXLc3sJiqd3jMKAY= -github.com/onsi/ginkgo/v2 v2.14.0/go.mod h1:JkUdW7JkN0V6rFvsHcJ478egV3XH9NxpD27Hal/PhZw= -github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8= -github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ= +github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8= +github.com/onsi/ginkgo/v2 v2.17.1/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs= +github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk= +github.com/onsi/gomega v1.32.0/go.mod h1:a4x4gW6Pz2yK1MAmvluYme5lvYTn61afQ2ETw/8n4Lg= github.com/open-telemetry/opamp-go v0.14.0 h1:KoziIK+wsFojhUXNTkCSTnCPf0eCMqFAaccOs0HrWIY= github.com/open-telemetry/opamp-go v0.14.0/go.mod h1:XOGCigljsLSTZ8FfLwvat0M1QDj3conIIgRa77BWrKs= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= @@ -502,8 +502,8 @@ github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e h1:cxgCNo/R769CO23AK github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc= -github.com/operator-framework/operator-lib v0.12.0 h1:OzpMU5N7mvFgg/uje8FUUeD24Ahq64R6TdN25uswCYA= -github.com/operator-framework/operator-lib v0.12.0/go.mod h1:ClpLUI7hctEF7F5DBe/kg041dq/4NLR7XC5tArY7bG4= +github.com/operator-framework/operator-lib v0.13.0 h1:+TWgJhbJqyNix9m1LmHK5gY/lb3CGqZX3Wvl7K0k+6I= +github.com/operator-framework/operator-lib v0.13.0/go.mod h1:RDs1wGdOKWSMCO+BYSbqmmKGnD5jOP7TVP+KvoX8jMg= github.com/ovh/go-ovh v1.4.3 h1:Gs3V823zwTFpzgGLZNI6ILS4rmxZgJwJCz54Er9LwD0= github.com/ovh/go-ovh v1.4.3/go.mod h1:AkPXVtgwB6xlKblMjRKJJmjRp+ogrE7fz2lVgcQY8SY= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= @@ -545,8 +545,8 @@ github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1: github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/prometheus/client_model v0.6.0 h1:k1v3CzpSRUTrKMppY35TLwPvxHqBu0bYgxZzqGIgaos= -github.com/prometheus/client_model v0.6.0/go.mod h1:NTQHnmxFpouOD0DpvP4XujX3CdOAGQPoaGhyTchlyt8= +github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E= +github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY= github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4= github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo= @@ -688,8 +688,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= -golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA= -golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= +golang.org/x/exp v0.0.0-20240213143201-ec583247a57a h1:HinSgX1tJRX3KsL//Gxynpw5CTOAIPhgL4W8PNiIpVE= +golang.org/x/exp v0.0.0-20240213143201-ec583247a57a/go.mod h1:CxmFvTBINI24O/j8iY7H1xHzx2i4OsyguNBmN/uPtqc= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -1048,8 +1048,8 @@ k8s.io/component-base v0.29.3 h1:Oq9/nddUxlnrCuuR2K/jp6aflVvc0uDvxMzAWxnGzAo= k8s.io/component-base v0.29.3/go.mod h1:Yuj33XXjuOk2BAaHsIGHhCKZQAgYKhqIxIjIr2UXYio= k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= -k8s.io/kube-openapi v0.0.0-20240209001042-7a0d5b415232 h1:MMq4iF9pHuAz/9dLnHwBQKEoeigXClzs3MFh/seyqtA= -k8s.io/kube-openapi v0.0.0-20240209001042-7a0d5b415232/go.mod h1:Pa1PvrP7ACSkuX6I7KYomY6cmMA0Tx86waBhDUgoKPw= +k8s.io/kube-openapi v0.0.0-20240221221325-2ac9dc51f3f1 h1:rtdnaWfP40MTKv7izH81gkWpZB45pZrwIxyZdPSn1mI= +k8s.io/kube-openapi v0.0.0-20240221221325-2ac9dc51f3f1/go.mod h1:Pa1PvrP7ACSkuX6I7KYomY6cmMA0Tx86waBhDUgoKPw= k8s.io/kubectl v0.29.3 h1:RuwyyIU42MAISRIePaa8Q7A3U74Q9P4MoJbDFz9o3us= k8s.io/kubectl v0.29.3/go.mod h1:yCxfY1dbwgVdEt2zkJ6d5NNLOhhWgTyrqACIoFhpdd4= k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCfRziVtos3ofG/sQ= From 0e96e1fb07e135c36321b996cdc75d85235e42d6 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Mon, 6 May 2024 09:31:17 +0200 Subject: [PATCH 03/10] Check for the permissions instead of using a CLI flag (#2787) * Check for the permissions instead of using a CLI flag. #2588 Signed-off-by: Israel Blancas * Fix bundle Signed-off-by: Israel Blancas * Fix log message Signed-off-by: Israel Blancas * Add E2E tests and fix #2833 Signed-off-by: Israel Blancas * Fix tests Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Fix changelog Signed-off-by: Israel Blancas * Fix yaml Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Fix k8sattributes processor Signed-off-by: Israel Blancas * Fix test Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Remove checked error Signed-off-by: Israel Blancas * Revert change Signed-off-by: Israel Blancas * Fix workflow Signed-off-by: Israel Blancas * Fix E2E test Signed-off-by: Israel Blancas * Fix bundle Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Fix docs Signed-off-by: Israel Blancas * Fixes #2862 Signed-off-by: Israel Blancas * Fix E2E tests Signed-off-by: Israel Blancas * Apply changes requested in code review Signed-off-by: Israel Blancas * Remove kustomization Signed-off-by: Israel Blancas * Update bundle Signed-off-by: Israel Blancas * Fix typo Signed-off-by: Israel Blancas --------- Signed-off-by: Israel Blancas --- ...x-detector-resourcedetectionprocessor.yaml | 16 +++ .../2862-fix-clusterrolebinding-names.yaml | 16 +++ ...ssions-by-checking-the-sa-permissions.yaml | 16 +++ .github/workflows/e2e.yaml | 4 +- .gitignore | 1 + Makefile | 24 ++-- apis/v1beta1/collector_webhook.go | 25 +--- ...emetry-operator.clusterserviceversion.yaml | 7 +- config/manager/manager.yaml | 5 + controllers/opampbridge_controller_test.go | 4 + .../opentelemetrycollector_controller.go | 3 +- controllers/suite_test.go | 9 ++ internal/autodetect/main.go | 26 +++- internal/autodetect/main_test.go | 116 +++++++++++++++++- internal/autodetect/rbac/check.go | 82 +++++++++++++ internal/autodetect/rbac/operator.go | 30 +++++ internal/config/main.go | 28 +++-- internal/config/main_test.go | 13 ++ internal/config/options.go | 14 ++- .../collector/adapters/config_to_rbac_test.go | 2 +- internal/manifests/collector/collector.go | 3 +- .../processor/processor_k8sattributes.go | 18 ++- .../processor/processor_k8sattributes_test.go | 60 ++++++++- .../processor/processor_resourcedetection.go | 2 +- internal/manifests/collector/rbac.go | 2 +- .../rbac_resourcedetectionprocessor_k8s.yaml | 2 +- internal/naming/main.go | 4 +- internal/rbac/format.go | 44 +++++++ internal/rbac/format_test.go | 74 +++++++++++ main.go | 115 ++++++++--------- .../namespaces.yaml | 15 +++ .../extra-permissions-operator/nodes.yaml | 12 ++ .../extra-permissions-operator/rbac.yaml | 16 +++ .../replicaset.yaml | 11 ++ .../processor-k8sattributes/00-install.yaml | 4 + .../processor-k8sattributes/01-assert.yaml | 40 ++++++ .../processor-k8sattributes/01-install.yaml | 22 ++++ .../processor-k8sattributes/02-assert.yaml | 40 ++++++ .../processor-k8sattributes/02-install.yaml | 25 ++++ .../chainsaw-test.yaml | 24 ++++ .../00-install.yaml | 4 + .../01-assert.yaml | 33 +++++ .../01-install.yaml | 25 ++++ .../02-assert.yaml | 31 +++++ .../02-install.yaml | 25 ++++ .../chainsaw-test.yaml | 24 ++++ 46 files changed, 989 insertions(+), 127 deletions(-) create mode 100755 .chloggen/2833-fix-detector-resourcedetectionprocessor.yaml create mode 100755 .chloggen/2862-fix-clusterrolebinding-names.yaml create mode 100755 .chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml create mode 100644 internal/autodetect/rbac/check.go create mode 100644 internal/autodetect/rbac/operator.go create mode 100644 internal/rbac/format.go create mode 100644 internal/rbac/format_test.go create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/rbac.yaml create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/replicaset.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/00-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/01-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/02-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-k8sattributes/chainsaw-test.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/00-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/01-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/02-install.yaml create mode 100644 tests/e2e-automatic-rbac/processor-resourcedetection/chainsaw-test.yaml diff --git a/.chloggen/2833-fix-detector-resourcedetectionprocessor.yaml b/.chloggen/2833-fix-detector-resourcedetectionprocessor.yaml new file mode 100755 index 0000000000..effa51536f --- /dev/null +++ b/.chloggen/2833-fix-detector-resourcedetectionprocessor.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# 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: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Use the k8snode detector instead of kubernetes for the automatic RBAC creation for the resourcedetector" + +# One or more tracking issues related to the change +issues: [2833] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/.chloggen/2862-fix-clusterrolebinding-names.yaml b/.chloggen/2862-fix-clusterrolebinding-names.yaml new file mode 100755 index 0000000000..44307f7670 --- /dev/null +++ b/.chloggen/2862-fix-clusterrolebinding-names.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# 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: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "When two Collectors are created with the same name but different namespaces, the ClusterRoleBinding created by the first will be overriden by the second one." + +# One or more tracking issues related to the change +issues: [2862] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/.chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml b/.chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml new file mode 100755 index 0000000000..ab5895bb16 --- /dev/null +++ b/.chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +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: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Automatically enable RBAC creation if operator SA can create clusterroles and bindings. --create-rbac-permissions flag is noop and deprecated now. + +# One or more tracking issues related to the change +issues: [2588] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: \ No newline at end of file diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index fd876dbb98..9bd466986f 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -24,6 +24,7 @@ jobs: - "1.29" group: - e2e + - e2e-automatic-rbac - e2e-autoscale - e2e-instrumentation - e2e-opampbridge @@ -40,7 +41,8 @@ jobs: setup: "add-multi-instrumentation-params prepare-e2e" - group: e2e-metadata-filters setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=*filter.out --labels=*filter.out' prepare-e2e" - + - group: e2e-automatic-rbac + setup: "add-rbac-permissions-to-operator prepare-e2e" steps: - name: Check out code into the Go module directory uses: actions/checkout@v4 diff --git a/.gitignore b/.gitignore index 88abe862c0..1438657894 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ config/manager/kustomization.yaml # test resources kubeconfig tests/_build/ +config/rbac/extra-permissions-operator/ # autoinstrumentation artifacts build diff --git a/Makefile b/Makefile index d169d3c38b..c5a601fe0a 100644 --- a/Makefile +++ b/Makefile @@ -157,9 +157,15 @@ add-multi-instrumentation-params: add-image-opampbridge: @$(MAKE) add-operator-arg OPERATOR_ARG=--operator-opamp-bridge-image=$(OPERATOROPAMPBRIDGE_IMG) -.PHONY: enable-operator-featuregates -enable-operator-featuregates: OPERATOR_ARG = --feature-gates=$(FEATUREGATES) -enable-operator-featuregates: add-operator-arg +.PHONY: add-rbac-permissions-to-operator +add-rbac-permissions-to-operator: manifests kustomize + # Kustomize only allows patches in the folder where the kustomization is located + # This folder is ignored by .gitignore + cp -r tests/e2e-automatic-rbac/extra-permissions-operator/ config/rbac/extra-permissions-operator + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/namespaces.yaml + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/nodes.yaml + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/rbac.yaml + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/replicaset.yaml # Deploy controller in the current Kubernetes context, configured in ~/.kube/config .PHONY: deploy @@ -217,6 +223,11 @@ generate: controller-gen e2e: chainsaw $(CHAINSAW) test --test-dir ./tests/e2e +# end-to-end-test for testing automatic RBAC creation +.PHONY: e2e-automatic-rbac +e2e-automatic-rbac: chainsaw + $(CHAINSAW) test --test-dir ./tests/e2e-automatic-rbac + # end-to-end-test for testing autoscale .PHONY: e2e-autoscale e2e-autoscale: chainsaw @@ -272,9 +283,6 @@ e2e-upgrade: undeploy chainsaw .PHONY: prepare-e2e prepare-e2e: chainsaw set-image-controller add-image-targetallocator add-image-opampbridge container container-target-allocator container-operator-opamp-bridge start-kind cert-manager install-metrics-server install-targetallocator-prometheus-crds load-image-all deploy -.PHONY: prepare-e2e-with-featuregates -prepare-e2e-with-featuregates: chainsaw enable-operator-featuregates prepare-e2e - .PHONY: scorecard-tests scorecard-tests: operator-sdk $(OPERATOR_SDK) scorecard -w=5m bundle || (echo "scorecard test failed" && exit 1) @@ -320,10 +328,6 @@ endif install-metrics-server: ./hack/install-metrics-server.sh -.PHONY: install-prometheus-operator -install-prometheus-operator: - ./hack/install-prometheus-operator.sh - # This only installs the CRDs Target Allocator supports .PHONY: install-targetallocator-prometheus-crds install-targetallocator-prometheus-crds: diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index e26f4e232c..99fefdaee8 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -20,7 +20,6 @@ import ( "strings" "github.com/go-logr/logr" - authorizationv1 "k8s.io/api/authorization/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/runtime" @@ -359,7 +358,7 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r * if subjectAccessReviews, err := c.reviewer.CheckPolicyRules(ctx, r.GetNamespace(), r.Spec.TargetAllocator.ServiceAccount, targetAllocatorCRPolicyRules...); err != nil { return nil, fmt.Errorf("unable to check rbac rules %w", err) } else if allowed, deniedReviews := rbac.AllSubjectAccessReviewsAllowed(subjectAccessReviews); !allowed { - return warningsGroupedByResource(deniedReviews), nil + return rbac.WarningsGroupedByResource(deniedReviews), nil } } @@ -407,28 +406,6 @@ func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error { return nil } -// warningsGroupedByResource is a helper to take the missing permissions and format them as warnings. -func warningsGroupedByResource(reviews []*authorizationv1.SubjectAccessReview) []string { - fullResourceToVerbs := make(map[string][]string) - for _, review := range reviews { - if review.Spec.ResourceAttributes != nil { - key := fmt.Sprintf("%s/%s", review.Spec.ResourceAttributes.Group, review.Spec.ResourceAttributes.Resource) - if len(review.Spec.ResourceAttributes.Group) == 0 { - key = review.Spec.ResourceAttributes.Resource - } - fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.ResourceAttributes.Verb) - } else if review.Spec.NonResourceAttributes != nil { - key := fmt.Sprintf("nonResourceURL: %s", review.Spec.NonResourceAttributes.Path) - fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.NonResourceAttributes.Verb) - } - } - var warnings []string - for fullResource, verbs := range fullResourceToVerbs { - warnings = append(warnings, fmt.Sprintf("missing the following rules for %s: [%s]", fullResource, strings.Join(verbs, ","))) - } - return warnings -} - func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer) error { cvw := &CollectorWebhook{ reviewer: reviewer, diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index a200bdd9a0..c14ddf4e76 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-04-30T12:37:39Z" + createdAt: "2024-05-03T15:21:44Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -499,6 +499,11 @@ spec: - --zap-log-level=info - --zap-time-encoding=rfc3339nano - --enable-nginx-instrumentation=true + env: + - name: SERVICE_ACCOUNT_NAME + valueFrom: + fieldRef: + fieldPath: spec.serviceAccountName image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.99.0 livenessProbe: httpGet: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index a329ad65ab..b15e4abfd6 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -51,5 +51,10 @@ spec: requests: cpu: 100m memory: 64Mi + env: + - name: SERVICE_ACCOUNT_NAME + valueFrom: + fieldRef: + fieldPath: spec.serviceAccountName serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 diff --git a/controllers/opampbridge_controller_test.go b/controllers/opampbridge_controller_test.go index 269a43c745..99f4f43a78 100644 --- a/controllers/opampbridge_controller_test.go +++ b/controllers/opampbridge_controller_test.go @@ -37,6 +37,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/controllers" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/config" ) @@ -48,6 +49,9 @@ var opampBridgeMockAutoDetector = &mockAutoDetect{ PrometheusCRsAvailabilityFunc: func() (prometheus.Availability, error) { return prometheus.Available, nil }, + RBACPermissionsFunc: func(ctx context.Context) (rbac.Availability, error) { + return rbac.Available, nil + }, } func TestNewObjectsOnReconciliation_OpAMPBridge(t *testing.T) { diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 258c5fadfc..a713e15dfd 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -39,6 +39,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" @@ -239,7 +240,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er Owns(&autoscalingv2.HorizontalPodAutoscaler{}). Owns(&policyV1.PodDisruptionBudget{}) - if r.config.CreateRBACPermissions() { + if r.config.CreateRBACPermissions() == rbac.Available { builder.Owns(&rbacv1.ClusterRoleBinding{}) builder.Owns(&rbacv1.ClusterRole{}) } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index d20e7be8b2..a9d82248e9 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -56,6 +56,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata" @@ -95,6 +96,7 @@ var _ autodetect.AutoDetect = (*mockAutoDetect)(nil) type mockAutoDetect struct { OpenShiftRoutesAvailabilityFunc func() (openshift.RoutesAvailability, error) PrometheusCRsAvailabilityFunc func() (prometheus.Availability, error) + RBACPermissionsFunc func(ctx context.Context) (autoRBAC.Availability, error) } func (m *mockAutoDetect) PrometheusCRsAvailability() (prometheus.Availability, error) { @@ -111,6 +113,13 @@ func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailabi return openshift.RoutesNotAvailable, nil } +func (m *mockAutoDetect) RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) { + if m.RBACPermissionsFunc != nil { + return m.RBACPermissionsFunc(ctx) + } + return autoRBAC.NotAvailable, nil +} + func TestMain(m *testing.M) { ctx, cancel = context.WithCancel(context.TODO()) defer cancel() diff --git a/internal/autodetect/main.go b/internal/autodetect/main.go index e0dec72245..359843dcd0 100644 --- a/internal/autodetect/main.go +++ b/internal/autodetect/main.go @@ -16,11 +16,16 @@ package autodetect import ( + "context" + "fmt" + "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" + "github.com/open-telemetry/opentelemetry-operator/internal/rbac" ) var _ AutoDetect = (*autoDetect)(nil) @@ -29,14 +34,16 @@ var _ AutoDetect = (*autoDetect)(nil) type AutoDetect interface { OpenShiftRoutesAvailability() (openshift.RoutesAvailability, error) PrometheusCRsAvailability() (prometheus.Availability, error) + RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) } type autoDetect struct { - dcl discovery.DiscoveryInterface + dcl discovery.DiscoveryInterface + reviewer *rbac.Reviewer } // New creates a new auto-detection worker, using the given client when talking to the current cluster. -func New(restConfig *rest.Config) (AutoDetect, error) { +func New(restConfig *rest.Config, reviewer *rbac.Reviewer) (AutoDetect, error) { dcl, err := discovery.NewDiscoveryClientForConfig(restConfig) if err != nil { // it's pretty much impossible to get into this problem, as most of the @@ -46,7 +53,8 @@ func New(restConfig *rest.Config) (AutoDetect, error) { } return &autoDetect{ - dcl: dcl, + dcl: dcl, + reviewer: reviewer, }, nil } @@ -83,3 +91,15 @@ func (a *autoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailability return openshift.RoutesNotAvailable, nil } + +func (a *autoDetect) RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) { + w, err := autoRBAC.CheckRBACPermissions(ctx, a.reviewer) + if err != nil { + return autoRBAC.NotAvailable, err + } + if w != nil { + return autoRBAC.NotAvailable, fmt.Errorf("missing permissions: %s", w) + } + + return autoRBAC.Available, nil +} diff --git a/internal/autodetect/main_test.go b/internal/autodetect/main_test.go index 6b1e22369a..d5dbbc707e 100644 --- a/internal/autodetect/main_test.go +++ b/internal/autodetect/main_test.go @@ -15,19 +15,28 @@ package autodetect_test import ( + "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" + kubeTesting "k8s.io/client-go/testing" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" + "github.com/open-telemetry/opentelemetry-operator/internal/rbac" ) func TestDetectPlatformBasedOnAvailableAPIGroups(t *testing.T) { @@ -61,7 +70,7 @@ func TestDetectPlatformBasedOnAvailableAPIGroups(t *testing.T) { })) defer server.Close() - autoDetect, err := autodetect.New(&rest.Config{Host: server.URL}) + autoDetect, err := autodetect.New(&rest.Config{Host: server.URL}, nil) require.NoError(t, err) // test @@ -104,7 +113,7 @@ func TestDetectPlatformBasedOnAvailableAPIGroupsPrometheus(t *testing.T) { })) defer server.Close() - autoDetect, err := autodetect.New(&rest.Config{Host: server.URL}) + autoDetect, err := autodetect.New(&rest.Config{Host: server.URL}, nil) require.NoError(t, err) // test @@ -115,3 +124,106 @@ func TestDetectPlatformBasedOnAvailableAPIGroupsPrometheus(t *testing.T) { assert.Equal(t, tt.expected, ora) } } + +type fakeClientGenerator func() kubernetes.Interface + +const ( + createVerb = "create" + sarResource = "subjectaccessreviews" +) + +func reactorFactory(status v1.SubjectAccessReviewStatus) fakeClientGenerator { + return func() kubernetes.Interface { + c := fake.NewSimpleClientset() + c.PrependReactor(createVerb, sarResource, func(action kubeTesting.Action) (handled bool, ret runtime.Object, err error) { + // check our expectation here + if !action.Matches(createVerb, sarResource) { + return false, nil, fmt.Errorf("must be a create for a SAR") + } + sar, ok := action.(kubeTesting.CreateAction).GetObject().DeepCopyObject().(*v1.SubjectAccessReview) + if !ok || sar == nil { + return false, nil, fmt.Errorf("bad object") + } + sar.Status = status + return true, sar, nil + }) + return c + } +} + +func TestDetectRBACPermissionsBasedOnAvailableClusterRoles(t *testing.T) { + + for _, tt := range []struct { + description string + expectedAvailability autoRBAC.Availability + shouldError bool + namespace string + serviceAccount string + clientGenerator fakeClientGenerator + }{ + { + description: "Not possible to read the namespace", + namespace: "default", + shouldError: true, + expectedAvailability: autoRBAC.NotAvailable, + clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{ + Allowed: true, + }), + }, + { + description: "Not possible to read the service account", + serviceAccount: "default", + shouldError: true, + clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{ + Allowed: true, + }), + }, + { + description: "RBAC resources are NOT there", + + shouldError: true, + namespace: "default", + serviceAccount: "defaultSA", + clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{ + Allowed: false, + }), + expectedAvailability: autoRBAC.NotAvailable, + }, + { + description: "RBAC resources are there", + + shouldError: false, + namespace: "default", + serviceAccount: "defaultSA", + clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{ + Allowed: true, + }), + expectedAvailability: autoRBAC.Available, + }, + } { + t.Run(tt.description, func(t *testing.T) { + // These settings can be get from env vars + t.Setenv(autoRBAC.NAMESPACE_ENV_VAR, tt.namespace) + t.Setenv(autoRBAC.SA_ENV_VAR, tt.serviceAccount) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {})) + defer server.Close() + + r := rbac.NewReviewer(tt.clientGenerator()) + + aD, err := autodetect.New(&rest.Config{Host: server.URL}, r) + require.NoError(t, err) + + // test + rAuto, err := aD.RBACPermissions(context.Background()) + + // verify + assert.Equal(t, tt.expectedAvailability, rAuto) + if tt.shouldError { + require.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/autodetect/rbac/check.go b/internal/autodetect/rbac/check.go new file mode 100644 index 0000000000..1e133ebf49 --- /dev/null +++ b/internal/autodetect/rbac/check.go @@ -0,0 +1,82 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rbac + +import ( + "context" + "fmt" + "os" + + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/open-telemetry/opentelemetry-operator/internal/rbac" +) + +const ( + SA_ENV_VAR = "SERVICE_ACCOUNT_NAME" + NAMESPACE_ENV_VAR = "NAMESPACE" + NAMESPACE_FILE_PATH = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" +) + +func getOperatorNamespace() (string, error) { + namespace := os.Getenv(NAMESPACE_ENV_VAR) + if namespace != "" { + return namespace, nil + } + + nsBytes, err := os.ReadFile(NAMESPACE_FILE_PATH) + if err != nil { + return "", err + } + return string(nsBytes), nil +} + +func getOperatorServiceAccount() (string, error) { + sa := os.Getenv(SA_ENV_VAR) + if sa == "" { + return sa, fmt.Errorf("%s env variable not found", SA_ENV_VAR) + } + return sa, nil +} + +// CheckRBACPermissions checks if the operator has the needed permissions to create RBAC resources automatically. +// If the RBAC is there, no errors nor warnings are returned. +func CheckRBACPermissions(ctx context.Context, reviewer *rbac.Reviewer) (admission.Warnings, error) { + namespace, err := getOperatorNamespace() + if err != nil { + return nil, fmt.Errorf("%s: %w", "not possible to check RBAC rules", err) + } + + serviceAccount, err := getOperatorServiceAccount() + if err != nil { + return nil, fmt.Errorf("%s: %w", "not possible to check RBAC rules", err) + } + + rules := []*rbacv1.PolicyRule{ + { + APIGroups: []string{"rbac.authorization.k8s.io"}, + Resources: []string{"clusterrolebindings", "clusterroles"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update"}, + }, + } + + if subjectAccessReviews, err := reviewer.CheckPolicyRules(ctx, serviceAccount, namespace, rules...); err != nil { + return nil, fmt.Errorf("%s: %w", "unable to check rbac rules", err) + } else if allowed, deniedReviews := rbac.AllSubjectAccessReviewsAllowed(subjectAccessReviews); !allowed { + return rbac.WarningsGroupedByResource(deniedReviews), nil + } + return nil, nil +} diff --git a/internal/autodetect/rbac/operator.go b/internal/autodetect/rbac/operator.go new file mode 100644 index 0000000000..03d0b892ea --- /dev/null +++ b/internal/autodetect/rbac/operator.go @@ -0,0 +1,30 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rbac + +// Availability represents that the opeerator service account has permissions to create RBAC resources. +type Availability int + +const ( + // NotAvailable RBAC permissions are not available. + NotAvailable Availability = iota + + // Available NotAvailable RBAC permissions are available. + Available +) + +func (p Availability) String() string { + return [...]string{"NotAvailable", "Available"}[p] +} diff --git a/internal/config/main.go b/internal/config/main.go index 4430f3d618..a0dbe533b9 100644 --- a/internal/config/main.go +++ b/internal/config/main.go @@ -16,6 +16,7 @@ package config import ( + "context" "time" "github.com/go-logr/logr" @@ -24,6 +25,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/version" ) @@ -43,7 +45,7 @@ type Config struct { autoInstrumentationPythonImage string collectorImage string collectorConfigMapEntry string - createRBACPermissions bool + createRBACPermissions autoRBAC.Availability enableMultiInstrumentation bool enableApacheHttpdInstrumentation bool enableDotNetInstrumentation bool @@ -60,10 +62,11 @@ type Config struct { operatorOpAMPBridgeConfigMapEntry string autoInstrumentationNodeJSImage string autoInstrumentationJavaImage string - openshiftRoutesAvailability openshift.RoutesAvailability - prometheusCRAvailability prometheus.Availability - labelsFilter []string - annotationsFilter []string + + openshiftRoutesAvailability openshift.RoutesAvailability + prometheusCRAvailability prometheus.Availability + labelsFilter []string + annotationsFilter []string } // New constructs a new configuration based on the given options. @@ -72,6 +75,7 @@ func New(opts ...Option) Config { o := options{ prometheusCRAvailability: prometheus.NotAvailable, openshiftRoutesAvailability: openshift.RoutesNotAvailable, + createRBACPermissions: autoRBAC.NotAvailable, collectorConfigMapEntry: defaultCollectorConfigMapEntry, targetAllocatorConfigMapEntry: defaultTargetAllocatorConfigMapEntry, operatorOpAMPBridgeConfigMapEntry: defaultOperatorOpAMPBridgeConfigMapEntry, @@ -80,6 +84,7 @@ func New(opts ...Option) Config { enableJavaInstrumentation: true, annotationsFilter: []string{"kubectl.kubernetes.io/last-applied-configuration"}, } + for _, opt := range opts { opt(&o) } @@ -88,7 +93,6 @@ func New(opts ...Option) Config { autoDetect: o.autoDetect, collectorImage: o.collectorImage, collectorConfigMapEntry: o.collectorConfigMapEntry, - createRBACPermissions: o.createRBACPermissions, enableMultiInstrumentation: o.enableMultiInstrumentation, enableApacheHttpdInstrumentation: o.enableApacheHttpdInstrumentation, enableDotNetInstrumentation: o.enableDotNetInstrumentation, @@ -125,12 +129,22 @@ func (c *Config) AutoDetect() error { return err } c.openshiftRoutesAvailability = ora + c.logger.V(2).Info("openshift routes detected", "availability", ora) pcrd, err := c.autoDetect.PrometheusCRsAvailability() if err != nil { return err } c.prometheusCRAvailability = pcrd + c.logger.V(2).Info("prometheus cr detected", "availability", pcrd) + + rAuto, err := c.autoDetect.RBACPermissions(context.Background()) + if err != nil { + c.logger.V(2).Info("the rbac permissions are not set for the operator", "reason", err) + } + c.createRBACPermissions = rAuto + c.logger.V(2).Info("create rbac permissions detected", "availability", rAuto) + return nil } @@ -185,7 +199,7 @@ func (c *Config) CollectorConfigMapEntry() string { } // CreateRBACPermissions is true when the operator can create RBAC permissions for SAs running a collector instance. Immutable. -func (c *Config) CreateRBACPermissions() bool { +func (c *Config) CreateRBACPermissions() autoRBAC.Availability { return c.createRBACPermissions } diff --git a/internal/config/main_test.go b/internal/config/main_test.go index 7c22e65687..1f3886f776 100644 --- a/internal/config/main_test.go +++ b/internal/config/main_test.go @@ -15,6 +15,7 @@ package config_test import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -23,6 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/config" ) @@ -51,6 +53,9 @@ func TestConfigChangesOnAutoDetect(t *testing.T) { PrometheusCRsAvailabilityFunc: func() (prometheus.Availability, error) { return prometheus.Available, nil }, + RBACPermissionsFunc: func(ctx context.Context) (rbac.Availability, error) { + return rbac.Available, nil + }, } cfg := config.New( config.WithAutoDetect(mock), @@ -74,6 +79,7 @@ var _ autodetect.AutoDetect = (*mockAutoDetect)(nil) type mockAutoDetect struct { OpenShiftRoutesAvailabilityFunc func() (openshift.RoutesAvailability, error) PrometheusCRsAvailabilityFunc func() (prometheus.Availability, error) + RBACPermissionsFunc func(ctx context.Context) (rbac.Availability, error) } func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailability, error) { @@ -89,3 +95,10 @@ func (m *mockAutoDetect) PrometheusCRsAvailability() (prometheus.Availability, e } return prometheus.NotAvailable, nil } + +func (m *mockAutoDetect) RBACPermissions(ctx context.Context) (rbac.Availability, error) { + if m.RBACPermissionsFunc != nil { + return m.RBACPermissionsFunc(ctx) + } + return rbac.NotAvailable, nil +} diff --git a/internal/config/options.go b/internal/config/options.go index 40ce1dade9..7635059413 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -23,6 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus" + autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/version" ) @@ -42,7 +43,7 @@ type options struct { autoInstrumentationNginxImage string collectorImage string collectorConfigMapEntry string - createRBACPermissions bool + createRBACPermissions autoRBAC.Availability enableMultiInstrumentation bool enableApacheHttpdInstrumentation bool enableDotNetInstrumentation bool @@ -86,11 +87,6 @@ func WithCollectorConfigMapEntry(s string) Option { o.collectorConfigMapEntry = s } } -func WithCreateRBACPermissions(s bool) Option { - return func(o *options) { - o.createRBACPermissions = s - } -} func WithEnableMultiInstrumentation(s bool) Option { return func(o *options) { o.enableMultiInstrumentation = s @@ -206,6 +202,12 @@ func WithPrometheusCRAvailability(pcrd prometheus.Availability) Option { } } +func WithRBACPermissions(rAuto autoRBAC.Availability) Option { + return func(o *options) { + o.createRBACPermissions = rAuto + } +} + func WithLabelFilters(labelFilters []string) Option { return func(o *options) { diff --git a/internal/manifests/collector/adapters/config_to_rbac_test.go b/internal/manifests/collector/adapters/config_to_rbac_test.go index 28774ca846..8260d23648 100644 --- a/internal/manifests/collector/adapters/config_to_rbac_test.go +++ b/internal/manifests/collector/adapters/config_to_rbac_test.go @@ -51,7 +51,7 @@ service: desc: "resourcedetection-processor k8s", config: `processors: resourcedetection: - detectors: [kubernetes] + detectors: [k8snode] service: pipelines: traces: diff --git a/internal/manifests/collector/collector.go b/internal/manifests/collector/collector.go index 3b984d6918..9cb2302bba 100644 --- a/internal/manifests/collector/collector.go +++ b/internal/manifests/collector/collector.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -60,7 +61,7 @@ func Build(params manifests.Params) ([]client.Object, error) { } } - if params.Config.CreateRBACPermissions() { + if params.Config.CreateRBACPermissions() == rbac.Available { manifestFactories = append(manifestFactories, manifests.Factory(ClusterRole), manifests.Factory(ClusterRoleBinding), diff --git a/internal/manifests/collector/parser/processor/processor_k8sattributes.go b/internal/manifests/collector/parser/processor/processor_k8sattributes.go index 293411acbf..76fb45e08f 100644 --- a/internal/manifests/collector/parser/processor/processor_k8sattributes.go +++ b/internal/manifests/collector/parser/processor/processor_k8sattributes.go @@ -57,15 +57,19 @@ func (o *K8sAttributesParser) GetRBACRules() []rbacv1.PolicyRule { Resources: []string{"pods", "namespaces"}, Verbs: []string{"get", "watch", "list"}, }, - { - APIGroups: []string{"apps"}, - Resources: []string{"replicasets"}, - Verbs: []string{"get", "watch", "list"}, - }, + } + + replicasetPolicy := rbacv1.PolicyRule{ + APIGroups: []string{"apps"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, } extractCfg, ok := o.config["extract"] if !ok { + // k8s.deployment.name is enabled by default so, replicasets permissions are needed + // https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32248#discussion_r1560077826 + prs = append(prs, replicasetPolicy) return prs } @@ -81,7 +85,9 @@ func (o *K8sAttributesParser) GetRBACRules() []rbacv1.PolicyRule { for _, m := range metadata { metadataField := fmt.Sprint(m) - if strings.Contains(metadataField, "k8s.node") { + if metadataField == "k8s.deployment.uid" || metadataField == "k8s.deployment.name" { + prs = append(prs, replicasetPolicy) + } else if strings.Contains(metadataField, "k8s.node") { prs = append(prs, rbacv1.PolicyRule{ APIGroups: []string{""}, diff --git a/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go b/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go index c6328cc51f..7274ffaa3d 100644 --- a/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go +++ b/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go @@ -48,11 +48,11 @@ func TestK8sAttributesRBAC(t *testing.T) { }, }, { - name: "extract k8s.node", + name: "extract k8s.deployment.name", config: map[interface{}]interface{}{ "extract": map[interface{}]interface{}{ "metadata": []interface{}{ - "k8s.node", + "k8s.deployment.name", }, }, }, @@ -67,6 +67,62 @@ func TestK8sAttributesRBAC(t *testing.T) { Resources: []string{"replicasets"}, Verbs: []string{"get", "watch", "list"}, }, + }, + }, + { + name: "extract k8s.deployment.uid", + config: map[interface{}]interface{}{ + "extract": map[interface{}]interface{}{ + "metadata": []interface{}{ + "k8s.deployment.uid", + }, + }, + }, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, + }, + }, + }, + { + name: "extract k8s.pod.name", + config: map[interface{}]interface{}{ + "extract": map[interface{}]interface{}{ + "metadata": []interface{}{ + "k8s.pod.name", + }, + }, + }, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, + }, + }, + { + name: "extract k8s.node", + config: map[interface{}]interface{}{ + "extract": map[interface{}]interface{}{ + "metadata": []interface{}{ + "k8s.node", + }, + }, + }, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, { APIGroups: []string{""}, Resources: []string{"nodes"}, diff --git a/internal/manifests/collector/parser/processor/processor_resourcedetection.go b/internal/manifests/collector/parser/processor/processor_resourcedetection.go index b9038733c5..4145f6baa6 100644 --- a/internal/manifests/collector/parser/processor/processor_resourcedetection.go +++ b/internal/manifests/collector/parser/processor/processor_resourcedetection.go @@ -63,7 +63,7 @@ func (o *ResourceDetectionParser) GetRBACRules() []rbacv1.PolicyRule { for _, d := range detectors { detectorName := fmt.Sprint(d) switch detectorName { - case "kubernetes": + case "k8snode": policy := rbacv1.PolicyRule{ APIGroups: []string{""}, Resources: []string{"nodes"}, diff --git a/internal/manifests/collector/rbac.go b/internal/manifests/collector/rbac.go index 4af9223996..25bad9dcb1 100644 --- a/internal/manifests/collector/rbac.go +++ b/internal/manifests/collector/rbac.go @@ -70,7 +70,7 @@ func ClusterRoleBinding(params manifests.Params) (*rbacv1.ClusterRoleBinding, er return nil, nil } - name := naming.ClusterRoleBinding(params.OtelCol.Name) + name := naming.ClusterRoleBinding(params.OtelCol.Name, params.OtelCol.Namespace) labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) return &rbacv1.ClusterRoleBinding{ diff --git a/internal/manifests/collector/testdata/rbac_resourcedetectionprocessor_k8s.yaml b/internal/manifests/collector/testdata/rbac_resourcedetectionprocessor_k8s.yaml index 9cd7c5790d..4027b74a15 100644 --- a/internal/manifests/collector/testdata/rbac_resourcedetectionprocessor_k8s.yaml +++ b/internal/manifests/collector/testdata/rbac_resourcedetectionprocessor_k8s.yaml @@ -4,7 +4,7 @@ receivers: grpc: processors: resourcedetection: - detectors: [kubernetes] + detectors: [k8snode] exporters: otlp: endpoint: "otlp:4317" diff --git a/internal/naming/main.go b/internal/naming/main.go index 8b801ce75e..def5adbf2a 100644 --- a/internal/naming/main.go +++ b/internal/naming/main.go @@ -136,8 +136,8 @@ func ClusterRole(otelcol string, namespace string) string { } // ClusterRoleBinding builds the cluster role binding name based on the instance. -func ClusterRoleBinding(otelcol string) string { - return DNSName(Truncate("%s-collector", 63, otelcol)) +func ClusterRoleBinding(otelcol, namespace string) string { + return DNSName(Truncate("%s-%s-collector", 63, otelcol, namespace)) } // TAService returns the name to use for the TargetAllocator service. diff --git a/internal/rbac/format.go b/internal/rbac/format.go new file mode 100644 index 0000000000..784bcc39c2 --- /dev/null +++ b/internal/rbac/format.go @@ -0,0 +1,44 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rbac + +import ( + "fmt" + "strings" + + v1 "k8s.io/api/authorization/v1" +) + +// WarningsGroupedByResource is a helper to take the missing permissions and format them as warnings. +func WarningsGroupedByResource(reviews []*v1.SubjectAccessReview) []string { + fullResourceToVerbs := make(map[string][]string) + for _, review := range reviews { + if review.Spec.ResourceAttributes != nil { + key := fmt.Sprintf("%s/%s", review.Spec.ResourceAttributes.Group, review.Spec.ResourceAttributes.Resource) + if len(review.Spec.ResourceAttributes.Group) == 0 { + key = review.Spec.ResourceAttributes.Resource + } + fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.ResourceAttributes.Verb) + } else if review.Spec.NonResourceAttributes != nil { + key := fmt.Sprintf("nonResourceURL: %s", review.Spec.NonResourceAttributes.Path) + fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.NonResourceAttributes.Verb) + } + } + var warnings []string + for fullResource, verbs := range fullResourceToVerbs { + warnings = append(warnings, fmt.Sprintf("missing the following rules for %s: [%s]", fullResource, strings.Join(verbs, ","))) + } + return warnings +} diff --git a/internal/rbac/format_test.go b/internal/rbac/format_test.go new file mode 100644 index 0000000000..82f97d25fe --- /dev/null +++ b/internal/rbac/format_test.go @@ -0,0 +1,74 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rbac + +import ( + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/authorization/v1" +) + +func TestWarningsGroupedByResource(t *testing.T) { + tests := []struct { + desc string + reviews []*v1.SubjectAccessReview + expected []string + }{ + { + desc: "No access reviews", + reviews: nil, + expected: nil, + }, + { + desc: "One access review with resource attributes", + reviews: []*v1.SubjectAccessReview{ + { + Spec: v1.SubjectAccessReviewSpec{ + ResourceAttributes: &v1.ResourceAttributes{ + Verb: "get", + Group: "", + Resource: "namespaces", + }, + }, + }, + }, + expected: []string{"missing the following rules for namespaces: [get]"}, + }, + { + desc: "One access review with non resource attributes", + reviews: []*v1.SubjectAccessReview{ + { + Spec: v1.SubjectAccessReviewSpec{ + ResourceAttributes: &v1.ResourceAttributes{ + Verb: "watch", + Group: "apps", + Resource: "replicasets", + }, + }, + }, + }, + expected: []string{"missing the following rules for apps/replicasets: [watch]"}, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + w := WarningsGroupedByResource(tt.reviews) + assert.Equal(t, tt.expected, w) + }) + } + +} diff --git a/main.go b/main.go index fb12b5eccf..dbf9be361f 100644 --- a/main.go +++ b/main.go @@ -139,7 +139,7 @@ func main() { pflag.BoolVar(&enableLeaderElection, "enable-leader-election", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") - pflag.BoolVar(&createRBACPermissions, "create-rbac-permissions", false, "Automatically create RBAC permissions needed by the processors") + pflag.BoolVar(&createRBACPermissions, "create-rbac-permissions", false, "Automatically create RBAC permissions needed by the processors (deprecated)") pflag.BoolVar(&enableMultiInstrumentation, "enable-multi-instrumentation", false, "Controls whether the operator supports multi instrumentation") pflag.BoolVar(&enableApacheHttpdInstrumentation, constants.FlagApacheHttpd, true, "Controls whether the operator supports Apache HTTPD auto-instrumentation") pflag.BoolVar(&enableDotNetInstrumentation, constants.FlagDotNet, true, "Controls whether the operator supports dotnet auto-instrumentation") @@ -199,57 +199,6 @@ func main() { restConfig := ctrl.GetConfigOrDie() - // builds the operator's configuration - ad, err := autodetect.New(restConfig) - if err != nil { - setupLog.Error(err, "failed to setup auto-detect routine") - os.Exit(1) - } - - cfg := config.New( - config.WithLogger(ctrl.Log.WithName("config")), - config.WithVersion(v), - config.WithCollectorImage(collectorImage), - config.WithCreateRBACPermissions(createRBACPermissions), - config.WithEnableMultiInstrumentation(enableMultiInstrumentation), - config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation), - config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation), - config.WithEnableGoInstrumentation(enableGoInstrumentation), - config.WithEnableNginxInstrumentation(enableNginxInstrumentation), - config.WithEnablePythonInstrumentation(enablePythonInstrumentation), - config.WithEnableNodeJSInstrumentation(enableNodeJSInstrumentation), - config.WithEnableJavaInstrumentation(enableJavaInstrumentation), - config.WithTargetAllocatorImage(targetAllocatorImage), - config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage), - config.WithAutoInstrumentationJavaImage(autoInstrumentationJava), - config.WithAutoInstrumentationNodeJSImage(autoInstrumentationNodeJS), - config.WithAutoInstrumentationPythonImage(autoInstrumentationPython), - config.WithAutoInstrumentationDotNetImage(autoInstrumentationDotNet), - config.WithAutoInstrumentationGoImage(autoInstrumentationGo), - config.WithAutoInstrumentationApacheHttpdImage(autoInstrumentationApacheHttpd), - config.WithAutoInstrumentationNginxImage(autoInstrumentationNginx), - config.WithAutoDetect(ad), - config.WithLabelFilters(labelsFilter), - config.WithAnnotationFilters(annotationsFilter), - ) - err = cfg.AutoDetect() - if err != nil { - setupLog.Error(err, "failed to autodetect config variables") - } - // Only add these to the scheme if they are available - if cfg.PrometheusCRAvailability() == prometheus.Available { - setupLog.Info("Prometheus CRDs are installed, adding to scheme.") - utilruntime.Must(monitoringv1.AddToScheme(scheme)) - } else { - setupLog.Info("Prometheus CRDs are not installed, skipping adding to scheme.") - } - if cfg.OpenShiftRoutesAvailability() == openshift.RoutesAvailable { - setupLog.Info("Openshift CRDs are installed, adding to scheme.") - utilruntime.Must(routev1.Install(scheme)) - } else { - setupLog.Info("Openshift CRDs are not installed, skipping adding to scheme.") - } - var namespaces map[string]cache.Config watchNamespace, found := os.LookupEnv("WATCH_NAMESPACE") if found { @@ -298,17 +247,69 @@ func main() { os.Exit(1) } + clientset, clientErr := kubernetes.NewForConfig(mgr.GetConfig()) + if err != nil { + setupLog.Error(clientErr, "failed to create kubernetes clientset") + } + + reviewer := rbac.NewReviewer(clientset) ctx := ctrl.SetupSignalHandler() - err = addDependencies(ctx, mgr, cfg, v) + + // builds the operator's configuration + ad, err := autodetect.New(restConfig, reviewer) if err != nil { - setupLog.Error(err, "failed to add/run bootstrap dependencies to the controller manager") + setupLog.Error(err, "failed to setup auto-detect routine") os.Exit(1) } - clientset, clientErr := kubernetes.NewForConfig(mgr.GetConfig()) + + cfg := config.New( + config.WithLogger(ctrl.Log.WithName("config")), + config.WithVersion(v), + config.WithCollectorImage(collectorImage), + config.WithEnableMultiInstrumentation(enableMultiInstrumentation), + config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation), + config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation), + config.WithEnableGoInstrumentation(enableGoInstrumentation), + config.WithEnableNginxInstrumentation(enableNginxInstrumentation), + config.WithEnablePythonInstrumentation(enablePythonInstrumentation), + config.WithEnableNodeJSInstrumentation(enableNodeJSInstrumentation), + config.WithEnableJavaInstrumentation(enableJavaInstrumentation), + config.WithTargetAllocatorImage(targetAllocatorImage), + config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage), + config.WithAutoInstrumentationJavaImage(autoInstrumentationJava), + config.WithAutoInstrumentationNodeJSImage(autoInstrumentationNodeJS), + config.WithAutoInstrumentationPythonImage(autoInstrumentationPython), + config.WithAutoInstrumentationDotNetImage(autoInstrumentationDotNet), + config.WithAutoInstrumentationGoImage(autoInstrumentationGo), + config.WithAutoInstrumentationApacheHttpdImage(autoInstrumentationApacheHttpd), + config.WithAutoInstrumentationNginxImage(autoInstrumentationNginx), + config.WithAutoDetect(ad), + config.WithLabelFilters(labelsFilter), + config.WithAnnotationFilters(annotationsFilter), + ) + err = cfg.AutoDetect() if err != nil { - setupLog.Error(clientErr, "failed to create kubernetes clientset") + setupLog.Error(err, "failed to autodetect config variables") + } + // Only add these to the scheme if they are available + if cfg.PrometheusCRAvailability() == prometheus.Available { + setupLog.Info("Prometheus CRDs are installed, adding to scheme.") + utilruntime.Must(monitoringv1.AddToScheme(scheme)) + } else { + setupLog.Info("Prometheus CRDs are not installed, skipping adding to scheme.") + } + if cfg.OpenShiftRoutesAvailability() == openshift.RoutesAvailable { + setupLog.Info("Openshift CRDs are installed, adding to scheme.") + utilruntime.Must(routev1.Install(scheme)) + } else { + setupLog.Info("Openshift CRDs are not installed, skipping adding to scheme.") + } + + err = addDependencies(ctx, mgr, cfg, v) + if err != nil { + setupLog.Error(err, "failed to add/run bootstrap dependencies to the controller manager") + os.Exit(1) } - reviewer := rbac.NewReviewer(clientset) if err = controllers.NewReconciler(controllers.Params{ Client: mgr.GetClient(), diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml new file mode 100644 index 0000000000..99a57e7386 --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/namespaces.yaml @@ -0,0 +1,15 @@ +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - namespaces + verbs: + - create + - delete + - get + - list + - patch + - update + - watch diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml new file mode 100644 index 0000000000..3971ded1a4 --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml @@ -0,0 +1,12 @@ +--- +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/rbac.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/rbac.yaml new file mode 100644 index 0000000000..c36e0c2213 --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/rbac.yaml @@ -0,0 +1,16 @@ +- op: add + path: /rules/- + value: + apiGroups: + - rbac.authorization.k8s.io + resources: + - clusterrolebindings + - clusterroles + verbs: + - create + - delete + - get + - list + - patch + - update + - watch diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/replicaset.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/replicaset.yaml new file mode 100644 index 0000000000..887c17da54 --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/replicaset.yaml @@ -0,0 +1,11 @@ +- op: add + path: /rules/- + value: + apiGroups: + - apps + resources: + - replicasets + verbs: + - get + - list + - watch diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/00-install.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/00-install.yaml new file mode 100644 index 0000000000..274fa212c1 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/00-install.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: chainsaw-k8sattributes diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml new file mode 100644 index 0000000000..ae72093f6f --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml @@ -0,0 +1,40 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-k8sattributes-cluster-role +rules: +- apiGroups: + - "" + resources: + - pods + - namespaces + verbs: + - get + - watch + - list +- apiGroups: + - "apps" + resources: + - replicasets + verbs: + - get + - watch + - list +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-k8sattributes.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-k8sattributes-collector + name: simplest-chainsaw-k8sattributes-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-k8sattributes-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-k8sattributes \ No newline at end of file diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/01-install.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/01-install.yaml new file mode 100644 index 0000000000..f498429a54 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/01-install.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-k8sattributes +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + k8sattributes: + exporters: + debug: + service: + pipelines: + traces: + receivers: [otlp] + processors: [k8sattributes] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml new file mode 100644 index 0000000000..b1a5b4a58d --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml @@ -0,0 +1,40 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-k8sattributes-cluster-role +rules: +- apiGroups: + - "" + resources: + - pods + - namespaces + verbs: + - get + - watch + - list +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - watch + - list +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-k8sattributes.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-k8sattributes-collector + name: simplest-chainsaw-k8sattributes-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-k8sattributes-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-k8sattributes diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/02-install.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/02-install.yaml new file mode 100644 index 0000000000..612e9676b3 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/02-install.yaml @@ -0,0 +1,25 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-k8sattributes +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + k8sattributes: + extract: + metadata: + - k8s.node.name + exporters: + debug: + service: + pipelines: + traces: + receivers: [otlp] + processors: [k8sattributes] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/chainsaw-test.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/chainsaw-test.yaml new file mode 100644 index 0000000000..efceb94533 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-k8sattributes/chainsaw-test.yaml @@ -0,0 +1,24 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: processor-k8sattributes +spec: + steps: + - name: create-namespace + try: + - apply: + file: 00-install.yaml + - name: default-conf + try: + - apply: + file: 01-install.yaml + - assert: + file: 01-assert.yaml + - name: k8s.node + try: + - apply: + file: 02-install.yaml + - assert: + file: 02-assert.yaml diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/00-install.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/00-install.yaml new file mode 100644 index 0000000000..412a68b477 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/00-install.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: chainsaw-resourcedetection \ No newline at end of file diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml new file mode 100644 index 0000000000..077ce1a5e3 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml @@ -0,0 +1,33 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-resourcedetection-cluster-role +rules: +- apiGroups: + - config.openshift.io + resources: + - infrastructures + - infrastructures/status + verbs: + - get + - watch + - list +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-resourcedetection.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-resourcedetection-collector + app.kubernetes.io/part-of: opentelemetry + name: simplest-chainsaw-resourcedetection-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-resourcedetection-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-resourcedetection diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/01-install.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/01-install.yaml new file mode 100644 index 0000000000..139126ad34 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/01-install.yaml @@ -0,0 +1,25 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-resourcedetection +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + resourcedetection: + detectors: [openshift] + timeout: 2s + override: false + exporters: + debug: + service: + pipelines: + traces: + receivers: [otlp] + processors: [resourcedetection] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml new file mode 100644 index 0000000000..ecdea43a56 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml @@ -0,0 +1,31 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-resourcedetection-cluster-role +rules: +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-resourcedetection.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-resourcedetection-collector + app.kubernetes.io/part-of: opentelemetry + name: simplest-chainsaw-resourcedetection-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-resourcedetection-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-resourcedetection diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/02-install.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/02-install.yaml new file mode 100644 index 0000000000..f155149391 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/02-install.yaml @@ -0,0 +1,25 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-resourcedetection +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + resourcedetection: + detectors: [k8snode] + timeout: 2s + override: false + exporters: + debug: + service: + pipelines: + traces: + receivers: [otlp] + processors: [resourcedetection] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/chainsaw-test.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/chainsaw-test.yaml new file mode 100644 index 0000000000..8d0e7ccbd9 --- /dev/null +++ b/tests/e2e-automatic-rbac/processor-resourcedetection/chainsaw-test.yaml @@ -0,0 +1,24 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: processor-resourcedetection +spec: + steps: + - name: create-namespace + try: + - apply: + file: 00-install.yaml + - name: openshift-detector + try: + - apply: + file: 01-install.yaml + - assert: + file: 01-assert.yaml + - name: k8snode-detector + try: + - apply: + file: 02-install.yaml + - assert: + file: 02-assert.yaml From 9944af63606b36e9e9ad328268304e466c2f2769 Mon Sep 17 00:00:00 2001 From: Yuri Sa <48062171+yuriolisa@users.noreply.github.com> Date: Mon, 6 May 2024 16:44:11 +0200 Subject: [PATCH 04/10] Check labels annotations filter (#2923) * Fixed Annotations/Labels filter Signed-off-by: Yuri Sa * Fixed Annotations/Labels filter Signed-off-by: Yuri Sa --------- Signed-off-by: Yuri Sa --- .chloggen/fix-labels-annotations-filter.yaml | 16 ++++++++++++++ .github/workflows/e2e.yaml | 2 +- internal/manifests/manifestutils/labels.go | 5 +++-- main.go | 23 +++++++++++++++++--- 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100755 .chloggen/fix-labels-annotations-filter.yaml diff --git a/.chloggen/fix-labels-annotations-filter.yaml b/.chloggen/fix-labels-annotations-filter.yaml new file mode 100755 index 0000000000..bde0808c84 --- /dev/null +++ b/.chloggen/fix-labels-annotations-filter.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# 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: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix of Labels and Annotations filter + +# One or more tracking issues related to the change +issues: [2770] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 9bd466986f..ea6a3d7ead 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -40,7 +40,7 @@ jobs: - group: e2e-multi-instrumentation setup: "add-multi-instrumentation-params prepare-e2e" - group: e2e-metadata-filters - setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=*filter.out --labels=*filter.out' prepare-e2e" + setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=.*filter.out --labels=.*filter.out' prepare-e2e" - group: e2e-automatic-rbac setup: "add-rbac-permissions-to-operator prepare-e2e" steps: diff --git a/internal/manifests/manifestutils/labels.go b/internal/manifests/manifestutils/labels.go index e70c018b22..3605e38c12 100644 --- a/internal/manifests/manifestutils/labels.go +++ b/internal/manifests/manifestutils/labels.go @@ -25,8 +25,9 @@ import ( ) func IsFilteredSet(sourceSet string, filterSet []string) bool { - for _, pattern := range filterSet { - if match, _ := regexp.MatchString(pattern, sourceSet); match { + for _, basePattern := range filterSet { + pattern, _ := regexp.Compile(basePattern) + if match := pattern.MatchString(sourceSet); match { return match } } diff --git a/main.go b/main.go index dbf9be361f..fb856fd123 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "os" + "regexp" "runtime" "strings" "time" @@ -158,9 +159,8 @@ func main() { stringFlagOrEnv(&autoInstrumentationGo, "auto-instrumentation-go-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_GO", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-go-instrumentation/autoinstrumentation-go:%s", v.AutoInstrumentationGo), "The default OpenTelemetry Go instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationApacheHttpd, "auto-instrumentation-apache-httpd-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_APACHE_HTTPD", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationApacheHttpd), "The default OpenTelemetry Apache HTTPD instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationNginx, "auto-instrumentation-nginx-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_NGINX", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationNginx), "The default OpenTelemetry Nginx instrumentation image. This image is used when no image is specified in the CustomResource.") - pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys") - pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --annotations-filter=*filter.out will filter out annotations that looks like: annotation.filter.out: true") - pflag.IntVar(&webhookPort, "webhook-port", 9443, "The port the webhook endpoint binds to.") + pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --labels-filter=.*filter.out will filter out labels that looks like: label.filter.out: true") + pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --annotations-filter=.*filter.out will filter out annotations that looks like: annotation.filter.out: true") pflag.StringVar(&tlsOpt.minVersion, "tls-min-version", "VersionTLS12", "Minimum TLS version supported. Value must match version names from https://golang.org/pkg/crypto/tls/#pkg-constants.") pflag.StringSliceVar(&tlsOpt.cipherSuites, "tls-cipher-suites", nil, "Comma-separated list of cipher suites for the server. Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). If omitted, the default Go cipher suites will be used") pflag.Parse() @@ -305,6 +305,23 @@ func main() { setupLog.Info("Openshift CRDs are not installed, skipping adding to scheme.") } + if cfg.AnnotationsFilter() != nil { + for _, basePattern := range cfg.AnnotationsFilter() { + _, compileErr := regexp.Compile(basePattern) + if compileErr != nil { + setupLog.Error(compileErr, "could not compile the regexp pattern for Annotations filter") + } + } + } + if cfg.LabelsFilter() != nil { + for _, basePattern := range cfg.LabelsFilter() { + _, compileErr := regexp.Compile(basePattern) + if compileErr != nil { + setupLog.Error(compileErr, "could not compile the regexp pattern for Labels filter") + } + } + } + err = addDependencies(ctx, mgr, cfg, v) if err != nil { setupLog.Error(err, "failed to add/run bootstrap dependencies to the controller manager") From d42a0ce1166efe86c95f85268f265e338d0002f8 Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 7 May 2024 17:59:42 +0200 Subject: [PATCH 05/10] [autoinstrumentation/nodejs] Update to v0.46.0 (#2930) --- autoinstrumentation/nodejs/package.json | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/autoinstrumentation/nodejs/package.json b/autoinstrumentation/nodejs/package.json index e10d280b29..fc31b49676 100644 --- a/autoinstrumentation/nodejs/package.json +++ b/autoinstrumentation/nodejs/package.json @@ -11,20 +11,20 @@ "devDependencies": { "copyfiles": "^2.4.1", "rimraf": "^5.0.5", - "typescript": "^5.4.2" + "typescript": "^5.4.5" }, "dependencies": { "@opentelemetry/api": "1.8.0", - "@opentelemetry/auto-instrumentations-node": "0.45.0", - "@opentelemetry/exporter-metrics-otlp-grpc": "0.49.1", - "@opentelemetry/exporter-prometheus": "0.49.1", - "@opentelemetry/exporter-trace-otlp-grpc": "0.49.1", - "@opentelemetry/resource-detector-alibaba-cloud": "0.28.7", - "@opentelemetry/resource-detector-aws": "1.4.0", - "@opentelemetry/resource-detector-container": "0.3.7", - "@opentelemetry/resource-detector-gcp": "0.29.7", - "@opentelemetry/resources": "1.22.0", - "@opentelemetry/sdk-metrics": "1.22.0", - "@opentelemetry/sdk-node": "0.49.1" + "@opentelemetry/auto-instrumentations-node": "0.46.0", + "@opentelemetry/exporter-metrics-otlp-grpc": "0.51.0", + "@opentelemetry/exporter-prometheus": "0.51.0", + "@opentelemetry/exporter-trace-otlp-grpc": "0.51.0", + "@opentelemetry/resource-detector-alibaba-cloud": "0.28.9", + "@opentelemetry/resource-detector-aws": "1.4.2", + "@opentelemetry/resource-detector-container": "0.3.9", + "@opentelemetry/resource-detector-gcp": "0.29.9", + "@opentelemetry/resources": "1.24.0", + "@opentelemetry/sdk-metrics": "1.24.0", + "@opentelemetry/sdk-node": "0.51.0" } } From 76a0ab5419ecb63a9f0021667d2c8c6ba8bf31ac Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Wed, 8 May 2024 12:02:44 +0530 Subject: [PATCH 06/10] [Chore] Test case for scraping OpenShift in-cluster monitroing stack (#2844) * Test case for scraping OpenShift in-cluster monitroing stack * Bump Chainsaw version --- tests/e2e-openshift/Dockerfile | 2 +- .../chainsaw-test.yaml | 26 +++++++++ .../check_logs.sh | 57 +++++++++++++++++++ .../create-clusterrolebinding-assert.yaml | 19 +++++++ .../create-clusterrolebinding.yaml | 21 +++++++ .../create-otel-instance-assert.yaml | 27 +++++++++ .../create-otel-instance.yaml | 46 +++++++++++++++ 7 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 tests/e2e-openshift/scrape-in-cluster-monitoring/chainsaw-test.yaml create mode 100755 tests/e2e-openshift/scrape-in-cluster-monitoring/check_logs.sh create mode 100644 tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding-assert.yaml create mode 100644 tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding.yaml create mode 100644 tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance-assert.yaml create mode 100644 tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance.yaml diff --git a/tests/e2e-openshift/Dockerfile b/tests/e2e-openshift/Dockerfile index 5a9d46944d..5efd816d1f 100644 --- a/tests/e2e-openshift/Dockerfile +++ b/tests/e2e-openshift/Dockerfile @@ -26,7 +26,7 @@ RUN curl -LO https://github.com/kudobuilder/kuttl/releases/download/v0.15.0/kube && mv kubectl-kuttl_0.15.0_linux_x86_64 /usr/local/bin/kuttl # Install Chainsaw e2e -RUN go install github.com/kyverno/chainsaw@v0.1.7 +RUN go install github.com/kyverno/chainsaw@v0.2.0 # Install kubectl and oc RUN curl -LO https://mirror.openshift.com/pub/openshift-v4/x86_64/clients/ocp/latest/openshift-client-linux.tar.gz \ diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/chainsaw-test.yaml b/tests/e2e-openshift/scrape-in-cluster-monitoring/chainsaw-test.yaml new file mode 100644 index 0000000000..e7616fa735 --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/chainsaw-test.yaml @@ -0,0 +1,26 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: scrape-in-cluster-monitoring +spec: + namespace: chainsaw-scrape-in-cluster-monitoring + steps: + - name: Create OTEL collector with Prometheus receiver to scrape in-cluster metrics + try: + - apply: + file: create-clusterrolebinding.yaml + - assert: + file: create-clusterrolebinding-assert.yaml + - apply: + file: create-otel-instance.yaml + - assert: + file: create-otel-instance-assert.yaml + - name: Wait for the metrics to be collected + try: + - sleep: + duration: 10s + - name: Check the presence of metrics in the OTEL collector + try: + - script: + timeout: 5m + content: ./check_logs.sh diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/check_logs.sh b/tests/e2e-openshift/scrape-in-cluster-monitoring/check_logs.sh new file mode 100755 index 0000000000..1531be4d2c --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/check_logs.sh @@ -0,0 +1,57 @@ +#!/bin/bash +# This script checks the OpenTelemetry collector pod for the presence of Metrics. + +# Define the label selector +LABEL_SELECTOR="app.kubernetes.io/component=opentelemetry-collector" +NAMESPACE=chainsaw-scrape-in-cluster-monitoring + +# Define the search strings +SEARCH_STRING1='-> container' +SEARCH_STRING2='-> label_pod_security_kubernetes_io_audit: Str(restricted)' +SEARCH_STRING3='-> label_pod_security_kubernetes_io_enforce: Str(privileged)' +SEARCH_STRING4='-> label_kubernetes_io_metadata_name:' +SEARCH_STRING5='-> namespace:' + +# Initialize flags to track if strings are found +FOUND1=false +FOUND2=false +FOUND3=false +FOUND4=false +FOUND5=false + +# Loop until all strings are found +while ! $FOUND1 || ! $FOUND2 || ! $FOUND3 || ! $FOUND4 || ! $FOUND5; do + # Get the list of pods with the specified label + PODS=($(kubectl -n $NAMESPACE get pods -l $LABEL_SELECTOR -o jsonpath='{.items[*].metadata.name}')) + + # Loop through each pod and search for the strings in the logs + for POD in "${PODS[@]}"; do + # Search for the first string + if ! $FOUND1 && kubectl -n $NAMESPACE --tail=500 logs $POD | grep -q -- "$SEARCH_STRING1"; then + echo "\"$SEARCH_STRING1\" found in $POD" + FOUND1=true + fi + # Search for the second string + if ! $FOUND2 && kubectl -n $NAMESPACE --tail=500 logs $POD | grep -q -- "$SEARCH_STRING2"; then + echo "\"$SEARCH_STRING2\" found in $POD" + FOUND2=true + fi + # Search for the third string + if ! $FOUND3 && kubectl -n $NAMESPACE --tail=500 logs $POD | grep -q -- "$SEARCH_STRING3"; then + echo "\"$SEARCH_STRING3\" found in $POD" + FOUND3=true + fi + # Search for the fourth string + if ! $FOUND4 && kubectl -n $NAMESPACE --tail=500 logs $POD | grep -q -- "$SEARCH_STRING4"; then + echo "\"$SEARCH_STRING4\" found in $POD" + FOUND4=true + fi + # Search for the fifth string + if ! $FOUND5 && kubectl -n $NAMESPACE --tail=500 logs $POD | grep -q -- "$SEARCH_STRING5"; then + echo "\"$SEARCH_STRING5\" found in $POD" + FOUND5=true + fi + done +done + +echo "Found the matched metrics in collector" diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding-assert.yaml b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding-assert.yaml new file mode 100644 index 0000000000..5b9aac803e --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding-assert.yaml @@ -0,0 +1,19 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: chainsaw-scrape-in-cluster-monitoring-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: cluster-monitoring-view +subjects: +- kind: ServiceAccount + name: otel-collector + namespace: chainsaw-scrape-in-cluster-monitoring + +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cabundle + namespace: chainsaw-scrape-in-cluster-monitoring \ No newline at end of file diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding.yaml b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding.yaml new file mode 100644 index 0000000000..80e00a48b5 --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-clusterrolebinding.yaml @@ -0,0 +1,21 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: chainsaw-scrape-in-cluster-monitoring-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: cluster-monitoring-view +subjects: + - kind: ServiceAccount + name: otel-collector + namespace: chainsaw-scrape-in-cluster-monitoring + +--- +kind: ConfigMap +apiVersion: v1 +metadata: + name: cabundle + namespce: chainsaw-scrape-in-cluster-monitoring + annotations: + service.beta.openshift.io/inject-cabundle: "true" diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance-assert.yaml b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance-assert.yaml new file mode 100644 index 0000000000..ae6c0cc554 --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance-assert.yaml @@ -0,0 +1,27 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: otel-collector + namespace: chainsaw-scrape-in-cluster-monitoring +status: + availableReplicas: 1 + readyReplicas: 1 + replicas: 1 + +--- +apiVersion: v1 +kind: Service +metadata: + name: otel-collector-monitoring + namespace: chainsaw-scrape-in-cluster-monitoring +spec: + ports: + - name: monitoring + port: 8888 + protocol: TCP + targetPort: 8888 + selector: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-scrape-in-cluster-monitoring.otel + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry \ No newline at end of file diff --git a/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance.yaml b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance.yaml new file mode 100644 index 0000000000..f8d53af4ab --- /dev/null +++ b/tests/e2e-openshift/scrape-in-cluster-monitoring/create-otel-instance.yaml @@ -0,0 +1,46 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: otel + namespace: chainsaw-scrape-in-cluster-monitoring +spec: + volumeMounts: + - name: cabundle-volume + mountPath: /etc/pki/ca-trust/source/service-ca + readOnly: true + volumes: + - name: cabundle-volume + configMap: + name: cabundle + mode: deployment + config: | + receivers: + prometheus: + config: + scrape_configs: + - job_name: 'federate' + scrape_interval: 15s + scheme: https + tls_config: + ca_file: /etc/pki/ca-trust/source/service-ca/service-ca.crt + bearer_token_file: /var/run/secrets/kubernetes.io/serviceaccount/token + # honor_labels needs to be set to false due to bug https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/32555 + honor_labels: false + params: + 'match[]': + - '{__name__="kube_namespace_labels"}' + metrics_path: '/federate' + static_configs: + - targets: + - "prometheus-k8s.openshift-monitoring.svc.cluster.local:9091" + + exporters: + debug: + verbosity: detailed + + service: + pipelines: + metrics: + receivers: [prometheus] + processors: [] + exporters: [debug] From e98b7a2c42c52136bc1ccd4a49e67c2f00eb9482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraci=20Paix=C3=A3o=20Kr=C3=B6hling?= Date: Wed, 8 May 2024 16:21:27 +0200 Subject: [PATCH 07/10] [chore] fix typo on warning, improved reading of the message (#2934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [chore] fix typo on warning, improved reading of the message Signed-off-by: Juraci Paixão Kröhling * fixed tests Signed-off-by: Juraci Paixão Kröhling --------- Signed-off-by: Juraci Paixão Kröhling --- apis/v1beta1/collector_webhook.go | 2 +- apis/v1beta1/collector_webhook_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index 99fefdaee8..8780ffa7e3 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -190,7 +190,7 @@ func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollecto nullObjects := r.Spec.Config.nullObjects() if len(nullObjects) > 0 { - warnings = append(warnings, fmt.Sprintf("Collector config spec.config has null objects: %s. For compatibility tooling (kustomize and kubectl edit) it is recommended to use empty obejects e.g. batch: {}.", strings.Join(nullObjects, ", "))) + warnings = append(warnings, fmt.Sprintf("Collector config spec.config has null objects: %s. For compatibility with other tooling, such as kustomize and kubectl edit, it is recommended to use empty objects e.g. batch: {}.", strings.Join(nullObjects, ", "))) } // validate volumeClaimTemplates diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 253ae99971..99d127b3ef 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -72,7 +72,7 @@ func TestValidate(t *testing.T) { }, warnings: []string{ - "Collector config spec.config has null objects: extensions.foo:, processors.batch:, processors.foo:. For compatibility tooling (kustomize and kubectl edit) it is recommended to use empty obejects e.g. batch: {}.", + "Collector config spec.config has null objects: extensions.foo:, processors.batch:, processors.foo:. For compatibility with other tooling, such as kustomize and kubectl edit, it is recommended to use empty objects e.g. batch: {}.", }, }, } From 84fc72ab778be70be0f977d8bfd3e38c99014b38 Mon Sep 17 00:00:00 2001 From: Jason Crouse Date: Wed, 8 May 2024 11:38:03 -0400 Subject: [PATCH 08/10] opamp bridge - set healthy bool at collector pool level (#2937) * set healthy bool at collector pool level * add changelog --- .chloggen/add-collector-pool-healthy.yaml | 16 ++++++++++++++++ cmd/operator-opamp-bridge/agent/agent.go | 6 ++++++ cmd/operator-opamp-bridge/agent/agent_test.go | 8 ++++---- 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100755 .chloggen/add-collector-pool-healthy.yaml diff --git a/.chloggen/add-collector-pool-healthy.yaml b/.chloggen/add-collector-pool-healthy.yaml new file mode 100755 index 0000000000..1bc8ca48ed --- /dev/null +++ b/.chloggen/add-collector-pool-healthy.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +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: opamp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add healthy field at collector pool level in opamp bridge heartbeat + +# One or more tracking issues related to the change +issues: [2936] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/cmd/operator-opamp-bridge/agent/agent.go b/cmd/operator-opamp-bridge/agent/agent.go index cab16f7728..792c2d23de 100644 --- a/cmd/operator-opamp-bridge/agent/agent.go +++ b/cmd/operator-opamp-bridge/agent/agent.go @@ -119,11 +119,17 @@ func (agent *Agent) generateCollectorPoolHealth() (map[string]*protobufs.Compone if err != nil { return nil, err } + + isPoolHealthy := true + for _, pod := range podMap { + isPoolHealthy = isPoolHealthy && pod.Healthy + } healthMap[key.String()] = &protobufs.ComponentHealth{ StartTimeUnixNano: uint64(col.ObjectMeta.GetCreationTimestamp().UnixNano()), StatusTimeUnixNano: uint64(agent.clock.Now().UnixNano()), Status: col.Status.Scale.StatusReplicas, ComponentHealthMap: podMap, + Healthy: isPoolHealthy, } } return healthMap, nil diff --git a/cmd/operator-opamp-bridge/agent/agent_test.go b/cmd/operator-opamp-bridge/agent/agent_test.go index d883e49965..75d533eae1 100644 --- a/cmd/operator-opamp-bridge/agent/agent_test.go +++ b/cmd/operator-opamp-bridge/agent/agent_test.go @@ -272,7 +272,7 @@ func TestAgent_getHealth(t *testing.T) { StatusTimeUnixNano: uint64(fakeClock.Now().UnixNano()), ComponentHealthMap: map[string]*protobufs.ComponentHealth{ "testnamespace/collector": { - Healthy: false, // we're working with mocks so the status will never be reconciled. + Healthy: true, StartTimeUnixNano: collectorStartTime, LastError: "", Status: "", @@ -305,7 +305,7 @@ func TestAgent_getHealth(t *testing.T) { StatusTimeUnixNano: uint64(fakeClock.Now().UnixNano()), ComponentHealthMap: map[string]*protobufs.ComponentHealth{ "testnamespace/collector": { - Healthy: false, // we're working with mocks so the status will never be reconciled. + Healthy: true, StartTimeUnixNano: collectorStartTime, LastError: "", Status: "", @@ -313,7 +313,7 @@ func TestAgent_getHealth(t *testing.T) { ComponentHealthMap: map[string]*protobufs.ComponentHealth{}, }, "testnamespace/other": { - Healthy: false, // we're working with mocks so the status will never be reconciled. + Healthy: true, StartTimeUnixNano: collectorStartTime, LastError: "", Status: "", @@ -345,7 +345,7 @@ func TestAgent_getHealth(t *testing.T) { StatusTimeUnixNano: uint64(fakeClock.Now().UnixNano()), ComponentHealthMap: map[string]*protobufs.ComponentHealth{ "other/third": { - Healthy: false, // we're working with mocks so the status will never be reconciled. + Healthy: true, StartTimeUnixNano: collectorStartTime, LastError: "", Status: "", From 5d5139372e477b22d32512be5406e1e45b8b0780 Mon Sep 17 00:00:00 2001 From: ItielOlenick <67790309+ItielOlenick@users.noreply.github.com> Date: Wed, 8 May 2024 19:25:35 +0300 Subject: [PATCH 09/10] Ta https server (#2921) * Added https server, tests, secret marshalling * Added changelog * Moved TLS config. Cleaned up server. * TLS config as struct method. Go Idioms * Removed default tls file paths. Minor cleanup. --- .chloggen/ta-add-https.yaml | 18 ++++ cmd/otel-allocator/config/config.go | 59 ++++++++++++ cmd/otel-allocator/config/config_test.go | 6 ++ cmd/otel-allocator/config/flags.go | 40 +++++++-- cmd/otel-allocator/config/flags_test.go | 12 +++ .../config/testdata/config_test.yaml | 5 ++ cmd/otel-allocator/main.go | 26 +++++- cmd/otel-allocator/server/server.go | 89 +++++++++++++++---- cmd/otel-allocator/server/server_test.go | 71 ++++++++++++++- 9 files changed, 303 insertions(+), 23 deletions(-) create mode 100755 .chloggen/ta-add-https.yaml diff --git a/.chloggen/ta-add-https.yaml b/.chloggen/ta-add-https.yaml new file mode 100755 index 0000000000..1c231c3c18 --- /dev/null +++ b/.chloggen/ta-add-https.yaml @@ -0,0 +1,18 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +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 + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Added option for creating an mTLS-configured HTTPS server to fetch scrape config with real secret values. + +# One or more tracking issues related to the change +issues: [1669] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + The change introduces an option to create an additional HTTPS server with mTLS configuration. + This server is specifically utilized for obtaining the scrape configuration with actual secret values. diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index 03368d1b60..e772eadc45 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -15,6 +15,8 @@ package config import ( + "crypto/tls" + "crypto/x509" "errors" "fmt" "io/fs" @@ -53,6 +55,7 @@ type Config struct { AllocationStrategy string `yaml:"allocation_strategy,omitempty"` FilterStrategy string `yaml:"filter_strategy,omitempty"` PrometheusCR PrometheusCRConfig `yaml:"prometheus_cr,omitempty"` + HTTPS HTTPSServerConfig `yaml:"https,omitempty"` } type PrometheusCRConfig struct { @@ -64,6 +67,14 @@ type PrometheusCRConfig struct { ScrapeInterval model.Duration `yaml:"scrape_interval,omitempty"` } +type HTTPSServerConfig struct { + Enabled bool `yaml:"enabled,omitempty"` + ListenAddr string `yaml:"listen_addr,omitempty"` + CAFilePath string `yaml:"ca_file_path,omitempty"` + TLSCertFilePath string `yaml:"tls_cert_file_path,omitempty"` + TLSKeyFilePath string `yaml:"tls_key_file_path,omitempty"` +} + func LoadFromFile(file string, target *Config) error { return unmarshal(target, file) } @@ -103,6 +114,31 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error { return err } + target.HTTPS.Enabled, err = getHttpsEnabled(flagSet) + if err != nil { + return err + } + + target.HTTPS.ListenAddr, err = getHttpsListenAddr(flagSet) + if err != nil { + return err + } + + target.HTTPS.CAFilePath, err = getHttpsCAFilePath(flagSet) + if err != nil { + return err + } + + target.HTTPS.TLSCertFilePath, err = getHttpsTLSCertFilePath(flagSet) + if err != nil { + return err + } + + target.HTTPS.TLSKeyFilePath, err = getHttpsTLSKeyFilePath(flagSet) + if err != nil { + return err + } + return nil } @@ -164,3 +200,26 @@ func ValidateConfig(config *Config) error { } return nil } + +func (c HTTPSServerConfig) NewTLSConfig() (*tls.Config, error) { + cert, err := tls.LoadX509KeyPair(c.TLSCertFilePath, c.TLSKeyFilePath) + if err != nil { + return nil, err + } + + caCert, err := os.ReadFile(c.CAFilePath) + if err != nil { + return nil, err + } + + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{cert}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: caCertPool, + MinVersion: tls.VersionTLS12, + } + return tlsConfig, nil +} diff --git a/cmd/otel-allocator/config/config_test.go b/cmd/otel-allocator/config/config_test.go index fda73793e2..fec1bcdd36 100644 --- a/cmd/otel-allocator/config/config_test.go +++ b/cmd/otel-allocator/config/config_test.go @@ -61,6 +61,12 @@ func TestLoad(t *testing.T) { PrometheusCR: PrometheusCRConfig{ ScrapeInterval: model.Duration(time.Second * 60), }, + HTTPS: HTTPSServerConfig{ + Enabled: true, + CAFilePath: "/path/to/ca.pem", + TLSCertFilePath: "/path/to/cert.pem", + TLSKeyFilePath: "/path/to/key.pem", + }, PromConfig: &promconfig.Config{ GlobalConfig: promconfig.GlobalConfig{ ScrapeInterval: model.Duration(60 * time.Second), diff --git a/cmd/otel-allocator/config/flags.go b/cmd/otel-allocator/config/flags.go index bb7dbbb344..9a928cb958 100644 --- a/cmd/otel-allocator/config/flags.go +++ b/cmd/otel-allocator/config/flags.go @@ -25,11 +25,16 @@ import ( // Flag names. const ( - targetAllocatorName = "target-allocator" - configFilePathFlagName = "config-file" - listenAddrFlagName = "listen-addr" - prometheusCREnabledFlagName = "enable-prometheus-cr-watcher" - kubeConfigPathFlagName = "kubeconfig-path" + targetAllocatorName = "target-allocator" + configFilePathFlagName = "config-file" + listenAddrFlagName = "listen-addr" + prometheusCREnabledFlagName = "enable-prometheus-cr-watcher" + kubeConfigPathFlagName = "kubeconfig-path" + httpsEnabledFlagName = "enable-https-server" + listenAddrHttpsFlagName = "listen-addr-https" + httpsCAFilePathFlagName = "https-ca-file" + httpsTLSCertFilePathFlagName = "https-tls-cert-file" + httpsTLSKeyFilePathFlagName = "https-tls-key-file" ) // We can't bind this flag to our FlagSet, so we need to handle it separately. @@ -41,6 +46,11 @@ func getFlagSet(errorHandling pflag.ErrorHandling) *pflag.FlagSet { flagSet.String(listenAddrFlagName, ":8080", "The address where this service serves.") flagSet.Bool(prometheusCREnabledFlagName, false, "Enable Prometheus CRs as target sources") flagSet.String(kubeConfigPathFlagName, filepath.Join(homedir.HomeDir(), ".kube", "config"), "absolute path to the KubeconfigPath file") + flagSet.Bool(httpsEnabledFlagName, false, "Enable HTTPS additional server") + flagSet.String(listenAddrHttpsFlagName, ":8443", "The address where this service serves over HTTPS.") + flagSet.String(httpsCAFilePathFlagName, "", "The path to the HTTPS server TLS CA file.") + flagSet.String(httpsTLSCertFilePathFlagName, "", "The path to the HTTPS server TLS certificate file.") + flagSet.String(httpsTLSKeyFilePathFlagName, "", "The path to the HTTPS server TLS key file.") zapFlagSet := flag.NewFlagSet("", flag.ErrorHandling(errorHandling)) zapCmdLineOpts.BindFlags(zapFlagSet) flagSet.AddGoFlagSet(zapFlagSet) @@ -62,3 +72,23 @@ func getListenAddr(flagSet *pflag.FlagSet) (string, error) { func getPrometheusCREnabled(flagSet *pflag.FlagSet) (bool, error) { return flagSet.GetBool(prometheusCREnabledFlagName) } + +func getHttpsListenAddr(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(listenAddrHttpsFlagName) +} + +func getHttpsEnabled(flagSet *pflag.FlagSet) (bool, error) { + return flagSet.GetBool(httpsEnabledFlagName) +} + +func getHttpsCAFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsCAFilePathFlagName) +} + +func getHttpsTLSCertFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsTLSCertFilePathFlagName) +} + +func getHttpsTLSKeyFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsTLSKeyFilePathFlagName) +} diff --git a/cmd/otel-allocator/config/flags_test.go b/cmd/otel-allocator/config/flags_test.go index b1bf11b6ce..fe83863569 100644 --- a/cmd/otel-allocator/config/flags_test.go +++ b/cmd/otel-allocator/config/flags_test.go @@ -70,6 +70,18 @@ func TestFlagGetters(t *testing.T) { expectedErr: true, getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getConfigFilePath(fs) }, }, + { + name: "HttpsServer", + flagArgs: []string{"--" + httpsEnabledFlagName, "true"}, + expectedValue: true, + getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getHttpsEnabled(fs) }, + }, + { + name: "HttpsServerKey", + flagArgs: []string{"--" + httpsTLSKeyFilePathFlagName, "/path/to/tls.key"}, + expectedValue: "/path/to/tls.key", + getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getHttpsTLSKeyFilePath(fs) }, + }, } for _, tt := range tests { diff --git a/cmd/otel-allocator/config/testdata/config_test.yaml b/cmd/otel-allocator/config/testdata/config_test.yaml index 8d4a427133..6f55a99dac 100644 --- a/cmd/otel-allocator/config/testdata/config_test.yaml +++ b/cmd/otel-allocator/config/testdata/config_test.yaml @@ -4,6 +4,11 @@ collector_selector: app.kubernetes.io/managed-by: opentelemetry-operator prometheus_cr: scrape_interval: 60s +https: + enabled: true + ca_file_path: /path/to/ca.pem + tls_cert_file_path: /path/to/cert.pem + tls_key_file_path: /path/to/key.pem config: scrape_configs: - job_name: prometheus diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index efb43e541c..f9531d6740 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -86,7 +86,17 @@ func main() { setupLog.Error(err, "Unable to initialize allocation strategy") os.Exit(1) } - srv := server.NewServer(log, allocator, cfg.ListenAddr) + + httpOptions := []server.Option{} + if cfg.HTTPS.Enabled { + tlsConfig, confErr := cfg.HTTPS.NewTLSConfig() + if confErr != nil { + setupLog.Error(confErr, "Unable to initialize TLS configuration") + os.Exit(1) + } + httpOptions = append(httpOptions, server.WithTLSConfig(tlsConfig, cfg.HTTPS.ListenAddr)) + } + srv := server.NewServer(log, allocator, cfg.ListenAddr, httpOptions...) discoveryCtx, discoveryCancel := context.WithCancel(ctx) sdMetrics, err := discovery.CreateAndRegisterSDMetrics(prometheus.DefaultRegisterer) @@ -189,6 +199,20 @@ func main() { setupLog.Error(shutdownErr, "Error on server shutdown") } }) + if cfg.HTTPS.Enabled { + runGroup.Add( + func() error { + err := srv.StartHTTPS() + setupLog.Info("HTTPS Server failed to start") + return err + }, + func(_ error) { + setupLog.Info("Closing HTTPS server") + if shutdownErr := srv.ShutdownHTTPS(ctx); shutdownErr != nil { + setupLog.Error(shutdownErr, "Error on HTTPS server shutdown") + } + }) + } runGroup.Add( func() error { for { diff --git a/cmd/otel-allocator/server/server.go b/cmd/otel-allocator/server/server.go index 90771839a8..33e845103f 100644 --- a/cmd/otel-allocator/server/server.go +++ b/cmd/otel-allocator/server/server.go @@ -16,6 +16,7 @@ package server import ( "context" + "crypto/tls" "encoding/json" "fmt" "net/http" @@ -32,6 +33,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promhttp" + promcommconfig "github.com/prometheus/common/config" promconfig "github.com/prometheus/prometheus/config" "gopkg.in/yaml.v2" @@ -63,28 +65,36 @@ type Server struct { logger logr.Logger allocator allocation.Allocator server *http.Server + httpsServer *http.Server jsonMarshaller jsoniter.API // Use RWMutex to protect scrapeConfigResponse, since it // will be predominantly read and only written when config // is applied. - mtx sync.RWMutex - scrapeConfigResponse []byte + mtx sync.RWMutex + scrapeConfigResponse []byte + ScrapeConfigMarshalledSecretResponse []byte } -func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr string) *Server { - s := &Server{ - logger: log, - allocator: allocator, - jsonMarshaller: jsonConfig, +type Option func(*Server) + +// Option to create an additional https server with mTLS configuration. +// Used for getting the scrape config with real secret values. +func WithTLSConfig(tlsConfig *tls.Config, httpsListenAddr string) Option { + return func(s *Server) { + httpsRouter := gin.New() + s.setRouter(httpsRouter) + + s.httpsServer = &http.Server{Addr: httpsListenAddr, Handler: httpsRouter, ReadHeaderTimeout: 90 * time.Second, TLSConfig: tlsConfig} } +} - gin.SetMode(gin.ReleaseMode) - router := gin.New() +func (s *Server) setRouter(router *gin.Engine) { router.Use(gin.Recovery()) router.UseRawPath = true router.UnescapePathValues = false router.Use(s.PrometheusMiddleware) + router.GET("/scrape_configs", s.ScrapeConfigsHandler) router.GET("/jobs", s.JobHandler) router.GET("/jobs/:job_id/targets", s.TargetsHandler) @@ -92,8 +102,25 @@ func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr strin router.GET("/livez", s.LivenessProbeHandler) router.GET("/readyz", s.ReadinessProbeHandler) registerPprof(router.Group("/debug/pprof/")) +} + +func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr string, options ...Option) *Server { + s := &Server{ + logger: log, + allocator: allocator, + jsonMarshaller: jsonConfig, + } + + gin.SetMode(gin.ReleaseMode) + router := gin.New() + s.setRouter(router) s.server = &http.Server{Addr: listenAddr, Handler: router, ReadHeaderTimeout: 90 * time.Second} + + for _, opt := range options { + opt(s) + } + return s } @@ -107,6 +134,16 @@ func (s *Server) Shutdown(ctx context.Context) error { return s.server.Shutdown(ctx) } +func (s *Server) StartHTTPS() error { + s.logger.Info("Starting HTTPS server...") + return s.httpsServer.ListenAndServeTLS("", "") +} + +func (s *Server) ShutdownHTTPS(ctx context.Context) error { + s.logger.Info("Shutting down HTTPS server...") + return s.httpsServer.Shutdown(ctx) +} + // RemoveRegexFromRelabelAction is needed specifically for keepequal/dropequal actions because even though the user doesn't specify the // regex field for these actions the unmarshalling implementations of prometheus adds back the default regex fields // which in turn causes the receiver to error out since the unmarshaling of the json response doesn't expect anything in the regex fields @@ -151,15 +188,14 @@ func RemoveRegexFromRelabelAction(jsonConfig []byte) ([]byte, error) { return jsonConfigNew, nil } -// UpdateScrapeConfigResponse updates the scrape config response. The target allocator first marshals these -// configurations such that the underlying prometheus marshaling is used. After that, the YAML is converted -// in to a JSON format for consumers to use. -func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.ScrapeConfig) error { - var configBytes []byte +func (s *Server) MarshalScrapeConfig(configs map[string]*promconfig.ScrapeConfig, marshalSecretValue bool) error { + promcommconfig.MarshalSecretValue = marshalSecretValue + configBytes, err := yaml.Marshal(configs) if err != nil { return err } + var jsonConfig []byte jsonConfig, err = yaml2.YAMLToJSON(configBytes) if err != nil { @@ -172,8 +208,28 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap } s.mtx.Lock() - s.scrapeConfigResponse = jsonConfigNew + if marshalSecretValue { + s.ScrapeConfigMarshalledSecretResponse = jsonConfigNew + } else { + s.scrapeConfigResponse = jsonConfigNew + } s.mtx.Unlock() + + return nil +} + +// UpdateScrapeConfigResponse updates the scrape config response. The target allocator first marshals these +// configurations such that the underlying prometheus marshaling is used. After that, the YAML is converted +// in to a JSON format for consumers to use. +func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.ScrapeConfig) error { + err := s.MarshalScrapeConfig(configs, false) + if err != nil { + return err + } + err = s.MarshalScrapeConfig(configs, true) + if err != nil { + return err + } return nil } @@ -181,6 +237,9 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap func (s *Server) ScrapeConfigsHandler(c *gin.Context) { s.mtx.RLock() result := s.scrapeConfigResponse + if c.Request.TLS != nil { + result = s.ScrapeConfigMarshalledSecretResponse + } s.mtx.RUnlock() // We don't use the jsonHandler method because we don't want our bytes to be re-encoded diff --git a/cmd/otel-allocator/server/server_test.go b/cmd/otel-allocator/server/server_test.go index 1a83875cf5..88b8ad9368 100644 --- a/cmd/otel-allocator/server/server_test.go +++ b/cmd/otel-allocator/server/server_test.go @@ -15,6 +15,7 @@ package server import ( + "crypto/tls" "encoding/json" "fmt" "io" @@ -194,11 +195,14 @@ func TestServer_TargetsHandler(t *testing.T) { } func TestServer_ScrapeConfigsHandler(t *testing.T) { + svrConfig := allocatorconfig.HTTPSServerConfig{} + tlsConfig, _ := svrConfig.NewTLSConfig() tests := []struct { description string scrapeConfigs map[string]*promconfig.ScrapeConfig expectedCode int expectedBody []byte + serverOptions []Option }{ { description: "nil scrape config", @@ -455,17 +459,67 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { }, expectedCode: http.StatusOK, }, + { + description: "https secret handling", + scrapeConfigs: map[string]*promconfig.ScrapeConfig{ + "serviceMonitor/testapp/testapp3/0": { + JobName: "serviceMonitor/testapp/testapp3/0", + HonorTimestamps: true, + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(30 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + HTTPClientConfig: config.HTTPClientConfig{ + FollowRedirects: true, + BasicAuth: &config.BasicAuth{ + Username: "test", + Password: "P@$$w0rd1!?", + }, + }, + }, + }, + expectedCode: http.StatusOK, + serverOptions: []Option{ + WithTLSConfig(tlsConfig, ""), + }, + }, + { + description: "http secret handling", + scrapeConfigs: map[string]*promconfig.ScrapeConfig{ + "serviceMonitor/testapp/testapp3/0": { + JobName: "serviceMonitor/testapp/testapp3/0", + HonorTimestamps: true, + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(30 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + HTTPClientConfig: config.HTTPClientConfig{ + FollowRedirects: true, + BasicAuth: &config.BasicAuth{ + Username: "test", + Password: "P@$$w0rd1!?", + }, + }, + }, + }, + expectedCode: http.StatusOK, + }, } for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { listenAddr := ":8080" - s := NewServer(logger, nil, listenAddr) + s := NewServer(logger, nil, listenAddr, tc.serverOptions...) assert.NoError(t, s.UpdateScrapeConfigResponse(tc.scrapeConfigs)) request := httptest.NewRequest("GET", "/scrape_configs", nil) w := httptest.NewRecorder() - s.server.Handler.ServeHTTP(w, request) + if s.httpsServer != nil { + request.TLS = &tls.ConnectionState{} + s.httpsServer.Handler.ServeHTTP(w, request) + } else { + s.server.Handler.ServeHTTP(w, request) + } result := w.Result() assert.Equal(t, tc.expectedCode, result.StatusCode) @@ -478,6 +532,19 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { scrapeConfigs := map[string]*promconfig.ScrapeConfig{} err = yaml.Unmarshal(bodyBytes, scrapeConfigs) require.NoError(t, err) + + for _, c := range scrapeConfigs { + if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil { + assert.Equal(t, c.HTTPClientConfig.BasicAuth.Password, config.Secret("")) + } + } + + for _, c := range tc.scrapeConfigs { + if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil { + c.HTTPClientConfig.BasicAuth.Password = "" + } + } + assert.Equal(t, tc.scrapeConfigs, scrapeConfigs) }) } From edae5b4907b072f25006b7b73502e5c53a85a445 Mon Sep 17 00:00:00 2001 From: Anthony Mirabella Date: Wed, 8 May 2024 12:42:37 -0400 Subject: [PATCH 10/10] Move Aneurysm9 to emeritus status (#2935) Signed-off-by: Anthony J Mirabella --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9d8a1e40d9..d08b684c09 100644 --- a/README.md +++ b/README.md @@ -777,10 +777,13 @@ Emeritus Approvers: Target Allocator Maintainers ([@open-telemetry/operator-ta-maintainers](https://github.com/orgs/open-telemetry/teams/operator-ta-maintainers)): -- [Anthony Mirabella](https://github.com/Aneurysm9), AWS - [Kristina Pathak](https://github.com/kristinapathak), Lightstep - [Sebastian Poxhofer](https://github.com/secustor) +Emeritus Target Allocator Maintainers + +- [Anthony Mirabella](https://github.com/Aneurysm9), AWS + Maintainers ([@open-telemetry/operator-maintainers](https://github.com/orgs/open-telemetry/teams/operator-maintainers)): - [Jacob Aronoff](https://github.com/jaronoff97), Lightstep