Remove duplicated Sidecar mode validations #4469
Conversation
| return warnings, fmt.Errorf("the OpenTelemetry Spec Ingress configuiration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s", | ||
| ModeDeployment, ModeDaemonSet, ModeStatefulSet, | ||
| ) | ||
| } |
There was a problem hiding this comment.
This was simply duplicated with the validation just above.
|
Note that we have E2E tests checking both options - the CRD and the webhook - here. These were originally done as E2E tests because we were already running them for the oldest and latest K8s version we support. I think using envtest for this is more ergonomic, though. So how about:
WDYT @open-telemetry/operator-approvers ? |
|
Thank you so much for the feedback! I’ve updated the Makefile so that we can run the unit tests with the minimum supported version in CI and removed the e2e tests. Please let me know what you think 🙏 |
| expectedErr: fmt.Sprintf("Ingress can only be used in combination with the modes: %s, %s, %s", v1beta1.ModeDeployment, v1beta1.ModeDaemonSet, v1beta1.ModeStatefulSet), | ||
| }, | ||
| { | ||
| name: "invalid mode with priorityClassName", |
There was a problem hiding this comment.
Are we removing these tests because we are not longer able to test the CEL expressions?
There was a problem hiding this comment.
They’re now being tested in TestValidationViaCRDAnnotations using envtest 🙂
3 participants
Description:
This PR is part of the migration from webhook-based validation to CEL. As a first step, I removed the duplicate validations from the webhook. Thank you for reviewing!
Link to tracking Issue(s):
Testing:
Documentation: