Add forceDeploy property in Cognito User Pools

[TsimpDim]

I tried to implement the proposed solution which can be found here. I added a forceDeploy flag that once set will set a random property in the custom resource that is used to attach triggers - if the flag is set to false then it sets the property to 0. I updated tests as well, and made sure the linter is happy.

Addresses: #9635

[medikoo]

Thank you @TsimpDim, that looks very promising. Have you confirmed locally it does the job?

[medikoo]

Let's maybe set it to Date.now(), and do not set ForceDeploy (we can assign undefined) if no forceDeploy was set

[TsimpDim]

Updated!

[TsimpDim]

Have you confirmed locally it does the job?

During deployment the Triggers still get removed for a little while until CF updates all the resources, but they eventually get added back. I'm not sure if we could improve this at all, but either way it is a way better solution then what one has to implement to circumvent this issue currently :D

[medikoo]

Sorry, didn't notice that one earlier.

In new tests we always rely on runServerless util (we have already a tons of tests configured for it, which can self as reference).

Can you update that one, so it's also runServerless based?

[TsimpDim]

Sure! I had to do some reading because I'm not familiar at all with testing in Javascript (or the internals of Serverless) but I came up with this:

    it('should create the necessary resources for the most minimal configuration with forceDeploy', () =>
      runServerless({
        fixture: 'cognitoUserPool',
        configExt: {
          functions: {
            existingSimple: {
              events: [
                {
                  cognitoUserPool: {
                    forceDeploy: true
                  },
                },
              ],
            }
          }
        },
        command: 'package',
      }).then(({ awsNaming, cfTemplate }) => {
        const { Resources } = cfTemplate;
        let customResource = Resources[awsNaming.getCustomResourceCognitoUserPoolResourceLogicalId('existingSimple')];
        
        expect(customResource.Properties.ForceDeploy).to.not.deep.equal(undefined);
    }));

(only one expect() statement instead of the previous three, for now at least). Will something like this do?

[medikoo]

Yes, that looks very good.
Just use async/await (instead of .then()), and rather confirm on property as expect(typeof ..).to.equal('string')

[TsimpDim]

I updated the test:

I could not find a better way of specifying what ForceDeploy's value should be (especially since it varies from execution to execution) so I opted for a value range assessment.
I also tried to add
expect(addCustomResourceToServiceStub).to.have.been.calledOnce; which I would expect to run fine, but it always failed (0 calls instead of the expected one - not sure if that's expected or I did something wrong)

I also updated the documentation to show the new flag.

[medikoo]

Thank you @TsimpDim I have just two minor suggestions, and otherwise, it looks good to me

[codecov]

Codecov Report

Merging #10435 (4ad509c) into master (f893772) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #10435   +/-   ##
=======================================
  Coverage   85.43%   85.44%           
=======================================
  Files         336      336           
  Lines       14003    14004    +1     
=======================================
+ Hits        11964    11966    +2     
+ Misses       2039     2038    -1     

Impacted Files	Coverage Δ
...gins/aws/package/compile/events/cognitoUserPool.js	100.00% <100.00%> (ø)
lib/plugins/aws/package/compile/functions.js	95.14% <0.00%> (+0.32%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f893772...4ad509c. Read the comment docs.

[medikoo]

Thank you @TsimpDim that looks great!

[meirgbinahai]

I see that this feature was pushed to version 2.71.0 - can anyone confirm that it works in versions 2.71.0+ ?

[halx4]

I see that this feature was pushed to version 2.71.0 - can anyone confirm that it works in versions 2.71.0+ ?

It works for me!
This is how I set it:

...
functions:
  postConfirmation-cognitoTrigger:
    handler: foo.bar
    events:
      - cognitoUserPool:
          pool: ${self:custom.userPoolName}
          existing: true
          trigger: PostConfirmation
          forceDeploy: true
...

[chris]

One followup question, related to @TsimpDim 's comment about how the triggers seem to get removed for a little bit during the deploy, then added back in. If that's the case, it seems like unless you change your Cognito pool frequently, it'd be best to only add the forceDeploy: true part in when you know this issue will happen? i.e. if we are only rarely changing user pool config (so this issue would rarely occur), I wouldn't want to have the triggers disappear on every deploy, and risk problems in my system (e.g. triggers not getting called) for the more common deploy case. Does that make sense - i.e. would you agree that if you infrequently change your user pool, you should only use forceDeploy: true when making those occasional changes, and otherwise leave it out of serverless.yml?

[AEsmerio]

I have been struggling with this issue for a very long time, but finally been resolved! THANKS! By adding "forceDeploy: true" it solved the problem. 🍻