Skip to content

Commit 827ad72

Browse files
committed
address comments
1 parent b8be3e9 commit 827ad72

5 files changed

Lines changed: 248 additions & 48 deletions

File tree

ansible/roles/operator-pipeline/templates/openshift/tasks/merge-pr.yml

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ spec:
6969
7070
- name: merge-pull-request
7171
image: "$(params.pipeline_image)"
72+
volumeMounts:
73+
- name: merge-guard-git-scratch
74+
mountPath: /tmp/merge-guard-git
7275
env:
7376
- name: HOME
7477
value: "$(params.userHome)"
@@ -97,6 +100,8 @@ spec:
97100
if [[ "$(params.enable_merge_base_lane_guard)" == "true" ]] \
98101
&& [[ "$(workspaces.merge-base-guard.bound)" == "true" ]] \
99102
&& [[ -f "$(workspaces.merge-base-guard.path)/snapshot.json" ]]; then
103+
SCRATCH_GIT_DIR="/tmp/merge-guard-git/repo"
104+
rm -rf "${SCRATCH_GIT_DIR}"
100105
mkdir -p "${HOME}"
101106
if [[ "${WORKSPACE_SSH_DIRECTORY_BOUND}" == "true" ]]; then
102107
cp -R "${WORKSPACE_SSH_DIRECTORY_PATH}" "${HOME}/.ssh"
@@ -112,10 +117,10 @@ spec:
112117
# Inject safe.directory without mutating $HOME/.gitconfig.
113118
export GIT_CONFIG_COUNT=1
114119
export GIT_CONFIG_KEY_0=safe.directory
115-
export GIT_CONFIG_VALUE_0="$(workspaces.source.path)"
120+
export GIT_CONFIG_VALUE_0="${SCRATCH_GIT_DIR}"
116121
VERIFY_RC=0
117122
merge-base-lane-fingerprint verify \
118-
--git-dir "$(workspaces.source.path)" \
123+
--git-dir "${SCRATCH_GIT_DIR}" \
119124
--git-repo-url "$(params.git_repo_url)" \
120125
--git-base-branch "$(params.git_base_branch)" \
121126
--snapshot-file "$(workspaces.merge-base-guard.path)/snapshot.json" \
@@ -170,3 +175,6 @@ spec:
170175
exit 1
171176
fi
172177
fi
178+
volumes:
179+
- name: merge-guard-git-scratch
180+
emptyDir: {}

