serverless-application-model icon indicating copy to clipboard operation
serverless-application-model copied to clipboard

Allow to pass parameter value for 'Enabled' property in ScheduledEvent of 'AWS::Serverless::Function'

Open jeswanthamazon opened this issue 5 years ago • 25 comments

When you reference a parameter for 'Enabled' property in ScheduledEvent of 'AWS::Serverless::Function', it is not honoring the parameter value passed and setting the "State": "ENABLED" for Events::Rule in the processed template.

'Enabled' property in ScheduledEvent of 'AWS::Serverless::Function' currently only accepts 'true' or 'false' bool values hard-coded as string in template.

in the background when composing the processed template, 'AWS::Serverless-2016-10-31' transform sees:

  • if 'Enabled' property is set in the ScheduledEvent,
  • if it is set, 'State' Is set to 'DISABLED' only if the 'Enabled' property is 'False'. If it is anything else, 'State' Is set to 'ENABLED'. You can reference the transform code here: https://github.com/awslabs/serverless-application-model/blob/93b6ac0afb6f322e047d6e4c4887f0d829e54b88/samtranslator/model/eventsources/push.py#L113

Currently Serverless transform is not capable of reading the parameter value to set the 'State' of Event::Rule.

Please add support to parse the parameter value and set the 'State' property value to 'DISABLED' /'Enabled' accordingly.

jeswanthamazon avatar Dec 16 '19 09:12 jeswanthamazon

This is related to #1360

SydneyUni-Jim avatar Jan 04 '20 03:01 SydneyUni-Jim

Thanks for the issue report! We have passed this along to our product team for possible prioritization. Please +1 on the feature to help with prioritization.

praneetap avatar Jan 08 '20 18:01 praneetap

It has been 10 months, when is this going to be fixed? Even Enabled: False doesn't even work anymore.

RichieRunner avatar Oct 09 '20 00:10 RichieRunner

@jfuss is this still under review?

jwalsh2me avatar Oct 12 '20 14:10 jwalsh2me

this is such a shame. you have a core feature broken, and nobody who's paid to work on it even cares.

RichieRunner avatar Nov 03 '20 18:11 RichieRunner

Same issue here, lost a few hours on troubleshooting this...

z0ph avatar Nov 24 '20 13:11 z0ph

Since it's abundantly clear now that AWS and the maintainers of this codebase are refusing to prioritize this to be a critical bug, what is everyone's recommendation for disabling the Enabled flag if you are building resources in NP?

I am trying to achieve this kind of conditional logic: image

RichieRunner avatar Nov 27 '20 00:11 RichieRunner

Since it's abundantly clear now that AWS and the maintainers of this codebase are refusing to prioritize this to be a critical bug, what is everyone's recommendation for disabling the Enabled flag if you are building resources in NP?

I am trying to achieve this kind of conditional logic: image

Same here, what do you think @RichieRunner if we switch to the standard CloudFormation template (not SAM), should it be fine?

z0ph avatar Nov 27 '20 07:11 z0ph

That was the same conditional logic I had tried. @victor

On Fri, Nov 27, 2020 at 1:36 AM Victor GRENU [email protected] wrote:

Since it's abundantly clear now that AWS and the maintainers of this codebase are refusing to prioritize this to be a critical bug, what is everyone's recommendation for disabling the Enabled flag if you are building resources in NP?

I am trying to achieve this kind of conditional logic: [image: image] https://user-images.githubusercontent.com/20289967/100397949-496e5480-3001-11eb-8c2a-8b8bdcf572b8.png

Same here, what do you think @RichieRunner https://github.com/RichieRunner if we switch to the standard CloudFormation template (not SAM), should it be fine?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aws/serverless-application-model/issues/1329#issuecomment-734691438, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFODGAKIN3LZPGIC4YHHJYDSR5JGPANCNFSM4J3GXJOA .

jwalsh2me avatar Nov 27 '20 16:11 jwalsh2me

@RichieRunner @z0ph @jwalsh2me thanks for bumping this issue and discussing workarounds. I've just now added this to the backlog internally, and I apologize that we struggle with communicating status on issues like this where there are internal and external trackers involved.

Is there a workaround people have found for this particular issue? I was hoping for some cases like this that SAM CLI's recent addition of environments would help, but I can see why it fails here.

awsjeffg avatar Nov 27 '20 20:11 awsjeffg

A workaround would be much appreciated. I'm running into the same issue using the same Condition logic.

jewelsjacobs avatar Nov 29 '20 14:11 jewelsjacobs

Hi there,

Do you think we can apply similar logic to the parent lambda event source?

image

I don't know where we can set this up in SAM. Any clues ?

z0ph avatar Nov 30 '20 09:11 z0ph

FWIW, I have a bash script function workaround. It will remove the lambda CloudWatch Event rule trigger and disable the rule itself for SAM rules with the default serverlessrepo*. names. It should be easy to add parameters / variables to.

I can't find an AWS CLI way to just disable a CloudWatch Event in the Lambda so I remove it.

