Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add scriptworker signing transforms #7

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions src/mozilla_taskgraph/transforms/scriptworker/signing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

import re

from taskgraph.transforms.base import TransformSequence
from taskgraph.util.schema import (
Schema,
optionally_keyed_by,
resolve_keyed_by,
)
from voluptuous import ALLOW_EXTRA, Any, Optional, Required

SIGNING_FORMATS = ["autograph_gpg"]
SIGNING_TYPES = ["dep", "release"]
DETACHED_SIGNATURE_EXTENSION = ".asc"

signing_schema = Schema(
{
Required("attributes"): {
Optional("artifacts"): dict,
Required("build-type"): str,
},
Required("signing"): optionally_keyed_by(
"build-type",
"level",
{
Required("format"): optionally_keyed_by(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we sometimes sign with multiple formats at once. Do you want to handle this now, or is that going to come later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that will come later. For now it's easier for me if I just focus on a single project at a time (firefox-android). But thanks for the heads up that this will be needed at some point.

"build-type", "level", Any(*SIGNING_FORMATS)
),
Optional("type"): optionally_keyed_by(
"build-type", "level", Any(*SIGNING_TYPES)
),
Optional("ignore-artifacts"): list,
},
),
Required("worker"): {
Required("upstream-artifacts"): [
{
# Paths to the artifacts to sign
Required("paths"): [str],
}
],
},
},
extra=ALLOW_EXTRA,
)

transforms = TransformSequence()
transforms.add_validate(signing_schema)


@transforms.add
def resolve_signing_keys(config, tasks):
for task in tasks:
for key in (
"signing",
"signing.format",
"signing.type",
):
resolve_keyed_by(
task,
key,
item_name=task["name"],
**{
"build-type": task["attributes"]["build-type"],
"level": config.params["level"],
},
)
yield task


@transforms.add
def set_signing_attributes(_, tasks):
for task in tasks:
task["attributes"]["signed"] = True
yield task


@transforms.add
def set_signing_format(_, tasks):
for task in tasks:
for upstream_artifact in task["worker"]["upstream-artifacts"]:
upstream_artifact["formats"] = [task["signing"]["format"]]
yield task


@transforms.add
def set_signing_and_worker_type(config, tasks):
for task in tasks:
signing_type = task["signing"].get("type")
if not signing_type:
signing_type = "release" if config.params["level"] == "3" else "dep"

task.setdefault("worker", {})["signing-type"] = f"{signing_type}-signing"

if "worker-type" not in task:
worker_type = "signing"
build_type = task["attributes"]["build-type"]

if signing_type == "dep":
worker_type = f"dep-{worker_type}"
if build_type == "macos":
worker_type = f"{build_type}-{worker_type}"
task["worker-type"] = worker_type

yield task


@transforms.add
def filter_out_ignored_artifacts(_, tasks):
for task in tasks:
ignore = task["signing"].get("ignore-artifacts")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we have to ignore specific things (as opposed to having the upstream tasks explicitly define what needs signing)? It's easier for debugging and other inspection to have the things we're actually going to be signing as the explicit part IMO. Not a blocker (and I'm guessing there's some reason it has to be done this way...), just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yes this is a good point.. I did it this way simply because the transforms in firefox-android use a list of extensions to ignore and adjusting the logic from that to a regex to ignore was trivial.. However, you are absolutely correct that being explicit about what we are signing seems much better than implicitly signing everything! So I will make this a blocker!

Do you think we should always enforce being explicit about which artifacts are signed? Or should it be an optional key and default to signing all artifacts? I'm leaning towards the former.

Maybe we could allow either a list of upstream artifacts to sign, or a regex whereupon any upstream artifact that matches will be signed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the most important part is that it should be an allowlist instead of a denylist. With that in mind, I think an explicit list of names, a regex of things to sign, or even a "sign all artifacts" mode are all fine.

So - what you're proposing sounds great!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, that's helpful. I think instead of rushing this PR into the module, I'm going to try and go straight to doing things properly by fixing parts of taskcluster/taskgraph#227 first.

Because if we're going to need the artifact names up front, then we'll require multi_dep-like logic to resolve them. Initially I was going to move multi_dep (or similar) over later, but now I'm seeing that we should do it first.. So this PR is going to be on hold for a little bit while I sort that out over in the Taskgraph repo..

if not ignore:
yield task
continue

def is_ignored(artifact):
return not any(re.search(i, artifact) for i in ignore)

if task["attributes"].get("artifacts"):
task["attributes"]["artifacts"] = {
extension: path
for extension, path in task["attributes"]["artifacts"].items()
if is_ignored(path)
}

for upstream_artifact in task["worker"]["upstream-artifacts"]:
upstream_artifact["paths"] = [
path for path in upstream_artifact["paths"] if is_ignored(path)
]

yield task


