openshift-mcp-server: add periodic for upstream sync#80316
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughA new OpenShift CI periodic configuration file is created for the openshift-mcp-server release 5.0. It configures repository-based image building from ChangesOpenShift MCP Server Release 5.0 CI Periodic Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-merge-bot
Bot
added
the
rehearsals-ack
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml (2)
30-34: ⚡ Quick winAdd checksum verification for downloaded gh CLI binary.
The script downloads the gh CLI binary from GitHub releases without verifying its checksum. While the pinned version and HTTPS help, adding checksum verification provides defense-in-depth against supply chain attacks (compromised releases, MITM, etc.).
🔒 Recommended addition: Verify checksum before using gh CLI
# Install gh CLI GH_VERSION="2.62.0" GH_TARBALL="gh_${GH_VERSION}_linux_amd64.tar.gz" + GH_CHECKSUM="f33ee8a3e3c8e5fc1c1fa3c7069caa4bb21b6f5e50df59b9d5a9f3d42f0f2ca8" # Update with actual checksum curl -sL "https://github.com/cli/cli/releases/download/v${GH_VERSION}/${GH_TARBALL}" -o "/tmp/${GH_TARBALL}" + echo "${GH_CHECKSUM} /tmp/${GH_TARBALL}" | sha256sum -c - tar -xzf "/tmp/${GH_TARBALL}" -C /tmp export PATH="/tmp/gh_${GH_VERSION}_linux_amd64/bin:${PATH}"Obtain the checksum from the gh CLI v2.62.0 release page.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml` around lines 30 - 34, Add checksum verification for the downloaded gh CLI tarball: after you set GH_VERSION and GH_TARBALL and download the tarball with curl, also download or embed the expected checksum for that GH_VERSION (e.g., from the gh release page), compute the tarball's checksum (sha256sum or shasum -a 256) and verify it matches the expected value, and abort (non-zero exit) if the verification fails before running tar and exporting PATH; update the block around GH_VERSION, GH_TARBALL, curl, tar and PATH to include this verification step.
3-11: 💤 Low valueConsider whether the images section is needed.
The configuration builds an
openshift-mcp-serverimage fromDockerfile.cibut:
- Promotion is disabled (line 9), so the image won't be pushed
- The test step uses
from: src(line 49), running directly from source code- The periodic job only executes
make sync-upstream-prfrom sourceIf the image is not required for this periodic job, you could remove the
imagesandpromotionsections to simplify the configuration and avoid unnecessary image builds. However, if these sections are intentionally included to match the main config structure (per variant periodic patterns), they can remain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml` around lines 3 - 11, The images and promotion blocks currently configure building an openshift-mcp-server image from Dockerfile.ci but promotion is disabled and the job runs from source (uses from: src and executes make sync-upstream-pr), so remove the images and promotion sections (or comment them out) if the image build is not required for this periodic; otherwise leave them as-is to preserve variant parity. Locate the top-level images: and promotion: entries (references: Dockerfile.ci, openshift-mcp-server, promotion -> name "5.0") and either delete those blocks or document why they must remain for consistency with other variant configs while ensuring the job still runs from source using make sync-upstream-pr.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml`:
- Around line 38-42: Replace embedding GH_TOKEN in the remote URL by using
GIT_ASKPASS: keep reading GH_TOKEN as before, write a small helper script that
echoes the token, make it executable, export GIT_ASKPASS pointing to that helper
and set GIT_TERMINAL_PROMPT=0, then call git remote set-url origin with the
normal https://github.com/openshift/openshift-mcp-server.git URL so git will
obtain credentials via GIT_ASKPASS instead of exposing the token in the remote
URL; reference GH_TOKEN, GIT_ASKPASS, and the git remote set-url origin
invocation when locating where to change the script.
- Around line 1-53: Add the required top-level sections base_images and releases
to the __periodics variant (mirror the structure used in non-periodic configs)
so the config validates; in the test steps replace embedding GH_TOKEN in the git
remote URL by configuring git credential helper or exporting
GIT_ASKPASS/GIT_HTTP_TOKEN_ENV to supply credentials to git (stop using the
literal "git remote set-url origin
\"https://openshift-ci-robot:${GH_TOKEN}@...\"" pattern), and harden the gh CLI
install by downloading the tarball to /tmp, validating its integrity (compare a
checksum or signature) before extracting and adding to PATH (adjust the
GH_TARBALL/ GH_VERSION variables and the tar extraction step accordingly).
Ensure these changes touch the keys base_images and releases, the test step that
runs make sync-upstream-pr, the git remote set-url invocation, and the gh
download/extract commands.
---
Nitpick comments:
In
`@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml`:
- Around line 30-34: Add checksum verification for the downloaded gh CLI
tarball: after you set GH_VERSION and GH_TARBALL and download the tarball with
curl, also download or embed the expected checksum for that GH_VERSION (e.g.,
from the gh release page), compute the tarball's checksum (sha256sum or shasum
-a 256) and verify it matches the expected value, and abort (non-zero exit) if
the verification fails before running tar and exporting PATH; update the block
around GH_VERSION, GH_TARBALL, curl, tar and PATH to include this verification
step.
- Around line 3-11: The images and promotion blocks currently configure building
an openshift-mcp-server image from Dockerfile.ci but promotion is disabled and
the job runs from source (uses from: src and executes make sync-upstream-pr), so
remove the images and promotion sections (or comment them out) if the image
build is not required for this periodic; otherwise leave them as-is to preserve
variant parity. Locate the top-level images: and promotion: entries (references:
Dockerfile.ci, openshift-mcp-server, promotion -> name "5.0") and either delete
those blocks or document why they must remain for consistency with other variant
configs while ensuring the job still runs from source using make
sync-upstream-pr.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 10b73921-61ad-4b08-a53c-cd2469215cd9
📒 Files selected for processing (1)
ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml
| GH_TOKEN="$(cat /etc/github-token/token)" | ||
| export GH_TOKEN | ||
| git config user.name "OpenShift CI Robot" | ||
| git config user.email "noreply@openshift.io" | ||
| git remote set-url origin "https://openshift-ci-robot:${GH_TOKEN}@github.com/openshift/openshift-mcp-server.git" |
There was a problem hiding this comment.
Consider using GIT_ASKPASS to avoid embedding token in remote URL.
Embedding the GitHub token directly in the git remote URL (line 42) could expose it in git error messages or logs if git commands print the remote URL. While the script correctly avoids set -x to prevent tracing, git itself might output URLs in certain error conditions.
Recommended mitigation:
Use GIT_ASKPASS or a credential helper instead:
- GH_TOKEN="$(cat /etc/github-token/token)"
- export GH_TOKEN
- git config user.name "OpenShift CI Robot"
- git config user.email "noreply@openshift.io"
- git remote set-url origin "https://openshift-ci-robot:${GH_TOKEN}`@github.com/openshift/openshift-mcp-server.git`"
+ # Configure git to use token without embedding in URL
+ cat /etc/github-token/token > /tmp/gh-token
+ echo -e '#!/bin/sh\ncat /tmp/gh-token' > /tmp/git-askpass.sh
+ chmod +x /tmp/git-askpass.sh
+ export GIT_ASKPASS=/tmp/git-askpass.sh
+ export GH_TOKEN="$(cat /etc/github-token/token)"
+ git config user.name "OpenShift CI Robot"
+ git config user.email "noreply@openshift.io"
+ git remote set-url origin "https://openshift-ci-robot@github.com/openshift/openshift-mcp-server.git"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-release-5.0__periodics.yaml`
around lines 38 - 42, Replace embedding GH_TOKEN in the remote URL by using
GIT_ASKPASS: keep reading GH_TOKEN as before, write a small helper script that
echoes the token, make it executable, export GIT_ASKPASS pointing to that helper
and set GIT_TERMINAL_PROMPT=0, then call git remote set-url origin with the
normal https://github.com/openshift/openshift-mcp-server.git URL so git will
obtain credentials via GIT_ASKPASS instead of exposing the token in the remote
URL; reference GH_TOKEN, GIT_ASKPASS, and the git remote set-url origin
invocation when locating where to change the script.
Source: Coding guidelines
There was a problem hiding this comment.
Obsolete file target, but a reasonable concern.
There was a problem hiding this comment.
@grokspawn, thanks for acknowledging the concern! Since the file target is obsolete, would you like me to open a follow-up GitHub issue to track the recommendation of using GIT_ASKPASS instead of embedding the token in the remote URL, so it can be addressed whenever the relevant configuration lands?
🧠 Learnings used
Learnt from: PillaiManish
Repo: openshift/release PR: 79231
File: ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml:0-0
Timestamp: 2026-05-20T05:41:25.164Z
Learning: When reviewing OpenShift/Kubernetes operator manifests/CSVs, if a ClusterServiceVersion (CSV) defines an `emptyDir` volume mounted at `/tmp` with `medium: Memory`, treat that data as scoped to the *pod lifecycle* (not the container lifecycle). Container restarts within the same pod (e.g., restart after SIGTERM/termination and recreation of the container) should not wipe the `emptyDir` contents; data written to something like `/tmp/e2e-cover` should still be present and can be collected afterward (e.g., via `oc cp`). The `emptyDir` is cleared when the pod is deleted/recreated/rescheduled (pod lifecycle ends), not on container restarts.
Learnt from: rdiscala
Repo: openshift/release PR: 79357
File: ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-sno-day2-worker-4.18.yaml:23-23
Timestamp: 2026-05-20T11:54:20.424Z
Learning: In the openshift/release repository CI operator configuration YAMLs, do not treat an intentionally impossible cron schedule (e.g., `0 0 31 2 *` for “Feb 31”) as a misconfiguration. This pattern is an intentional mechanism used to disable/park a periodic CI job temporarily (e.g., while an upstream bug is being fixed). Only flag cron syntax errors when the schedule is not an intentional “impossible date” disable pattern used for job parking.
Learnt from: dhaiducek
Repo: openshift/release PR: 79563
File: ci-operator/config/stolostron/cluster-proxy/stolostron-cluster-proxy-main.yaml:41-43
Timestamp: 2026-05-20T19:31:57.286Z
Learning: In the openshift/release repo, for ci-operator container test step config YAML files under ci-operator/config/, remember that the `commands` field is read by ci-operator at runtime from the config YAML and is not embedded into the generated ProwJob YAML under `ci-operator/jobs/`. Therefore, if a PR changes only `commands` (and not any ProwJob-spec affecting structural fields such as test/job names, job types/spec, or metadata that changes the resulting ProwJob spec), you generally do NOT need to run `make update` and regenerating artifacts should be a no-op diff. Run `make update` only when the changes affect the generated ProwJob spec (e.g., new/renamed tests, new job types, or other spec/metadata changes that alter `ci-operator/jobs/` output).
Learnt from: tthvo
Repo: openshift/release PR: 79564
File: ci-operator/config/openshift/multiarch/openshift-multiarch-main__nightly-4.20-upgrade-from-nightly-4.19.yaml:94-94
Timestamp: 2026-05-20T20:01:14.481Z
Learning: In the openshift/release repo, when editing ci-operator config YAMLs under ci-operator/config/, changes that only modify the *values* of environment variables inside test step `env` blocks (e.g., COMPUTE_NODE_TYPE, ADDITIONAL_WORKER_VM_TYPE, MIGRATION_INFRA_MACHINE_TYPE) do not change the generated ProwJob specs under ci-operator/jobs/. For these value-only env tweaks, reviewers should not require running `make update` because it should produce no diff. Require `make update` only for structural/semantic changes that can affect the generated ProwJob spec (e.g., adding/renaming tests, changing job types, or altering metadata/fields that feed into the ProwJob spec).
Learnt from: mtulio
Repo: openshift/release PR: 79439
File: ci-operator/config/openshift/hypershift/openshift-hypershift-release-5.0__periodics.yaml:159-175
Timestamp: 2026-05-21T04:10:01.133Z
Learning: In the openshift/release repository, treat ci-operator periodic *variant* YAMLs (files ending with `__periodics.yaml`, e.g. `*__periodics.yaml`) as a different schema from the main ci-operator config. These periodic variant files are NOT expected to contain `build_root`, `images`, or `promotion` sections. Only `base_images`, `releases`, `resources`, `tests`, and `zz_generated_metadata` are required/expected. During reviews, do not flag missing `build_root`, `images`, or `promotion` in periodic variant `__periodics.yaml` files as structural/schema issues.
Learnt from: karampok
Repo: openshift/release PR: 79537
File: ci-operator/config/openshift/bond-cni/openshift-bond-cni-main.yaml:32-33
Timestamp: 2026-05-21T09:04:21.025Z
Learning: In openshift/release, when modifying a ci-operator config YAML under ci-operator/config/ by adding an `env` block (key/value environment variables such as `TEST_TYPE`, `TEST_SUITE`, `TEST_ARGS`) to an existing test step, this change does not affect the generated ProwJob specs under ci-operator/jobs/. In this case, `make update` is a no-op—do not request or flag that `make update` was (or needs to be) run for these env-only edits.
Learnt from: feichashao
Repo: openshift/release PR: 79740
File: ci-operator/config/openshift/backplane-cli/openshift-backplane-cli-main.yaml:2-10
Timestamp: 2026-05-27T06:35:08.802Z
Learning: In the openshift/release repo, for ci-operator config YAMLs under `ci-operator/config/`, changes that only modify these fields do not require running `make update` or regenerating ProwJob spec YAMLs under `ci-operator/jobs/`: `base_images` and `build_root.image_stream_tag`. These values are read by ci-operator at runtime and are not embedded into the generated ProwJob specs. Only “structural/semantic” changes that would change the resulting ProwJob specs themselves (e.g., adding/renaming tests, changing job types, or changing metadata that feeds into the ProwJob spec) require `make update`.
Learnt from: sivaramsingana
Repo: openshift/release PR: 79753
File: ci-operator/config/quay/quay-tests/quay-quay-tests-master__ocp-421-s390x-quay.yaml:1-41
Timestamp: 2026-05-27T11:27:23.893Z
Learning: In the openshift/release repo, for CI-operator configuration YAMLs located under `ci-operator/config/`, do not require `images` or `promotion` sections when the configuration is test-only and does not build any new container images.
- Include `images` only if the config actually builds new images.
- Include `promotion` only if any built images must be promoted.
- Test-only configs that only consume `base_images` in `workflow/test` steps (and do not build new images) should be considered complete without `images` or `promotion`.
Learnt from: rafabene
Repo: openshift/release PR: 79722
File: ci-operator/config/openshift-hyperfleet/hyperfleet-e2e/openshift-hyperfleet-hyperfleet-e2e-main__e2e.yaml:28-28
Timestamp: 2026-05-27T14:34:48.023Z
Learning: When reviewing `ci-operator` presubmit test YAMLs in `ci-operator/config/**/*.yaml`, do NOT automatically flag `skip_if_only_changed: .` as a misconfiguration. In this repo it can be an intentional pattern for “manual-only” presubmit jobs that are designed to run only when explicitly requested (e.g., triggered via `/test <job-name>`, similar to `pj-rehearse`) rather than auto-running on every PR. Only flag `skip_if_only_changed: .` if the surrounding job configuration does not indicate it is meant to be a manually triggered presubmit.
Learnt from: LuboTerifaj
Repo: openshift/release PR: 79928
File: ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-stable-5.0-upgrade-from-stable-5.0.yaml:46-62
Timestamp: 2026-06-02T10:55:27.606Z
Learning: In the openshift/release repo’s ci-operator config YAMLs, the `relative` field is valid when used inside a `prerelease` release block (it is not limited to `candidate` blocks). If you see `relative` set under `prerelease` (e.g., `relative: 1`), do not flag it as a schema violation. Reviewers should treat this as the standard OpenShift incremental z-stream upgrade testing pattern where `ci-operator` uses `relative` to append `&rel=<N>` to the release API request, selecting the Nth build position within the configured `version_bounds` (e.g., `5.0.0-ec.1` → `5.0.0-ec.2`).
Learnt from: smg247
Repo: openshift/release PR: 79989
File: ci-operator/config/openshift/sippy/openshift-sippy-main.yaml:21-22
Timestamp: 2026-06-02T17:01:07.630Z
Learning: In this repo, when reviewing ci-operator config YAMLs under `ci-operator/config/`, treat `dockerfile_path` entries as paths resolved relative to the upstream source repository being tested (e.g., `openshift/sippy`), not relative to the `openshift/release` repo clone. Do not flag `dockerfile_path` as missing/broken just because the referenced file is not present in the `openshift/release` checkout; verify the Dockerfile exists in the upstream source repo referenced by the test configuration before raising a finding.
Learnt from: smg247
Repo: openshift/release PR: 79989
File: ci-operator/config/openshift/sippy/openshift-sippy-main.yaml:21-29
Timestamp: 2026-06-02T17:01:09.558Z
Learning: In openshift/release, for ci-operator config YAMLs under ci-operator/config/, changes limited to the `images` and/or `promotion` sections (including adding/removing image build entries or promotion targets) should not trigger running `make update` or regenerating ProwJob spec YAMLs under ci-operator/jobs/. `ci-operator` reads `images` and `promotion` directly from these config YAMLs at runtime, and those fields are not embedded into the generated ProwJob specs. Only changes that affect ProwJob spec generation/structure (e.g., adding/renaming tests, changing job types, or modifying metadata that feeds into ProwJob spec generation) require `make update` and regenerated ProwJob YAMLs.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: grokspawn The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
removed
the
rehearsals-ack
|
/pj-rehearse periodic-openshift-mcp-server-upstream-pr-sync |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Signed-off-by: grokspawn <jordan@nimblewidget.com>
|
/pj-rehearse periodic-openshift-mcp-server-upstream-pr-sync |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@grokspawn: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
This PR proposes a new periodic job which will manage upstream/downstream sync. Since this is only ever operating from HEAD, making the change in the infra-periodics.yaml space.
The image may need to be changed/updated/tweaked. I'm going to try the rehearsals to see what happens here.
Summary by CodeRabbit
This PR adds a periodic CI job configuration for the
openshift-mcp-serverrepository to manage automated upstream synchronization.What's changing:
A new periodic job
upstream-pr-syncis being configured to run automatically on weekdays at 8 AM UTC. This job:make sync-upstream-prto synchronize upstream changesInfrastructure impact:
The configuration is added to OpenShift's CI system and specifies:
Dockerfile.ciThis automation reduces manual effort in keeping the downstream MCP server repository synchronized with upstream changes during the normal business week.