https://gist.github.com/jewelsjacobs/2ea1b37dd4db5279d038111e77d9d813

jewelsjacobs avatar Dec 01 '20 16:12 jewelsjacobs

FWIW, I have a bash script function workaround. It will remove the lambda CloudWatch Event rule trigger and disable the rule itself for SAM rules with the default serverlessrepo*. names. It should be easy to add parameters / variables to.

I can't find an AWS CLI way to just disable a CloudWatch Event in the Lambda so I remove it.

https://gist.github.com/jewelsjacobs/2ea1b37dd4db5279d038111e77d9d813

Thanks, so...you just run this as a last step in every build? 😖

RichieRunner avatar Dec 04 '20 20:12 RichieRunner

Thanks, so...you just run this as a last step in every build? 😖

Pretty much @RichieRunner 😞

jewelsjacobs avatar Jan 19 '21 15:01 jewelsjacobs

+1 waiting for a fix

diegogurpegui avatar Jan 19 '21 19:01 diegogurpegui

@jwalsh2me I don't know what you're talking about

victor avatar Jan 21 '21 12:01 victor

I could work around this by adding a separate resource for AWS::Events::Rule and excluded 'Events' property from AWS::Serverless::Function. Example template:-

Transform: AWS::Serverless-2016-10-31
Description: >
  sam-app

  Sample SAM Template for sam-app

Globals:
  Function:
    Timeout: 3

Parameters:
  RuleEnabled:
    Description: ENABLED or DISABLED
    Default: ENABLED
    Type: String
    # AllowedValues: [True, False]

Resources:
  HelloWorldFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: hello_world/
      Handler: app.lambda_handler
      Runtime: python2.7
      # Events:
        # HelloWorld:
        #   Type: Api # More info about API Event Source: https://github.com/awslabs/serverless-application-model/blob/master/versions/2016-10-31.md#api
        #   Properties:
        #     Path: /hello
        #     Method: get
        # ScheduledEvent:
        #   Type: Schedule
        #   Properties:
        #     Schedule: cron(30 3 * * ? *)
        #     Description: Execute the application as batch (Run at 9:00 am (UTC) every day)
        #     Enabled: !Ref RuleEnabled

  HelloWorldFunctionScheduledEvent:
    Type: AWS::Events::Rule
    Properties:
      State: !Ref RuleEnabled
      ScheduleExpression: cron(30 3 * * ? *)
      Description: Execute the application as batch (Run at 9:00 am (UTC) every day)
      Targets:
      - Id: HelloWorldFunctionScheduledEventLambdaTarget
        Arn:
          Fn::GetAtt:
          - HelloWorldFunction
          - Arn

nbrational avatar Jan 28 '21 00:01 nbrational

I could work around this by adding a separate resource for AWS::Events::Rule and excluded 'Events' property from AWS::Serverless::Function. Example template:-

Ahh, that's smart! This is worth a try as well, thanks!

RichieRunner avatar Jan 28 '21 18:01 RichieRunner

I could work around this by adding a separate resource for AWS::Events::Rule and excluded 'Events' property from AWS::Serverless::Function. Example template:-

It's working as a workaround. Thanks @nbrational

z0ph avatar Mar 29 '21 08:03 z0ph

I could work around this by adding a separate resource for AWS::Events::Rule and excluded 'Events' property from AWS::Serverless::Function. Example template:-

Thank you @nbrational for the workaround.

I think you also have to grant Events the permission to invoke the lambda, using AWS::Lambda::Permission resource :

  HelloWorldFunctionLambdaPermission:
    Type: AWS::Lambda::Permission
    Properties:
      Action: lambda:InvokeFunction
      FunctionName: !GetAtt HelloWorldFunction.Arn
      Principal: events.amazonaws.com
      SourceArn: !GetAtt HelloWorldFunctionScheduledEvent.Arn

forzagreen avatar May 31 '21 15:05 forzagreen

Closing in on the 2 year mark with still no fix from the SAM team on this 🎉

RichieRunner avatar Dec 19 '21 23:12 RichieRunner

Could we please get some attention on this?

jfalkenstein avatar Jan 10 '22 22:01 jfalkenstein

Hey all, I am moving through our backlog of PRs and this PR (https://github.com/aws/serverless-application-model/pull/1666) was looking to address this.

I am in the middle of updating it but realized a couple things, if you are using Intrinsic Functions as the value, this will break because SAM expects "true" or "false" while the underlying CFN resource uses "enabled" or "disabled".

SAM's history with intrinsic functions are hit or miss. Mostly due to us needing to re-implement how CloudFormation handles these, as there is no library provided by CloudFormation. Instead of going down that path (which has a bunch of flaws/concerns), I am thinking about adding a new property that will be a passthrough to the underlying CloudFormation resource directly. This would require customers to change their templates but existing ones would behave the same and people running into issues (as described here) could be unblocked.

Looking for some thoughts from the community on this.

jfuss avatar Jun 16 '22 14:06 jfuss

I like the idea of a new property!

jwalsh2me avatar Jun 28 '22 14:06 jwalsh2me