Skip to content

fix: use correct name when running dns create-or-update#1944

Closed
OhMyMndy wants to merge 3 commits intocloudflare:masterfrom
OhMyMndy:create-or-update-fix
Closed

fix: use correct name when running dns create-or-update#1944
OhMyMndy wants to merge 3 commits intocloudflare:masterfrom
OhMyMndy:create-or-update-fix

Conversation

@OhMyMndy
Copy link
Copy Markdown

@OhMyMndy OhMyMndy commented May 5, 2024

When running flarectl dns create-or-update the update fails because it uses name + '.' + zone instead of just name

Description

Change the name which is used to fetch the DNS record when it needs to be updated

Has your change been tested?

Yes, tested this with my previously failing command flarectl dns create-or-update --zone mydomain.com --name '*.mydomain.com' --content 127.0.0.1 --type A

Screenshots (if appropriate):

Types of changes

What sort of change does your code introduce/modify?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented in cloudflare/api-schemas
    and relies on stable APIs.

@OhMyMndy OhMyMndy requested a review from jacobbednarz as a code owner May 5, 2024 19:35
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2024

changelog detected ✅

Comment thread cmd/flarectl/dns.go
}

records, _, err := api.ListDNSRecords(context.Background(), cloudflare.ZoneIdentifier(zoneID), cloudflare.ListDNSRecordsParams{Name: name + "." + zone})
records, _, err := api.ListDNSRecords(context.Background(), cloudflare.ZoneIdentifier(zoneID), cloudflare.ListDNSRecordsParams{Name: name})
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 117, the record itself is create with just Name: name, not zone appended to it

@jacobbednarz
Copy link
Copy Markdown
Contributor

thanks for the PR, however, i'm hesitant to merge this as i think the intended use here is to omit the zone name from your --name value. merging this would break the (from memory) intentional use where the zone name is not included in the --name value.

the CLI is in maintenance only mode so i don't want to change the functionality here but perhaps a better in between is to strip the zone name if it is supplied as part of --name?

@OhMyMndy
Copy link
Copy Markdown
Author

OhMyMndy commented Aug 4, 2024

Omitting zone name from the name value works perfectly indeed!

@OhMyMndy OhMyMndy closed this Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants