-
Notifications
You must be signed in to change notification settings - Fork 76
OAPE-698: Integration of codecov on the must-gather-operator #350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # E2E coverage lifecycle script for CI and local use. | ||
| # | ||
| # Usage: | ||
| # hack/e2e-coverage.sh setup Prepare the operator for coverage collection | ||
| # hack/e2e-coverage.sh collect Collect, convert, and optionally upload coverage data | ||
| # | ||
| # Environment variables: | ||
| # COVERAGE_IMAGE (setup) Full pullspec of the coverage-instrumented image | ||
| # CODECOV_TOKEN (collect) Codecov upload token; skip upload if unset | ||
| # ARTIFACT_DIR (collect) Directory for CI artifacts; defaults to "." | ||
| set -euo pipefail | ||
|
|
||
| NAMESPACE="must-gather-operator" | ||
| DEPLOYMENT="must-gather-operator" | ||
| CONTAINER="must-gather-operator" | ||
| GOCOVERDIR_PATH="/tmp/e2e-cover" | ||
| CODECOV_SECRET_PATH="/var/run/secrets/must-gather-operator/ci-secrets/CODECOV_TOKEN" | ||
| POD_LABEL="name=must-gather-operator" | ||
|
|
||
| setup() { | ||
| echo "--- E2E Coverage Setup ---" | ||
|
|
||
| if [[ -z "${COVERAGE_IMAGE:-}" ]]; then | ||
| echo "Error: COVERAGE_IMAGE env var must be set" | ||
| exit 1 | ||
| fi | ||
| echo "Coverage image: ${COVERAGE_IMAGE}" | ||
|
|
||
| local csv | ||
| csv=$(oc get deployment "${DEPLOYMENT}" -n "${NAMESPACE}" \ | ||
| -o jsonpath='{.metadata.ownerReferences[?(@.kind=="ClusterServiceVersion")].name}' 2>/dev/null) | ||
|
|
||
| if [[ -n "${csv}" ]]; then | ||
| echo "Found CSV: ${csv} -- patching via CSV" | ||
| # Assumes deployments/0 and containers/0 are the must-gather-operator. | ||
| oc patch csv "${csv}" -n "${NAMESPACE}" --type=json -p "[ | ||
| {\"op\": \"replace\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/containers/0/image\", \"value\": \"${COVERAGE_IMAGE}\"}, | ||
| {\"op\": \"add\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-\", \"value\": {\"name\": \"GOCOVERDIR\", \"value\": \"${GOCOVERDIR_PATH}\"}}, | ||
| {\"op\": \"add\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-\", \"value\": {\"name\": \"OPERATOR_IMAGE\", \"value\": \"${COVERAGE_IMAGE}\"}}, | ||
| {\"op\": \"add\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/containers/0/volumeMounts/-\", \"value\": {\"name\": \"coverage-data\", \"mountPath\": \"${GOCOVERDIR_PATH}\"}}, | ||
| {\"op\": \"add\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/volumes/-\", \"value\": {\"name\": \"coverage-data\", \"emptyDir\": {}}} | ||
| ]" | ||
| else | ||
| echo "No CSV found -- patching deployment directly" | ||
| oc set image "deployment/${DEPLOYMENT}" -n "${NAMESPACE}" \ | ||
| "${CONTAINER}=${COVERAGE_IMAGE}" | ||
| oc set env "deployment/${DEPLOYMENT}" -n "${NAMESPACE}" \ | ||
| -c "${CONTAINER}" GOCOVERDIR="${GOCOVERDIR_PATH}" OPERATOR_IMAGE="${COVERAGE_IMAGE}" | ||
|
|
||
| local has_vol | ||
| has_vol=$(oc get "deployment/${DEPLOYMENT}" -n "${NAMESPACE}" \ | ||
| -o jsonpath='{.spec.template.spec.volumes[?(@.name=="coverage-data")].name}' 2>/dev/null) | ||
| if [[ -z "${has_vol}" ]]; then | ||
| oc patch "deployment/${DEPLOYMENT}" -n "${NAMESPACE}" --type=json -p "[ | ||
| {\"op\": \"add\", \"path\": \"/spec/template/spec/containers/0/volumeMounts/-\", \"value\": {\"name\": \"coverage-data\", \"mountPath\": \"${GOCOVERDIR_PATH}\"}}, | ||
| {\"op\": \"add\", \"path\": \"/spec/template/spec/volumes/-\", \"value\": {\"name\": \"coverage-data\", \"emptyDir\": {}}} | ||
| ]" | ||
| else | ||
| echo "Volume 'coverage-data' already exists -- skipping volume patch" | ||
| fi | ||
| fi | ||
|
|
||
| echo "Waiting for operator rollout with coverage image..." | ||
| oc rollout status "deployment/${DEPLOYMENT}" -n "${NAMESPACE}" --timeout=180s | ||
|
|
||
| echo "Verifying GOCOVERDIR is set in the running pod..." | ||
| oc exec -n "${NAMESPACE}" "deploy/${DEPLOYMENT}" -- env | grep GOCOVERDIR || \ | ||
| echo "Warning: GOCOVERDIR not found in pod env (non-fatal)" | ||
|
|
||
| echo "--- Coverage setup complete ---" | ||
| } | ||
|
|
||
| collect() { | ||
| echo "--- E2E Coverage Collection ---" | ||
|
|
||
| local artifact_dir="${ARTIFACT_DIR:-.}" | ||
| local coverage_dir="${artifact_dir}/e2e-cover-data" | ||
| local coverage_profile="${artifact_dir}/coverage-e2e.out" | ||
|
|
||
| if [[ -z "${CODECOV_TOKEN:-}" ]] && [[ -f "${CODECOV_SECRET_PATH}" ]]; then | ||
| CODECOV_TOKEN=$(cat "${CODECOV_SECRET_PATH}") | ||
| export CODECOV_TOKEN | ||
| fi | ||
|
|
||
| local pod | ||
| pod=$(oc get pod -n "${NAMESPACE}" -l "${POD_LABEL}" \ | ||
| --field-selector=status.phase=Running \ | ||
| -o jsonpath='{.items[0].metadata.name}' 2>/dev/null) | ||
| if [[ -z "${pod}" ]]; then | ||
| echo "Error: no operator pod found in namespace ${NAMESPACE}" | ||
| exit 1 | ||
| fi | ||
| echo "Operator pod: ${pod}" | ||
|
|
||
| echo "Sending SIGTERM to operator process to flush coverage data..." | ||
| oc exec -n "${NAMESPACE}" "${pod}" -c "${CONTAINER}" -- /bin/sh -c 'kill -TERM 1' || true | ||
|
|
||
| echo "Waiting for container to restart..." | ||
| oc wait pod/"${pod}" --for=condition=Ready=False -n "${NAMESPACE}" --timeout=30s 2>/dev/null || true | ||
| oc wait pod/"${pod}" --for=condition=Ready -n "${NAMESPACE}" --timeout=120s | ||
|
|
||
| mkdir -p "${coverage_dir}" | ||
| echo "Copying coverage data from operator pod..." | ||
| oc cp "${NAMESPACE}/${pod}:${GOCOVERDIR_PATH}/." "${coverage_dir}" -c "${CONTAINER}" | ||
|
|
||
| echo "Coverage files:" | ||
| ls -la "${coverage_dir}/" 2>/dev/null || true | ||
|
|
||
| if ls "${coverage_dir}"/covmeta.* >/dev/null 2>&1; then | ||
| echo "Converting coverage data to Go profile format..." | ||
| go tool covdata textfmt -i="${coverage_dir}" -o="${coverage_profile}" | ||
|
|
||
| echo "" | ||
| echo "=== E2E Coverage Summary ===" | ||
| go tool covdata percent -i="${coverage_dir}" | ||
| echo "=============================" | ||
| echo "" | ||
| echo "Coverage profile: ${coverage_profile} ($(wc -l < "${coverage_profile}") lines)" | ||
|
|
||
| if [[ -n "${CODECOV_TOKEN:-}" ]]; then | ||
| echo "Uploading to Codecov..." | ||
| local codecov_version="v0.8.0" | ||
| local codecov_bin="${artifact_dir}/codecov" | ||
| local codecov_os codecov_asset | ||
| codecov_os="$(uname -s)" | ||
| case "${codecov_os}" in | ||
| Linux) | ||
| codecov_asset="linux/codecov" | ||
| ;; | ||
| Darwin) | ||
| # Official macos asset supports Intel and Apple Silicon. | ||
| codecov_asset="macos/codecov" | ||
| ;; | ||
| *) | ||
| echo "Error: unsupported OS for Codecov uploader: ${codecov_os}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| echo "Downloading Codecov uploader for ${codecov_asset}" | ||
| curl -sS -o "${codecov_bin}" \ | ||
| "https://uploader.codecov.io/${codecov_version}/${codecov_asset}" | ||
| curl -sS -o "${codecov_bin}.SHA256SUM" \ | ||
| "https://uploader.codecov.io/${codecov_version}/${codecov_asset}.SHA256SUM" | ||
|
|
||
| # macOS ships shasum; Linux ships sha256sum. | ||
| if !( | ||
| cd "$(dirname "${codecov_bin}")" | ||
| if command -v sha256sum >/dev/null 2>&1; then | ||
| sha256sum -c "$(basename "${codecov_bin}").SHA256SUM" | ||
| else | ||
| shasum -a 256 -c "$(basename "${codecov_bin}").SHA256SUM" | ||
| fi | ||
| ); then | ||
| echo "Error: Codecov binary checksum verification failed" | ||
| exit 1 | ||
| fi | ||
| chmod +x "${codecov_bin}" | ||
|
|
||
| local -a codecov_args=( | ||
| --file="${coverage_profile}" | ||
| --flags=e2e | ||
| --name="E2E Coverage" | ||
| --verbose | ||
| ) | ||
|
|
||
| local job_type="${JOB_TYPE:-local}" | ||
| if [[ "${job_type}" == "presubmit" ]]; then | ||
| echo "Detected presubmit (PR #${PULL_NUMBER:-unknown})" | ||
| [[ -n "${PULL_NUMBER:-}" ]] && codecov_args+=(--pr "${PULL_NUMBER}") | ||
| [[ -n "${PULL_PULL_SHA:-}" ]] && codecov_args+=(--sha "${PULL_PULL_SHA}") | ||
| [[ -n "${PULL_BASE_REF:-}" ]] && codecov_args+=(--branch "${PULL_BASE_REF}") | ||
| [[ -n "${REPO_OWNER:-}" && -n "${REPO_NAME:-}" ]] && codecov_args+=(--slug "${REPO_OWNER}/${REPO_NAME}") | ||
| elif [[ "${job_type}" == "postsubmit" ]]; then | ||
| echo "Detected postsubmit (branch ${PULL_BASE_REF:-unknown})" | ||
| [[ -n "${PULL_BASE_SHA:-}" ]] && codecov_args+=(--sha "${PULL_BASE_SHA}") | ||
| [[ -n "${PULL_BASE_REF:-}" ]] && codecov_args+=(--branch "${PULL_BASE_REF}") | ||
| [[ -n "${REPO_OWNER:-}" && -n "${REPO_NAME:-}" ]] && codecov_args+=(--slug "${REPO_OWNER}/${REPO_NAME}") | ||
| else | ||
| echo "Local run -- no Prow context, Codecov will auto-detect from git" | ||
| fi | ||
|
|
||
| "${codecov_bin}" "${codecov_args[@]}" || echo "Warning: Codecov upload failed (non-fatal)" | ||
| rm -f "${codecov_bin}" "${codecov_bin}.SHA256SUM" | ||
| else | ||
| echo "CODECOV_TOKEN not set -- skipping Codecov upload." | ||
| echo "Coverage profile saved as artifact: ${coverage_profile}" | ||
| fi | ||
| else | ||
| echo "Warning: No coverage data found in ${coverage_dir}" | ||
| echo "The operator may not have been built with coverage instrumentation," | ||
| echo "or it may not have exited cleanly (SIGKILL instead of SIGTERM)." | ||
| fi | ||
|
|
||
| echo "--- Coverage collection complete ---" | ||
| } | ||
|
|
||
| case "${1:-}" in | ||
| setup) | ||
| setup | ||
| ;; | ||
| collect) | ||
| collect | ||
| ;; | ||
| *) | ||
| echo "Usage: $0 {setup|collect}" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Build the must-gather-operator binary with coverage instrumentation. | ||
| # This mirrors build/Dockerfile / Dockerfile.openshift but adds Go coverage flags | ||
| # so the binary records which lines are executed during E2E tests. | ||
| FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS builder | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- images/ci/Dockerfile.coverage ---\n'
nl -ba images/ci/Dockerfile.coverage | sed -n '1,220p'
printf '\n--- references to e2e-cover / coverage dir contract ---\n'
rg -n "e2e-cover|e2e-coverage|/tmp/e2e-cover|coverage" hack images/ci -g '!**/*.png' || true
printf '\n--- any policy docs mentioning image reference or digest/tag rules ---\n'
rg -n "floating tag|pin by digest|Red Hat images|non-RH images|HEALTHCHECK|non-root|COPY specific files" . -g '!**/*.png' || trueRepository: openshift/must-gather-operator Length of output: 259 🏁 Script executed: #!/bin/bash
set -euo pipefail
git ls-files images/ci/Dockerfile.coverage images/ci | sed -n '1,200p'
printf '\n--- file ---\n'
nl -ba images/ci/Dockerfile.coverage | sed -n '1,220p'
printf '\n--- related references ---\n'
rg -n "registry\.ci\.openshift\.org/ocp/builder|registry\.access\.redhat\.com|ubi|minimal|HEALTHCHECK|USER 65534|COPY .*" images/ci hack -g '!**/*.png' || trueRepository: openshift/must-gather-operator Length of output: 252 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- images/ci/Dockerfile.coverage ---'
cat -n images/ci/Dockerfile.coverage | sed -n '1,220p'
echo
echo '--- coverage-related references ---'
rg -n "e2e-cover|e2e-coverage|/tmp/e2e-cover|coverage" hack images/ci -g '!**/*.png' || true
echo
echo '--- image reference / security policy references ---'
rg -n "floating tag|pin by digest|Red Hat images|non-RH images|HEALTHCHECK|non-root|COPY specific files" . -g '!**/*.png' || trueRepository: openshift/must-gather-operator Length of output: 8032 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- similar Dockerfiles ---'
for f in build/Dockerfile Dockerfile.openshift images/ci/Dockerfile.coverage; do
if [ -f "$f" ]; then
echo
echo "### $f"
cat -n "$f" | sed -n '1,220p'
fi
done
echo
echo '--- all registry.ci.openshift.org / registry.access.redhat.com references in repo ---'
rg -n "registry\.ci\.openshift\.org|registry\.access\.redhat\.com" . -g '!vendor/**' -g '!**/*.png' || trueRepository: openshift/must-gather-operator Length of output: 5585 🌐 Web query:
💡 Result: The registry registry.ci.openshift.org is an internal OpenShift CI registry used primarily by OpenShift's internal automation and build processes, not for general user or production consumption [1][2]. Key points regarding its policy and usage: 1. Authoritative CI Registry: While registry.ci.openshift.org is used for internal processes like Release Controllers and mirroring, the authoritative CI registry and source of truth for all OpenShift CI images is quay.io/openshift/ci (often called QCI) [1][2]. 2. Usage Policy for Clients: Users and external integrations should not reference images from registry.ci.openshift.org [1]. Instead, they are directed to use the corresponding images hosted in QCI [1]. 3. Builder Images: Red Hat's ART (Architecture, Release, and Tools) team manages and pushes "builder images" (e.g., golang, rhel-based images) to registry.ci.openshift.org [3][1]. These images are specifically mirrored to ensure that CI image builds and productized image builds are consistent [3]. 4. Access and Authentication: Access to the registry is restricted and managed via internal CI infrastructure [1]. Because it is not a public registry, pull requests and automation scripts often require specific authentication (e.g., via pull secrets) or configuration to interact with it [4][5]. 5. Image Policy Configurations: OpenShift clusters allow for complex image policies (such as ClusterImagePolicy or ImagePolicy) that control which registries are allowed for importing images [6][7]. When configuring these policies, users must ensure they do not inadvertently block necessary OpenShift release or infrastructure images [6][7]. Users are responsible for configuring their own cluster's image policies to align with their security requirements while ensuring critical component connectivity [8][6]. In summary, registry.ci.openshift.org is a managed component of the internal OpenShift CI pipeline [1]. It is not intended for general use, and production workflows should rely on official, supported registries such as registry.redhat.io or quay.io [1][8]. Citations:
🌐 Web query:
💡 Result: In OpenShift CI, there is no single, explicit global policy that forces the use of digest tags over standard tags in Dockerfiles. Instead, the behavior is managed by the specific tooling, primarily the Citations:
Use a floating tag for the final UBI base image. The builder image is a managed OpenShift CI reference, so digest pinning isn’t needed here; the 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| ARG SRC_DIR=/go/src/github.com/openshift/must-gather-operator | ||
|
|
||
| WORKDIR $SRC_DIR | ||
|
|
||
| COPY . . | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Do not copy the entire build context.
As per path instructions, “COPY specific files, not entire context” and “No secrets in ENV, ARG, or COPY.” 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| RUN go build -cover -covermode=count -coverpkg=./... \ | ||
| -o must-gather-operator . | ||
|
|
||
| FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7-1771346502 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can keep it same as |
||
|
|
||
| RUN microdnf install -y tar gzip openssh-clients wget shadow-utils procps sshpass nc findutils && \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is |
||
| microdnf clean all | ||
|
|
||
| ARG SRC_DIR=/go/src/github.com/openshift/must-gather-operator | ||
| COPY --from=builder $SRC_DIR/must-gather-operator /usr/local/bin/must-gather-operator | ||
| COPY --from=builder $SRC_DIR/build/bin /usr/local/bin | ||
|
|
||
| # 65534 is the 'nobody' user/group - a standard unprivileged user for containers | ||
| RUN mkdir -p /tmp/e2e-cover && chown 65534:65534 /tmp/e2e-cover && chmod 700 /tmp/e2e-cover | ||
| USER 65534:65534 | ||
| ENV GOCOVERDIR=/tmp/e2e-cover | ||
| ENTRYPOINT ["/usr/local/bin/must-gather-operator"] | ||
|
Comment on lines
+15
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Define a container This image has no healthcheck, so container runtimes cannot detect a wedged operator process through the image metadata. Add a healthcheck using the operator’s supported health signal. As per path instructions, “HEALTHCHECK defined”. 🤖 Prompt for AI AgentsSource: Path instructions |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a space after
!in the negation.if !(is flagged by ShellCheck (SC1035); the!negation operator must be a standalone token, otherwise it collides with extglob!(...)pattern syntax and can be misparsed.🐛 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 148-148: You are missing a required space after the !.
(SC1035)
🤖 Prompt for AI Agents
Source: Linters/SAST tools