ecs: example in managed instances documentation does not work

[isker]

Describe the bug

This example uses a managed instances capacity provider with an Ec2Service. It fails to synth because the cluster lacks EC2 capacity providers.
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs-readme.html#managed-instances-capacity-providers

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Library Version

No response

Expected Behavior

Examples work. Examples are tested in integration tests. (This one is not, the integ test only tests managed instances capacity provider with a Fargate Service.)

Current Behavior

λ npx cdk synth
/private/tmp/foo/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:3
  `);throw new(errors_1()).UnscopedValidationError(`Validation failed with the following errors:
           ^
UnscopedValidationError: Validation failed with the following errors:
  [FooStack/EC2Service] Cluster for this service needs Ec2 capacity. Call addXxxCapacity() on the cluster.
    at path [undefined]

  [FooStack/EC2Service] Cluster for this service needs Ec2 capacity. Call addXxxCapacity() on the cluster.
    at validateTree (/private/tmp/foo/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:3:12)
    at synthesize (/private/tmp/foo/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:1893)
    at App.synth (/private/tmp/foo/node_modules/aws-cdk-lib/core/lib/stage.js:1:2912)
    at process.<anonymous> (/private/tmp/foo/node_modules/aws-cdk-lib/core/lib/app.js:1:2041)
    at Object.onceWrapper (node:events:633:26)
    at process.emit (node:events:518:28)
    at process.emit (node:domain:489:12)
    at process.emit.sharedData.processEmitHook.installedValue [as emit] (/private/tmp/foo/node_modules/@cspotcode/source-map-support/source-map-support.js:745:40)
npx ts-node --prefer-ts-exts bin/foo.ts: Subprocess exited with error 1

Reproduction Steps

I ran npx cdk init app --language typescript, put that example in the stack, and filled in the "blanks" of the declare consts in the example.

Resulting stack:

import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as ecs from 'aws-cdk-lib/aws-ecs';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import { Size } from 'aws-cdk-lib';

export class FooStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const managedInstancesInstanceProfile = new iam.InstanceProfile(
      this,
      "ManagedInstancesInstanceProfile",
    );
    managedInstancesInstanceProfile.role?.addManagedPolicy(
      iam.ManagedPolicy.fromAwsManagedPolicyName(
        "AmazonECSInstanceRolePolicyForManagedInstances",
      ),
    );
    const managedInstancesInfrastructureRole = new iam.Role(
      this,
      "ManagedInstancesInfrastructureRole",
      {
        assumedBy: new iam.ServicePrincipal("ecs.amazonaws.com"),
        managedPolicies: [
          iam.ManagedPolicy.fromAwsManagedPolicyName(
            "AmazonECSInfrastructureRolePolicyForManagedInstances",
          ),
        ],
      },
    );
    managedInstancesInstanceProfile.role?.grantPassRole(
      managedInstancesInfrastructureRole,
    );

    const cluster = new ecs.Cluster(this, 'Cluster',);

    // Create a Managed Instances Capacity Provider
    const miCapacityProvider = new ecs.ManagedInstancesCapacityProvider(this, 'MICapacityProvider', {
      infrastructureRole: managedInstancesInfrastructureRole,
      ec2InstanceProfile: managedInstancesInstanceProfile,
      subnets: cluster.vpc.privateSubnets,
      securityGroups: [new ec2.SecurityGroup(this, 'MISecurityGroup', { vpc: cluster.vpc })],
      instanceRequirements: {
        vCpuCountMin: 1,
        memoryMin: Size.gibibytes(2),
        cpuManufacturers: [ec2.CpuManufacturer.INTEL],
        acceleratorManufacturers: [ec2.AcceleratorManufacturer.NVIDIA],
      },
      propagateTags: ecs.PropagateManagedInstancesTags.CAPACITY_PROVIDER,
    });

    // Add the capacity provider to the cluster
    cluster.addManagedInstancesCapacityProvider(miCapacityProvider);

    const taskDefinition = new ecs.Ec2TaskDefinition(this, 'TaskDef');

    taskDefinition.addContainer('web', {
      image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
      memoryReservationMiB: 256,
    });

    new ecs.Ec2Service(this, 'EC2Service', {
      cluster,
      taskDefinition,
      minHealthyPercent: 100,
      capacityProviderStrategies: [
        {
          capacityProvider: miCapacityProvider.capacityProviderName,
          weight: 1,
        },
      ],
    });
  }
}

Possible Solution

No response

Additional Information/Context

No response

AWS CDK Library version (aws-cdk-lib)

2.220.0

AWS CDK CLI version

N/A

Node.js Version

N/A

OS

N/A

Language

TypeScript

Language Version

No response

Other information

@kg-aws

While I've got your attention:

