From b8d6d7b83e8452d3296b7312649490250211b31f Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Wed, 20 Nov 2024 13:35:03 -0600 Subject: [PATCH 1/4] investigation --- .../controller/operators/catalog/operator.go | 24 ++++++++++++++++++- .../controller/operators/catalog/operator.go | 24 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 10d3187096..10924cdd84 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2242,6 +2242,26 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc return fmt.Errorf("error creating validator for schema version %s: %s", version, err) } gvr := schema.GroupVersionResource{Group: gr.Group, Version: version, Resource: gr.Resource} + + results := make(map[string]*unstructured.Unstructured) + + crList, err := dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing resources in GroupVersionResource %#v: %s", gvr, err) + } + for _, cr := range crList.Items { + var namespacedName string + if cr.GetNamespace() == "" { + namespacedName = cr.GetName() + } else { + namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) + } + if err := validation.ValidateCustomResource(field.NewPath(""), &cr, validator).ToAggregate(); err != nil { + return validationError{fmt.Errorf("blocking lister: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + } + results[namespacedName] = &cr + } + pager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) { return dynamicClient.Resource(gvr).List(context.TODO(), opts) })) @@ -2257,7 +2277,9 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("error validating %s %q: simple list succeeded where paginated failed:\nsimple:\n%#v\npaginated:\n%#v\n %v", + gvr, namespacedName, results[namespacedName], obj, err)} + // return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 10d3187096..10924cdd84 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2242,6 +2242,26 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc return fmt.Errorf("error creating validator for schema version %s: %s", version, err) } gvr := schema.GroupVersionResource{Group: gr.Group, Version: version, Resource: gr.Resource} + + results := make(map[string]*unstructured.Unstructured) + + crList, err := dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing resources in GroupVersionResource %#v: %s", gvr, err) + } + for _, cr := range crList.Items { + var namespacedName string + if cr.GetNamespace() == "" { + namespacedName = cr.GetName() + } else { + namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) + } + if err := validation.ValidateCustomResource(field.NewPath(""), &cr, validator).ToAggregate(); err != nil { + return validationError{fmt.Errorf("blocking lister: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + } + results[namespacedName] = &cr + } + pager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) { return dynamicClient.Resource(gvr).List(context.TODO(), opts) })) @@ -2257,7 +2277,9 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("error validating %s %q: simple list succeeded where paginated failed:\nsimple:\n%#v\npaginated:\n%#v\n %v", + gvr, namespacedName, results[namespacedName], obj, err)} + // return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } From 7629e0979ca9b746a338c6a2db6cc6117a4a8447 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 3 Dec 2024 17:39:33 -0600 Subject: [PATCH 2/4] type assertion to force interface conversion Signed-off-by: Jordan Keister --- .../controller/operators/catalog/operator.go | 31 +++---------------- .../controller/operators/catalog/operator.go | 31 +++---------------- 2 files changed, 10 insertions(+), 52 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 10924cdd84..0d52c6e3e6 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2243,43 +2243,22 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } gvr := schema.GroupVersionResource{Group: gr.Group, Version: version, Resource: gr.Resource} - results := make(map[string]*unstructured.Unstructured) - - crList, err := dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) - if err != nil { - return fmt.Errorf("error listing resources in GroupVersionResource %#v: %s", gvr, err) - } - for _, cr := range crList.Items { - var namespacedName string - if cr.GetNamespace() == "" { - namespacedName = cr.GetName() - } else { - namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) - } - if err := validation.ValidateCustomResource(field.NewPath(""), &cr, validator).ToAggregate(); err != nil { - return validationError{fmt.Errorf("blocking lister: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} - } - results[namespacedName] = &cr - } - pager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) { return dynamicClient.Resource(gvr).List(context.TODO(), opts) })) validationFn := func(obj runtime.Object) error { - err = validation.ValidateCustomResource(field.NewPath(""), obj, validator).ToAggregate() + // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert + // if it does, it's a programming error + cr := obj.(*unstructured.Unstructured) + err = validation.ValidateCustomResource(field.NewPath(""), cr, validator).ToAggregate() if err != nil { - // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert - // if it does, it's a programming error - cr := obj.(*unstructured.Unstructured) var namespacedName string if cr.GetNamespace() == "" { namespacedName = cr.GetName() } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: simple list succeeded where paginated failed:\nsimple:\n%#v\npaginated:\n%#v\n %v", - gvr, namespacedName, results[namespacedName], obj, err)} - // return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 10924cdd84..0d52c6e3e6 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2243,43 +2243,22 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } gvr := schema.GroupVersionResource{Group: gr.Group, Version: version, Resource: gr.Resource} - results := make(map[string]*unstructured.Unstructured) - - crList, err := dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) - if err != nil { - return fmt.Errorf("error listing resources in GroupVersionResource %#v: %s", gvr, err) - } - for _, cr := range crList.Items { - var namespacedName string - if cr.GetNamespace() == "" { - namespacedName = cr.GetName() - } else { - namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) - } - if err := validation.ValidateCustomResource(field.NewPath(""), &cr, validator).ToAggregate(); err != nil { - return validationError{fmt.Errorf("blocking lister: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} - } - results[namespacedName] = &cr - } - pager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) { return dynamicClient.Resource(gvr).List(context.TODO(), opts) })) validationFn := func(obj runtime.Object) error { - err = validation.ValidateCustomResource(field.NewPath(""), obj, validator).ToAggregate() + // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert + // if it does, it's a programming error + cr := obj.(*unstructured.Unstructured) + err = validation.ValidateCustomResource(field.NewPath(""), cr, validator).ToAggregate() if err != nil { - // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert - // if it does, it's a programming error - cr := obj.(*unstructured.Unstructured) var namespacedName string if cr.GetNamespace() == "" { namespacedName = cr.GetName() } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: simple list succeeded where paginated failed:\nsimple:\n%#v\npaginated:\n%#v\n %v", - gvr, namespacedName, results[namespacedName], obj, err)} - // return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } From a9f410f192de830dfa3b005b33d113315d8c063c Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 3 Dec 2024 21:00:58 -0600 Subject: [PATCH 3/4] mod error message Signed-off-by: Jordan Keister --- .../pkg/controller/operators/catalog/operator.go | 2 +- .../pkg/controller/operators/catalog/operator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 0d52c6e3e6..5335c5aa96 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2258,7 +2258,7 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("JEK: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 0d52c6e3e6..5335c5aa96 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2258,7 +2258,7 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc } else { namespacedName = fmt.Sprintf("%s/%s", cr.GetNamespace(), cr.GetName()) } - return validationError{fmt.Errorf("error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} + return validationError{fmt.Errorf("JEK: error validating %s %q: updated validation is too restrictive: %v", cr.GroupVersionKind(), namespacedName, err)} } return nil } From 2757afda7f0ba9b254a17501388edc2cf1a622e0 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Thu, 5 Dec 2024 12:06:53 -0600 Subject: [PATCH 4/4] this fixed the upstream reproducer, but broke unit test Signed-off-by: Jordan Keister --- .../pkg/controller/operators/catalog/operator.go | 2 +- .../pkg/controller/operators/catalog/operator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 5335c5aa96..ce51163853 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2250,7 +2250,7 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert // if it does, it's a programming error cr := obj.(*unstructured.Unstructured) - err = validation.ValidateCustomResource(field.NewPath(""), cr, validator).ToAggregate() + err = validation.ValidateCustomResource(field.NewPath(""), *cr, validator).ToAggregate() if err != nil { var namespacedName string if cr.GetNamespace() == "" { diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 5335c5aa96..ce51163853 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -2250,7 +2250,7 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc // lister will only provide unstructured objects as runtime.Object, so this should never fail to convert // if it does, it's a programming error cr := obj.(*unstructured.Unstructured) - err = validation.ValidateCustomResource(field.NewPath(""), cr, validator).ToAggregate() + err = validation.ValidateCustomResource(field.NewPath(""), *cr, validator).ToAggregate() if err != nil { var namespacedName string if cr.GetNamespace() == "" {