From 98dbf45ea282d91c87ef571312b82b7293d8113e Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Tue, 18 Aug 2026 14:02:55 +0200 Subject: [PATCH 1/4] :bug: fix(olm): correct installModes and make OperatorHub submission portable The OperatorHub submission path had never executed, so three bugs survived CI and review. Found by running hack/operatorhub-pr.sh for real. - installModes advertised OwnNamespace/SingleNamespace as supported, but the controller never reads olm.targetNamespaces and sets no WATCH_NAMESPACE, so a namespace-scoped install would silently collect cluster-wide. Advertise AllNamespaces only until that plumbing exists. - sed -i is GNU-only; on BSD/macOS sed -i consumes the next argument as a backup suffix, so the OpenShift annotation step died with 'invalid command code' on every manual re-run. Rewritten with awk. - The existing-PR lookup filtered on "${FORK_OWNER}:${BRANCH}", a literal match against GitHub's canonical org casing (PlatformRelay vs platformrelay), so it never matched and the script tried to open a duplicate PR. Match the head owner via headRepositoryOwner + ascii_downcase instead. - gh pr edit resolves assignees/labels/reviewers over GraphQL and requires read:org, which would force a broader PAT than this needs. PATCH via gh api. - ci.yaml reviewers listed the platformrelay org; the field takes usernames. Each is covered by a regression gate in dist_operatorhub_pr_test.sh. --- config/olm/ci.yaml | 6 +++ .../kollect.clusterserviceversion.yaml | 8 ++- hack/operatorhub-pr.sh | 50 +++++++++++++++---- hack/test/dist_operatorhub_pr_test.sh | 27 ++++++++++ 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/config/olm/ci.yaml b/config/olm/ci.yaml index 13dd941b..e16efb0d 100644 --- a/config/olm/ci.yaml +++ b/config/olm/ci.yaml @@ -1,2 +1,8 @@ --- +# NOTE: this file is NOT what gets submitted. hack/operatorhub-pr.sh writes its own +# copy of ci.yaml into the community-operators PR. Keep the two in sync. +# `reviewers` takes GitHub usernames (an org does not resolve) — the PR author must +# appear in the upstream copy for `authorized-changes` / self-merge to apply. updateGraph: semver-mode +reviewers: + - konih diff --git a/config/olm/template/manifests/kollect.clusterserviceversion.yaml b/config/olm/template/manifests/kollect.clusterserviceversion.yaml index 5b0f287e..8036686a 100644 --- a/config/olm/template/manifests/kollect.clusterserviceversion.yaml +++ b/config/olm/template/manifests/kollect.clusterserviceversion.yaml @@ -121,11 +121,15 @@ spec: - base64data: __ICON_BASE64__ mediatype: image/svg+xml + # Kollect's controller watches cluster-wide: it never reads olm.targetNamespaces + # and sets no WATCH_NAMESPACE, so a namespace-scoped install would silently + # collect from the whole cluster. Advertise AllNamespaces only until that + # plumbing exists (ADR-0708 follow-up D9). installModes: - type: OwnNamespace - supported: true + supported: false - type: SingleNamespace - supported: true + supported: false - type: MultiNamespace supported: false - type: AllNamespaces diff --git a/hack/operatorhub-pr.sh b/hack/operatorhub-pr.sh index d35801f3..1daeb96e 100755 --- a/hack/operatorhub-pr.sh +++ b/hack/operatorhub-pr.sh @@ -8,7 +8,11 @@ # Required env vars: # VERSION - Release version without 'v' prefix (e.g., 0.17.0) # IMAGE_DIGEST - Controller image digest (e.g., sha256:abc...) -# GH_TOKEN - PAT with public_repo scope for fork push and upstream PRs +# GH_TOKEN - classic PAT for fork push and upstream PRs. Scopes: `public_repo` +# (clone/push the public forks, open + edit upstream PRs) AND +# `workflow` — the submission branch is cut from upstream/main, which +# carries .github/workflows/*, and GitHub rejects a PAT push that +# introduces workflow files without it once the fork drifts behind. # # Optional env vars: # FORK_OWNER - GitHub org owning the forks (default: platformrelay) @@ -96,14 +100,27 @@ submit_bundle() { cp "${CHECKOUT_DIR}/${BUNDLE_DIR}/metadata/"* "${OPERATOR_DIR}/${VERSION}/metadata/" if [[ -n "${openshift_versions}" ]]; then - sed -i '/^annotations:/a\ com.redhat.openshift.versions: "'"${openshift_versions}"'"' \ - "${OPERATOR_DIR}/${VERSION}/metadata/annotations.yaml" + # Portable insert-after. GNU and BSD/macOS sed disagree on BOTH `-i` (BSD reads the + # next argument as the backup suffix) and the one-line `a\text` form, so the previous + # `sed -i` worked on the CI runner and died locally with "invalid command code". + # awk behaves identically on both, which matters because docs/RELEASE.md and the + # workflow's own failure warning both tell operators to re-run this script by hand. + local annotations_file="${OPERATOR_DIR}/${VERSION}/metadata/annotations.yaml" + local tmp_annotations + tmp_annotations="$(mktemp)" + awk -v line=" com.redhat.openshift.versions: \"${openshift_versions}\"" \ + '{ print } /^annotations:/ { print line }' \ + "${annotations_file}" >"${tmp_annotations}" + mv "${tmp_annotations}" "${annotations_file}" fi + # `reviewers` takes GitHub USERNAMES, not orgs. The PR author (the OPERATORHUB_PAT + # owner) must appear here on the UPSTREAM default branch for the pipeline to set + # `authorized-changes` and self-merge later version bumps; an org never resolves. cat > "${OPERATOR_DIR}/ci.yaml" <<'CIEOF' updateGraph: semver-mode reviewers: - - platformrelay + - konih CIEOF git add "${OPERATOR_DIR}/" @@ -130,20 +147,31 @@ See [release notes](https://github.com/platformrelay/kollect/releases/tag/v${VER --- *This PR was automatically created by the Kollect release workflow.*" + # Look the PR up by branch name and match the owner case-INSENSITIVELY. GitHub stores + # the canonical org casing ("PlatformRelay"), so the old `--head "${FORK_OWNER}:${BRANCH}"` + # filter silently returned nothing whenever FORK_OWNER differed in case, and the script + # then tried to create a duplicate PR and died with "a pull request already exists". + local fork_owner_lc + fork_owner_lc="$(printf '%s' "${FORK_OWNER}" | tr '[:upper:]' '[:lower:]')" local existing_pr existing_pr=$(GH_TOKEN="${GH_TOKEN}" gh pr list \ --repo "${upstream_repo}" \ - --head "${FORK_OWNER}:${BRANCH}" \ + --head "${BRANCH}" \ --state open \ - --json number \ - --jq '.[0].number // empty' 2>/dev/null || true) + --json number,headRepositoryOwner \ + --jq "[.[] | select((.headRepositoryOwner.login // \"\" | ascii_downcase) == \"${fork_owner_lc}\")] | .[0].number // empty" 2>/dev/null || true) if [[ -n "${existing_pr}" ]]; then echo "Updating existing PR #${existing_pr}" - GH_TOKEN="${GH_TOKEN}" gh pr edit "${existing_pr}" \ - --repo "${upstream_repo}" \ - --title "${pr_title}" \ - --body "${pr_body}" + # Use the REST endpoint, NOT `gh pr edit`. The latter resolves assignees/labels/ + # reviewers via GraphQL and so demands `read:org` ("The 'login' field requires one of + # the following scopes: ['read:org']"), which would force a broader PAT than this + # script needs. PATCH .../pulls/{n} updates title+body with `public_repo` alone. + GH_TOKEN="${GH_TOKEN}" gh api \ + --method PATCH \ + "repos/${upstream_repo}/pulls/${existing_pr}" \ + -f title="${pr_title}" \ + -f body="${pr_body}" >/dev/null echo "PR updated: https://github.com/${upstream_repo}/pull/${existing_pr}" else echo "Creating new PR..." diff --git a/hack/test/dist_operatorhub_pr_test.sh b/hack/test/dist_operatorhub_pr_test.sh index ecfaa025..cd00c98e 100755 --- a/hack/test/dist_operatorhub_pr_test.sh +++ b/hack/test/dist_operatorhub_pr_test.sh @@ -25,6 +25,33 @@ grep -Fq 'redhat-openshift-ecosystem/community-operators-prod' "${SCRIPT}" || fail "operatorhub-pr.sh must submit to community-operators-prod" grep -Fq 'v4.19' "${SCRIPT}" || fail "operatorhub-pr.sh must annotate OpenShift v4.19 for prod catalog" + +# The three checks below inspect EXECUTABLE lines only — the comments in operatorhub-pr.sh +# legitimately name the very anti-patterns being banned, and a naive grep matches its own docs. +CODE="$(grep -v '^[[:space:]]*#' "${SCRIPT}")" + +# GNU-only `sed -i