Skip to content

deployment: add native node-config builder and wire it into the ConfigMap#14609

Open
nimrod-starkware wants to merge 1 commit into
nimrod/jsonnet/config-format-argfrom
nimrod/jsonnet/native-config-builder
Open

deployment: add native node-config builder and wire it into the ConfigMap#14609
nimrod-starkware wants to merge 1 commit into
nimrod/jsonnet/config-format-argfrom
nimrod/jsonnet/native-config-builder

Conversation

@nimrod-starkware

Copy link
Copy Markdown
Contributor

Add src/config/native.py: it assembles the nested SequencerNodeConfig for a
service by deep-merging (null-preserving) the per-layer sequencer_config.jsonnet
override files and evaluating jsonnet build('', overrides), then selecting
the service's section. ConfigMapConstruct selects this native path when
config_format == "native"; preset remains the default. Add the jsonnet (_jsonnet)
Pipfile dependency and unit tests for the deep-merge and the overlay-service-name
-> build-key mapping. The override-layer data and the parity/mirror tests that
exercise the full build land in a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

nimrod-starkware commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nimrod-starkware made 9 comments.
Reviewable status: 0 of 8 files reviewed, 9 unresolved discussions.


deployments/sequencer/src/config/native.py line 69 at r1 (raw file):

    Returns a new dict that is fully independent of `base` (deep-copied once, up front); neither
    input is mutated.
    """

Suggestion:

    """
    Recursively deep-merge `overlay` onto `base`, preserving explicit `null` values.
    """

deployments/sequencer/src/config/native.py line 81 at r1 (raw file):

    it is free to write into it. Two dicts merge key-by-key; any non-dict overlay value (including
    explicit `null`) replaces the target value outright. `overlay` is only read, never mutated.
    """

Suggestion:

    """
    Recursively merge `overlay` into `target`, MUTATING `target` in place.
    """