• I wish the documentation would explain the tradeoffs between using Ec2Service and FargateService. Why did this feature not gets its own ManagedInstancesService? Why do we have to use one of these other constructs instead?
• Why does the user have to explicitly grant required iam permissions to the relevant infrastructure/instance profile roles? CDK typically provides these kinds of things automatically.
• I don't see why managed instances capacity providers have to drive creation of ClusterCapacityProviderAssociations: https://github.com/aws/aws-cdk/blame/cfbc6fbc1d6e4b960b48613b97ffd8dc79727882/packages/aws-cdk-lib/aws-ecs/lib/cluster.ts#L1966. Managed Instances capacity providers do not use capacity provider associations.
• Why doesn't ManagedInstancesCapacityProvider implement things like IConnectable? Most CDK L2 constructs don't require users to "drop down" to raw SecurityGroups.
• Similar question about subnets. Most L2 constructs accept ec2.SubnetSelection instead of a raw array of subnets.

[aemada-aws]

Why does the user have to explicitly grant required iam permissions to the relevant infrastructure/instance profile roles? CDK typically provides these kinds of things automatically.

You can provide your own infrastructure role or leave the construct to create one with the AmazonECSInfrastructureRolePolicyForManagedInstances managed policy. If the user brings their own role, the construct assumes they want to configure the permissions themselves. If we auto configure policies on a user provided role, this will create conflicts where users might not want to use managed policies or want more explicit control.

For instance profile, I will have a PR soon to make this optional and have the construct also create a default one if the user does not provide one to the construct.

I don't see why managed instances capacity providers have to drive creation of ClusterCapacityProviderAssociations: https://github.com/aws/aws-cdk/blame/cfbc6fbc1d6e4b960b48613b97ffd8dc79727882/packages/aws-cdk-lib/aws-ecs/lib/cluster.ts#L1966. Managed Instances capacity providers do not use capacity provider associations.

Checking this, i think you are right and I will have a PR soon to remove it. It was also called out in #35699

Why doesn't ManagedInstancesCapacityProvider implement things like IConnectable? Most CDK L2 constructs don't require users to "drop down" to raw SecurityGroups.

I have opened a PR to add it, this was missed from the initial implementation. Thanks for calling it out. #35745

Similar question about subnets. Most L2 constructs accept ec2.SubnetSelection instead of a raw array of subnets.

You are also right, this was a miss. However, you can still use vpc.selectSubnets({ subnetType: ec2.SubnetType.PUBLIC }).subnets to provide the subnets from the selection to the construct instead of providing the selection directly to the construct. Unfortunately the type cannot be changed because it is now part of the public API.

[isker]

You can provide your own infrastructure role or leave the construct to create one with the AmazonECSInfrastructureRolePolicyForManagedInstances managed policy.

You’re right, I misread the code.

For instance profile, I will have a PR soon to make this optional and have the construct also create a default one if the user does not provide one to the construct.

Please implement IGrantable, or at least provide access to that profile’s role, so more permissions may be granted to it.

[kg-aws]

I wish the documentation would explain the tradeoffs between using Ec2Service and FargateService. Why did this feature not gets its own ManagedInstancesService? Why do we have to use one of these other constructs instead?

EC2Service and FargateService are both constructs used in the AWS CDK for deploying containerized applications, but they have different underlying infrastructure models:

1. EC2Service runs containers on EC2 instances that customers manage
2. FargateService is a serverless option that abstracts away the need to manage EC2 instances

Regarding why there isn't a separate ManagedInstancesService, the managed instances services falls within the bucket of managed infrastructure similar to FargateService. The decision to use FargateService for managed instances stems from the overlapping characteristics and underlying validations between these managed services. By utilizing the FargateService construct, it can provide a familiar interface for users while still accommodating the specific needs of managed instances.

I don't see why managed instances capacity providers have to drive creation of ClusterCapacityProviderAssociations: https://github.com/aws/aws-cdk/blame/cfbc6fbc1d6e4b960b48613b97ffd8dc79727882/packages/aws-cdk-lib/aws-ecs/lib/cluster.ts#L1966. Managed Instances capacity providers do not use capacity provider associations.

As @aemada-aws mentioned that we probably can remove the CCPA resource for customer that are just relying on ManagedInstances, but I will work with @aemada-aws if there are any consequences of changing the logic here: https://github.com/aws/aws-cdk/blame/cfbc6fbc1d6e4b960b48613b97ffd8dc79727882/packages/aws-cdk-lib/aws-ecs/lib/cluster.ts#L349C3-L349C3

[isker]

Regarding why there isn't a separate ManagedInstancesService, the managed instances services falls within the bucket of managed infrastructure similar to FargateService. The decision to use FargateService for managed instances stems from the overlapping characteristics and underlying validations between these managed services. By utilizing the FargateService construct, it can provide a familiar interface for users while still accommodating the specific needs of managed instances.

Okay, well, the example in the docs uses an Ec2Service and not a FargateService.