Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config with api keys with provided values and multiple usage plans fails #9440

Closed
iPherian opened this issue May 6, 2021 · 8 comments
Closed

Comments

@iPherian
Copy link

iPherian commented May 6, 2021

Serverless.yml with api keys with provided values and multiple usage plans fails.

serverless.yml
service: apiKeysConfigBug

configValidationMode: error

provider:
  name: aws
  stage: ${opt:stage}
  apiGateway:
    apiKeys:
      - paid:
          - name: ${self:provider.stage}-paid
            value: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM1
      - free:
          - name: ${self:provider.stage}-free
            value: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM2
    usagePlan:
      - paid: {}
      - free:
          quota:
            limit: 5000
            offset: 1
            period: MONTH
          throttle:
            burstLimit: 200
            rateLimit: 100

functions:
  func:
    handler: dist/main.func
    events:
      - http:
          path: main
          method: get
          private: true
./node_modules/.bin/serverless deploy --stage apiKeysConfigBug output
Serverless: To ensure safe major version upgrades ensure "frameworkVersion" setting in service configuration (recommended setup: "frameworkVersion: ^2.40.0")

Serverless: Load command interactiveCli
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command config:tabcompletion
Serverless: Load command config:tabcompletion:install
Serverless: Load command config:tabcompletion:uninstall
Serverless: Load command create
Serverless: Load command install
Serverless: Load command package
Serverless: Load command deploy
Serverless: Load command deploy:function
Serverless: Load command deploy:list
Serverless: Load command deploy:list:functions
Serverless: Load command invoke
Serverless: Load command invoke:local
Serverless: Load command info
Serverless: Load command logs
Serverless: Load command metrics
Serverless: Load command print
Serverless: Load command remove
Serverless: Load command rollback
Serverless: Load command rollback:function
Serverless: Load command slstats
Serverless: Load command plugin
Serverless: Load command plugin
Serverless: Load command plugin:install
Serverless: Load command plugin
Serverless: Load command plugin:uninstall
Serverless: Load command plugin
Serverless: Load command plugin:list
Serverless: Load command plugin
Serverless: Load command plugin:search
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command upgrade
Serverless: Load command uninstall
Serverless: Load command login
Serverless: Load command logout
Serverless: Load command generate-event
Serverless: Load command test
Serverless: Load command dashboard
Serverless: Load command output
Serverless: Load command output:get
Serverless: Load command output:list
Serverless: Load command param
Serverless: Load command param:get
Serverless: Load command param:list
Serverless: Load command studio
Serverless: Skipping variables resolution with old resolver (new resolver reported no more variables to resolve)

 Serverless Error ----------------------------------------

  ServerlessError: Configuration error:
       at 'provider.apiGateway.apiKeys[0].paid[0]': should be string
       at 'provider.apiGateway.apiKeys[1].free[0]': should be string
      at ConfigSchemaHandler.handleErrorMessages (C:\serverless-api-keys-config-bug\node_modules\serverless\lib\classes\ConfigSchemaHandler\index.js:125:15)
      at ConfigSchemaHandler.validateConfig (C:\serverless-api-keys-config-bug\node_modules\serverless\lib\classes\ConfigSchemaHandler\index.js:113:12)
      at Service.validate (C:\serverless-api-keys-config-bug\node_modules\serverless\lib\classes\Service.js:226:41)
      at Serverless.run (C:\serverless-api-keys-config-bug\node_modules\serverless\lib\Serverless.js:322:39)
      at C:\serverless-api-keys-config-bug\node_modules\serverless\scripts\serverless.js:653:26

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com

  Your Environment Information ---------------------------
     Operating System:          win32
     Node Version:              14.16.1
     Framework Version:         2.40.0 (local)
     Plugin Version:            4.5.3
     SDK Version:               4.2.2
     Components Version:        3.9.2

Installed version

Framework Core: 2.40.0 (local)
Plugin: 4.5.3
SDK: 4.2.2
Components: 3.9.2
@pgrzesik
Copy link
Contributor

pgrzesik commented May 9, 2021

Hello @iPherian, thanks a lot for reporting and sorry you've run into trouble. I've managed to reproduce your use case and I believe that the underlying logic is correct, however, the schema does not accept object-based configuration for the keys when multiple usage plans are involved - it will need to be adjusted here:

awsApiGatewayApiKeys: {
type: 'array',
items: {
anyOf: [
{ type: 'string' },
{
type: 'object',
properties: {
name: { type: 'string' },
value: { type: 'string' },
description: { type: 'string' },
customerId: { type: 'string' },
},
anyOf: [{ required: ['name'] }, { required: ['value'] }],
additionalProperties: false,
},
{
type: 'object',
maxProperties: 1,
additionalProperties: {
type: 'array',
items: { type: 'string' },
},
},
],
},
},
and the problem should be resolved.

We'd be more than happy to accept a PR with the change implementing it. 🙌

@prithvijit-dasgupta
Copy link

prithvijit-dasgupta commented May 12, 2021

@pgrzesik would a schema block using patternProperties ( in an anyOf block ) suffice?. This seems like a trivial issue which I would be happy to raise a PR for

@pgrzesik
Copy link
Contributor

Hey @prithvijit-dasgupta - could you propose how that could look like? I'm not sure about patternProperties, but I might be missing something 👍

@prithvijit-dasgupta
Copy link

@pgrzesik
So from the given example

apiGateway:
    apiKeys:
      - paid:
          - name: ${self:provider.stage}-paid
            value: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM1
      - free:
          - name: ${self:provider.stage}-free
            value: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM2

would translate to essentially to this JSON

{
  "apiGateway": {
    "apiKeys": [
      {
        "paid": [
          {
            "name": "${self:provider.stage}-paid",
            "value": "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM1"
          }
        ]
      },
      {
        "free": [
          {
            "name": "${self:provider.stage}-free",
            "value": "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLM2"
          }
        ]
      }
    ]
  }
}

Assuming paid and free are just examples key names, the key can have any value

Using JSON Schema 7.0 patternProperties, the schema would end up something like

  awsApiGatewayApiKeys: {
    type: 'array',
    items: {
      anyOf: [
        { type: 'string' },
        {                           //new code starts here
          type: 'object',
          patternProperties: {
            '^.+$': {
              type: 'array',
              items: {
                type: 'object',
                properties: {
                  name: { type: 'string' },
                  value: { type: 'string' },
                  description: { type: 'string' },
                  customerId: { type: 'string' },
                },
                anyOf: [{ required: ['name'] }, { required: ['value'] }],
                additionalProperties: false,
              },
            },
          },
          additionalProperties: false,
        },                     //new code ends here
        {
          type: 'object',
          properties: {
            name: { type: 'string' },
            value: { type: 'string' },
            description: { type: 'string' },
            customerId: { type: 'string' },
          },
          anyOf: [{ required: ['name'] }, { required: ['value'] }],
          additionalProperties: false,
        },
        {
          type: 'object',
          maxProperties: 1,
          additionalProperties: {
            type: 'array',
            items: { type: 'string' },
          },
        },
      ],
    },
  },

However, I am not sure if the internal processing takes care of the example structure. I could have misunderstood the example and the use case

@pgrzesik
Copy link
Contributor

Hey @prithvijit-dasgupta - yeah, something along those lines should work just fine 👍 Feel free to propose a PR if you're interested 🙌

@iPherian
Copy link
Author

Hmm that has got me thinking. Wouldn't something like this be better? This way one avoids repeating the definition of the api key object, plus you retain the previous behavior of how usage plan objects should have one property of an array to differentiate them from objects directly describing an individual api key. So like this:

  awsApiGatewayApiKey: {
    anyOf: [
      { type: "string" },
      {
        type: "object",
        properties: {
          name: { type: "string" },
          value: { type: "string" },
          description: { type: "string" },
          customerId: { type: "string" },
        },
        anyOf: [{ required: ["name"] }, { required: ["value"] }],
        additionalProperties: false,
      },
    ],
  },
  awsApiGatewayApiKeys: {
    type: "array",
    items: {
      anyOf: [
        { $ref: "#/definitions/awsApiGatewayApiKey" },
        {
          type: "object",
          maxProperties: 1,
          additionalProperties: {
            type: "array",
            items: { $ref: "#/definitions/awsApiGatewayApiKey" },
          },
        },
      ],
    },
  },

@iPherian
Copy link
Author

Looks like we've got a nice PR, and it's got tests too. PR #9489

@pgrzesik
Copy link
Contributor

Yup, looks like the changes implemented in #9489 should adress this issue - I'm going to close this one, it will be a part of an upcoming release which will go out in a few days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants