Skip to content

Commit 04899a2

Browse files
🌱 chore(deps): upgrade k8s to 1.35 and Go dependencies (edge) (#2486)
* chore(deps): upgrade k8s to 1.35 and Go dependencies (edge) Assisted-by: Cursor/Claude * fix breaking type-safe webhook API introduced by controller-runtime v0.23.0 controller-runtime v0.23.0 introduced type-safe webhook builders using generics. Webhook methods now receive typed objects directly instead of runtime.Object, eliminating the need for type assertions and providing compile-time type safety. Changes: - Update ClusterCatalog webhook Default method to use *ocv1.ClusterCatalog - Update SetupWebhookWithManager to pass type to NewWebhookManagedBy - Simplify tests to leverage type safety (no type assertion tests needed) Assisted-by: Cursor/Claude * fix breaking ENVTEST teardown introduced by controller-runtime v0.23.0 controller-runtime v0.23.0 changed test environment teardown timing behavior. Direct calls to testEnv.Stop() can fail intermittently due to graceful shutdown timing, requiring retry logic to ensure proper cleanup. Changes: - Add StopWithRetry helper function to test/utils.go with retry logic - Wrap testEnv.Stop() calls with retry helper (1-minute timeout, 1-second interval) - Add error logging for each retry attempt to aid debugging - Update api/v1/suite_test.go to use test.StopWithRetry - Update internal/operator-controller/controllers/suite_test.go to use test.StopWithRetry This shared utility follows DRY principle and ensures consistent teardown behavior across all test suites, preventing flaky test failures during teardown. Assisted-by: Cursor/Claude Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # api/v1/suite_test.go # internal/operator-controller/controllers/suite_test.go * fix breaking StatusWriter interface introduced by controller-runtime v0.23.0 controller-runtime v0.23.0 added Apply method to the StatusWriter interface for server-side apply support on status subresources. Changes: - Add Apply method to statusWriterMock to satisfy StatusWriter interface - Method signature: Apply(context.Context, runtime.ApplyConfiguration, ...SubResourceApplyOption) Assisted-by: Cursor/Claude * fix: document server-side apply requirement with nolint for client.Apply The client.Apply patch type constant is deprecated in controller-runtime v0.23.0+ in favor of typed Apply methods that require generated apply configurations. However, server-side apply with field ownership management is essential for this controller to properly manage ClusterExtensionRevision lifecycle. The deprecated client.Apply provides: - Field ownership via client.FieldOwner() - Force ownership via client.ForceOwnership - Automatic create-or-update semantics Alternative approaches (MergeFrom, StrategicMerge) lack field management support, causing upgrade test failures where ownership conflicts prevent proper updates during controller upgrades. Changes: - Keep client.Apply usage with comprehensive nolint explanation - Document why server-side apply semantics are required - Note migration path: generate apply configurations project-wide This ensures controllers work correctly during upgrades while documenting the technical debt for future migration. Assisted-by: Cursor/Claude * fix: replace deprecated NewSimpleClientset with NewClientset The fake.NewSimpleClientset is deprecated in client-go in favor of fake.NewClientset which provides better support for field management and server-side apply testing. The migration is straightforward for tests that don't require apply configurations - simply replace NewSimpleClientset() with NewClientset(). The new implementation works seamlessly with existing reactor-based mocking. Changes: - Replace fake.NewSimpleClientset() with fake.NewClientset() - Remove nolint suppression comment - All tests pass without modifications Assisted-by: Cursor/Claude * fix: improve StatusWriter.Apply mock robustness in boxcutter tests Address code review feedback by making the Apply method implementation more defensive: - Add applyCalled tracking field to statusWriterMock - Return error if Apply is unexpectedly called during tests - This ensures tests fail immediately if the code starts using Apply, rather than silently passing with incorrect assumptions The error message clearly indicates this method is not expected to be used, making debugging easier if the interface usage changes in the future. Assisted-by: Cursor/Claude * fix: improve upgrade e2e test robustness for controller-runtime v0.23.1 Controller-runtime v0.23.1+ has faster leader election startup, causing timing issues with the upgrade e2e tests that watch pod logs for leader election messages. The log watching approach fails because it uses Follow=true which only streams NEW logs, missing messages emitted before the test starts watching. With faster startup in v0.23.1+, leader election often completes before the test begins watching. Changes: - Replace log watching with direct lease object checking for leader election - Poll lease.Spec.HolderIdentity to confirm leader election occurred - Refactor watchPodLogsForSubstring to use polling with TailLines instead of Follow - Add per-poll context timeout (5s) to prevent hanging on slow log streams - Add scanner error checking to detect and handle stream errors - Fix context cancellation timing - pollCancel() called after stream operations complete - Add k8s.io/utils/ptr import for log options This approach is timing-independent and directly verifies the desired state rather than relying on log message observation. The per-poll timeout prevents tests from hanging indefinitely while still respecting the outer context timeout. Fixes address Copilot review comments about context management and error handling. Assisted-by: Cursor/Claude Co-authored-by: Cursor <cursoragent@cursor.com> * fix: restore CEL validation error message format test updates for Kubernetes v1.35 Kubernetes v1.35 changed CEL validation error message format to include the actual validated value instead of just the type ("string" or "object"). This provides more helpful error messages for debugging validation failures. This commit was accidentally dropped during interactive rebase. Changes: - Update test expectations to include actual values in error messages - Change from 'Invalid value: "string"' to 'Invalid value: "actual-value"' - Update 20+ test cases in TestImageSourceCELValidationRules and related tests - Fix indentation consistency across all test cases Assisted-by: Cursor/Claude Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address StopWithRetry false negative issue Address code review feedback from @perdasilva: the "final attempt" after timeout creates a confusing false negative. If the final attempt succeeds after we've logged "timeout reached", we return success despite logging a timeout message. Changes: - Remove final attempt after timeout - Return error immediately when timeout is reached - Clearer error message with timeout duration - Prevents false negatives where timeout is logged but success is returned This makes the function behavior match its log messages - if we say we timed out, we actually return an error. Assisted-by: Cursor/Claude Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8927559 commit 04899a2

12 files changed

Lines changed: 459 additions & 405 deletions

File tree

api/v1/clustercatalog_types_test.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestImageSourceCELValidationRules(t *testing.T) {
3838
PollIntervalMinutes: ptr.To(1),
3939
},
4040
wantErrs: []string{
41-
"openAPIV3Schema.properties.spec.properties.source.properties.image: Invalid value: \"object\": cannot specify pollIntervalMinutes while using digest-based image",
41+
"openAPIV3Schema.properties.spec.properties.source.properties.image: Invalid value: cannot specify pollIntervalMinutes while using digest-based image",
4242
},
4343
},
4444
"valid digest based image ref, poll interval not allowed, poll interval not specified": {
@@ -52,63 +52,63 @@ func TestImageSourceCELValidationRules(t *testing.T) {
5252
Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc",
5353
},
5454
wantErrs: []string{
55-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
55+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
5656
},
5757
},
5858
"invalid digest based image ref, invalid name": {
5959
spec: ImageSource{
6060
Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc",
6161
},
6262
wantErrs: []string{
63-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
63+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
6464
},
6565
},
6666
"invalid digest based image ref, invalid digest algorithm": {
6767
spec: ImageSource{
6868
Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc",
6969
},
7070
wantErrs: []string{
71-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
71+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
7272
},
7373
},
7474
"invalid digest based image ref, too short digest encoding": {
7575
spec: ImageSource{
7676
Ref: "docker.io/foo/bar@sha256:abcdef123456789",
7777
},
7878
wantErrs: []string{
79-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest is not valid. the encoded string must be at least 32 characters",
79+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar@sha256:abcdef123456789\": digest is not valid. the encoded string must be at least 32 characters",
8080
},
8181
},
8282
"invalid digest based image ref, invalid characters in digest encoding": {
8383
spec: ImageSource{
8484
Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc",
8585
},
8686
wantErrs: []string{
87-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
87+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
8888
},
8989
},
9090
"invalid image ref, no tag or digest": {
9191
spec: ImageSource{
9292
Ref: "docker.io/foo/bar",
9393
},
9494
wantErrs: []string{
95-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": must end with a digest or a tag",
95+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar\": must end with a digest or a tag",
9696
},
9797
},
9898
"invalid tag based image ref, tag too long": {
9999
spec: ImageSource{
100100
Ref: fmt.Sprintf("docker.io/foo/bar:%s", strings.Repeat("x", 128)),
101101
},
102102
wantErrs: []string{
103-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": tag is invalid. the tag must not be more than 127 characters",
103+
fmt.Sprintf("openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar:%s\": tag is invalid. the tag must not be more than 127 characters", strings.Repeat("x", 128)),
104104
},
105105
},
106106
"invalid tag based image ref, tag contains invalid characters": {
107107
spec: ImageSource{
108108
Ref: "docker.io/foo/bar:-foo_bar-",
109109
},
110110
wantErrs: []string{
111-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": tag is invalid. valid tags must begin with a word character (alphanumeric + \"_\") followed by word characters or \".\", and \"-\" characters",
111+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io/foo/bar:-foo_bar-\": tag is invalid. valid tags must begin with a word character (alphanumeric + \"_\") followed by word characters or \".\", and \"-\" characters",
112112
},
113113
},
114114
"valid tag based image ref": {
@@ -129,7 +129,7 @@ func TestImageSourceCELValidationRules(t *testing.T) {
129129
Ref: "docker.io:8080",
130130
},
131131
wantErrs: []string{
132-
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
132+
"openAPIV3Schema.properties.spec.properties.source.properties.image.ref: Invalid value: \"docker.io:8080\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
133133
},
134134
},
135135
"valid image ref, domain with port": {
@@ -179,64 +179,64 @@ func TestResolvedImageSourceCELValidation(t *testing.T) {
179179
Ref: "-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc",
180180
},
181181
wantErrs: []string{
182-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
182+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"-quay+docker/foo/bar@sha256:abcdef123456789abcdef123456789abc\": must start with a valid domain. valid domains must be alphanumeric characters (lowercase and uppercase) separated by the \".\" character.",
183183
},
184184
},
185185
"invalid digest based image ref, invalid name": {
186186
spec: ImageSource{
187187
Ref: "docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc",
188188
},
189189
wantErrs: []string{
190-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
190+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/FOO/BAR@sha256:abcdef123456789abcdef123456789abc\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
191191
},
192192
},
193193
"invalid digest based image ref, invalid digest algorithm": {
194194
spec: ImageSource{
195195
Ref: "docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc",
196196
},
197197
wantErrs: []string{
198-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
198+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/foo/bar@99-problems:abcdef123456789abcdef123456789abc\": digest algorithm is not valid. valid algorithms must start with an uppercase or lowercase alpha character followed by alphanumeric characters and may contain the \"-\", \"_\", \"+\", and \".\" characters.",
199199
},
200200
},
201201
"invalid digest based image ref, too short digest encoding": {
202202
spec: ImageSource{
203203
Ref: "docker.io/foo/bar@sha256:abcdef123456789",
204204
},
205205
wantErrs: []string{
206-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest is not valid. the encoded string must be at least 32 characters",
206+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/foo/bar@sha256:abcdef123456789\": digest is not valid. the encoded string must be at least 32 characters",
207207
},
208208
},
209209
"invalid digest based image ref, invalid characters in digest encoding": {
210210
spec: ImageSource{
211211
Ref: "docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc",
212212
},
213213
wantErrs: []string{
214-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
214+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/foo/bar@sha256:XYZxy123456789abcdef123456789abc\": digest is not valid. the encoded string must only contain hex characters (A-F, a-f, 0-9)",
215215
},
216216
},
217217
"invalid image ref, no digest": {
218218
spec: ImageSource{
219219
Ref: "docker.io/foo/bar",
220220
},
221221
wantErrs: []string{
222-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
222+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/foo/bar\": must end with a digest",
223223
},
224224
},
225225
"invalid image ref, only domain with port": {
226226
spec: ImageSource{
227227
Ref: "docker.io:8080",
228228
},
229229
wantErrs: []string{
230-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
231-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
230+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io:8080\": a valid name is required. valid names must contain lowercase alphanumeric characters separated only by the \".\", \"_\", \"__\", \"-\" characters.",
231+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io:8080\": must end with a digest",
232232
},
233233
},
234234
"invalid image ref, tag-based ref": {
235235
spec: ImageSource{
236236
Ref: "docker.io/foo/bar:latest",
237237
},
238238
wantErrs: []string{
239-
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"string\": must end with a digest",
239+
"openAPIV3Schema.properties.status.properties.resolvedSource.properties.image.properties.ref: Invalid value: \"docker.io/foo/bar:latest\": must end with a digest",
240240
},
241241
},
242242
} {
@@ -271,15 +271,15 @@ func TestClusterCatalogURLsCELValidation(t *testing.T) {
271271
Base: "file://somefilepath",
272272
},
273273
wantErrs: []string{
274-
fmt.Sprintf("%s: Invalid value: \"string\": scheme must be either http or https", pth),
274+
fmt.Sprintf("%s: Invalid value: \"file://somefilepath\": scheme must be either http or https", pth),
275275
},
276276
},
277277
"base is invalid": {
278278
urls: ClusterCatalogURLs{
279279
Base: "notevenarealURL",
280280
},
281281
wantErrs: []string{
282-
fmt.Sprintf("%s: Invalid value: \"string\": must be a valid URL", pth),
282+
fmt.Sprintf("%s: Invalid value: \"notevenarealURL\": must be a valid URL", pth),
283283
},
284284
},
285285
} {
@@ -309,7 +309,7 @@ func TestSourceCELValidation(t *testing.T) {
309309
Type: SourceTypeImage,
310310
},
311311
wantErrs: []string{
312-
fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
312+
fmt.Sprintf("%s: Invalid value: image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
313313
},
314314
},
315315
"image source with required image field": {
@@ -351,7 +351,7 @@ func TestResolvedSourceCELValidation(t *testing.T) {
351351
Type: SourceTypeImage,
352352
},
353353
wantErrs: []string{
354-
fmt.Sprintf("%s: Invalid value: \"object\": image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
354+
fmt.Sprintf("%s: Invalid value: image is required when source type is %s, and forbidden otherwise", pth, SourceTypeImage),
355355
},
356356
},
357357
"image source with required image field": {

api/v1/suite_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"log"
2121
"os"
2222
"testing"
23+
"time"
2324

2425
"github.com/stretchr/testify/require"
2526
apimachineryruntime "k8s.io/apimachinery/pkg/runtime"
@@ -56,6 +57,9 @@ func TestMain(m *testing.M) {
5657
}
5758

5859
code := m.Run()
59-
utilruntime.Must(testEnv.Stop())
60+
// Use Eventually wrapper for graceful test environment teardown
61+
// controller-runtime v0.23.0+ requires this to prevent timing-related errors
62+
stopErr := test.StopWithRetry(testEnv, time.Minute, time.Second)
63+
utilruntime.Must(stopErr)
6064
os.Exit(code)
6165
}

0 commit comments

Comments
 (0)