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

change-resource-record-sets uses only record name and type as unique key but requires full details when deleting #8241

Closed
sdrabblescripta opened this issue Oct 16, 2023 · 11 comments
Assignees
Labels
bug This issue is a bug. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. route53

Comments

@sdrabblescripta
Copy link

sdrabblescripta commented Oct 16, 2023

Describe the bug

I attempt to UPSERT a record to change where it points:

# r53.json
{
    "Comment": "An example change",
    "Changes": [
        {
            "Action": "UPSERT",
            "ResourceRecordSet": {
                "Name": "foo.bar.com",
                "Type": "A",
                "Region": "us-east-1",
                "SetIdentifier": "foo.bar.com",
                "AliasTarget": {   # this is the block I am trying to change
                    "HostedZoneId": "ABC",
                    "DNSName": "XYZ",
                    "EvaluateTargetHealth": false
                }
            }
        }
    ]
}

AWS complains:

$ aws route53 change-resource-record-sets --hosted-zone-id=XYZ --change-batch file://r53.json
An error occurred (InvalidChangeBatch) when calling the ChangeResourceRecordSets operation: [RRSet with DNS name foo.bar.com., type A, SetIdentifier foo.bar.com, and Region Name=us-east-1 cannot be created because a non-latency RRSet with the same name and type already exists.]

Fair enough, it uses name + type to identify the record... except:

#r53-delete.json
{
    "Comment": "An example change",
    "Changes": [
        {
            "Action": "DELETE",
            "ResourceRecordSet": {
                "Name": "foo.bar.com",
                "Type": "A"
            }
        }
    ]
}
$ aws route53 change-resource-record-sets --hosted-zone-id=XYZ --change-batch file://r53-delete.json
An error occurred (InvalidInput) when calling the ChangeResourceRecordSets operation: Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords], or TrafficPolicyInstanceId], but found none in Change with [Action=DELETE, Name=foo.bar.com, Type=A, SetIdentifier=null]

Having to look up the existing values just to stuff them into the DELETE change is a waste of time and other resources. Either (name+type) refers to a unique record, or it doesn't, and in the latter case I should therefore not have to provide the other specific record details when deleting. In the former case, an UPSERT should cause the unique record to be updated, as per the docs:

| UPSERT : If a resource set doesn’t exist, Route 53 creates it. If a resource set exists Route 53 updates it with the values in the request.

Expected Behavior

I expect one of the following:

  1. I can DELETE a record by name+type alone, or
  2. I can UPSERT a record by name+type alone (preferred)

Current Behavior

See above.

Reproduction Steps

See above.

Possible Solution

Allow me to UPSERT a record using name+type as the key.

Additional Information/Context

No response

CLI version used

aws-cli/2.13.26 Python/3.11.6 Linux/5.15.49-linuxkit exe/aarch64.debian.11 prompt/off

Environment details (OS name and version, etc.)

Linux f6a3cbd6774c 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 GNU/Linux

@sdrabblescripta sdrabblescripta added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 16, 2023
@RyanFitzSimmonsAK RyanFitzSimmonsAK self-assigned this Oct 16, 2023
@RyanFitzSimmonsAK RyanFitzSimmonsAK added investigating This issue is being investigated and/or work is in progress to resolve the issue. route53 p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Oct 16, 2023
@RyanFitzSimmonsAK
Copy link
Contributor

Hi @sdrabblescripta, thanks for reaching out. Could you clarify the inconsistency here? Right now, it looks like both UPSERT and DELETE operations require the same level of detail to be successful. They both return An error occurred (InvalidInput) when calling the ChangeResourceRecordSets operation: Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords]... if that level of detail is not met.

@RyanFitzSimmonsAK RyanFitzSimmonsAK added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 17, 2023
@sdrabblescripta
Copy link
Author

Hi @RyanFitzSimmonsAK thanks for responding, but that's exactly my issue. The error from UPSERTing a record (changing the target and other params) is:

a non-latency RRSet with the same name and type already exists.

implying only the name and type are used for the key, however since both delete and the update side of an upsert fail unless the existing parameters are provided kinda puts the lie to that statement. Plus, if i can't actually update a record (change target e.g.) what's the point of an UPSERT? it should really just be an INSERT.

@RyanFitzSimmonsAK
Copy link
Contributor

Using the sample code that you provided, I was able to change the target alias of a latency-based record. What is the routing policy of your alias record? Also, could you provide debug logs? You can get debug logs by adding --debug to the command, and redacting any sensitive information.

@sdrabblescripta
Copy link
Author

sdrabblescripta commented Oct 18, 2023

