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