ansible/roles/operator-pipeline/templates/openshift/tasks/record-merge-base-lane-snapshot.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ spec:
4343
Clone of the repo at git_commit_base (detect-changes). Reused for fingerprinting
4444
when the live remote tip still matches git_commit_base (no second clone).
4545
- name: output
46-
description: Writes snapshot.json and ephemeral tip clone (e.g. results/merge-base-guard).
46+
description: Writes snapshot.json for merge-pr guard input.
4747
- name: ssh-directory
4848
optional: true
4949
description: >
@@ -52,6 +52,9 @@ spec:
5252
- name: record-snapshot
5353
image: "$(params.pipeline_image)"
5454
workingDir: $(workspaces.output.path)
55+
volumeMounts:
56+
- name: merge-guard-git-scratch
57+
mountPath: /tmp/merge-guard-git
5558
env:
5659
- name: HOME
5760
value: "$(params.userHome)"
@@ -87,7 +90,7 @@ spec:
8790
fi
8891
8992
BASE_CLONE="$(workspaces.base.path)"
90-
TIP_CLONE="$(workspaces.output.path)/tip-clone"
93+
TIP_CLONE="/tmp/merge-guard-git/tip-clone"
9194
9295
REMOTE_URL="$(params.git_repo_url)"
9396
if [[ "${REMOTE_URL}" == git@* ]] || [[ "${REMOTE_URL}" == ssh://* ]]; then
@@ -129,3 +132,6 @@ spec:
129132
--operator-path "$(params.operator_path)" \
130133
--added-or-modified-catalog-operators "$(params.added_or_modified_catalog_operators)" \
131134
--output "$(workspaces.output.path)/snapshot.json"
135+
volumes:
136+
- name: merge-guard-git-scratch
137+
emptyDir: {}

operatorcert/entrypoints/merge_base_lane_fingerprint.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@
1515

1616

1717
def _parser() -> argparse.ArgumentParser:
18+
"""
19+
Create the CLI argument parser.
20+
21+
Returns:
22+
argparse.ArgumentParser: Configured parser with `record` and `verify`
23+
subcommands.
24+
"""
1825
parser = argparse.ArgumentParser(
1926
description="Record or verify a fingerprint of operator/catalog paths on the base branch."
2027
)
@@ -85,7 +92,12 @@ def _parser() -> argparse.ArgumentParser:
8592

8693

8794
def main() -> None:
88-
"""CLI entrypoint for record or verify merge-base lane fingerprint snapshots."""
95+
"""
96+
Run the merge-base lane fingerprint CLI entrypoint.
97+
98+
Parses arguments, configures logging, and dispatches to either
99+
`record_snapshot` or `verify_snapshot`.
100+
"""
89101
parser = _parser()
90102
args = parser.parse_args()
91103
setup_logger(level="DEBUG" if args.verbose else "INFO")

operatorcert/merge_base_lane.py

Lines changed: 165 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,18 @@
1616

1717
def _git_global_args_for_ssh_remote(remote_url: str) -> list[str]:
1818
"""
19-
Extra `git -c ...` options so SSH matches Tekton git-clone (git-init) behavior.
19+
Build optional git config args for SSH remotes.
2020
21-
git-init uses StrictHostKeyChecking=accept-new, so clone works when the
22-
ssh-directory secret has a key but no known_hosts. Plain `git ls-remote` /
23-
`git fetch` in the pipeline image use OpenSSH defaults and fail with
24-
"Host key verification failed" in that setup.
21+
Adds `git -c ...` options so SSH behavior matches Tekton git-init. This
22+
keeps host key checking compatible with setups where the ssh-directory
23+
secret has keys but no `known_hosts` file.
24+
25+
Args:
26+
remote_url (str): Git remote URL.
27+
28+
Returns:
29+
list[str]: Extra arguments for a git command, or an empty list for
30+
non-SSH remotes.
2531
"""
2632
u = remote_url.strip()
2733
if u.startswith("git@") or u.startswith("ssh://"):
@@ -33,19 +39,36 @@ def _git_global_args_for_ssh_remote(remote_url: str) -> list[str]:
3339

3440

3541
def split_csv(value: str) -> list[str]:
36-
"""Split comma-separated Tekton params into stripped non-empty segments."""
42+
"""
43+
Split a comma-separated value into normalized segments.
44+
45+
Args:
46+
value (str): Comma-separated text.
47+
48+
Returns:
49+
list[str]: Non-empty, whitespace-trimmed segments.
50+
"""
3751
if not value or not value.strip():
3852
return []
3953
return [p.strip() for p in value.split(",") if p.strip()]
4054

4155

4256
def catalog_operator_segment_to_repo_path(segment: str) -> str:
4357
"""
44-
Map detect-changes catalog operator segments to repo-root-relative paths.
58+
Map a detect-changes catalog segment to a repo-relative path.
4559
46-
`detect-changed-operators` emits `version/operator` (e.g. `v4.14/foo`)
47-
without the `catalogs/` prefix. Entries may also already be under
48-
`catalogs/...` in which case they are normalized as-is.
60+
`detect-changed-operators` emits `version/operator` values (for example
61+
`v4.14/foo`) without a `catalogs/` prefix. Inputs already under
62+
`catalogs/...` are normalized and returned as-is.
63+
64+
Args:
65+
segment (str): Catalog operator segment.
66+
67+
Returns:
68+
str: Normalized path under `catalogs/`.
69+
70+
Raises:
71+
ValueError: If the segment is empty, absolute, unsafe, or malformed.
4972
"""
5073
s = segment.strip()
5174
if not s:
@@ -71,10 +94,22 @@ def build_lane_paths(
7194
operator_path: str, added_or_modified_catalog_operators: str
7295
) -> list[str]:
7396
"""
74-
Union and de-duplicate path prefixes that define the merge guard lane on the base tree.
97+
Build sorted unique path prefixes for the merge guard lane.
98+
99+
Combines `operator_path` (`operators/...`) and
100+
`added_or_modified_catalog_operators` from detect-changes
101+
(comma-separated `version/operator` or `catalogs/...`).
102+
103+
Args:
104+
operator_path (str): Operator path reported by detect-changes.
105+
added_or_modified_catalog_operators (str): Comma-separated catalog
106+
operator segments from detect-changes.
75107
76-
Uses `operator_path` (`operators/...`) and `added_or_modified_catalog_operators`
77-
from detect-changes (comma-separated `version/operator` or `catalogs/...`).
108+
Returns:
109+
list[str]: Sorted, de-duplicated lane paths.
110+
111+
Raises:
112+
ValueError: If any resolved path is unsafe.
78113
"""
79114
paths: set[str] = set()
80115

@@ -102,7 +137,18 @@ def _add_operator_path(raw: str) -> None:
102137

103138
def git_ls_tree_lines(git_dir: Path, tree_ish: str, paths: Sequence[str]) -> list[str]:
104139
"""
105-
Return sorted lines from `git ls-tree -r` for the given paths (stable fingerprint input).
140+
Return sorted `git ls-tree -r` output lines for selected paths.
141+
142+
Args:
143+
git_dir (Path): Repository directory.
144+
tree_ish (str): Commit, ref, or tree expression to inspect.
145+
paths (Sequence[str]): Path prefixes to include.
146+
147+
Returns:
148+
list[str]: Sorted non-empty `git ls-tree` lines.
149+
150+
Raises:
151+
RuntimeError: If `git ls-tree` fails.
106152
"""
107153
if not paths:
108154
return []
@@ -125,7 +171,15 @@ def git_ls_tree_lines(git_dir: Path, tree_ish: str, paths: Sequence[str]) -> lis
125171

126172
def fingerprint_lane(git_dir: Path, tree_ish: str, paths: Sequence[str]) -> str:
127173
"""
128-
SHA256 hex digest of canonical ls-tree output for paths at tree_ish.
174+
Compute a SHA256 fingerprint for the lane paths at a tree-ish.
175+
176+
Args:
177+
git_dir (Path): Repository directory.
178+
tree_ish (str): Commit, ref, or tree expression to fingerprint.
179+
paths (Sequence[str]): Path prefixes that define the lane.
180+
181+
Returns:
182+
str: Hex-encoded SHA256 digest of canonical `git ls-tree` output.
129183
"""
130184
lines = git_ls_tree_lines(git_dir, tree_ish, paths)
131185
blob = "\n".join(lines).encode("utf-8")
@@ -134,14 +188,24 @@ def fingerprint_lane(git_dir: Path, tree_ish: str, paths: Sequence[str]) -> str:
134188

135189
def git_ls_remote_tip(remote_url: str, branch: str) -> str:
136190
"""
137-
Resolve refs/heads/<branch> OID via `git ls-remote`.
138-
139-
Uses the current process environment (`HOME`, `SSH_AUTH_SOCK`, etc.).
140-
For SSH `remote_url` values, callers must install the same credentials as
141-
`git-clone` (for example Tekton `ssh-directory` copied to `$HOME/.ssh`)
142-
before invoking the CLI. SSH host-key handling matches Tekton git-init
143-
(`StrictHostKeyChecking=accept-new`) so missing `known_hosts` in the secret
144-
does not break verify after a successful clone.
191+
Resolve the current object ID for a remote branch tip.
192+
193+
Uses `git ls-remote` against `refs/heads/<branch>` in the current process
194+
environment (`HOME`, `SSH_AUTH_SOCK`, and related settings).
195+
196+
For SSH remotes, callers must provide credentials equivalent to the clone
197+
step (for example Tekton `ssh-directory` copied to `$HOME/.ssh`). SSH host
198+
key handling mirrors Tekton git-init (`StrictHostKeyChecking=accept-new`).
199+
200+
Args:
201+
remote_url (str): Git remote URL.
202+
branch (str): Branch name.
203+
204+
Returns:
205+
str: Object ID for `refs/heads/<branch>`.
206+
207+
Raises:
208+
RuntimeError: If `git ls-remote` fails or the branch ref is missing.
145209
"""
146210
ref = f"refs/heads/{branch}"
147211
cmd = [
@@ -173,10 +237,40 @@ def git_fetch_branch_tip(
173237
git_dir: Path, remote_name: str, remote_url: str, branch: str, local_ref: str
174238
) -> None:
175239
"""
176-
Shallow-fetch branch tip from `remote_url` into `local_ref`.
240+
Shallow-fetch a remote branch tip into a local ref.
241+
242+
This uses the same SSH/HTTPS credential requirements as
243+
`git_ls_remote_tip()`.
244+
245+
Args:
246+
git_dir (Path): Repository directory.
247+
remote_name (str): Temporary remote name to create.
248+
remote_url (str): Git remote URL.
249+
branch (str): Branch name to fetch.
250+
local_ref (str): Destination local ref.
177251
178-
Same SSH/HTTPS credential requirements as :func:`git_ls_remote_tip`.
252+
Raises:
253+
RuntimeError: If remote add or fetch fails.
179254
"""
255+
git_dir.mkdir(parents=True, exist_ok=True)
256+
probe = subprocess.run(
257+
["git", "-C", str(git_dir), "rev-parse", "--git-dir"],
258+
check=False,
259+
capture_output=True,
260+
text=True,
261+
)
262+
if probe.returncode != 0:
263+
init = subprocess.run(
264+
["git", "init", str(git_dir)],
265+
check=False,
266+
capture_output=True,
267+
text=True,
268+
)
269+
if init.returncode != 0:
270+
raise RuntimeError(
271+
f"git init failed ({init.returncode}): "
272+
f"{init.stderr.strip() or init.stdout.strip()}"
273+
)
180274
# remove the local remote if it exists
181275
subprocess.run(
182276
["git", "-C", str(git_dir), "remote", "remove", remote_name],
@@ -221,13 +315,27 @@ def git_fetch_branch_tip(
221315

222316

223317
def load_snapshot(path: Path) -> Any:
224-
"""Load a snapshot from a file."""
318+
"""
319+
Load a snapshot JSON file.
320+
321+
Args:
322+
path (Path): Snapshot file path.
323+
324+
Returns:
325+
Any: Parsed JSON snapshot data.
326+
"""
225327
with path.open(encoding="utf-8") as f:
226328
return json.load(f)
227329

228330

229331
def write_snapshot(path: Path, data: dict[str, Any]) -> None:
230-
"""Write a snapshot to a file."""
332+
"""
333+
Write snapshot data to a JSON file.
334+
335+
Args:
336+
path (Path): Output snapshot path.
337+
data (dict[str, Any]): Snapshot payload to serialize.
338+
"""
231339
path.parent.mkdir(parents=True, exist_ok=True)
232340
with path.open("w", encoding="utf-8") as f:
233341
json.dump(data, f, indent=2, sort_keys=True)
@@ -241,7 +349,17 @@ def record_snapshot(
241349
added_or_modified_catalog_operators: str,
242350
output: Path,
243351
) -> None:
244-
"""Record a snapshot of the merge-base lane fingerprint."""
352+
"""
353+
Record a snapshot of the merge-base lane fingerprint.
354+
355+
Args:
356+
git_dir (Path): Repository directory.
357+
tree_ish (str): Commit or ref used as the base snapshot point.
358+
operator_path (str): Operator path from detect-changes.
359+
added_or_modified_catalog_operators (str): Comma-separated catalog
360+
operator segments from detect-changes.
361+
output (Path): Snapshot JSON output path.
362+
"""
245363
paths = build_lane_paths(operator_path, added_or_modified_catalog_operators)
246364
if not paths:
247365
write_snapshot(
@@ -275,11 +393,26 @@ def verify_snapshot(
275393
snapshot_file: Path,
276394
) -> int:
277395
"""
278-
Returns 0 if merge is allowed, EXIT_LANE_MISMATCH if lane changed on base, 1 on error.
396+
Verify snapshot validity against the current base branch lane.
397+
398+
Returns 0 if merge is allowed, `EXIT_LANE_MISMATCH` if the lane changed on
399+
the base branch, and 1 when verification fails.
400+
401+
If `remote_url` is not reachable from clone-local configuration alone,
402+
configure git credentials in the process environment as required by
403+
`git_ls_remote_tip()`.
404+
405+
Args:
406+
git_dir (Path): Repository directory.
407+
remote_url (str): Upstream repository URL.
408+
branch (str): Base branch name.
409+
snapshot_file (Path): Snapshot JSON path written during record.
410+
411+
Returns:
412+
int: Verification status code.
279413
280-
When `remote_url` is not reachable from the local clone alone, `git ls-remote` /
281-
`git fetch` run in the process environment; configure git credentials accordingly
282-
(see :func:`git_ls_remote_tip`).
414+
Raises:
415+
ValueError: If snapshot content is structurally invalid.
283416
"""
284417
try:
285418
snap = load_snapshot(snapshot_file)

0 commit comments

Comments
 (0)