dnsimple adapter produces invalid URL in create_record

Bug #1825049 reported by Leon Miller-Out on 2019-04-16
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
lexicon (Debian)
Fix Released
Unknown
lexicon (Ubuntu)
Undecided
Unassigned
Bionic
Undecided
Unassigned
Cosmic
Undecided
Unassigned

Bug Description

[racb: see comment 1 for SRU information; regression potential: likely to be in the DNSimpple.com API for DNS record creation; however there is good existing test coverage, including for this bug being added, which I think is sufficient for regression testing during SRU verification]

The bug fixed by the following pull request is also present in lexicon v2.2.1-2, and the fix should be backported.
https://github.com/AnalogJ/lexicon/pull/389/files?file-filters%5B%5D=.py

This causes requests to the DNSimple.com API for DNS record creation to fail because the generated URL is ill-formed.

Line 64 of is
        payload = self._post('{0}/zones/{1}/records'.format(self.account_id, self.options.get('domain')), record)
but should be
        payload = self._post('/{0}/zones/{1}/records'.format(self.account_id, self.options.get('domain')), record)

$ lsb_release -rd
Description: Ubuntu 18.04.2 LTS
Release: 18.04

$ apt-cache policy lexicon
lexicon:
  Installed: (none)
  Candidate: 2.2.1-2
  Version table:
     2.2.1-2 500
        500 http://mirror.rackspace.com/ubuntu bionic/universe amd64 Packages
        500 http://mirror.rackspace.com/ubuntu bionic/universe i386 Packages
        500 http://archive.ubuntu.com/ubuntu bionic/universe amd64 Packages
        500 http://archive.ubuntu.com/ubuntu bionic/universe i386 Packages

Adrien Ferrand (adferrand) wrote :

I am a maintainer of the Lexicon project, and want to propose a SRU for Lexicon 2.2.1 on Bionic and Lexicon 2.7.0 on Cosmic to fix that issue.

[Impact]

 * Due to an undisclosed bug in Lexicon on the DNSimple provider, the creation of new DNS records
   on DNSimple is not possible anymore since one month.
 * This bug was present in Lexicon since 2.2.0, so it impacts Bionic and Cosmic.
 * Its effect is to generate an invalid URL for DNSimple API when trying to create a new DNS record.
 * For unkown reasons, the invalid URL was still accepted by DNSimple API, until something change
   one month ago, and these URLs started to be rejected.

[Test Case]

 * To reproduce the bug one needs a valid DNSimple account, generates an authentication token,
   and calls this typical DNS record creation command:
   `lexicon dnsimple create example.com TXT --name test --value content --auth-token [TOKEN]` to
   see Lexicon raising an exception about a malformed request.
 * Without a valid DNSimple account, one can still see the wrong behavior by checking the VCRpy
   cassettes at tests/fixtures/cassettes/dnsimple/IntegrationTests: typical malformed requests
   like `https://api.sandbox.dnsimple.com/v2762/` instead of
   `https://api.sandbox.dnsimple.com/v2/762/` can be seen.
 * Fix proposed here both makes the live execution of Lexicon be successful, and make VCRpy have
   only valid URLs referenced.

[Regression Potential]

 * Besides the modifications done on the VCRpy cassettes, that are for test purposes, the fix
   modifies only one line in Lexicon code, to fix the URL in case of record creation. Upstream
   is already fixed, and correct behavior after it is confirmed at
   https://github.com/AnalogJ/lexicon/issues/387.
 * Fix is already applied to Disco.
 * So I have no possible regression in mind.

[Other Info]

 * Two separate debdiff are provided: one for Bionic, one for Cosmic. They have been generated from
   the latest respective docker OS versions.

Adrien Ferrand (adferrand) wrote :
Adrien Ferrand (adferrand) wrote :
Robie Basak (racb) wrote :

Examining the source, the fix looks present in 3.0.8-2 which is in Disco and Eoan, and debian/changelog reports it fixed in that upload also.

Changed in lexicon (Ubuntu):
status: New → Fix Released
Robie Basak (racb) wrote :

Thank you for preparing these updates, providing all the SRU information, etc!

I made the following adjustments:

Corrected the changelog version according to https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

Ran the update-maintainer tool to adjust the maintainer field according to https://wiki.ubuntu.com/DebianMaintainerField

Added a bug link to the changelog entry as required by SRU policy.

Ran "dch -r ''" to provide a proper sign-off line in debian/changelog

Tested builds locally for both Cosmic and Bionic.

I didn't add dep3 headers to the quilt patch (https://dep-team.pages.debian.net/deps/dep3/) - not strictly required, the bug link has links to everything needed, and in this case there will be no ongoing delta. However, it is useful to generally have these, as a note for next time.

The Bionic package built successfully. Unfortunately the Cosmic package fails to build. Please could you take a look? I pasted my build log to http://paste.ubuntu.com/p/VfDDrbR9qF/. It may be that the Cosmic package as-is doesn't rebuild successfully because of unrelated changes. If that is so, unfortunately it will need to be fixed before we can land the fix in Cosmic.

I'll attach my updated debdiffs. Feel free to adjust them further as needed.

Robie Basak (racb) wrote :
Robie Basak (racb) wrote :
Robie Basak (racb) wrote :

I'm curious. Are there tests missing from the Cosmic patch, or are they already present and so not required in the patch?

Changed in lexicon (Ubuntu Bionic):
status: New → Triaged
Changed in lexicon (Ubuntu Cosmic):
status: New → Triaged
Changed in lexicon (Debian):
status: Unknown → Fix Released
Adrien Ferrand (adferrand) wrote :

