Skip to content

Commit 6405543

Browse files
committed
fixed to pass map[string]interface instead of unstructured.Unstructured
We started seeing some issues with folks who had spurious CRD incompatibility claims when updating operators. It is a failure in OLM code which validates existing CRs against incoming CRDs, recently updated in operator-framework#3387. This manifested in `InstallPlan` `.status.Message` something like: ``` retrying execution due to error: error validating existing CRs against new CRD's schema for \"pgadmins.postgres-operator.crunchydata.com\": error validating postgres-operator.crunchydata.com/v1beta1, Kind=PGAdmin \"openshift-operators/example-pgadmin\": updated validation is too restrictive: [].spec.tolerations[0].tolerationSeconds: Invalid value: \"number\": spec.tolerations[0].tolerationSeconds in body must be of type integer: \"number\" ``` The difference between the predecessor calling convention and the one introduced in operator-framework#3387 appears to be that one is a pointer and the other is concrete. old ```golang unstructured.Unstructured{Object:map[string]interface... ``` new ```golang &unstructured.Unstructured{Object:map[string]interface... ``` so it would seem that merely type-asserting the value and de-referencing it would yield the appropriate result, but it appears instead that it effectively disables all CR vs CRD reconciliation checks (evidenced by the fact that the unit tests multiply fail). But k8s already dereferences pointer parameters [here](https://github.com/kubernetes/kube-openapi/blob/master/pkg/validation/validate/schema.go#L139-L141) during validation. So that isn't it. And the `validate.ValidateCustomResource` interface is terrifyingly permissive in allowing `customResource` as `interface{}` [here](https://pkg.go.dev/k8s.io/[email protected]/pkg/apiserver/validation#ValidateCustomResource). So we cannot derive guidance from it. Taking a page from k8s' use of the validation API, which uses `unstructured.UnstructuredContent()` to convert the `unstructured.Unstructured` into a `map[string]interface{}` [here](https://github.com/kubernetes/kubernetes/blob/1504f10e7946f95a8b1da35e28e4c7453ff62775/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go#L54) then we achieve the desired results. Signed-off-by: Jordan Keister <[email protected]>
1 parent 3911cac commit 6405543

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

pkg/controller/operators/catalog/operator.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -2246,11 +2246,11 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gr schema.GroupResourc
22462246
return dynamicClient.Resource(gvr).List(context.TODO(), opts)
22472247
}))
22482248
validationFn := func(obj runtime.Object) error {
2249-
err = validation.ValidateCustomResource(field.NewPath(""), obj, validator).ToAggregate()
2249+
// lister will only provide unstructured objects as runtime.Object, so this should never fail to convert
2250+
// if it does, it's a programming error
2251+
cr := obj.(*unstructured.Unstructured)
2252+
err = validation.ValidateCustomResource(field.NewPath(""), cr.UnstructuredContent(), validator).ToAggregate()
22502253
if err != nil {
2251-
// lister will only provide unstructured objects as runtime.Object, so this should never fail to convert
2252-
// if it does, it's a programming error
2253-
cr := obj.(*unstructured.Unstructured)
22542254
var namespacedName string
22552255
if cr.GetNamespace() == "" {
22562256
namespacedName = cr.GetName()

0 commit comments

Comments
 (0)