chore(chart-deps): update otel-operator to version 0.120.0 - #3446
chore(chart-deps): update otel-operator to version 0.120.0#3446svcAPLBot wants to merge 24 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the vendored opentelemetry-operator Helm chart dependency to 0.120.0 and refreshes related chart artifacts (CRDs, schema, test hooks) for the newer upstream version.
Changes:
- Bump
opentelemetry-operatorchart dependency/version references to0.120.0. - Update included CRDs and values schema to match the updated chart.
- Adjust Helm test pods/scripts and update the collector image tag.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/otel-operator/values.yaml | Updates default collector image tag. |
| charts/otel-operator/values.schema.json | Adds a new feature gate entry to the chart values schema. |
| charts/otel-operator/templates/tests/test-service-connection.yaml | Renames test pods and changes the service connectivity test loop/script. |
| charts/otel-operator/templates/tests/test-certmanager-connection.yaml | Updates cert-manager webhook address and changes the connectivity test loop/script. |
| charts/otel-operator/templates/clusterrole.yaml | Adds RBAC permissions for operators.coreos.com clusterserviceversions. |
| charts/otel-operator/crds/crd-opentelemetrycollector.yaml | Updates collector CRD schema (status fields). |
| charts/otel-operator/crds/crd-opentelemetry.io_opampbridges.yaml | Updates opampbridge CRD schema (TLS fields). |
| charts/otel-operator/Chart.yaml | Updates chart version and appVersion. |
| chart/chart-index/Chart.yaml | Bumps the umbrella chart dependency on otel-operator to 0.120.0. |
| apps.yaml | Updates the otel app metadata version string. |
| @@ -1,5 +1,5 @@ | |||
| apiVersion: v2 | |||
| appVersion: 0.153.0 | |||
| appVersion: 0.156.0 | |||
| - get | ||
| - list | ||
| - patch | ||
| - update | ||
| - watch |
| wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true) | ||
| if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then | ||
| exit 0 | ||
| fi |
| wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true) | ||
| if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then | ||
| exit 0 | ||
| fi |
| wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true) | ||
| if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then | ||
| exit 0 | ||
| fi |
| otel: | ||
| title: OpenTelemetry Operator | ||
| appVersion: 0.153.0 | ||
| appVersion: 0.156.0 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
charts/otel-operator/templates/tests/test-service-connection.yaml:32
- The test loop captures
wgetoutput and compares it to an error string, butbusybox wget -q(quiet) suppresses output (including errors). As a result,wget_outputwill be empty and this test will always time out/fail even when the service is reachable. Consider removing-qand checking for a status substring instead of exact full output.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-service-connection.yaml:91
- Same issue as the metrics test:
busybox wget -qsuppresses output (including errors), sowget_outputis likely empty and the string comparison never succeeds. Removing-qand matching on the status substring makes the test actually validate reachability.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:32
busybox wget -qsuppresses output (including errors), sowget_outputis likely empty and the loop will always fail even if cert-manager is reachable. Remove-qand check for a status substring so the test can succeed when the service responds.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/clusterrole.yaml:218
- This change grants cluster-wide
patch/updateon OLM ClusterServiceVersions (operators.coreos.com/clusterserviceversions). That is a significant permissions expansion and can allow mutating OLM-installed operators if OLM is present. If the operator only needs to read CSVs, consider limiting this rule toget/list/watch; otherwise, consider gating the mutating verbs behind a feature gate / values flag with a short rationale in the chart.
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- patch
- update
- watch
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
charts/otel-operator/templates/tests/test-service-connection.yaml:33
- The test currently compares the full wget error string exactly (including HTTP version). With
busybox:latestthe wording/version can vary (e.g., HTTP/1.1), which can make this Helm test flaky even when the service is up. Consider matching on the status substring instead of exact equality.
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:33
- The test currently compares the full wget error string exactly (including HTTP version). With
busybox:latestthe wording/version can vary (e.g., HTTP/1.1), which can make this Helm test flaky even when the service is up. Consider matching on the status substring instead of exact equality.
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
charts/otel-operator/templates/clusterrole.yaml:218
- This adds cluster-wide
patch/updatepermissions onoperators.coreos.com/clusterserviceversions, which is a significant RBAC expansion (write access to OLM resources). If this is only needed on OLM/OpenShift clusters, consider gating it behind an API presence check or a values flag; otherwise consider whether read-only verbs are sufficient.
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- patch
- update
- watch
charts/otel-operator/templates/tests/test-service-connection.yaml:92
- The test currently compares the full wget error string exactly (including HTTP version). With
busybox:latestthe wording/version can vary (e.g., HTTP/1.1), which can make this Helm test flaky even when the service is up. Consider matching on the status substring instead of exact equality.
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
charts/otel-operator/templates/clusterrole.yaml:218
- The added RBAC rule grants write access (
patch/update) to OLM ClusterServiceVersions. For the upstream opentelemetry-operator chart 0.120.0, the CSV permission is read-only (get/list/watch); giving the operator write access to cluster-scoped CSVs is unnecessarily permissive.
verbs:
- get
- list
- patch
- update
- watch
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
charts/otel-operator/templates/tests/test-service-connection.yaml:32
- The Helm test checks for an exact
wgeterror string (HTTP/1.0 + exact wording). This is brittle acrosswgetversions and may cause flakyhelm testfailures even when the service is reachable (e.g., HTTP/1.1 or slightly different phrasing). Prefer a looser match for a 400 response.
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-service-connection.yaml:91
- The Helm test checks for an exact
wgeterror string (HTTP/1.0 + exact wording). This is brittle acrosswgetversions and may cause flakyhelm testfailures even when the service is reachable. Prefer a looser match for a 400 response.
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:32
- The Helm test checks for an exact
wgeterror string (HTTP/1.0 + exact wording). This is brittle acrosswgetversions and may cause flakyhelm testfailures even when the cert-manager webhook is reachable. Prefer a looser match for a 400 response.
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/clusterrole.yaml:218
- This change grants the operator ClusterRole
patch/updateonoperators.coreos.com/clusterserviceversionscluster-wide. This is a significant privilege escalation; if it’s not strictly required, it should be removed or gated behind an explicit opt-in value (and documented why it’s needed).
- get
- list
- patch
- update
- watch
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
charts/otel-operator/templates/clusterrole.yaml:218
- This change expands the operator ClusterRole to include patch/update on OLM ClusterServiceVersions. That’s a high-privilege permission surface (it allows modifying OLM-managed operators) and it’s also irrelevant on clusters where the operators.coreos.com API isn’t present. Consider gating this rule on the API being available (or a chart value) to keep least-privilege by default.
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- patch
- update
- watch
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
charts/otel-operator/templates/clusterrole.yaml:218
- This change grants the operator ClusterRole permission to patch/update
operators.coreos.comClusterServiceVersions. That significantly increases privileges and is unrelated to typical OpenTelemetry Operator operation unless OLM integration is explicitly supported. Consider removing these verbs (or gating the rule behind a dedicated values flag) unless there is a documented, tested requirement for mutating CSVs.
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- patch
- update
- watch
charts/otel-operator/crds/crd-opentelemetry.io_opampbridges.yaml:883
- The new
tlssection introducesinsecure_skip_verify(snake_case) alongside otherwise camelCase fields (e.g.,serviceAccount). Kubernetes APIs and CRD schemas typically use camelCase JSON keys; a snake_case field can be surprising for clients and may not match the controller's expected field name. Please confirm this matches the upstream OpAMPBridge API; if not, rename it toinsecureSkipVerifyto align with conventions.
tls:
properties:
insecure:
type: boolean
insecure_skip_verify:
type: boolean
type: object
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
charts/otel-operator/templates/tests/test-service-connection.yaml:31
- The test script captures
wgetoutput to detect an expected HTTP 400, butwget -q(busybox) is likely to suppress the error text entirely. Also, comparing against the full exact message is brittle across wget implementations. Consider capturing stderr (without-q) and matching on the presence of400instead.
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:31
- The cert-manager connectivity test relies on capturing the HTTP 400 from
wget, but with busyboxwget -qcan suppress the error output entirely. Exact-string equality on the full message is also fragile; capture stderr and match for400.
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
charts/otel-operator/templates/clusterrole.yaml:218
- This update grants the operator ClusterRole
patch/updateon OLM ClusterServiceVersions cluster-wide. That’s a high-privilege permission (and not obviously required for a Helm-installed operator). If it’s only needed for OLM-managed deployments, consider making it conditional behind an explicit value/feature gate, or reduce it to read-only verbs if the operator only needs discovery.
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- patch
- update
- watch
charts/otel-operator/templates/tests/test-service-connection.yaml:90
- Same as the metrics test:
wget -qcan suppress the error text you’re trying to match, and exact-string matching makes the test fragile. Prefer capturing stderr and matching for400.
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:19
- Hard-coding the cert-manager webhook DNS name to the
cert-managernamespace will makehelm testfail in clusters where cert-manager runs in a different namespace (or uses a differently named service). It would be more robust to make the namespace/service name configurable via values (and add it to values.schema.json), defaulting tocert-manager-webhook.cert-manager.
- name: CERT_MANAGER_CLUSTERIP
value: "cert-manager-webhook.cert-manager"
- name: CERT_MANAGER_PORT
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
apps.yaml:224
appVersionin apps.yaml appears to represent the application/operator version for other entries (e.g., trivy, sealed-secrets). Updating this to0.156.0is inconsistent with the opentelemetry-operator chart version0.120.0and will likely confuse consumers reading app metadata.
otel:
title: OpenTelemetry Operator
appVersion: 0.156.0
repo: https://github.com/open-telemetry/opentelemetry-operator
charts/otel-operator/Chart.yaml:2
appVersionis used as the default tag for the operator image whenmanager.image.tagis empty (see templates/_helpers.tpl). SettingappVersionto0.156.0will cause the deployment to pullghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.156.0, which is very likely not the intended operator version for chart0.120.0.appVersionshould track the operator image version, while the collector image version is already controlled bymanager.collectorImage.tagin values.yaml.
apiVersion: v2
appVersion: 0.156.0
charts/otel-operator/templates/tests/test-service-connection.yaml:32
- This test captures and compares
wgeterror output, but it still uses-q(quiet). With BusyBoxwget(the configured test image),-qsuppresses diagnostics, sowget_outputcan be empty even when the service responds, causing the loop to time out and the Helm test to fail. Drop-qand match on the presence of an HTTP 400 status more robustly.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-service-connection.yaml:91
- This test captures and compares
wgeterror output, but it still uses-q(quiet). With BusyBoxwget(the configured test image),-qsuppresses diagnostics, sowget_outputcan be empty even when the service responds, causing the loop to time out and the Helm test to fail. Drop-qand match on the presence of an HTTP 400 status more robustly.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:32
- This test captures and compares
wgeterror output, but it still uses-q(quiet). With BusyBoxwget(the configured test image),-qsuppresses diagnostics, sowget_outputcan be empty even when the service responds, causing the loop to time out and the Helm test to fail. Drop-qand match on the presence of an HTTP 400 status more robustly.
while [ "$i" -lt 30 ]; do
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
charts/otel-operator/templates/tests/test-service-connection.yaml:33
- The Helm test currently relies on matching the full
wgeterror string (includingHTTP/1.0), which is brittle across wget implementations/versions and will also fail if the endpoint returns a non-400 response (even though the service is up). Since this chart uses the BusyBox image, it’s more robust to usewget’s exit code (0=success, 8=HTTP error response) and add a short network timeout to avoid hanging the test container.
wget_output=$(wget -q "$MANAGER_METRICS_SERVICE_CLUSTERIP:$MANAGER_METRICS_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
charts/otel-operator/templates/clusterrole.yaml:217
- This change grants the operator ClusterRole
patch/updateon OLM ClusterServiceVersions (operators.coreos.com/clusterserviceversions). That’s a significant expansion of cluster-wide write permissions; unless the operator actually mutates CSVs at runtime, it would be safer to keep this rule read-only (get/list/watch) or gate the write verbs behind an explicit value (e.g., only when deploying via OLM/OpenShift).
verbs:
- get
- list
- patch
- update
charts/otel-operator/templates/tests/test-service-connection.yaml:92
- This test hits the webhook Service (the Deployment mounts serving certs for the webhook server), but the current check uses
wgetwithout an explicithttps://URL and matches an exactHTTP/1.0 400 Bad Requeststring. Plain HTTP to a TLS port (or a different HTTP version/status line) can cause false negatives even when the webhook is healthy. Consider using anhttps://URL and checking wget’s exit code (0=success, 8=HTTP error response) instead of matching the full error string.
wget_output=$(wget -q "$WEBHOOK_SERVICE_CLUSTERIP:$WEBHOOK_SERVICE_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
charts/otel-operator/templates/tests/test-certmanager-connection.yaml:33
- The cert-manager webhook endpoint is on 443/TLS, but the test uses
wgetwithout an explicithttps://URL and depends on an exactHTTP/1.0 400 Bad Requeststring. This is likely to be flaky (TLS vs plain HTTP, HTTP version differences, different status lines) even when the service is up. Consider using an https URL and checking wget’s exit code instead of matching the full error string.
wget_output=$(wget -q "$CERT_MANAGER_CLUSTERIP:$CERT_MANAGER_PORT" 2>&1 || true)
if [ "$wget_output" = "wget: server returned error: HTTP/1.0 400 Bad Request" ]; then
exit 0
fi
i=$((i + 1))
This PR updates the dependency opentelemetry-operator to version 0.120.0.