nebraska: Deprecate the version handling
We originally added a version handling to detect the version mismatches
in full and delta payloads. But truly that is not really necessary. We
normally generate payloads for a specific version. But sometimes the
version number is not really obvious or we can't catch it easily. It is
just as fine to ignore the version checking. IMO the version checking is
a call for headache for a few reasons:
- We can't really check always for the uprevs in version number because
we will be testing enterprise rollback feature, and there we always
use a downreved version. So there is not reason to check if an
incoming request has lower/equal version than what the payloads
metadata says.
- In some instances we can't even correctly extract the payloads'
version. For example for DLC images, since they are build at
build_package time, the version number is non-existent in the image
itself. We might be able to somehow get the version number in the
release builders, but locally that assumption doesn't really hold.
- Version numbers are quite complicated and versions build locally have
weird assignment that can be complicated to parse and compare, etc.
We have other means of protecting against bad versions and nebraska
should not be the place for that. For example, if we send the wrong DLC
payload, the dlcservice will complain later. But for its worth,
currently, the devserver does not do any version checking anyway. It
always responds with version 99999.0.0.0. Update engine also doesn't
really care about the version anymore. Besides, originally we decided to
add these version so we can serve multiple different versioned payloads
in a directory and let nebraska choose one. But that is a bad idea. Not
only it introduces the randomness that is caused by nebraska decisions,
it shouldn't be the nebraska's business making those decisions. Also, we
probably would've never have this case anyway. We always update/install
a specific version. Serving different versions in one directory doesn't
really make sense.
So deprecate anything related to
that.
BUG=chromium:920404
TEST=unittest
TEST=manually running nebraska and query it.
Change-Id: Ifbb70f8e5484abdeed1856f847068e90d3ae3ae0
Reviewed-on: https://chromium-review.googlesource.com/1592554
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
diff --git a/nebraska/appindex_unittest.py b/nebraska/appindex_unittest.py
index 8e6c2d7..400f850 100755
--- a/nebraska/appindex_unittest.py
+++ b/nebraska/appindex_unittest.py
@@ -279,34 +279,6 @@
version="1.2.0")
self.assertFalse(app_data.MatchRequest(request))
- def testMatchRequestVersionMismatch(self):
- """Tests MatchRequest for install version mismatch."""
-
- app_data = AppDataGenerator(
- appid="foo",
- is_delta=False,
- target_version="2.2.0",
- source_version=None)
- request = nebraska.Request.AppRequest(
- request_type=nebraska.Request.AppRequest.RequestType.INSTALL,
- appid="foo",
- version="1.2.0")
- self.assertFalse(app_data.MatchRequest(request))
-
- def testMatchRequestDeltaVersionMismatch(self):
- """Test MatchRequest for delta version mismatch."""
- app_data = AppDataGenerator(
- appid="foo",
- is_delta=False,
- target_version="1.2.0",
- source_version="1.0.0")
- request = nebraska.Request.AppRequest(
- request_type=nebraska.Request.AppRequest.RequestType.UPDATE,
- appid="foo",
- version="1.2.0",
- delta_okay=True)
- self.assertFalse(app_data.MatchRequest(request))
-
def testMatchRequestDeltaMismatch(self):
"""Tests MatchRequest for delta mismatch."""
app_data = AppDataGenerator(
@@ -332,50 +304,5 @@
version="1.3.0")
self.assertFalse(app_data.MatchRequest(request))
- def testMatchRequestSourceMismatch(self):
- """Tests MatchRequest for delta source mismatch."""
- app_data = AppDataGenerator(
- appid="foo",
- is_delta=True,
- target_version="2.3.0",
- source_version="2.2.0")
- request = nebraska.Request.AppRequest(
- request_type=nebraska.Request.AppRequest.RequestType.UPDATE,
- appid="foo",
- version="1.2.0",
- delta_okay=True)
- self.assertFalse(app_data.MatchRequest(request))
-
- def testMatchRequestInvalidVersion(self):
- """Tests MatchRequest on invalid version string."""
- with mock.patch('nebraska.logging') as mock_log:
- app_data = AppDataGenerator(
- appid="foo",
- is_delta=True,
- target_version="2.3.0",
- source_version="2.2.0")
- request = nebraska.Request.AppRequest(
- request_type=nebraska.Request.AppRequest.RequestType.UPDATE,
- appid="foo",
- version="0xc0ffee",
- delta_okay=True)
- self.assertFalse(app_data.MatchRequest(request))
- mock_log.error.assert_called_once()
-
- with mock.patch('nebraska.logging') as mock_log:
- app_data = AppDataGenerator(
- appid="foo",
- is_delta=True,
- target_version="2,3,0",
- source_version="2.2.0")
- request = nebraska.Request.AppRequest(
- request_type=nebraska.Request.AppRequest.RequestType.UPDATE,
- appid="foo",
- version="2.2.0",
- delta_okay=True)
- self.assertFalse(app_data.MatchRequest(request))
- mock_log.error.assert_called_once()
-
-
if __name__ == '__main__':
unittest.main()
diff --git a/nebraska/nebraska.py b/nebraska/nebraska.py
index dbfba21..657bd82 100755
--- a/nebraska/nebraska.py
+++ b/nebraska/nebraska.py
@@ -25,37 +25,6 @@
from xml.etree import ElementTree
-def VersionCmp(version_a_str, version_b_str):
- """Compare two version strings.
-
- Currently we only match on major/minor versions.
-
- Args:
- version_a_str: String representing first version number.
- version_b_str: String representing second version number.
-
- Returns:
- < 0 if version_a is less than version_b.
- > 0 if version_a is greater than version_b.
- 0 if the version numbers are equal.
-
- Raises:
- ValueError if either version string is not valid.
- """
-
- try:
- version_a = tuple([int(i) for i in version_a_str.split('.')[0:2]])
- version_b = tuple([int(i) for i in version_b_str.split('.')[0:2]])
-
- if version_a[0] != version_b[0]:
- return version_a[0] - version_b[0]
-
- return version_a[1] - version_b[1]
-
- except (IndexError, ValueError):
- raise ValueError("Not a valid version string")
-
-
class Request(object):
"""Request consisting of a list of apps to update/install."""
@@ -491,9 +460,8 @@
"""Returns true iff the app matches a given client request.
An app matches a request if the appid matches the requested appid.
- Additionally, if the app describes a delta update payload, the request
- must be able to accept delta payloads, and the source versions must match.
- If the request is not an update, the versions must match.
+ Additionally, if the app describes a delta update payload, the request must
+ be able to accept delta payloads.
Args:
request: A request object describing a client request.
@@ -501,33 +469,19 @@
Returns:
True if the app matches the given request, False otherwise.
"""
- # TODO(http://crbug.com/914939): We only account for tip/branch versions. We
- # need to be able to handle full version strings as well as developer builds
- # that don't have a "real" final version component.
-
if self.appid != request.appid:
return False
- try:
- if request.request_type == request.RequestType.UPDATE:
- if self.is_delta:
- if not request.delta_okay:
- return False
- if VersionCmp(request.version, self.source_version) != 0:
- return False
- return VersionCmp(request.version, self.target_version) < 0
-
- if request.request_type == request.RequestType.INSTALL:
- if self.is_delta:
- return False
- return VersionCmp(request.version, self.target_version) == 0
-
+ if request.request_type == request.RequestType.UPDATE:
+ if self.is_delta:
+ return request.delta_okay
else:
- return False
+ return True
- except ValueError as err:
- logging.error("Unable to compare version strings (%s)", str(err))
- return False
+ if request.request_type == request.RequestType.INSTALL:
+ return not self.is_delta
+
+ return False
class AppIndex(object):
@@ -535,9 +489,9 @@
Index of available apps used to generate responses to Omaha requests. The
index consists of lists of payload information associated with a given appid,
- since we can have multiple payloads for a given app (different versions,
- delta/full payloads). The index is built by scanning a given directory for
- json files that describe the available payloads.
+ since we can have multiple payloads for a given app (delta/full payloads). The
+ index is built by scanning a given directory for json files that describe the
+ available payloads.
"""
def __init__(self, directory):
@@ -580,8 +534,8 @@
"""Search the index for a given appid.
Searches the index for the payloads matching a client request. Matching is
- based on appid, target_version, and whether the client is searching for an
- update and can handle delta payloads.
+ based on appid, and whether the client is searching for an update and can
+ handle delta payloads.
Args:
request: AppRequest describing the client request.
@@ -598,13 +552,6 @@
if not matches:
return None
- # Find the highest version out of the matching payloads.
- max_version = reduce(
- lambda a, b: a if VersionCmp(a.target_version, b.target_version) > 0
- else b, matches).target_version
-
- matches = [app for app in matches if app.target_version == max_version]
-
# If the client can handle a delta, prefer to send a delta.
if request.delta_okay:
match = next((x for x in matches if x.is_delta), None)
