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

Inconsistent ChangeId format in Route 53 #554

Closed
dacut opened this issue Jun 14, 2022 · 5 comments
Closed

Inconsistent ChangeId format in Route 53 #554

dacut opened this issue Jun 14, 2022 · 5 comments
Assignees
Labels
bug This issue is a bug.

Comments

@dacut
Copy link

dacut commented Jun 14, 2022

Describe the bug

When using the Route 53 client, you currently need to alter the change id coming out of the change_resource_record_sets() call when calling get_change(). The former returns change ids prefixed with /change/, while the latter expects this prefix to be omitted.

For example (pseudocode), this is what's required:

    let crrso = r53.change_resource_record_sets().hosted_zone_id(...).change_batch(...).send().await.unwrap();

    // orig_change_id is `/change/C01234567AAAAAAAAAAAA`
    let orig_change_id = crrso.change_info.unwrap().id.unwrap();

    // change_id is just the C01234... part
    let change_id = change_id.split_at(8).1.to_string();
    let gco = r53.get_change().id(change_id).send().await;

Expected Behavior

I expect to be able to pass the change id from one API call to the next unaltered.

Current Behavior

The call fails unless I remove /change/ from the change id.

The problem can be seen on line 518 of the attached run.log: it creates a URI of the form /2013-04-01/change/%252Fchange%252F<raw-id>.

In the call that succeeds, the URI is of the form /2013-04-01/change/<raw-id>.

I have redacted the hosted zone id from the logs.

run.log

Reproduction Steps

Attached is a small program that reproduces this issue. I've (again) redacted the Route 53 hosted zone id.

main.rs.txt

The output is:

Change ID: /change/C0472137U0LRYA30WCRM
Route 53 GetChange failed: NoSuchChange: Could not find resource with ID: /change/C0472137U0LRYA30WCRM
Route 53 GetChange: GetChangeOutput { change_info: Some(ChangeInfo { id: Some("/change/C0472137U0LRYA30WCRM"), status: Some(Pending), submitted_at: Some(DateTime { seconds: 1655228803, subsecond_nanos: 486000000 }), comment: None }) }

I would expect the second line (Route 53 GetChange failed) to not be present, and the mutating logic on lines 32-35 to not be needed.

Possible Solution

Boto doesn't seem to have this problem; it looks like they have some custom logic in a hook to munge the ids.

Looking at the botocore model file, GetChange looks like this:

    "GetChange":{
      "name":"GetChange",
      "http":{
        "method":"GET",
        "requestUri":"/2013-04-01/change/{Id}"
      },
      ...
    }

So they're just passing in the raw part of the id.

Then, in a global list of API mungers, they inject fix_route53_ids into a step called before-parameter-build for Route 53.

Some quick skimming suggests this is already being generated (?!) for /hostedzone/ ids in hosted_zone_preprocessor.rs. Maybe something needs to be tickled to tell the Smithy generator to do this for change ids, as well?

Additional Information/Context

No response

Version

0.13.0

Environment details (OS name and version, etc.)

MacOS 12.4, M1 (uname -a: Darwin dacBook2021.local 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:37 PDT 2022; root:xnu-8020.121.3~4/RELEASE_ARM64_T6000 arm64)

Logs

See above

@dacut dacut added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 14, 2022
@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label Jun 15, 2022
@Velfi
Copy link
Contributor

Velfi commented Jun 15, 2022

@dacut Thanks for submitting this issue, I'm going to see how this compares to other SDKs and the CLI.

@Velfi Velfi self-assigned this Jun 15, 2022
@dacut
Copy link
Author

dacut commented Jun 15, 2022

Hi @Velfi. It worked in the CLI for me, but I suspect that's because the CLI leverages Boto under the hood. My CLI version: aws-cli/2.7.4 Python/3.9.13 Darwin/21.5.0 source/arm64 prompt/off

@dacut
Copy link
Author

dacut commented Jun 15, 2022

It's buried in my wall of text above 😀, but this is the relevant bit from Boto3: https://github.com/boto/botocore/blob/develop/botocore/handlers.py#L621-L642

@Velfi
Copy link
Contributor

Velfi commented Jun 15, 2022

I'm working on tickling smithy to fix this and I should have a PR soon

@Velfi Velfi added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Jun 17, 2022
@rcoh rcoh closed this as completed Jun 23, 2022
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@rcoh rcoh removed the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants