Skip to content

Commit 5d036d4

Browse files
Fix(config): Allow duplicate keys in replication.config
Pass strict=False to configparser.ConfigParser() in get_config() so that Gerrit's git-style multi-valued options parse cleanly. Gerrit's pull-replication plugin writes one fetch refspec per line under the [remote "..."] section, e.g. [remote "onap"] url = https://gerrit.onap.org/r/a/${name}.git fetch = +refs/heads/*:refs/heads/* fetch = +refs/tags/*:refs/tags/* fetch = +refs/changes/*:refs/changes/* This is valid git config syntax for representing a list of values for the same key. Python's configparser defaults to strict=True and refuses repeated keys, raising DuplicateOptionError on the second 'fetch =' line. A diagnostic capture from a CI deployment showed every hook firing crash with that exception the moment find_and_dispatch() called get_replication_remotes() -> get_config(REPLICATION), dropping every patchset-created, comment-added and change-merged event before any workflow dispatch could run. g2p only reads the single-valued url, authgroup and remotenamestyle keys from each remote, never the multi-valued fetch key, so the 'last value wins' semantics that strict=False introduces for duplicate keys have no functional impact on g2p. Adds: * tests/fixtures/replication_multi_fetch.config covering the pull-replication shape that triggered the field crash; * test_get_config_tolerates_duplicate_keys asserting the parser loads the fixture without raising; * test_get_replication_remotes_tolerates_duplicate_keys asserting the higher-level helper still produces the expected remotes mapping for that fixture. Co-authored-by: Claude <claude@anthropic.com> Change-Id: I692e6e15c5253d8d7493ad688ac9247e38af6b4d Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
1 parent 6400566 commit 5d036d4

4 files changed

Lines changed: 127 additions & 1 deletion

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
fixes:
3+
- |
4+
Fix ``configparser.DuplicateOptionError`` raised from every hook
5+
invocation when ``replication.config`` contains multi-valued
6+
``fetch = ...`` refspecs under a single ``[remote "..."]`` section.
7+
This is the canonical shape Gerrit's pull-replication plugin
8+
writes (and is valid git-config syntax for representing a list),
9+
but Python's ``configparser`` defaults to ``strict=True`` and
10+
refuses to parse repeated keys. ``get_config()`` now constructs
11+
the parser with ``strict=False``, which allows the multi-valued
12+
keys through with "last value wins" semantics. ``g2p`` only
13+
reads the single-valued ``url``, ``authgroup`` and
14+
``remotenamestyle`` keys from the remote sections, never the
15+
multi-valued ``fetch`` key, so the relaxed semantics have no
16+
functional impact.
17+
upgrade:
18+
- |
19+
No operator action required. Existing deployments that did not
20+
trigger this crash (because their ``replication.config`` happened
21+
to keep ``fetch`` to a single line per remote) see no behaviour
22+
change. Deployments that did crash after every hook event —
23+
notably any pull-replication setup mirrored from a Gerrit source
24+
— will start dispatching workflows successfully after this
25+
release.

