Skip to content

docs: explain how to find the release ECP - #249

Merged
BohdanMar merged 5 commits into
conforma:mainfrom
BohdanMar:ec-2007-find-release-ecp
Sep 14, 2026
Merged

BohdanMar merged 5 commits into
conforma:mainfrom
BohdanMar:ec-2007-find-release-ecp

Conversation

@BohdanMar

@BohdanMar BohdanMar commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up from review on #246.

Adds instructions for finding the release ECP:

  • Look up the ReleasePlanAdmission name from ReleasePlan labels via oc get
  • Browse the konflux-release-data repo as an alternative
  • Fallback: ask your SRE team

Fixes: https://redhat.atlassian.net/browse/EC-2007

Add instructions for looking up the ReleasePlanAdmission name
from the ReleasePlan labels, and mention the konflux-release-data
repo as an alternative.

Follow-up from review feedback on PR conforma#246.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7bfa7e27-8436-4184-9e46-8bb52784f2e0


Comment @coderabbitai help to get the list of available commands.

Comment thread modules/ROOT/pages/early-policy-violations.adoc Outdated
Comment thread modules/ROOT/pages/early-policy-violations.adoc Outdated
Use grep releasePlanAdmission for cleaner output, and use <rpa-name>
placeholder since we're finding the ReleasePlanAdmission name.
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:30 PM UTC · Completed 1:43 PM UTC

Commit: ca5794c · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.15

@fullsend-ai-review fullsend-ai-review Bot added the risk/low PR risk: low label Sep 3, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Risk Assessment: low (1/5)

Details

Docs-only 29-line change to a single AsciiDoc page by a recurring contributor with no protected paths, security-sensitive files, CI, or dependency impact.

Previous run

Risk Assessment: low (1/5)

Details

Docs-only 28-line change to a single AsciiDoc page with no protected paths, no security-sensitive files, no CI/dependency changes, and a known contributor; Tier 2 shows modest recent churn but no coupling or regression concerns, yielding a composite well within the low band.

Previous run (2)

Risk Assessment: low (1/5)

Details

Minimal-risk documentation-only change: a single AsciiDoc file with 22 lines changed, no protected paths, no security-sensitive files, no CI/dependency changes, and a known non-first-time contributor; slightly elevated file churn and fix frequency are explained by an active review cycle on a recently-added guide.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review

Findings

Info

  • [provenance-warning] — Prior review context was discarded because provenance validation failed (PRIOR_REVIEW_PROVENANCE=unverifiable-wrong-app: the prior review comment was created by a different app than expected). This run treats all findings as first-time assessments; severity anchoring was skipped.