deployments/sequencer/src/config/native.py line 97 at r1 (raw file):

    `cp -rf` cannot clobber one with the other across repos). They are merged in sorted order; layer
    files in the same dir must have disjoint leaf keys, so the order is immaterial (see `LAYER_GLOB`).
    """

Suggestion:

    """
    Returns all *sequencer_config.jsonnet` override-layer file in `layer_dir`, in sorted order.
    """

deployments/sequencer/src/config/native.py line 153 at r1 (raw file):

    wins). Shared ancestors across multiple overlays are deduped, so the output is identical whether
    the caller passes only the leaf overlay or the leaf plus its explicit ancestors.
    """

Suggestion:

    """
    Resolves the ordered list of existing `*sequencer_config.jsonnet` layer files.
    """

deployments/sequencer/src/config/native.py line 231 at r1 (raw file):

    Matches the Rust eval form (`crates/apollo_deployments/src/jsonnet.rs`): `build` is a method on
    the imported object (no extVar / TLA). The overrides object is injected inline as a JSON literal.
    """

Suggestion:

    """
    Evaluates `(import 'lib/build.libsonnet').build(<layout>, <overrides>)` and return its JSON.
    """

deployments/sequencer/src/constructs/configmap.py line 19 at r1 (raw file):

        layout: str,
        overlays: list[str],
        config_format: str = "preset",

Suggestion:

config_format: str,

deployments/sequencer/test/test_native_config.py line 24 at r1 (raw file):

    assert merged == {"a": {"x": 1, "y": None}, "b": "keep"}
    # The null is a real value, present in the result.
    assert "y" in merged["a"] and merged["a"]["y"] is None

Suggestion:

def test_deep_merge_preserves_explicit_null():
    """A later layer setting a key to null overwrites an earlier non-null value (not skipped)."""
    base = {"a": {"x": 1, "y": 2}, "b": "keep"}
    overlay = {"a": {"y": None}}
    merged = deep_merge_preserving_null(base, overlay)
    assert merged == {"a": {"x": 1, "y": None}, "b": "keep"}
    assert base == base (not changed)
    assert overlay == overlay

deployments/sequencer/test/test_native_config.py line 47 at r1 (raw file):

    deep_merge_preserving_null(base, overlay)
    assert base == {"a": {"x": 1}}
    assert overlay == {"a": {"y": 2}}

delete


deployments/sequencer/test/test_native_config.py line 56 at r1 (raw file):

@pytest.mark.parametrize("name", ["core", "gateway", "l1", "mempool", "committer"])
def test_service_name_to_build_key_identity(name):
    assert service_name_to_build_key(name) == name

delete

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nimrod-starkware made 1 comment.
Reviewable status: 0 of 8 files reviewed, 10 unresolved discussions.


deployments/sequencer/src/config/native.py line 47 at r1 (raw file):

# Maps an overlay/deployment service name to the build key used by the jsonnet layout
# (`lib/layouts/hybrid.libsonnet` `services::`). Only `sierracompiler` differs from the build key;
# every other service name is identity.

clean these comments

Code quote:

# Repo root: this file is deployments/sequencer/src/config/native.py, root is 4 levels up.
REPO_ROOT = Path(__file__).resolve().parents[4]

# JPATH for the jsonnet evaluator, matching `crates/apollo_deployments/src/jsonnet.rs` (JSONNET_DIR).
# `import 'lib/build.libsonnet'` and build's internal relative imports resolve under this dir.
JSONNET_DIR = REPO_ROOT / "crates" / "apollo_deployments" / "jsonnet"

# Glob matching every override-layer file in an overlay layer directory. The pattern requires the
# `sequencer_config.jsonnet` suffix but allows a name prefix, so a directory may carry more than one
# layer file (e.g. the public repo's applicative `sequencer_config.jsonnet` and the devops-owned
# `common_sequencer_config.jsonnet`), authored with distinct names so the deploy's
# `cp -rf devops/overlays -> public/configs/overlays` (a file-level replace) cannot clobber one with
# the other. All matches in a dir are deep-merged in sorted order; layer files sharing a dir MUST
# have disjoint leaf keys (enforced by the cross-repo collision test) so the merge order is immaterial.
LAYER_GLOB = "*sequencer_config.jsonnet"

# Maps an overlay/deployment service name to the build key used by the jsonnet layout
# (`lib/layouts/hybrid.libsonnet` `services::`). Only `sierracompiler` differs from the build key;
# every other service name is identity.

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nimrod-starkware resolved 7 discussions.
Reviewable status: 0 of 8 files reviewed, 3 unresolved discussions (waiting on dan-starkware).

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nimrod-starkware resolved 3 discussions.
Reviewable status: 0 of 8 files reviewed, all discussions resolved (waiting on dan-starkware).

@cursor

cursor Bot commented Jun 28, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches deployment ConfigMap content for sequencer nodes when native format is enabled; preset remains default but mis-merged jsonnet could ship wrong node config without the deferred parity tests.

Overview
Adds a native ConfigMap generation path alongside the existing preset placeholder flow. When config_format == "native", ConfigMapConstruct builds nested SequencerNodeConfig JSON by deep-merging per-overlay chain_params.jsonnet / node_params.jsonnet / replacers.jsonnet along the same overlay chain as YAML, then evaluating jsonnet build(layout, params) and selecting the service section (with sierracompilersierra_compiler).

Overlay directory resolution is factored into src/config/overlays.py so YAML loading and native bucket resolution stay aligned. The jsonnet Python package is added for evaluation; CI now runs the full pytest test/ suite instead of only port-uniqueness tests.

Unit tests cover null-preserving deep-merge only; full parity / overlay jsonnet data tests are deferred to a follow-up.

Reviewed by Cursor Bugbot for commit e625740. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1a279e2. Configure here.

Comment thread deployments/sequencer/src/config/native.py Outdated
…gMap

Add src/config/native.py: it assembles the nested SequencerNodeConfig for a
service by deep-merging (null-preserving) the per-layer sequencer_config.jsonnet
override files and evaluating jsonnet build('<layout>', overrides), then selecting
the service's section. ConfigMapConstruct selects this native path when
config_format == "native"; preset remains the default. Add the jsonnet (_jsonnet)
Pipfile dependency and unit tests for the deep-merge and the overlay-service-name
-> build-key mapping. The override-layer data and the parity/mirror tests that
exercise the full build land in a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants