cdk-constructs icon indicating copy to clipboard operation
cdk-constructs copied to clipboard

Ensure unique resources ID are created

Open fasatrix opened this issue 4 years ago • 10 comments

Context: While deploying a multi pipeline with a single stack that will independently deploy FE and BE ECS services (under the same ALB) we noted the following issue.

Issues: Resources IDs aren't unique while being created as part of a single CDK stack.

image

image

Architectural Diagram of the target state image

fasatrix avatar Sep 08 '21 22:09 fasatrix

@hupe1980 any chances for this to be reviewed? Thanks

fasatrix avatar Sep 12 '21 22:09 fasatrix

@hupe1980 any change for this to be looked at? It should be quick as doesn't change anything major. Thanks

fasatrix avatar Sep 16 '21 21:09 fasatrix

@hupe1980 Hi, I also come across this issue if I try to deploy multiple services in one stack, is there any change for you to look at this PR

cao2504 avatar Oct 06 '21 01:10 cao2504

Actually, the cdk framework already attaches an id to the logical name. Even the stdlib does not take any further steps. See: https://github.com/aws/aws-cdk/blob/ae840ff1abb8283a1290dae5859f5729a9cf72b1/packages/%40aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts#L160

How is your stack structured exactly?

hupe1980 avatar Oct 09 '21 19:10 hupe1980

Actually, the cdk framework already attaches an id to the logical name. Even the stdlib does not take any further steps. See: https://github.com/aws/aws-cdk/blob/ae840ff1abb8283a1290dae5859f5729a9cf72b1/packages/%40aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts#L160

How is your stack structured exactly?

Of course it does , but if in the same stack you call that CDK construct twice (e.g. you deploy two services at once, for instance fe and be), CDK will complain as that resource exists a ready.

We have a stack like this

new CDPipelineStack(app, 'StackName', {
    certificateArn: values.certificateArn,
    url: `som- URL`,
    pipelines: [
      {
        cpu: 2048,
        memory: 4096,
        imageRepoName: 'image-be',
        imageRepoTag: values.tag,
        nextImageRepoTag: values.nextTag,
        path: '/api/*',
        containerEnvironment: [
          { name: 'ENVIRONMENT_NAME', value: values.environmentName },
          { name: 'FRONTEND_HOST', value: 'some-host-name' },
        ],
        containerSecrets: [
          { name: 'OAUTH_GITHUB_CLIENT_ID', valueFrom: `/service-catalogue/${values.parameterStoreKey}/OAUTH_GITHUB_CLIENT_ID` },
          { name: 'OAUTH_GITHUB_CLIENT_SECRET', valueFrom: `/service-catalogue/${values.parameterStoreKey}/OAUTH_GITHUB_CLIENT_SECRET` },
          { name: 'GIT_SVC_ACCOUNT_TOKEN', valueFrom: `/${values.parameterStoreKey}/GIT_SVC_ACCOUNT_TOKEN` },
        ],
        numberOfTasks: 2,
        // Due to session state being application server local, we need stickiness on the ALB
        sessionStickinessDuration: Duration.days(7),
      },
      {
        url: 'someurl',
        imageRepoName: 'image-fe,
        imageRepoTag: values.tag,
        nextImageRepoTag: values.nextTag,
        containerEnvironment: [
          { name: 'ENVIRONMENT_NAME', value: values.environmentName },
          { name: 'FE_HOST', value: `somehost` },
        ]
      }
    ],
  });

The construct then will loop through the above array of pipelines and creates independent codePipleine/CodeDeploy

fasatrix avatar Oct 12 '21 01:10 fasatrix

@hupe1980 as said in my PR we have already forked this and made the changes ourselves as per this PR and it works, problem is we don't w3ant to maintain the fork each time you change this project hence why we are requiring the change to be approved. I can walk you through our use case (which I believe it is quite unique) if you can let me know abut your availability in NZT (we can zeoom call)

fasatrix avatar Oct 13 '21 05:10 fasatrix

Do you set always new IDs during the loop? About as: https://github.com/cloudcomponents/cdk-constructs/blob/0ff07cbce1fe25b5ea6d57ce08553f4b1fff90c8/packages/cdk-static-website/src/website-alias-record.ts#L47

hupe1980 avatar Oct 13 '21 19:10 hupe1980

Do you set always new IDs during the loop? About as:

https://github.com/cloudcomponents/cdk-constructs/blob/0ff07cbce1fe25b5ea6d57ce08553f4b1fff90c8/packages/cdk-static-website/src/website-alias-record.ts#L47

HI @hupe1980 we definitely do when we create our own resources however as you can see from the above error stack the problem is with a resource we have no control of Custom:EcsDeploymentGroup as it is part of your lib. This is the one that fails because of a not unique name while called in parallel as part of multiple pipelines within the same stack, and hence why we asking for this PR. As you can see from the PR it is not the only one the affected recourse, and you could use the same strategy as in the example you indicate above to make it unique by using a has instead of passing the id as I do (this is unique though as it is carried over from the constract that instantiate the resource). Please try to create a deployment similar to the one I have explained above (two pipelines in a common stack, classic front and backend architecture which will create two separate CodePipeline and CodeDeploy) and hopefully you will witness it too.

Look at the following example from my PR, if I try to create those resources as part of the same stack twice (two different pipelines) AWS will complain as I have already one resource with that Name. You can use hash if you like instead of id I do not mind as long as it is unique and fixes the problem.

image

I am still keen to show while it is happening if you indicate a time suitable to attend a zoom?

fasatrix avatar Oct 14 '21 18:10 fasatrix

Ah okay. I understand it :

The problem is the singleton 'getOrCreate': https://github.com/cloudcomponents/cdk-constructs/blob/0ff07cbce1fe25b5ea6d57ce08553f4b1fff90c8/packages/cdk-blue-green-container-deployment/src/ecs-deployment-group.ts#L108

We have to replace the singleton with a normal construct

hupe1980 avatar Oct 14 '21 22:10 hupe1980

Ah okay. I understand it :

The problem is the singleton 'getOrCreate':

https://github.com/cloudcomponents/cdk-constructs/blob/0ff07cbce1fe25b5ea6d57ce08553f4b1fff90c8/packages/cdk-blue-green-container-deployment/src/ecs-deployment-group.ts#L108

We have to replace the singleton with a normal construct

Nice thx

fasatrix avatar Oct 14 '21 22:10 fasatrix