Skip to content

pull: complete detached meta fetch before scanning#873

Closed
jlebon wants to merge 2 commits intoostreedev:masterfrom
jlebon:pr/meta-before-scan
Closed

pull: complete detached meta fetch before scanning#873
jlebon wants to merge 2 commits intoostreedev:masterfrom
jlebon:pr/meta-before-scan

Conversation

@jlebon
Copy link
Member

@jlebon jlebon commented May 17, 2017

If somehow a repo has gpg verification on but doesn't have signatures
present for the existing commit, ostree would error out if it needs to
scan the commit object (e.g. if there are no updates available).

An instance of this is currently happening in Fedora AH, in which
signatures are not shipped in the ISO due to filesystem restrictions.
Another possible scenario is if a content provider switches from not
signing commits to signing them; even if older commits are retroactively
signed, clients' local commit objects would error out if they needed
scanning.

This patch adds a check to ensure that we always attempt to fetch the
detached metadata and wait for its result (whether it exists or not)
before moving on to scan their corresponding commit objects.

See also: coreos/rpm-ostree#630

jlebon added 2 commits May 17, 2017 14:13
If somehow a repo has gpg verification on but doesn't have signatures
present for the existing commit, ostree would error out if it needs to
scan the commit object (e.g. if there are no updates available).

An instance of this is currently happening in Fedora AH, in which
signatures are not shipped in the ISO due to filesystem restrictions.
Another possible scenario is if a content provider switches from not
signing commits to signing them; even if older commits are retroactively
signed, clients' local commit objects would error out if they needed
scanning.

This patch adds a check to ensure that we always attempt to fetch the
detached metadata and wait for its result (whether it exists or not)
before moving on to scan their corresponding commit objects.

See also: coreos/rpm-ostree#630
@cgwalters
Copy link
Member

This looks sane to me - let's do a release first though? I'll update #865

@cgwalters
Copy link
Member

(If you feel strongly though I could be convinced to review carefully, land this and wait a day or two for soak, then release)

@jlebon
Copy link
Member Author

jlebon commented May 17, 2017

Nah, I think when it comes to the pull code 🐙, it's better to play it extra safe.

@cgwalters
Copy link
Member

Nice!

@rh-atomic-bot r+ 0f569e3

@rh-atomic-bot
Copy link

⌛ Testing commit 0f569e3 with merge a8fd37b...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing a8fd37b to master...

@dustymabe
Copy link
Contributor

can we get a release out with this in it?

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.

4 participants