@transforms.add
def set_gpg_detached_signature_artifacts(_, tasks):
for task in tasks:
if task["signing"]["format"] != "autograph_gpg":
yield task
continue

task["attributes"]["artifacts"] = {
extension
+ DETACHED_SIGNATURE_EXTENSION: path
+ DETACHED_SIGNATURE_EXTENSION
for extension, path in task["attributes"]["artifacts"].items()
}

yield task


@transforms.add
def remove_signing_config(_, tasks):
for task in tasks:
del task["signing"]
yield task
28 changes: 28 additions & 0 deletions taskcluster/test/params/main-push.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
base_ref: refs/heads/main
base_repository: https://github.com/mozilla-releng/mozilla-taskgraph
base_rev: a76ea4308313211a99e8e501c5a97a5ce2c08cc1
build_date: 1681151087
build_number: 1
do_not_optimize: []
enable_always_target: true
existing_tasks: {}
filters:
- target_tasks_method
head_ref: refs/heads/main
head_repository: https://github.com/mozilla-releng/mozilla-taskgraph
head_rev: a0785edae4a841b6119925280c744000f59b903e
head_tag: ''
level: '1'
moz_build_date: '20230410182447'
next_version: null
optimize_strategies: null
optimize_target_tasks: true
owner: [email protected]
project: mozilla-taskgraph
pushdate: 0
pushlog_id: '0'
repository_type: git
target_tasks_method: default
tasks_for: github-push
version: null
28 changes: 28 additions & 0 deletions taskcluster/test/params/pull-request.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
base_ref: main
base_repository: https://github.com/mozilla-releng/mozilla-taskgraph
base_rev: a0785edae4a841b6119925280c744000f59b903e
build_date: 1681154438
build_number: 1
do_not_optimize: []
enable_always_target: true
existing_tasks: {}
filters:
- target_tasks_method
head_ref: codecov
head_repository: https://github.com/user/mozilla-taskgraph
head_rev: 06c766e8e9d558eed5ccf8029164120a27af5fb1
head_tag: ''
level: '1'
moz_build_date: '20230410192038'
next_version: null
optimize_strategies: null
optimize_target_tasks: true
owner: [email protected]
project: mozilla-taskgraph
pushdate: 0
pushlog_id: '0'
repository_type: git
target_tasks_method: default
tasks_for: github-pull-request
version: null
118 changes: 118 additions & 0 deletions test/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
from pathlib import Path

import pytest
from taskgraph.config import GraphConfig
from taskgraph.transforms.base import TransformConfig

here = Path(__file__).parent


@pytest.fixture(scope="session")
def datadir():
return here / "data"


def fake_load_graph_config(root_dir):
graph_config = GraphConfig(
{
"trust-domain": "test-domain",
"taskgraph": {
"repositories": {
"ci": {"name": "Taskgraph"},
}
},
"workers": {
"aliases": {
"b-linux": {
"provisioner": "taskgraph-b",
"implementation": "docker-worker",
"os": "linux",
"worker-type": "linux",
},
"t-linux": {
"provisioner": "taskgraph-t",
"implementation": "docker-worker",
"os": "linux",
"worker-type": "linux",
},
}
},
"task-priority": "low",
"treeherder": {"group-names": {"T": "tests"}},
},
root_dir,
)
graph_config.__dict__["register"] = lambda: None
return graph_config


@pytest.fixture
def graph_config(datadir):
return fake_load_graph_config(str(datadir / "taskcluster" / "ci"))


class FakeParameters(dict):
strict = True

def is_try(self):
return False

def file_url(self, path, pretty=False):
return path


@pytest.fixture
def parameters():
return FakeParameters(
{
"base_repository": "http://hg.example.com",
"build_date": 0,
"build_number": 1,
"enable_always_target": True,
"head_repository": "http://hg.example.com",
"head_rev": "abcdef",
"head_ref": "default",
"level": "1",
"moz_build_date": 0,
"next_version": "1.0.1",
"owner": "some-owner",
"project": "some-project",
"pushlog_id": 1,
"repository_type": "hg",
"target_tasks_method": "test_method",
"tasks_for": "hg-push",
"try_mode": None,
"version": "1.0.0",
}
)


@pytest.fixture
def make_transform_config(parameters, graph_config):
def inner(kind_config=None, kind_dependencies_tasks=None):
kind_config = kind_config or {}
kind_dependencies_tasks = kind_dependencies_tasks or {}
return TransformConfig(
"test",
str(here),
kind_config,
parameters,
kind_dependencies_tasks,
graph_config,
write_artifacts=False,
)

return inner


@pytest.fixture
def run_transform(make_transform_config):
def inner(func, tasks, config=None):
if not isinstance(tasks, list):
tasks = [tasks]

if not config:
config = make_transform_config()
return list(func(config, tasks))

return inner
Loading