fix(ec2): passing keypair to instance unexpectedly does nothing

[ayush-shah-1501]

Fixes by Specifying key pair reference in cfnInstance.

This will change behavior if both keyName and keyPair is set on an existing resource, since we will use keyPair.keyPairName instead of keyName now. However, there is no correct use case for specifying both keyPair and keyName, and given keyName is deprecated, this PR is introducing the correct behavior.

Closes #28478.
Closes #28569

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[ayush-shah-1501]

Exemption Request: This PR is quick fix of this PR, which didn't need changes to readme and tests.

[sumupitchayan]

@ayush-shah-1501 this PR title should be fix not chore and should also include tests to validate this functionality fixes the bug

[laurelmay]

@ayush-shah-1501 Thanks for catching this! So sorry for missing this in the original PR. I will momentarily open a PR on your repo that will add a possible test & updates the integration tests.

Additionally, so long as a user follows the docs for how to use an existing key pair, this should not be a breaking change.

Edit: Hopefully ayush-shah-1501#1 can help address some of the linter concerns. But it may be good to ask for an Exemption Request if the linter is still mad about the integration test.

[ayush-shah-1501]

Exemption Request: This PR is quick fix of #28138 PR

[laurelmay]

For what it's worth, I think it's safe to not call this a breaking change. The correct behavior is documented and it does work correctly for other resource types; this condition was just missed in the implementation for Instance. Honestly, the current behavior (with keyName deprecated and keyPair not working) is far more broken than the results of this change.

[kaizencc]

Thanks @ayush-shah-1501 and @kylelaker

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 256455c
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[cponfick]

I am not sure if this change is at fault here, but using the ´keyPair´ parameter does not set the Ref in CFN. It does nothing.

Example:

key_pair = aws_ec2.KeyPair(
            self,
            f"Keypair",
            public_key_material="some key",
        )
instance = aws_ec2.Instance(
            self,
            f"id",
            vpc=base.vpc,
            instance_type=aws_ec2.InstanceType.of(
                aws_ec2.InstanceClass.T3, aws_ec2.InstanceSize.MICRO
            ),
            machine_image=aws_ec2.MachineImage.generic_linux(
                {'eu-central-1': 'ami-0faab6bdbac9486fb'}
            ),
            key_pair=key_pair,
            # key_name=key_pair.key_pair_name,
            associate_public_ip_address=True,
            vpc_subnets=aws_ec2.SubnetSelection(
                subnet_type=aws_ec2.SubnetType.PUBLIC
            ),
        )

keyName works as expected.

[kaizencc]

@cponfick this hasn't been released yet. will be out in 2.119.0, which will probably go out today. let us know in a new issue if the problem persists!

[mmieluch]

I'm using a slightly higher version 2.121.1 and it's working fine for me. Thank you for the contribution!

[alastairmccormack]

Has this been regressed?

I just used aws_cdk.aws_ec2.KeyPair.from_key_pair_name() with version 2.128.0 (build d995261), and it did nothing leaving the instance with no key. I've used Instance.key_name instead and all is good.