All six review dimensions (correctness of oc/yq commands and Kubernetes label/field references, style/conventions against sibling .adoc files, intent-coherence with the linked Jira EC-2007 and PR #246 follow-up, and docs-currency across nav.adoc, partials, and other pages) returned no findings. Risk assessment: low (1/5) — docs-only 29-line change to a single AsciiDoc page, no protected paths, no security-sensitive files, no CI or dependency impact.

Previous run

Review

Docs-only change to a single AsciiDoc page (+23 / -5). The new oc get /
yq steps replace the previous "ask your SRE team" fallback and are a
substantive improvement. No blocking issues found. A few observations for
the author's consideration below — none require action to merge.

Findings

Low

  • [cli-invocation-consistency] modules/ROOT/pages/early-policy-violations.adoc:83 — The first command oc get releaseplan __<releaseplan-name>__ ... omits -n <tenant-namespace> while the follow-up command on line 90 uses -n __<managed-namespace>__. Users whose current kube-context namespace is not their tenant namespace will get a NotFound. The partial$oc_login.adoc include only appears at Step 2, so Step 1 runs against whatever context the user has. Advisory — the common Konflux workflow has the tenant namespace as the current context.
    Remediation: Consider adding -n __<tenant-namespace>__ for symmetry, or moving the oc_login include earlier and noting that Step 1 must run in the tenant namespace.
  • [command-output-usability] modules/ROOT/pages/early-policy-violations.adoc:83yq .metadata.labels | grep releasePlanAdmission emits a full YAML line (release.appstudio.openshift.io/releasePlanAdmission: my-rpa-name) rather than the bare RPA name that Step 2's __<rpa-name>__ placeholder wants. Readers can identify the value, but a direct key lookup would be cleaner and copy-pasteable.
    Remediation: Consider yq -r '.metadata.labels["release.appstudio.openshift.io/releasePlanAdmission"]' (or equivalent) so Step 1's output feeds Step 2 verbatim.

Info

  • [unverified-external-claim] modules/ROOT/pages/early-policy-violations.adoc:83 — The command assumes a ReleasePlan carries a release.appstudio.openshift.io/releasePlanAdmission label populated by the release-service controller. Consistent with the known Konflux convention, but not verifiable from this docs-only repo. If matching hasn't happened yet, the label is absent and the pipe produces no output; the doc does not mention this failure mode.
  • [unverified-external-claim] modules/ROOT/pages/early-policy-violations.adoc:90 — Claims yq .spec.policy on an RPA returns an ECP reference as a namespace/name string. Consistent with the release-service field being a string reference, but the current CRD shape could not be verified from this repo. If spec.policy is a structured object, the yq output would not be usable as-is for POLICY_CONFIGURATION.
  • [unverified-external-claim] modules/ROOT/pages/early-policy-violations.adoc:98 — Path tenants-config/auto-generated/cluster/<cluster>/tenants/<tenant>/ in konflux-release-data could not be verified from this repo. Worth a spot-check that the layout still matches.
  • [provenance-warning] N/A — Prior review context was discarded: PRIOR_REVIEW_PROVENANCE = unverifiable-wrong-app (the pre-existing review comment was authored by a different app than expected, so authorship — and any post-creation edits — cannot be reliably attributed). This review treats all findings as first-time assessments; severity anchoring was skipped for this run.
Previous run (2)

Review

Findings

Medium

  • [logic-error] modules/ROOT/pages/early-policy-violations.adoc:82 — Step 1 instructs the user to find the ReleasePlanAdmission (RPA) name and prepend the managed namespace to form a reference like rhtap-releng-tenant/<rpa-name>. However, Step 2's YAML template uses the placeholder <managed-namespace>/<ecp-name>, and the surrounding text says to "set POLICY_CONFIGURATION to the ECP used by your release pipeline," referencing an EnterpriseContractPolicy CR. This creates an inconsistency: Step 1 produces an RPA reference, but Step 2 asks for an ECP reference. If POLICY_CONFIGURATION accepts RPA references directly, the Step 2 placeholder and description should be updated to match Step 1's output. If it only accepts ECP references, Step 1 needs an additional command to extract the ECP name from within the RPA.
    Remediation: Align the two steps. Either (a) update the Step 2 placeholder from <managed-namespace>/<ecp-name> to <managed-namespace>/<rpa-name> and adjust the explanatory text to clarify that POLICY_CONFIGURATION accepts RPA references, or (b) add a command in Step 1 that extracts the ECP reference from the RPA (e.g., oc get releaseplanadmission <rpa-name> -n <managed-namespace> -o yaml | yq .spec.policy).

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

Step 1 found the RPA name but Step 2 needed the ECP reference.
Add a second command to extract spec.policy from the RPA.
@BohdanMar
BohdanMar requested a review from a team as a code owner September 7, 2026 09:19
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:21 AM UTC · Completed 9:30 AM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $4.13

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review September 7, 2026 09:29

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Sep 7, 2026
@BohdanMar
BohdanMar removed request for a team and simonbaird September 7, 2026 11:08
Add -n flags to both oc commands for explicit namespace targeting.
Use yq label lookup instead of grep for clean, copy-pasteable output.
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:11 AM UTC · Completed 11:16 AM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $2.80

@robnester-rh robnester-rh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@simonbaird simonbaird left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lgtm.

@BohdanMar
BohdanMar merged commit 4637dd5 into conforma:main Sep 14, 2026
3 checks passed
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:18 AM UTC · Completed 8:27 AM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $4.47

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already merged.

The /fs-review command only reviews open PRs/MRs.

Posted by fullsend pre-review check

@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 14, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:18 AM UTC · Completed 8:22 AM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $1.88

@fullsend-ai-retro

Copy link
Copy Markdown

Retro for PR #249 — a docs-only 29-line AsciiDoc change explaining how to find the release ECP. The workflow went well. The review agent caught a substantive Medium logic-error on the first review of commit 055ec455 (Step 1 produced an RPA reference but Step 2 required an ECP reference), which the author fixed in 3f670fe. A follow-up review then flagged two Low advisories (namespace flags, yq label lookup) that the author also adopted in ecd920d, before human reviewers approved and it was merged. The review agent added real value on a docs PR — no new proposals warranted.

Evidence for existing issues (noted here, not filed as duplicates):

  • Every re-review on this PR emitted a provenance-warning and skipped severity anchoring because scripts/pre-fetch-prior-review.sh compared the prior review's app client_id=Iv23li1nIorNLIQy6NWK against a blank expected value (expected env unset), producing PRIOR_REVIEW_PROVENANCE=unverifiable-wrong-app. Same fullsend-ai-review[bot] on both sides — misconfiguration, not a real mismatch. This caused each re-review to be treated as a first review and re-emit advisory findings the author had already seen. Already covered by fullsend-ai/agents#931, fullsend-ai/fullsend#6911, fullsend-ai/agents#203, and (security angle) fullsend-ai/fullsend#5463. Closed fullsend-ai/fullsend#5388 attempted a fix on 2026-08-21 but the misconfiguration persists on this repo — worth signaling on #931/#6911 that the deployment is still emitting the empty-expected-client_id path on conforma/user-guide as of 2026-09-07.
  • 15 seconds after merge (08:16:49Z → 08:17:04Z workflow_dispatch), the bot dispatched Review run 34821938328 against the just-merged PR, and the pre-review check posted a boilerplate "Review skipped — already merged" comment. Overlaps with fullsend-ai/fullsend#3728 ("Route job should check PR merge status before dispatching review"). Not filed as a new proposal.

Agents repo resolved during runs: fullsend-ai/agents (v0.40.0 for the 09-03 review, v0.41.0 for the 09-07 reviews).

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already merged.

The /fs-review command only reviews open PRs/MRs.

Posted by fullsend post-review check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge risk/low PR risk: low size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants