Skip to content

Commit 73f9a03

Browse files
committed
Address review: test, regroup const, document in operator chart
- Add TestShouldSkipUpdateChecks_SkipEnvVar covering true / TRUE / unset / false / unrecognized values. Clears CI env vars via t.Setenv so the opt-out path is exercised regardless of how the test binary is run. - Move EnvVarSkipUpdateCheck next to ciEnvVars so all "things that decide skip" live together. Expand the doc comment to spell out the usage-metrics coupling. - Document the env var on the operator chart's `operator.env` field so it is discoverable from values.yaml and the auto-generated chart README. Run helm-docs.
1 parent f3dd189 commit 73f9a03

4 files changed

Lines changed: 40 additions & 7 deletions

File tree

deploy/charts/operator/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ The command removes all the Kubernetes components associated with the chart and
5555
| operator.autoscaling.targetCPUUtilizationPercentage | int | `80` | Target CPU utilization percentage for autoscaling |
5656
| operator.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}}` | Container security context settings for the operator |
5757
| operator.defaultImagePullSecrets | list | `[]` | List of image pull secrets that the operator applies as defaults to every workload it spawns (proxy runners, vMCP servers, registry API, etc.). Per-CR `imagePullSecrets` take precedence on name collisions; chart-level entries are appended additively. The operator parses these once at startup from the TOOLHIVE_DEFAULT_IMAGE_PULL_SECRETS environment variable. The Secrets must exist in the namespace where each workload is created. Each entry may be either a plain string (the Secret name) or an object with a `name` field, e.g.: defaultImagePullSecrets: - regcred - name: otherscred The two shapes are equivalent; the object form matches `operator.imagePullSecrets` above for convenience. |
58-
| operator.env | list | `[]` | Environment variables to set in the operator container |
58+
| operator.env | list | `[]` | Environment variables to set in the operator container. Supported toolhive-specific variables include: - TOOLHIVE_SKIP_UPDATE_CHECK: set to "true" to disable the operator's periodic update check against the ToolHive update API. Also disables the usage-metrics collection that is gated on the same check. |
5959
| operator.features.experimental | bool | `false` | Enable experimental features |
6060
| operator.features.registry | bool | `true` | Enable registry controller (MCPRegistry). This automatically sets ENABLE_REGISTRY environment variable. |
6161
| operator.features.server | bool | `true` | Enable server-related controllers (MCPServer, MCPExternalAuthConfig, MCPRemoteProxy, and ToolConfig). This automatically sets ENABLE_SERVER environment variable. |

deploy/charts/operator/values.yaml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ operator:
5555
# -- Host for the proxy deployed by the operator
5656
proxyHost: 0.0.0.0
5757

58-
# -- Environment variables to set in the operator container
58+
# -- Environment variables to set in the operator container. Supported
59+
# toolhive-specific variables include:
60+
# - TOOLHIVE_SKIP_UPDATE_CHECK: set to "true" to disable the operator's
61+
# periodic update check against the ToolHive update API. Also disables
62+
# the usage-metrics collection that is gated on the same check.
5963
env: []
6064

6165
# -- List of ports to expose from the operator container

pkg/updates/client.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,14 @@ const (
5353

5454
buildTypeRelease = "release"
5555
buildTypeLocalBuild = "local_build"
56-
57-
// EnvVarSkipUpdateCheck disables update checks when set to "true"
58-
// (case-insensitive). Disabling update checks also disables update-derived
59-
// usage metrics.
60-
EnvVarSkipUpdateCheck = "TOOLHIVE_SKIP_UPDATE_CHECK"
6156
)
6257

58+
// EnvVarSkipUpdateCheck disables update checks when set to "true"
59+
// (case-insensitive). Disabling update checks also disables update-derived
60+
// usage metrics, because usagemetrics.ShouldEnableMetrics gates on
61+
// ShouldSkipUpdateChecks.
62+
const EnvVarSkipUpdateCheck = "TOOLHIVE_SKIP_UPDATE_CHECK"
63+
6364
// ciEnvVars contains environment variables that indicate CI environments
6465
var ciEnvVars = []string{
6566
"GITHUB_ACTIONS",

pkg/updates/client_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,31 @@ func TestNewVersionClientForComponent(t *testing.T) {
6767
})
6868
}
6969
}
70+
71+
func TestShouldSkipUpdateChecks_SkipEnvVar(t *testing.T) {
72+
// Not parallel: mutates process environment via t.Setenv, including the
73+
// CI variables that other tests in this binary may also rely on.
74+
tests := []struct {
75+
name string
76+
value string
77+
wantSet bool
78+
}{
79+
{name: "unset", value: "", wantSet: false},
80+
{name: "true", value: "true", wantSet: true},
81+
{name: "TRUE uppercase", value: "TRUE", wantSet: true},
82+
{name: "false does not skip", value: "false", wantSet: false},
83+
{name: "unrecognized value does not skip", value: "maybe", wantSet: false},
84+
}
85+
86+
for _, tt := range tests {
87+
t.Run(tt.name, func(t *testing.T) {
88+
// Clear CI vars so the opt-out path is exercised in isolation,
89+
// independent of whether the test runs under GitHub Actions.
90+
for _, v := range ciEnvVars {
91+
t.Setenv(v, "")
92+
}
93+
t.Setenv(EnvVarSkipUpdateCheck, tt.value)
94+
assert.Equal(t, tt.wantSet, ShouldSkipUpdateChecks())
95+
})
96+
}
97+
}

0 commit comments

Comments
 (0)