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.

Sign up for GitHub

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

Jump to bottom

Add forceDeploy property in Cognito User Pools #10435

Merged
merged 5 commits into from
Jan 5, 2022
Merged

Add forceDeploy property in Cognito User Pools #10435

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
26813bc
Add forceDeploy property in Cognito User Pools
Jan 2, 2022
e5f444f
Change from Math.random() to Date.now()
Jan 3, 2022
b682c37
Change test implementation to use runServerless
Jan 4, 2022
e85d1ce
Update documentation
Jan 4, 2022
4ad509c
Simplify test case
Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/plugins/aws/package/compile/events/cognitoUserPool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class AwsCompileCognitoUserPoolEvents {
pool: { type: 'string', maxLength: 128, pattern: '^[\\w\\s+=,.@-]+$' },
trigger: { enum: validTriggerSources },
existing: { type: 'boolean' },
forceDeploy: { type: 'boolean' },
},
required: ['pool', 'trigger'],
additionalProperties: false,
Expand Down Expand Up @@ -141,7 +142,7 @@ class AwsCompileCognitoUserPoolEvents {
functionObj.events.forEach((event) => {
if (event.cognitoUserPool && event.cognitoUserPool.existing) {
numEventsForFunc++;
const { pool, trigger } = event.cognitoUserPool;
const { pool, trigger, forceDeploy } = event.cognitoUserPool;
usesExistingCognitoUserPool = funcUsesExistingCognitoUserPool = true;

if (!currentPoolName) {
Expand Down Expand Up @@ -171,6 +172,7 @@ class AwsCompileCognitoUserPoolEvents {
}

let customCognitoUserPoolResource;
const forceDeployProperty = forceDeploy ? Date.now() : undefined;
if (numEventsForFunc === 1) {
customCognitoUserPoolResource = {
[customPoolResourceLogicalId]: {
Expand All @@ -188,6 +190,7 @@ class AwsCompileCognitoUserPoolEvents {
Trigger: trigger,
},
],
ForceDeploy: forceDeployProperty,
},
},
};
Expand Down
74 changes: 74 additions & 0 deletions test/unit/lib/plugins/aws/package/compile/events/cognitoUserPool.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,77 @@ describe('AwsCompileCognitoUserPoolEvents', () => {
Trigger: 'CustomMessage',
},
],
ForceDeploy: undefined,
},
});
});
});

it('should create the necessary resources for the most minimal configuration with forceDeploy', () => {
awsCompileCognitoUserPoolEvents.serverless.service.functions = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@TsimpDim TsimpDim Jan 4, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

first: {
name: 'first',
events: [
{
cognitoUserPool: {
pool: 'existing-cognito-user-pool',
trigger: 'CustomMessage',
existing: true,
forceDeploy: true,
},
},
],
},
};

return expect(
awsCompileCognitoUserPoolEvents.existingCognitoUserPools()
).to.be.fulfilled.then(() => {
const { Resources } =
awsCompileCognitoUserPoolEvents.serverless.service.provider
.compiledCloudFormationTemplate;

expect(addCustomResourceToServiceStub).to.have.been.calledOnce;
expect(addCustomResourceToServiceStub.args[0][1]).to.equal('cognitoUserPool');
expect(addCustomResourceToServiceStub.args[0][2]).to.deep.equal([
{
Action: [
'cognito-idp:ListUserPools',
'cognito-idp:DescribeUserPool',
'cognito-idp:UpdateUserPool',
],
Effect: 'Allow',
Resource: '*',
},
{
Action: ['lambda:AddPermission', 'lambda:RemovePermission'],
Effect: 'Allow',
Resource: { 'Fn::Sub': 'arn:${AWS::Partition}:lambda:*:*:function:first' },
},
{
Effect: 'Allow',
Resource: {
'Fn::Sub': 'arn:${AWS::Partition}:iam::*:role/*',
},
Action: ['iam:PassRole'],
},
]);
expect(Resources.FirstCustomCognitoUserPool1).to.not.deep.equal({
Type: 'Custom::CognitoUserPool',
Version: 1,
DependsOn: ['FirstLambdaFunction', 'CustomDashresourceDashexistingDashcupLambdaFunction'],
Properties: {
ServiceToken: {
'Fn::GetAtt': ['CustomDashresourceDashexistingDashcupLambdaFunction', 'Arn'],
},
FunctionName: 'first',
UserPoolName: 'existing-cognito-user-pool',
UserPoolConfigs: [
{
Trigger: 'CustomMessage',
},
],
ForceDeploy: undefined,
},
});
});
Expand Down Expand Up @@ -427,6 +498,7 @@ describe('AwsCompileCognitoUserPoolEvents', () => {
Trigger: 'DefineAuthChallenge',
},
],
ForceDeploy: undefined,
},
});
});
Expand Down Expand Up @@ -556,6 +628,7 @@ describe('AwsCompileCognitoUserPoolEvents', () => {
Trigger: 'DefineAuthChallenge',
},
],
ForceDeploy: undefined,
},
});
expect(Resources.SecondCustomCognitoUserPool1).to.deep.equal({
Expand Down Expand Up @@ -583,6 +656,7 @@ describe('AwsCompileCognitoUserPoolEvents', () => {
Trigger: 'PostAuthentication',
},
],
ForceDeploy: undefined,
},
});
});
Expand Down