The RoutingPolicy field doesn't appear settable via the CLI or boto3 but it seems to end up as Latency. I'm working on getting the debug logs but in the meantime.. should I be able to delete a record by name+type ONLY, as implied by these statements:

| a non-latency RRSet with the same name and type already exists.

(or this, from a DELETE op):

| Tried to delete resource record set [name='foo.bar.com.', type='A'] but it was not found

?

@RyanFitzSimmonsAK
Copy link
Contributor

should I be able to delete a record by name+type ONLY, as implied by these statements

No, I don't think so. The documentation for ChangeResourceRecordSets even specifies "To delete a resource record set, you must specify all the same values that you specified when you created it."

Those statements are suggesting that you're trying to create a latency record with the same name and type as an existing non-latency record. I'd double check that the record you are trying to modify is actually a latency record, and that all the other details are correct. Do you have any other records with the same name?

@sdrabblescripta
Copy link
Author

Thanks again. Maybe I'm just too dumb to understand but if I have to provide all details when deleting how can an upsert work? What exactly is being updated if the key is "all fields" ?

As far as the update I'm attempting goes, there's exactly one record matching (name, type). If I change the type as part of the upsert, there will be no records matching and so this should be considered an insert, no?

@RyanFitzSimmonsAK
Copy link
Contributor

Name + type is sufficient to identify a unique record, but you still need to provide other details because they would be required in the event the record does not exist and needs to be created. For example, if you wanted to just update the TTL of a simple routing record, you still need to provide ResourceRecords because it's needed for creating a record.

As far as the update I'm attempting goes, there's exactly one record matching (name, type). If I change the type as part of the upsert, there will be no records matching and so this should be considered an insert, no?

That is correct, as name + type identifies a unique record. Just make sure you're providing the correct parameters for whatever type of record you want to create.

@RyanFitzSimmonsAK RyanFitzSimmonsAK removed the bug This issue is a bug. label Nov 2, 2023
@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 2, 2023
@sdrabblescripta
Copy link
Author

| Name + type is sufficient to identify a unique record

Except when deleting, when I have to provide all details of the existing record.

This is my issue. The behaviour is definitely not "name + type identifies a unique record" when deleting. If it were, I wouldn't have to provide the entire current record, which requires a separate request to retrieve that same record just to delete it.

| if you wanted to just update the TTL of a simple routing record, you still need to provide ResourceRecords because it's needed for creating a record.

But in the event the record doesn't need to be created, you seem to be saying "(name + type) + TTL" is enough to update the existing record.. but this is not the case either.

TL;DR: there is no situation I have found where I can provide just the name + type for deleting, or the name + type + modified fields for upserting an existing (name + type) record.

@RyanFitzSimmonsAK RyanFitzSimmonsAK added the bug This issue is a bug. label Nov 2, 2023
@RyanFitzSimmonsAK
Copy link
Contributor

Name + type does identify a unique record, because Route53 does not allow you to have multiple records of the same name and type. However, the API is designed such that deleting a record requires more than that minimum amount of information. Considering that it is specifically called out in the documentation, I'd suspect it's a very purposeful decision.

In the event that the record needs to be updated (not created), it still needs both TTL and ResourceRecords, because that is the minimum amount of information required to create a simple routing record.

@RyanFitzSimmonsAK RyanFitzSimmonsAK added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 2, 2023
@sdrabblescripta
Copy link
Author

Thanks for your response. Given that there's already a CREATE command - where I would expect to pass ALL details of the new record - there seems to be no use for an UPSERT since it expects the same values, meaning a change is always treated as a create until the last possible moment. I found a different path to achieve my goals (DELETE + CREATE) however so this is not such a big issue for me at this time.

As to:

| Considering that it is specifically called out in the documentation, I'd suspect it's a very purposeful decision

Purposeful or not, the behaviour is inconsistent with declaring a unique key as the fields (name + type). Requiring ALL details be provided to delete an existing record is akin to saying (in python, or any other similar language) deleting a key from a dict requires the value be passed as well, which is.. uncommon to say the least. The doc states "name + key identifies a record uniquely" and then does not clearly indicate that the API flies in the face of this by requiring additional fields to uniquely identify a record for deletion; I believe that should be called out much more strongly.

That said, I think we're reached the point where there is no further value to be gained from continuing this discussion. While I think the docs could be clearer (or the API actually changed to make sense :) ) I truly value your input and am grateful for your perspective.

Copy link

github-actions bot commented Nov 3, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please 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.

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. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. route53
Projects
None yet
Development

No branches or pull requests

2 participants