aws / aws-sdk

Landing page for the AWS SDKs on GitHub
https://aws.amazon.com/tools/
Other
68 stars 12 forks source link

SDK doesn't offer a way to remove canary VPC Config #617

Open carcher5432 opened 9 months ago

carcher5432 commented 9 months ago

Describe the bug

There is no way in the SDK to send the correct request that AWS expects in order to remove VPC Config from an existing canary.

Expected Behavior

VPC Config can be removed from canary.

Current Behavior

AWS API request that successfully removes the vpc configuration:

{ # Other parameters removed because they are not relevant
   "requestParameters": {
        "VpcConfig": {
            "SubnetIds": "***",
            "SecurityGroupIds": "***"
        },
        "name": "canary-name"
    }
}

*output comes from CloudTrail

With this structure for the VPC Config Input and no additional logic to set the "***" that the AWS API expects, it doesn't seem possible to remove an existing VPC Config.

type VpcConfigInput struct {
    _ struct{} `type:"structure"`

    // The IDs of the security groups for this canary.
    SecurityGroupIds []*string `type:"list"`

    // The IDs of the subnets where this canary is to run.
    SubnetIds []*string `type:"list"`
}

Reproduction Steps

N/A

Possible Solution

I don't know what the best solution would be, given the current type constraints. It might require an additional field on the VpcConfigInput type and custom logic where the json body for the API request is created.

Additional Information/Context

No response

SDK version used

latest

Environment details (Version of Go (go version)? OS name and version, etc.)

N/A, core feature

RanVaknin commented 9 months ago

Hi @carcher5432 ,

If the service can accepts "***" as a valid input parameter, then it itself is deviating from the defined API model. If you didn't know, the SDK is code-generated from the various AWS Services' API models. In this case, the service defined SecurityGroupIds and SubnetIds as a list of strings. This means that the SDK's code generation platform, will use those API definition to generate types from.

So this is an issue that needs to be resolved upstream with the synthetics service team itself.

You can intercept and re-write the serialized request before its being sent by using request handlers:

package main

import (
    "fmt"
    "github.com/aws/aws-sdk-go/aws"
    "github.com/aws/aws-sdk-go/aws/request"
    "github.com/aws/aws-sdk-go/aws/session"
    "github.com/aws/aws-sdk-go/service/synthetics"
    "io"
    "strings"
)

func modifyRequestToAsterisk(r *request.Request) {
    if r.Operation.Name == "UpdateCanary" {
        body := r.GetBody()
        originalPayloadBytes, err := io.ReadAll(body)
        if err != nil {
            fmt.Println("Error reading request body:", err)
            return
        }
        originalPayload := string(originalPayloadBytes)

        modifiedPayload := strings.ReplaceAll(originalPayload, "\"SecurityGroupIds\":[]", "\"SecurityGroupIds\":\"***\"")
        modifiedPayload = strings.ReplaceAll(modifiedPayload, "\"SubnetIds\":[]", "\"SubnetIds\":\"***\"")

        r.HTTPRequest.Header.Set("Content-Length", fmt.Sprintf("%d", len(modifiedPayload)))
        r.SetBufferBody([]byte(modifiedPayload))
    }
}

func main() {
    sess, err := session.NewSession(&aws.Config{
        Region:   aws.String("us-east-1"),
        LogLevel: aws.LogLevel(aws.LogDebugWithHTTPBody),
    })

    if err != nil {
        fmt.Println("Error creating session:", err)
        return
    }

    svc := synthetics.New(sess)

    req, _ := svc.UpdateCanaryRequest(&synthetics.UpdateCanaryInput{
        Name: aws.String("canary-name"),
        VpcConfig: &synthetics.VpcConfigInput{
            SecurityGroupIds: []*string{},
            SubnetIds:        []*string{},
        },
    })

    req.Handlers.Send.PushFront(modifyRequestToAsterisk)

    err = req.Send()
    if err != nil {
        panic(err)
    }
}

This will result in this serialized request:

---[ REQUEST POST-SIGN ]-----------------------------
PATCH /canary/canary-name HTTP/1.1
Host: synthetics.us-west-2.amazonaws.com
User-Agent: aws-sdk-go/1.45.10 (go1.19.1; darwin; arm64)
Content-Length: 58
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20230929/us-east-1/synthetics/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date, Signature=REDACTED
Content-Type: application/json
X-Amz-Date: 20230929T212532Z
Accept-Encoding: gzip

{"VpcConfig":{"SecurityGroupIds":"***","SubnetIds":"***"}}

Please note that I did not actually reproduce this with a vpc configuration on my own. This solution is an example on how you can hook alter the serialization logic the SDK provides you.

Please let me know if this is helpful.

Thanks, Ran~

github-actions[bot] commented 9 months ago

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

carcher5432 commented 9 months ago

Thanks for the response! I'll reach out to the Synthetics team and see about that.

RanVaknin commented 9 months ago

Hi @carcher5432 ,

Can you please verify if that workaround works for you? Also if you have cut a ticket through the AWS console, please make sure to list me / someone from the SDK team as a point of contact as we will be able to provide additional info and help drive this forward.

Thanks, Ran~

github-actions[bot] commented 9 months ago

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

carcher5432 commented 9 months ago

Hi @RanVaknin,

That workaround does work, but the original issue I had found was in the Terraform aws provider and I am hesitant to implement it there if we might have a solution that doesn't require a workaround before too long. I have opened a ticket through the AWS console and pointed them here for reference.

Thanks!

tim-finnigan commented 1 day ago

Were there any updates on the internal ticket opened for this? I couldn't find anything referencing this issue. If in involves Terraform then this may something that would need to be handled on their end.