Draft
Conversation
added 28 commits
April 12, 2026 20:59
…openshift.io/v1 (openshift 4 only).
Adjust computation in central/graphql/resolvers/cluster_vulnerabilities_postgres_test.go
… given the new validation
|
Skipping CI for Draft Pull Request. |
Contributor
Author
|
/test ocp-4-21-qa-e2e-tests |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new OpenShift version validation relies on matching specific error message substrings in tests (e.g.,
"You have specified OpenShift version 3"); consider exposing a dedicated error type/value and asserting on that instead to make the tests less brittle to message wording changes. - The error message text for unsupported OpenShift versions is duplicated in multiple places (Helm templates,
roxctlcommands, renderer tests); factoring this into a shared constant or helper would reduce the risk of the messages drifting out of sync over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new OpenShift version validation relies on matching specific error message substrings in tests (e.g., `"You have specified OpenShift version 3"`); consider exposing a dedicated error type/value and asserting on that instead to make the tests less brittle to message wording changes.
- The error message text for unsupported OpenShift versions is duplicated in multiple places (Helm templates, `roxctl` commands, renderer tests); factoring this into a shared constant or helper would reduce the risk of the messages drifting out of sync over time.
## Individual Comments
### Comment 1
<location path="image/templates/helm/shared/templates/_openshift.tpl" line_range="10-11" />
<code_context>
- If "true" is passed for $._rox.env.openshift the OpenShift version is detected based on the Kubernetes cluster version.
- If the Kubernetes version is not available (i.e. when using Helm template) auto-sensing falls back on OpenShift 3 to be
- backward compatible.
+ If "true" is passed for $._rox.env.openshift, this is unconditionally mapped to OpenShift version "4", because that is the only
+ major version we currently support.
*/}}
</code_context>
<issue_to_address>
**issue (bug_risk):** The documented behavior for passing "true" does not match the implemented behavior.
This comment now states that passing "true" for `$._rox.env.openshift` always maps to OpenShift 4, but the code only special-cases `kindIs "bool" $env.openshift`. If a user sets `openshift: "true"` (string) in values, it will go through the `int` cast instead and likely fail rendering. Please either handle the string values "true"/"false" explicitly, or update the docs to clarify that `openshift` must be a boolean, not a string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+10
to
+11
| If "true" is passed for $._rox.env.openshift, this is unconditionally mapped to OpenShift version "4", because that is the only | ||
| major version we currently support. |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The documented behavior for passing "true" does not match the implemented behavior.
This comment now states that passing "true" for $._rox.env.openshift always maps to OpenShift 4, but the code only special-cases kindIs "bool" $env.openshift. If a user sets openshift: "true" (string) in values, it will go through the int cast instead and likely fail rendering. Please either handle the string values "true"/"false" explicitly, or update the docs to clarify that openshift must be a boolean, not a string.
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request removes OpenShift 3 support from the codebase. Changes include deprecating the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.69.3)Failed to read Trivy output file: ENOENT: no such file or directory, open '/inmem/1269/nsjail-db70a2cc-09f0-4323-962d-88456cf54a56/merged/.trivy-output.json' Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
roxctl/central/generate/k8s.go (1)
166-186:⚠️ Potential issue | 🟡 MinorReject explicit
--openshift-version=0as well.This still treats
0as a valid version, soroxctl central generate openshift --openshift-version=0succeeds even though Line 173 and Line 186 now advertise that only4is supported. The simplest fix is to default the flag to4and removecase 0, or otherwise track whether the flag was omitted versus explicitly set.Suggested fix
var openshiftVersion int c := k8sBasedOrchestrator(cliEnvironment, k8sConfig, "openshift", "Openshift", func() (storage.ClusterType, error) { clusterType := storage.ClusterType_OPENSHIFT4_CLUSTER switch openshiftVersion { - case 0: case 4: default: return 0, errox.InvalidArgs.Newf("invalid OpenShift version %d, only '4' is currently supported", openshiftVersion) } return clusterType, nil }) @@ - flagWrap.IntVar(&openshiftVersion, "openshift-version", 0, "The OpenShift major version to deploy on (currently only 4 is supported).") + flagWrap.IntVar(&openshiftVersion, "openshift-version", 4, "The OpenShift major version to deploy on (currently only 4 is supported).")As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@roxctl/central/generate/k8s.go` around lines 166 - 186, The OpenShift branch treats 0 as a valid version; update the flag handling so an explicit --openshift-version=0 is rejected: either change the IntVar default from 0 to 4 and remove the switch's case 0 in the k8sBasedOrchestrator closure, or add a bool/marker to detect whether the flag was set and if not default to 4; then ensure the switch in that closure only accepts case 4 and returns an InvalidArgs error for 0 and any other values. Target symbols: the openshiftVersion variable, the flagWrap.IntVar call that defines "--openshift-version", and the anonymous closure passed into k8sBasedOrchestrator.roxctl/sensor/generate/openshift.go (1)
22-29:⚠️ Potential issue | 🟡 Minor
0is still an accepted user value here.This switch keeps
case 0, soroxctl sensor generate openshift --openshift-version=0still passes even though the command now documents/runtime-errors as OpenShift-4-only. Please make the flag default to4and dropcase 0, or use a flag type that can distinguish “unset” from an explicit0.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@roxctl/sensor/generate/openshift.go` around lines 22 - 29, The switch in sensorGenerateOpenShiftCommand.ConstructOpenShift accepts 0 as a valid openshiftVersion, letting a user pass --openshift-version=0; remove the "case 0" branch and ensure the flag defaults to 4 (or change the flag to a nullable type like *int or a custom flag that can distinguish unset vs explicit 0) so that unset uses OpenShift 4 and explicit 0 is rejected; update validation in ConstructOpenShift to only accept case 4 and return the existing InvalidArgs error for other values, and ensure the openshiftVersion field is initialized to 4 when the flag is not provided.
🧹 Nitpick comments (2)
pkg/cluster/validation_test.go (1)
82-88: Test name may be misleading after validation change.The test name says "enabled admission controller webhooks should fail" but the new error message "OpenShift 3.x is not supported anymore" indicates all OpenShift 3 clusters fail regardless of webhook settings. Consider renaming the test to reflect the broader rejection:
Suggested rename
- "OpenShift3 cluster and enabled admission controller webhooks should fail": { + "OpenShift3 cluster should fail": { configureClusterFn: func(cluster *storage.Cluster) { - cluster.AdmissionControllerEvents = true cluster.Type = storage.ClusterType_OPENSHIFT_CLUSTER }, expectedErrors: []string{"OpenShift 3.x is not supported anymore"}, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cluster/validation_test.go` around lines 82 - 88, Rename the test case key string "OpenShift3 cluster and enabled admission controller webhooks should fail" to reflect that OpenShift 3 clusters are rejected regardless of webhook settings (e.g., "OpenShift3 cluster should fail validation regardless of admission controller webhooks"), leaving configureClusterFn and expectedErrors unchanged; update only the test name in the test map where the configureClusterFn and expectedErrors fields are defined so the test description matches the new "OpenShift 3.x is not supported anymore" validation behavior.pkg/helm/charts/tests/securedclusterservices/testdata/helmtest/scanner-v4.test.yaml (1)
32-34: Consider removing the now-redundant nested test case.The
"on openshift 4"test case (lines 32-34) only setsenv.openshift: 4and inherits the parent's expectations. With OpenShift 3 removed, there's no longer a contrasting case—this nested test may be unnecessary unless you want to explicitly verify OpenShift 4 behavior differs from the default/Kubernetes path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/helm/charts/tests/securedclusterservices/testdata/helmtest/scanner-v4.test.yaml` around lines 32 - 34, Remove the redundant nested test case named "on openshift 4" that only sets env.openshift: 4 and inherits the parent's expectations; delete the block with name: "on openshift 4" and its set: env.openshift: 4 entries from the test YAML (scanner-v4.test.yaml) so the redundant OpenShift 4-only scenario is no longer present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@central/telemetry/centralclient/client.go`:
- Around line 117-120: The code currently sets orchestrator =
storage.ClusterType_OPENSHIFT_CLUSTER.String(), which labels all OpenShift
installs as the deprecated OpenShift 3 type; change the mapping so OpenShift 4
installs use storage.ClusterType_OPENSHIFT4_CLUSTER.String() instead (update the
assignment of the orchestrator variable in client.go where
storage.ClusterType_OPENSHIFT_CLUSTER is used), and if compatibility logic is
required preserve any conditional branch by adding a detection/flag to choose
OPENSHIFT4_CLUSTER for v4 clusters while keeping OPENSHIFT_CLUSTER only for
explicit legacy/OpenShift3 cases.
---
Outside diff comments:
In `@roxctl/central/generate/k8s.go`:
- Around line 166-186: The OpenShift branch treats 0 as a valid version; update
the flag handling so an explicit --openshift-version=0 is rejected: either
change the IntVar default from 0 to 4 and remove the switch's case 0 in the
k8sBasedOrchestrator closure, or add a bool/marker to detect whether the flag
was set and if not default to 4; then ensure the switch in that closure only
accepts case 4 and returns an InvalidArgs error for 0 and any other values.
Target symbols: the openshiftVersion variable, the flagWrap.IntVar call that
defines "--openshift-version", and the anonymous closure passed into
k8sBasedOrchestrator.
In `@roxctl/sensor/generate/openshift.go`:
- Around line 22-29: The switch in
sensorGenerateOpenShiftCommand.ConstructOpenShift accepts 0 as a valid
openshiftVersion, letting a user pass --openshift-version=0; remove the "case 0"
branch and ensure the flag defaults to 4 (or change the flag to a nullable type
like *int or a custom flag that can distinguish unset vs explicit 0) so that
unset uses OpenShift 4 and explicit 0 is rejected; update validation in
ConstructOpenShift to only accept case 4 and return the existing InvalidArgs
error for other values, and ensure the openshiftVersion field is initialized to
4 when the flag is not provided.
---
Nitpick comments:
In `@pkg/cluster/validation_test.go`:
- Around line 82-88: Rename the test case key string "OpenShift3 cluster and
enabled admission controller webhooks should fail" to reflect that OpenShift 3
clusters are rejected regardless of webhook settings (e.g., "OpenShift3 cluster
should fail validation regardless of admission controller webhooks"), leaving
configureClusterFn and expectedErrors unchanged; update only the test name in
the test map where the configureClusterFn and expectedErrors fields are defined
so the test description matches the new "OpenShift 3.x is not supported anymore"
validation behavior.
In
`@pkg/helm/charts/tests/securedclusterservices/testdata/helmtest/scanner-v4.test.yaml`:
- Around line 32-34: Remove the redundant nested test case named "on openshift
4" that only sets env.openshift: 4 and inherits the parent's expectations;
delete the block with name: "on openshift 4" and its set: env.openshift: 4
entries from the test YAML (scanner-v4.test.yaml) so the redundant OpenShift
4-only scenario is no longer present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a8d61af-78c0-4368-846c-1149e462eff6
⛔ Files ignored due to path filters (2)
generated/storage/cluster.pb.gois excluded by!**/*.pb.go,!**/generated/**proto/storage/proto.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
CHANGELOG.mdcentral/cluster/datastore/datastore_impl_postgres_test.gocentral/clusters/zip/render_test.gocentral/graphql/resolvers/cluster_vulnerabilities_postgres_test.gocentral/graphql/resolvers/test_utils.gocentral/telemetry/centralclient/client.gocentral/views/platformcve/view_test.goimage/embed_charts.goimage/templates/helm/shared/templates/02-scanner-v4-01-security.yamlimage/templates/helm/shared/templates/_openshift.tplimage/templates/helm/stackrox-secured-cluster/internal/cluster-config.yaml.tpl.htplimage/templates/helm/stackrox-secured-cluster/internal/compatibility-translation.yamlimage/templates/helm/stackrox-secured-cluster/internal/defaults/30-base-config.yaml.htplimage/templates/helm/stackrox-secured-cluster/sensor-chart-upgrade.md.htplimage/templates/helm/stackrox-secured-cluster/templates/_init.tpl.htplimage/templates/helm/stackrox-secured-cluster/templates/admission-controller.yamlimage/templates/helm/stackrox-secured-cluster/values.yaml.htplpkg/cluster/validation.gopkg/cluster/validation_test.gopkg/helm/charts/tests/centralservices/testdata/helmtest/central.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/injected-cabundle-cm.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/openshift-auth.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/openshift-autosense.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/openshift-monitoring.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/scanner-v4.test.yamlpkg/helm/charts/tests/centralservices/testdata/helmtest/scanner.test.yamlpkg/helm/charts/tests/securedclusterservices/feature-flags/testdata/helmtest/admission-controller-config-disabled/admission-control.test.yamlpkg/helm/charts/tests/securedclusterservices/feature-flags/testdata/helmtest/admission-controller-config/admission-control.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/development_build-non-release/development_build.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/development_build-release/development_build.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/opensource-non-release/opensource.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/opensource-release/opensource.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/override/override.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/rhacs/rhacs.test.yamlpkg/helm/charts/tests/securedclusterservices/flavor/testdata/helmtest/stackrox/stackrox.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/admission-control.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/audit-logs.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/env.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/injected-cabundle-cm.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/legacy-settings.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/openshift-monitoring.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/scanner-slim.test.yamlpkg/helm/charts/tests/securedclusterservices/testdata/helmtest/scanner-v4.test.yamlpkg/renderer/central_db_test.gopkg/renderer/kubernetes_test.goproto/storage/cluster.protoroxctl/central/generate/generate_test.goroxctl/central/generate/k8s.goroxctl/sensor/generate/openshift.gotests/roxctl/bats-tests/cluster/scanner-generate.batstests/roxctl/bats-tests/cluster/sensor-generate-bundle.bats
💤 Files with no reviewable changes (8)
- central/graphql/resolvers/test_utils.go
- image/templates/helm/stackrox-secured-cluster/values.yaml.htpl
- pkg/helm/charts/tests/securedclusterservices/feature-flags/testdata/helmtest/admission-controller-config/admission-control.test.yaml
- image/templates/helm/stackrox-secured-cluster/internal/defaults/30-base-config.yaml.htpl
- image/templates/helm/stackrox-secured-cluster/templates/_init.tpl.htpl
- roxctl/central/generate/generate_test.go
- central/cluster/datastore/datastore_impl_postgres_test.go
- tests/roxctl/bats-tests/cluster/scanner-generate.bats
Comment on lines
+117
to
120
| // Should probably be changed to OPENSHIFT4_CLUSTER, but since this is production code, keeping it for now | ||
| // due to compatibility concerns. | ||
| orchestrator = storage.ClusterType_OPENSHIFT_CLUSTER.String() | ||
| } |
Contributor
There was a problem hiding this comment.
Telemetry still reports the deprecated OpenShift 3 platform label.
At Line 119, all OpenShift deployments are still tagged as OPENSHIFT_CLUSTER, which now represents deprecated/unsupported OpenShift 3 semantics. This will misclassify OpenShift 4 telemetry.
Suggested fix
if env.Openshift.BooleanSetting() {
- // Should probably be changed to OPENSHIFT4_CLUSTER, but since this is production code, keeping it for now
- // due to compatibility concerns.
- orchestrator = storage.ClusterType_OPENSHIFT_CLUSTER.String()
+ orchestrator = storage.ClusterType_OPENSHIFT4_CLUSTER.String()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Should probably be changed to OPENSHIFT4_CLUSTER, but since this is production code, keeping it for now | |
| // due to compatibility concerns. | |
| orchestrator = storage.ClusterType_OPENSHIFT_CLUSTER.String() | |
| } | |
| if env.Openshift.BooleanSetting() { | |
| orchestrator = storage.ClusterType_OPENSHIFT4_CLUSTER.String() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@central/telemetry/centralclient/client.go` around lines 117 - 120, The code
currently sets orchestrator = storage.ClusterType_OPENSHIFT_CLUSTER.String(),
which labels all OpenShift installs as the deprecated OpenShift 3 type; change
the mapping so OpenShift 4 installs use
storage.ClusterType_OPENSHIFT4_CLUSTER.String() instead (update the assignment
of the orchestrator variable in client.go where
storage.ClusterType_OPENSHIFT_CLUSTER is used), and if compatibility logic is
required preserve any conditional branch by adding a detection/flag to choose
OPENSHIFT4_CLUSTER for v4 clusters while keeping OPENSHIFT_CLUSTER only for
explicit legacy/OpenShift3 cases.
Comment on lines
7
to
11
| {{- if not ._rox.env.openshift }} | ||
| type: KUBERNETES_CLUSTER | ||
| {{- else }} | ||
| type: {{ if eq (int ._rox.env.openshift) 4 -}} OPENSHIFT4_CLUSTER {{- else -}} OPENSHIFT_CLUSTER {{ end }} | ||
| type: OPENSHIFT4_CLUSTER | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
Do not silently coerce deprecated OpenShift 3 input to OpenShift 4.
At Line 10, any truthy ._rox.env.openshift becomes OPENSHIFT4_CLUSTER. That lets env.openshift: 3 bypass the OpenShift 3 rejection (pkg/cluster/validation.go, Lines 64-66), which weakens the deprecation enforcement path.
Proposed fix
{{- if not ._rox.env.openshift }}
type: KUBERNETES_CLUSTER
- {{- else }}
+ {{- else if eq (._rox.env.openshift | int) 3 }}
+ {{- fail "OpenShift 3.x is not supported anymore" }}
+ {{- else }}
type: OPENSHIFT4_CLUSTER
{{- end }}Please also add one helmtest case asserting env.openshift: 3 errors, to prevent regressions.
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- if not ._rox.env.openshift }} | |
| type: KUBERNETES_CLUSTER | |
| {{- else }} | |
| type: {{ if eq (int ._rox.env.openshift) 4 -}} OPENSHIFT4_CLUSTER {{- else -}} OPENSHIFT_CLUSTER {{ end }} | |
| type: OPENSHIFT4_CLUSTER | |
| {{- end }} | |
| {{- if not ._rox.env.openshift }} | |
| type: KUBERNETES_CLUSTER | |
| {{- else if eq (._rox.env.openshift | int) 3 }} | |
| {{- fail "OpenShift 3.x is not supported anymore" }} | |
| {{- else }} | |
| type: OPENSHIFT4_CLUSTER | |
| {{- end }} |
Contributor
🚀 Build Images ReadyImages are ready for commit ea80cab. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-655-gea80cab06f |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19968 +/- ##
==========================================
- Coverage 49.56% 49.56% -0.01%
==========================================
Files 2764 2764
Lines 208357 208406 +49
==========================================
+ Hits 103269 103293 +24
- Misses 97436 97457 +21
- Partials 7652 7656 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@mclasmeier: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
change me!
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!