WIP: introduce per-role task representations #2014
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgeoghegan
added
the
allow-changed-migrations
tgeoghegan
added a commit
that referenced
this pull request
Sep 28, 2023
We want to enable aggregators to only store authentication token hashes
if they don't need the actual token (e.g., a helper only receives
aggregation sub-protocol messages). With the existing
`janus_aggregator_core::task::Task` structure, this is awkward, because
that struct was used both as a view into all of a task's parameters and
secrets, useful for test code that orchestrates the provisioning and
execution of a task, as well as a specific DAP participant's view of the
task, which is useful in Janus itself.
To resolve that tension, this commit introduces distinct structures for
those distinct views. `janus_aggregator_core::task::test_util::Task`
contains all of a task's parameters and secrets, regardless of any
protocol role. `janus_aggregatore_core::task::test_util::NewTaskBuilder`
is used in test contexts to build `test_util::Task` values, and then
`test_util::Task::{leader_view, helper_view}` then allow obtaining the
leader or helper specific view of the task as a
`janus_aggregator_core::task::AggregatorTask`. This holistic view of a
task is only used in test code, and thus we confine it to the
`test_util` module.
`AggregatorTask` is an aggregator's view of the task, and uses an enum
to represent the leader, helper or taskprov-provisioned helper specific
parameters. This task representation is what gets used by non-test
configurations of Janus. We also add `SerializediAggregatorTask`, which
much like `SerializedTask` is an adapter so that `AggregatorTask` values
can be serialized to and from YAML, for use in `janus_cli`.
The goal is to migrate all of Janus from the existing
`janus_aggregator_core::task::Task` and
`janus_aggregator_core::task::test_util::TaskBuilder` onto
`task::test_util::Task`, `task::test_util::NewTaskBuilder` and
`task::AggregatorTask`. Doing this all in one go would yield a huge diff
that is difficult to review, so in order to enable incremental adoption
of the new structures throughout Janus, we have them co-exist for now.
Once all usages of `task::Task` and `task::test_util::TaskBuilder` have
been removed, those types can be deleted and `NewTaskBuilder` can be
renamed to just `TaskBuilder`.
To see where all this is going, check out #2014.
Part of #1524
tgeoghegan
added a commit
that referenced
this pull request
Sep 29, 2023
We want to enable aggregators to only store authentication token hashes
if they don't need the actual token (e.g., a helper only receives
aggregation sub-protocol messages). With the existing
`janus_aggregator_core::task::Task` structure, this is awkward, because
that struct was used both as a view into all of a task's parameters and
secrets, useful for test code that orchestrates the provisioning and
execution of a task, as well as a specific DAP participant's view of the
task, which is useful in Janus itself.
To resolve that tension, this commit introduces distinct structures for
those distinct views. `janus_aggregator_core::task::test_util::Task`
contains all of a task's parameters and secrets, regardless of any
protocol role. `janus_aggregatore_core::task::test_util::NewTaskBuilder`
is used in test contexts to build `test_util::Task` values, and then
`test_util::Task::{leader_view, helper_view}` then allow obtaining the
leader or helper specific view of the task as a
`janus_aggregator_core::task::AggregatorTask`. This holistic view of a
task is only used in test code, and thus we confine it to the
`test_util` module.
`AggregatorTask` is an aggregator's view of the task, and uses an enum
to represent the leader, helper or taskprov-provisioned helper specific
parameters. This task representation is what gets used by non-test
configurations of Janus. We also add `SerializediAggregatorTask`, which
much like `SerializedTask` is an adapter so that `AggregatorTask` values
can be serialized to and from YAML, for use in `janus_cli`.
The goal is to migrate all of Janus from the existing
`janus_aggregator_core::task::Task` and
`janus_aggregator_core::task::test_util::TaskBuilder` onto
`task::test_util::Task`, `task::test_util::NewTaskBuilder` and
`task::AggregatorTask`. Doing this all in one go would yield a huge diff
that is difficult to review, so in order to enable incremental adoption
of the new structures throughout Janus, we have them co-exist for now.
Once all usages of `task::Task` and `task::test_util::TaskBuilder` have
been removed, those types can be deleted and `NewTaskBuilder` can be
renamed to just `TaskBuilder`.
To see where all this is going, check out #2014.
Part of #1524
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes work, but next I'm going to carve them up into pieces that are easier to review.