serverless / typescript

TypeScript definitions for Serverless Framework service configuration
140 stars 24 forks source link

Wrong definitions for resources.Resources properties #8

Closed fredericbarthelet closed 3 years ago

fredericbarthelet commented 3 years ago

Use case description

As of v2.13.0, the TS definitions generated for resources.Resources property states:

[k: string]: {
  Properties?: {
    [k: string]: unknown;
  };
  CreationPolicy?: {
    [k: string]: unknown;
  };
  DeletionPolicy?: string;
  DependsOn?: string[];
  Metadata?: {
    [k: string]: unknown;
  };
  UpdatePolicy?: {
    [k: string]: unknown;
  };
  UpdateReplacePolicy?: string;
  Condition?: string;
  [k: string]: {
    [k: string]: unknown;
  };
};

This definition lacks the mandatory Type property and allows any additional property, as long as they are objects (thus not fitting the requirement for Type property - which is a simple string).

Proposed solution

The issue comes from invalid JSON-schema definition in https://github.com/serverless/serverless/blob/739045d9bb03ddefcacceca0bd39f188517b187d/lib/plugins/aws/provider/awsProvider.js#L1075-L1083 and more specifically in additionalProperty definition which does not accept a property name as a valid JSON-schema property.

{
  type: 'object',
  properties: { foo: { type: 'string'}},
  additionalProperties: { type: 'string' }
}

is vallid since { type: 'string' } is a valid JSON-schema definition.

In our case, { Type: { type: 'string'}} is not a valid JSON-schema definition.

image

The schema must be changed at this location, specifying combined typed as explained in https://json-schema.org/understanding-json-schema/structuring.html#extending

In addition, since additionalProperties: false must be added to ensure no additional properties can be added, and this not being compatible with a combined schema using allOf, separate definitions must be used.

MorelSerge commented 3 years ago

Can a new release be made?

fredericbarthelet commented 3 years ago

Hi @Vluf , this repository releases follows serverless/serverless releases. Once the fix released on the framework side with v2.14, this repository will deploy the corresponding fixed types.

MorelSerge commented 3 years ago

Hi @fredericbarthelet, thanks for the clarification there. I installed the latest serverless and @serverless/typescript from master, but still receiving: TS2322: Type 'string' is not assignable to type '{ [k: string]: unknown; }'. for my Type property.

Resources section of my serverless.ts config:

resources: {
    Resources: {
      RenderBucket: {
        Type: "AWS::S3::Bucket",
        Properties: {
          BucketName: "${self:service}-render-bucket-${self:provider.stage}"
        }
      }
    }
  }
fredericbarthelet commented 3 years ago

Waiting for next serverless release, you can temporarily disable type checking on resources key with a //@ts-ignore marker