Skip to content

TRANSIP: Fix TXT quoting#2708

Merged
tlimoncelli merged 7 commits intoStackExchange:masterfrom
blackshadev:fix-quoting
Dec 11, 2023
Merged

TRANSIP: Fix TXT quoting#2708
tlimoncelli merged 7 commits intoStackExchange:masterfrom
blackshadev:fix-quoting

Conversation

@blackshadev
Copy link
Copy Markdown
Collaborator

Fixes !2702

  • Fix TXT quoting, by not quoting at all. TransIP doesn't need it (any more)
  • Fix and added some auditrecord entries to let integration tests pass
    • A TransIP TXT record cannot be empty
    • TransIP trims spaces from TXT records.

@blackshadev blackshadev marked this pull request as ready for review December 10, 2023 13:17
@cafferata
Copy link
Copy Markdown
Collaborator

cafferata commented Dec 10, 2023

Looks good! I did different test cases 👍

git remote add blackshadev [email protected]:blackshadev/dnscontrol.git
git fetch blackshadev
git checkout -b fix-quoting --track blackshadev/fix-quoting 
go build
dnscontrol preview --domains jcid.nl
******************** Domain: jcid.nl
Done. 0 corrections.

@blackshadev
Copy link
Copy Markdown
Collaborator Author

git remote add blackshadev [email protected]:blackshadev/dnscontrol.git
git fetch blackshadev
git checkout -b fix-quoting --track blackshadev/fix-quoting 
go build
dnscontrol preview --domains jcid.nl
******************** Domain: jcid.nl
Done. 0 corrections.

So is this a good thing?

Did jcid.nl contain a fixed record and is the 0 corrections as expected. Or did you expect it to remove some quoted TXT record?

@cafferata
Copy link
Copy Markdown
Collaborator

Your comment and my edit crossed paths. Fix looks good to me. I had already given a GitHub approval in advance. 👍

@tlimoncelli
Copy link
Copy Markdown
Contributor

Thanks!

@tlimoncelli tlimoncelli changed the title TransIP fix TXT quoting TRANSIP: Fix TXT quoting Dec 11, 2023
@tlimoncelli tlimoncelli merged commit a7e7643 into StackExchange:master Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants