cloudydeno / deno-aws_api

From-scratch Typescript client for accessing AWS APIs
https://deno.land/x/aws_api
59 stars 3 forks source link

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

Closed bacek closed 2 years ago

bacek commented 2 years ago
  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 commented 2 years ago

This is probably relevant for a second item: https://github.com/boto/botocore/blob/c920c8f40c6fb61b9b3f3b1d76ff3c14fdad3490/botocore/serialize.py#L319

paul-thompson-helix commented 2 years ago

So you mean:

Currently: interface InstanceNetworkInterfaceSpecification has Ipv4Prefixes: Ipv4PrefixSpecificationRequest[];

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

Refs:

danopia commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Hi, I've pushed a codegen fix https://github.com/cloudydeno/deno-aws_api/commit/efd6d33f7d2a85831d6be550b383905c9f0a98dd 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: https://github.com/cloudydeno/deno-aws_api/commit/0b94e4296eb29dd099ca54cda7ca5d9e66fcabd6

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 commented 2 years ago

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. :)