Skip to content

Commit

Permalink
Merge #2141
Browse files Browse the repository at this point in the history 
2141: Remove double publication on Remote Settings (fixes #2047) r=mythmon a=leplatrem

Fixes #2047

Co-authored-by: Mathieu Leplatre <mathieu@mozilla.com>
  • Loading branch information
@bors @leplatrem
bors[bot] and leplatrem committed Mar 17, 2020
2 parents 718b6e8 + 28e2252 commit 680e222
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 498 deletions.
4 changes: 2 additions & 2 deletions docs/ops/config.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ in other Django projects.

The account password to authenticate as DJANGO_REMOTE_SETTINGS_USERNAME.

.. envvar:: DJANGO_REMOTE_SETTINGS_COLLECTION_ID
.. envvar:: DJANGO_REMOTE_SETTINGS_CAPABILITIES_COLLECTION_ID

:default: ``normandy-recipes``
:default: ``normandy-recipes-capabilities``

The name of the Remote Settings collection where the recipes will be published.

Expand Down
146 changes: 35 additions & 111 deletions normandy/recipes/exports.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def check_config(self):
return # no check if disabled.

required_keys = [
"BASELINE_COLLECTION_ID",
"CAPABILITIES_COLLECTION_ID",
"WORKSPACE_BUCKET_ID",
"PUBLISH_BUCKET_ID",
Expand All @@ -110,49 +109,41 @@ def check_config(self):
raise ImproperlyConfigured("Invalid Remote Settings credentials")

# Test that collection is writable.
bucket_collection_pairs = [
(rs_settings.WORKSPACE_BUCKET_ID, rs_settings.BASELINE_COLLECTION_ID),
(rs_settings.WORKSPACE_BUCKET_ID, rs_settings.CAPABILITIES_COLLECTION_ID),
]
for bucket, collection in bucket_collection_pairs:
metadata = self.client.get_collection(id=collection, bucket=bucket)
if server_info["user"]["id"] not in metadata["permissions"].get("write", []):
raise ImproperlyConfigured(
f"Remote Settings collection {collection} is not writable in bucket {bucket}."
)
bucket = rs_settings.WORKSPACE_BUCKET_ID
collection = rs_settings.CAPABILITIES_COLLECTION_ID
metadata = self.client.get_collection(id=collection, bucket=bucket)
if server_info["user"]["id"] not in metadata["permissions"].get("write", []):
raise ImproperlyConfigured(
f"Remote Settings collection {collection} is not writable in bucket {bucket}."
)

# Test that collection has the proper review settings.
capabilities = server_info["capabilities"]
if "signer" in capabilities:
for collection in [
rs_settings.BASELINE_COLLECTION_ID,
rs_settings.CAPABILITIES_COLLECTION_ID,
]:
signer_config = capabilities["signer"]

# Since we use the rollback feature, make sure it's available.
if signer_config["version"] < "5.1.0":
raise ImproperlyConfigured("kinto-signer 5.1.0+ is required")

normandy_resource = [
r
for r in signer_config["resources"]
if r["source"]["bucket"] == rs_settings.WORKSPACE_BUCKET_ID
and r["source"]["collection"] == collection
]
review_disabled = (
len(normandy_resource) == 1
and not normandy_resource[0].get(
"to_review_enabled", signer_config["to_review_enabled"]
)
and not normandy_resource[0].get(
"group_check_enabled", signer_config["group_check_enabled"]
)
signer_config = capabilities["signer"]

# Since we use the rollback feature, make sure it's available.
if signer_config["version"] < "5.1.0":
raise ImproperlyConfigured("kinto-signer 5.1.0+ is required")

normandy_resource = [
r
for r in signer_config["resources"]
if r["source"]["bucket"] == bucket and r["source"]["collection"] == collection
]
review_disabled = (
len(normandy_resource) == 1
and not normandy_resource[0].get(
"to_review_enabled", signer_config["to_review_enabled"]
)
and not normandy_resource[0].get(
"group_check_enabled", signer_config["group_check_enabled"]
)
)
if not review_disabled:
raise ImproperlyConfigured(
f"Review was not disabled on Remote Settings collection {collection}."
)
if not review_disabled:
raise ImproperlyConfigured(
f"Review was not disabled on Remote Settings collection {collection}."
)

def published_recipes(self):
"""
Expand All @@ -164,34 +155,7 @@ def published_recipes(self):
capabilities_records = self.client.get_records(
bucket=rs_settings.PUBLISH_BUCKET_ID, collection=rs_settings.CAPABILITIES_COLLECTION_ID
)
baseline_records = self.client.get_records(
bucket=rs_settings.PUBLISH_BUCKET_ID, collection=rs_settings.BASELINE_COLLECTION_ID
)

capabilities_ids = set(r["id"] for r in capabilities_records)
baseline_ids = set(r["id"] for r in baseline_records)

# All baseline records *should* be present in the capabilities
# collection too. If not, that's concerning.
only_baseline_ids = baseline_ids - capabilities_ids
if only_baseline_ids:
logging.warn(
f"{len(only_baseline_ids)} records were found in the baseline collection "
f"that were not in the capabilities."
)
# Merge the list of records by id.
records_by_id = {}
for record in capabilities_records + baseline_records:
if record["id"] not in records_by_id:
records_by_id[record["id"]] = record

records = records_by_id.values()
else:
# All records in baseline_records are also in capabilities_records,
# so just use the ones from capabilities
records = capabilities_records

return records
return capabilities_records

def publish(self, recipe, approve_changes=True):
"""
Expand All @@ -200,27 +164,18 @@ def publish(self, recipe, approve_changes=True):
if self.client is None:
return # no-op if disabled.

baseline = False

# 1. Put the record.
record = recipe_as_record(recipe)
self.client.update_record(
data=record,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
collection=rs_settings.CAPABILITIES_COLLECTION_ID,
)
if recipe.approved_revision.uses_only_baseline_capabilities():
baseline = True
self.client.update_record(
data=record,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
collection=rs_settings.BASELINE_COLLECTION_ID,
)

# 2. Approve the changes immediately (multi-signoff is disabled).
log_action = "Batch published"
if approve_changes:
self.approve_changes(baseline)
self.approve_changes()
log_action = "Published"

logger.info(
Expand All @@ -234,9 +189,8 @@ def unpublish(self, recipe, approve_changes=True):
if self.client is None:
return # no-op if disabled.

# 1. Delete the record from both baseline and capabilities collections
# 1. Delete the record
either_existed = False
baseline = False

try:
self.client.delete_record(
Expand All @@ -253,33 +207,17 @@ def unpublish(self, recipe, approve_changes=True):
else:
raise

try:
self.client.delete_record(
id=str(recipe.id),
bucket=rs_settings.WORKSPACE_BUCKET_ID,
collection=rs_settings.BASELINE_COLLECTION_ID,
)
either_existed = True
baseline = True
except kinto_http.KintoException as e:
if e.response.status_code == 404:
logger.warning(
f"The recipe '{recipe.id}' was not published in the baseline collection. Skip."
)
else:
raise

# 2. Approve the changes immediately (multi-signoff is disabled).
log_action = "Batch deleted"
if either_existed and approve_changes:
self.approve_changes(baseline)
self.approve_changes()
log_action = "Deleted"

logger.info(
f"{log_action} record '{recipe.id}' of recipe {recipe.approved_revision.name!r}"
)

def approve_changes(self, baseline=True):
def approve_changes(self):
"""
Approve the changes made in the workspace collection.
Expand All @@ -296,13 +234,6 @@ def approve_changes(self, baseline=True):
data=APPROVE_CHANGES_FLAG,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
)
if baseline:
self.client.patch_collection(
id=rs_settings.BASELINE_COLLECTION_ID,
data=APPROVE_CHANGES_FLAG,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
)

logger.info("Changes were approved.")

except kinto_http.exceptions.KintoException:
Expand All @@ -313,11 +244,4 @@ def approve_changes(self, baseline=True):
data=ROLLBACK_CHANGES_FLAG,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
)
if baseline:
self.client.patch_collection(
id=rs_settings.BASELINE_COLLECTION_ID,
data=ROLLBACK_CHANGES_FLAG,
bucket=rs_settings.WORKSPACE_BUCKET_ID,
)

raise
5 changes: 1 addition & 4 deletions normandy/recipes/management/commands/update_recipe_signatures.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def handle(self, *args, force=False, **options):
recipes_to_update = self.get_outdated_recipes()

count = recipes_to_update.count()
any_baseline = False
if count == 0:
self.stdout.write("No out of date recipes to sign")
else:
Expand All @@ -45,10 +44,8 @@ def handle(self, *args, force=False, **options):
recipe.update_signature()
recipe.save()
remote_settings.publish(recipe, approve_changes=False)
if recipe.approved_revision.uses_only_baseline_capabilities():
any_baseline = True
# Approve all Remote Settings changes.
remote_settings.approve_changes(baseline=any_baseline)
remote_settings.approve_changes()

metrics.gauge("signed", count, tags=["force"] if force else [])

Expand Down
Loading

0 comments on commit 680e222

Please sign in to comment.