I fixed the issue observed in Cosmic as observed with http://paste.ubuntu.com/p/VfDDrbR9qF/

The update debdiff for Cosmic is attached after my post. The one for Bionic can stay at it is, since the package is building properly in this case.

About your question for the tests. I assume you are talking about the tests that are failing in Cosmic. In fact they were not present at the time of 2.2.1 for Bionic, they have been added after. However, the cassettes replaying the tests for the Route53 provider started to fail some months after. I did not handled this issue, and the associate PR does not provide useful insight about it. I can only assume from the logs that the requests contains an invalidation date that expired, making the boto package refuse them, and retry the requests against the real API. But without credentials, these requests fail to authenticate.

On upstream we are working on the Route53 provider to improve its capabilities. We will take care to avoid that the new cassettes expire in the future.

Adrien Ferrand (adferrand) wrote :
Robie Basak (racb) on 2019-04-30
description: updated
Robie Basak (racb) on 2019-04-30
description: updated
description: updated
Robie Basak (racb) wrote :

Uploaded to Bionic and Cosmic. I fixed my own previous mistake: s/xenial/bionic/ in debian/changelog of my Bionic debdiff from above. Apart from that I uploaded the latest debdiffs as-is.

Now awaiting SRU team review, which I should not do myself as I sponsored the upload.

Changed in lexicon (Ubuntu Bionic):
status: Triaged → In Progress
Changed in lexicon (Ubuntu Cosmic):
status: Triaged → In Progress

Hello Leon, or anyone else affected,

Accepted lexicon into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lexicon/2.7.0-1ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in lexicon (Ubuntu Cosmic):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-cosmic
Steve Langasek (vorlon) wrote :

Hello Leon, or anyone else affected,

Accepted lexicon into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lexicon/2.2.1-2ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in lexicon (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Leon Miller-Out (v-leon) on 2019-05-16
tags: added: verification-done-bionic
removed: verification-needed-bionic
Robie Basak (racb) wrote :

Leon, please could you confirm exactly what testing you performed and the exact package version number of the package that you tested? This information helps us avoid mistakes, such as anything specific that we think should be verified being missed. Otherwise we cannot make any judgement as to whether the testing that has been performed is sufficient, which is required to release the package to the updates pocket.

Yes, of course!

I installed python3-lexicon 2.2.1-2ubuntu0.1 from bionic-proposed:
$ sudo apt list python3-lexicon
Listing... Done
python3-lexicon/bionic-proposed,bionic-proposed,now 2.2.1-2ubuntu0.1 all
[installed]

I verified that the single-character fix is now included in the new version
of /usr/lib/python3/dist-packages/lexicon/providers/dnsimple.py. I also ran
the program that was leveraging the broken method and verified that it no
longer reports an error created by the malformed URL.

Thanks so much for working on this backport!

On Thu, May 16, 2019 at 10:05 AM Robie Basak <email address hidden>
wrote:

> Leon, please could you confirm exactly what testing you performed and
> the exact package version number of the package that you tested? This
> information helps us avoid mistakes, such as anything specific that we
> think should be verified being missed. Otherwise we cannot make any
> judgement as to whether the testing that has been performed is
> sufficient, which is required to release the package to the updates
> pocket.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1825049
>
> Title:
> dnsimple adapter produces invalid URL in create_record
>
> Status in lexicon package in Ubuntu:
> Fix Released
> Status in lexicon source package in Bionic:
> Fix Committed
> Status in lexicon source package in Cosmic:
> Fix Committed
> Status in lexicon package in Debian:
> Fix Released
>
> Bug description:
> [racb: see comment 1 for SRU information; regression potential: likely
> to be in the DNSimpple.com API for DNS record creation; however there
> is good existing test coverage, including for this bug being added,
> which I think is sufficient for regression testing during SRU
> verification]
>
> The bug fixed by the following pull request is also present in lexicon
> v2.2.1-2, and the fix should be backported.
> https://github.com/AnalogJ/lexicon/pull/389/files?file-filters%5B%5D=.py
>
> This causes requests to the DNSimple.com API for DNS record creation
> to fail because the generated URL is ill-formed.
>
> Line 64 of is
> payload =
> self._post('{0}/zones/{1}/records'.format(self.account_id,
> self.options.get('domain')), record)
> but should be
> payload =
> self._post('/{0}/zones/{1}/records'.format(self.account_id,
> self.options.get('domain')), record)
>
> $ lsb_release -rd
> Description: Ubuntu 18.04.2 LTS
> Release: 18.04
>
> $ apt-cache policy lexicon
> lexicon:
> Installed: (none)
> Candidate: 2.2.1-2
> Version table:
> 2.2.1-2 500
> 500 http://mirror.rackspace.com/ubuntu bionic/universe amd64
> Packages
> 500 http://mirror.rackspace.com/ubuntu bionic/universe i386
> Packages
> 500 http://archive.ubuntu.com/ubuntu bionic/universe amd64
> Packages
> 500 http://archive.ubuntu.com/ubuntu bionic/universe i386
> Packages
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/ubuntu/+source/lexicon/+bug/1825049/+subscriptions
>

Robie Basak (racb) wrote :

Great, thank you!

Would someone please also be able to test Cosmic in a similar way?

There is a minimum aging period of seven days for others to report issues. Once all the testing is done and the aging period has expired, I'll be able to release the updates.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.