src/gerrit_to_platform/config.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,23 @@ def get_config(config_type: str = DEFAULT_CONFIG) -> ConfigParser:
7474
Returns:
7575
ConfigParser: A loaded ConfigParser object with the requested
7676
configuration file
77+
78+
Notes:
79+
``replication.config`` follows git's config-file syntax, which
80+
permits the same option key to repeat inside a section (the
81+
canonical example is multiple ``fetch = ...`` refspecs under
82+
one ``[remote "..."]``). Python's ``configparser`` raises
83+
``DuplicateOptionError`` for that pattern unless ``strict`` is
84+
false, so we disable the strict check here. ``g2p`` only
85+
reads ``url``, ``authgroup`` and ``remotenamestyle`` from the
86+
remote sections, never the multi-valued ``fetch`` key, so the
87+
"last value wins" semantics that ``strict=False`` introduces
88+
for duplicates have no functional impact. The same parser is
89+
used for ``gerrit_to_platform.ini`` for consistency; that
90+
file is operator-controlled and does not contain duplicate
91+
keys in practice.
7792
"""
78-
config = configparser.ConfigParser()
93+
config = configparser.ConfigParser(strict=False)
7994
conf_file = CONFIG_FILES[config_type]
8095
with open(conf_file) as config_file:
8196
config.read_file(config_file, conf_file)
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Pull-replication configuration with multi-valued fetch refspecs.
2+
# Mirrors the shape Gerrit's pull-replication plugin emits in the
3+
# field; configparser raises DuplicateOptionError on this without
4+
# strict=False.
5+
6+
[gerrit]
7+
replicateOnStartup = true
8+
autoReload = true
9+
10+
[replication]
11+
lockErrorMaxRetries = 5
12+
maxRetries = 5
13+
useCGitClient = false
14+
refsBatchSize = 50
15+
16+
[remote "onap"]
17+
url = https://gerrit.onap.org/r/a/${name}.git
18+
fetchEvery = 60s
19+
timeout = 600
20+
connectionTimeout = 600000
21+
replicationDelay = 0
22+
replicationRetry = 60
23+
threads = 4
24+
createMissingRepositories = true
25+
replicateHiddenProjects = false
26+
fetch = +refs/heads/*:refs/heads/*
27+
fetch = +refs/tags/*:refs/tags/*
28+
fetch = +refs/changes/*:refs/changes/*
29+
30+
[remote "github-g2p"]
31+
url = git@github.com:modeseven-gerrit-onap/${name}.git
32+
authGroup = GitHub Replication
33+
remoteNameStyle = dash

tests/test_config.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232

3333
TEST_CONFIG = os.path.join(FIXTURE_DIR, "testconfig.ini")
3434
REPLICATION_CONFIG = os.path.join(FIXTURE_DIR, "replication.config")
35+
REPLICATION_MULTI_FETCH_CONFIG = os.path.join(
36+
FIXTURE_DIR, "replication_multi_fetch.config"
37+
)
3538

3639
MOCK_CONFIG_FILES = {
3740
CONFIG: TEST_CONFIG,
@@ -50,6 +53,56 @@ def test_get_config(mocker):
5053
assert get_config(REPLICATION).has_section('remote "github"')
5154

5255

56+
def test_get_config_tolerates_duplicate_keys(mocker):
57+
"""Multi-valued ``fetch`` refspecs in replication.config must parse.
58+
59+
Gerrit's pull-replication plugin writes multiple ``fetch = ...``
60+
lines under a single ``[remote "..."]`` section (valid git config
61+
syntax for representing a list). Python's ``configparser`` defaults
62+
to ``strict=True`` and raises ``DuplicateOptionError`` for that
63+
pattern, which previously crashed every g2p hook the moment
64+
``find_and_dispatch`` called ``get_replication_remotes()``. This
65+
test loads such a config and asserts it parses without raising.
66+
"""
67+
mocker.patch.object(
68+
gerrit_to_platform.config,
69+
"CONFIG_FILES",
70+
{
71+
CONFIG: TEST_CONFIG,
72+
REPLICATION: REPLICATION_MULTI_FETCH_CONFIG,
73+
},
74+
)
75+
config = get_config(REPLICATION)
76+
assert config.has_section('remote "onap"')
77+
# ``strict=False`` keeps the last value when keys repeat; verify
78+
# the URL field (which is single-valued) survives intact, and the
79+
# multi-valued ``fetch`` field at least returns a string rather
80+
# than raising.
81+
assert config.get('remote "onap"', "url") == (
82+
"https://gerrit.onap.org/r/a/${name}.git"
83+
)
84+
assert config.has_option('remote "onap"', "fetch")
85+
86+
87+
def test_get_replication_remotes_tolerates_duplicate_keys(mocker):
88+
"""``get_replication_remotes`` must succeed against a multi-fetch
89+
replication.config (regression for the field-observed crash).
90+
"""
91+
mocker.patch.object(
92+
gerrit_to_platform.config,
93+
"CONFIG_FILES",
94+
{
95+
CONFIG: TEST_CONFIG,
96+
REPLICATION: REPLICATION_MULTI_FETCH_CONFIG,
97+
},
98+
)
99+
remotes = get_replication_remotes()
100+
assert "github" in remotes
101+
assert "github-g2p" in remotes["github"]
102+
assert remotes["github"]["github-g2p"]["owner"] == ("modeseven-gerrit-onap")
103+
assert remotes["github"]["github-g2p"]["remotenamestyle"] == "dash"
104+
105+
53106
def test_get_mapping(mocker):
54107
"""Test get_mapping"""
55108
mocker.patch.object(

0 commit comments

Comments
 (0)