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.

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

Can't run new EC2 instance with providing NetworkInterfaces #16

Closed
bacek opened this issue Dec 10, 2021 · 7 comments · Fixed by #21
Closed

Can't run new EC2 instance with providing NetworkInterfaces #16

bacek opened this issue Dec 10, 2021 · 7 comments · Fixed by #21
Labels
bug Something isn't working

Comments

@bacek
Copy link

bacek commented Dec 10, 2021

  1. EC2 InstanceNetworkInterfaceSpecification is slightly incorrect comparing to https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-network-iface-embedded.html. Fields such as Ipv4Prefixes, Ivp4Addresses are optional according to documentation. Groups are actually spelled GroupSet.
  2. Running runInstnaces with NetworkInterfaces supplied in request lead to exception from AWS
Interrupted AwsServiceError: UnknownParameter: The parameter networkInterface is not recognized [Request ID: 4453910d-60c4-4c8a-8edb-169631ff8fdf]
@bacek
Copy link
Author

bacek commented Dec 10, 2021

@paul-thompson-helix
Copy link

So you mean:

Currently:
interface InstanceNetworkInterfaceSpecification has Ipv4Prefixes: Ipv4PrefixSpecificationRequest[];

Expected:
interface InstanceNetworkInterfaceSpecification has Ipv4Prefixes?: Ipv4PrefixSpecificationRequest[];

Refs:

@danopia
Copy link
Member

danopia commented Dec 10, 2021

Hey, thanks for the issue! Ec2 is a pretty weird API out of the bunch. If you could include an awscli or /x/aws_api etc. invocation that you expect to work, I should be able to compare the payloads sometime today.

@danopia
Copy link
Member

danopia commented Dec 10, 2021

Following up more directly:

  1. Typings. This will depend on where you're importing your EC2 client from. I don't ship a pregenerated ec2.ts to /x/aws_api anymore, so I'll assume you're importing from aws-api.deno.dev/v0.2/services/ec2.ts.

    1. Fields such as Ipv4Prefixes, Ivp4Addresses are optional according to documentation

      I see Ipv4Prefixes as optional with this library. Here's what I'm seeing on doc.deno.land. Maybe we're looking at different things? Update in following comment.

    2. Groups are actually spelled GroupSet.

      This is just a discrepancy between the official AWS JS SDK, and the AWS CloudFormation structures that you are looking at. Here's the official v3 JS SDK docs showing Groups. So this shouldn't be causing any issues.

      Here's the upstream definitions from v2 JS SDK showing the mapping from Groups to groupSet:

        InstanceNetworkInterface: {
          type: "structure",
          members: {
            # other members removed for brevity
            Groups: {
              shape: "GroupIdentifierList",
              documentation: "<p>One or more security groups.</p>",
              locationName: "groupSet"
            },
  2. Running runInstnaces with NetworkInterfaces supplied in request lead to exception from AWS

    Yea this definitely looks like a bug in this library. It'll be easiest to reproduce with an example invocation or code sample in hand 😄 I don't currently specify NetworkInterfaces in examples/ec2-launch-instance.ts, so when I have some time I can add one there to have another reproduction.

@danopia
Copy link
Member

danopia commented Dec 10, 2021

Oh dear, the Ipv4Prefixes thing is totally a codegen issue. The generated structures look correct when an API action filter is used to slim down the returned TypeScript module, which I highly recommend anyway, but when the whole EC2 API surface is generated, it does in fact wrongly require some fields.

I'll be able to work on this one point on my own but in the meantime try adding an action filter to your import. For example, my lib/examples/ec2.ts demo works with:

import { EC2, Instance } from 'https://aws-api.deno.dev/v0.2/services/ec2.ts?actions=RunInstances,DescribeInstances,DescribeAvailabilityZones,DescribeVpcs,DescribeSubnets,DescribeImages,TerminateInstances,GetConsoleOutput';

danopia referenced this issue Dec 10, 2021
The way we populate lists from XML means that you'll always get an
array, even if it's empty.
@danopia danopia added this to AWS APIs Dec 21, 2021
@danopia danopia added the bug Something isn't working label Dec 21, 2021
@danopia danopia moved this to Todo in AWS APIs Dec 21, 2021
@danopia
Copy link
Member

danopia commented Dec 21, 2021

Hi, I've pushed a codegen fix efd6d33 which addresses a few different issues in the EC2 API, including point 2 in your original message. It turns out my list encoding handled EC2's special-case wrongly, and the aws-sdk-js test fixtures didn't stress that aspect.

These changes impact only the request payloads. I've already applied it to both published codegen versions (v0.1 & v0.2) since the change is strictly fixing things and only in ec2.ts. Try a deno run --reload to pick this change up.

I also updated my EC2 demo to use NetworkInterfaces as a test: 0b94e42

For the most accurate typings, remember to import only the actions you need, as I showed in my previous comment. That issue is still not resolved.

EC2 is weird, so if you end up looping back to using this module, I'd love to hear how it goes, successful or not. Thanks!

@danopia danopia moved this from Todo to Investigating in AWS APIs Dec 22, 2021
@danopia danopia moved this from Investigating to In Progress in AWS APIs Dec 26, 2021
Repository owner moved this from In Progress to Done in AWS APIs Dec 26, 2021
@danopia
Copy link
Member

danopia commented Dec 26, 2021

The reported issues here should be resolved in the upcoming codegeneration v0.3. The prior codegens are left alone because this change changes typings to make more fields nullable.

This brief sample now runs for me:

import { ApiFactory } from 'https://deno.land/x/aws_api/client/mod.ts';
import { EC2 } from 'https://aws-api.deno.dev/v0.3/services/ec2.ts';
const ec2 = new ApiFactory().makeNew(EC2);

const {Instances: [instance]} = await ec2.runInstances({
  "MinCount": 1,
  "MaxCount": 1,
  "InstanceType": "t4g.nano",
  "ImageId": "ami-0e8eba75fce52c37c", // replace this
  "NetworkInterfaces": [{
    "DeviceIndex": 0,
    "AssociatePublicIpAddress": true,
    "SubnetId": "subnet-506cd939" // replace this
  }],
});

I'll be promoting v0.3 to default soon, in sync with releasing v0.6.0 of the client library. I look forward to hearing how your usecase goes. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants