From 5815592f8d6f3ed75f458bd800fa0de0c54db997 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 25 Aug 2026 07:38:30 -0400 Subject: [PATCH] Report an XRD schema Kubernetes would reject as non-structural A property with no type makes the derived CRD schema non-structural. BuildOpenAPIV3 does not fail on one: it reduces the whole component to a bare "type: object", so every generated language type for that XRD comes out as an empty stub. The one bad field is not the only thing lost. Kubernetes would reject the derived CRD with the same complaint, so an XRD in this state never worked on a cluster either; the only difference is that the CLI accepted it in silence. Run the same structural check the API server runs and fail with the path it reports, for example: properties[spec].properties[parameters].properties[acl].type: Required value: must not be empty for specified object fields Fixes #278 Signed-off-by: Arpit Jain --- internal/crd/generator.go | 35 ++++++++++++++++++++ internal/crd/generator_test.go | 19 +++++++++++ internal/crd/testdata/untyped-field-xrd.yaml | 32 ++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 internal/crd/testdata/untyped-field-xrd.yaml diff --git a/internal/crd/generator.go b/internal/crd/generator.go index fd79145..bed7540 100644 --- a/internal/crd/generator.go +++ b/internal/crd/generator.go @@ -20,7 +20,9 @@ import ( "path/filepath" "github.com/spf13/afero" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "sigs.k8s.io/yaml" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -44,6 +46,10 @@ func createCRDFromXRD(xrd xpv1.CompositeResourceDefinition) (*apiextensionsv1.Cu xrCrd.Spec.Names.ListKind = xrCrd.Spec.Names.Kind + "List" } + if err := validateStructural(xrCrd); err != nil { + return nil, nil, errors.Wrapf(err, "composite CRD derived from XRD %q has an unusable schema", xrd.GetName()) + } + if xrd.Spec.ClaimNames != nil { claimCrd, err = xcrd.ForCompositeResourceClaim(&xrd) if err != nil { @@ -59,6 +65,35 @@ func createCRDFromXRD(xrd xpv1.CompositeResourceDefinition) (*apiextensionsv1.Cu return xrCrd, claimCrd, nil } +// validateStructural reports schemas that Kubernetes would not accept as +// structural. Such a schema is silently reduced to an empty object when it is +// converted to OpenAPI, which leaves the generated language types with no +// fields at all rather than with the one bad field missing, so it is worth +// stopping on rather than passing through. +func validateStructural(crd *apiextensionsv1.CustomResourceDefinition) error { + for _, ver := range crd.Spec.Versions { + if ver.Schema == nil || ver.Schema.OpenAPIV3Schema == nil { + continue + } + + internal := &apiextensions.JSONSchemaProps{} + if err := apiextensionsv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(ver.Schema.OpenAPIV3Schema, internal, nil); err != nil { + return errors.Wrapf(err, "cannot read the schema of version %q", ver.Name) + } + + s, err := structuralschema.NewStructural(internal) + if err != nil { + return errors.Wrapf(err, "version %q", ver.Name) + } + + if err := structuralschema.ValidateStructural(nil, s).ToAggregate(); err != nil { + return errors.Wrapf(err, "version %q", ver.Name) + } + } + + return nil +} + // ProcessXRD generates associated CRDs from an XRD. func ProcessXRD(fs afero.Fs, bs []byte, path, baseFolder string) (string, string, error) { var xrd xpv1.CompositeResourceDefinition diff --git a/internal/crd/generator_test.go b/internal/crd/generator_test.go index 7e1f3aa..5e294ce 100644 --- a/internal/crd/generator_test.go +++ b/internal/crd/generator_test.go @@ -17,6 +17,7 @@ limitations under the License. package crd import ( + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -33,6 +34,9 @@ var claimableXRDBytes []byte //go:embed testdata/unclaimable-xrd.yaml var unclaimableXRDBytes []byte +//go:embed testdata/untyped-field-xrd.yaml +var untypedFieldXRDBytes []byte + func TestProcessXRD(t *testing.T) { t.Parallel() @@ -44,6 +48,8 @@ func TestProcessXRD(t *testing.T) { expectedClaimKind string expectedClaimListKind string + + expectedErr string }{ "ClaimableXRD": { xrdBytes: claimableXRDBytes, @@ -57,6 +63,10 @@ func TestProcessXRD(t *testing.T) { expectedXRKind: "XInternalBucket", expectedXRListKind: "XInternalBucketList", }, + "XRDWithAnUntypedField": { + xrdBytes: untypedFieldXRDBytes, + expectedErr: `properties[spec].properties[parameters].properties[acl].type: Required value: must not be empty for specified object fields`, + }, } for name, tc := range tcs { @@ -65,6 +75,15 @@ func TestProcessXRD(t *testing.T) { outFS := afero.NewMemMapFs() xrPath, claimPath, err := ProcessXRD(outFS, tc.xrdBytes, "output", "/") + if tc.expectedErr != "" { + if err == nil { + t.Fatalf("expected an error mentioning %q, got none", tc.expectedErr) + } + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("expected the error to mention %q, got: %v", tc.expectedErr, err) + } + return + } if err != nil { t.Fatal(err) } diff --git a/internal/crd/testdata/untyped-field-xrd.yaml b/internal/crd/testdata/untyped-field-xrd.yaml new file mode 100644 index 0000000..81d7592 --- /dev/null +++ b/internal/crd/testdata/untyped-field-xrd.yaml @@ -0,0 +1,32 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xuntypedbuckets.platform.example.com +spec: + group: platform.example.com + names: + categories: + - crossplane + kind: XUntypedBucket + plural: xuntypedbuckets + versions: + - name: v1alpha1 + referenceable: true + schema: + openAPIV3Schema: + description: UntypedBucket is the Schema for the UntypedBucket API. + properties: + spec: + description: UntypedBucketSpec defines the desired state of UntypedBucket. + properties: + parameters: + properties: + acl: + description: This property has no type, which is not structural. + type: object + type: object + status: + description: UntypedBucketStatus defines the observed state of UntypedBucket. + type: object + type: object + served: true