Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Dockerfile.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS builder
WORKDIR /go/src/github.com/openshift/secrets-store-csi-driver-operator
COPY . .
RUN make build-coverage

FROM registry.ci.openshift.org/ocp/4.22:base-rhel9
COPY --from=builder /go/src/github.com/openshift/secrets-store-csi-driver-operator/secrets-store-csi-driver-operator /usr/bin/
ENV GOCOVERDIR=/tmp/e2e-cover
ENTRYPOINT ["/bin/sh", "-c", "mkdir -p /tmp/e2e-cover && exec /usr/bin/secrets-store-csi-driver-operator \"$@\"", "--"]
LABEL io.k8s.display-name="OpenShift Secrets Store CSI Driver Operator" \
io.k8s.description="The Secrets Store CSI Driver Operator installs and maintains the Secrets Store CSI Driver on a cluster."
23 changes: 23 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,26 @@ test-e2e:
hack/e2e.sh

.PHONY: test-e2e

##@ E2E Coverage

.PHONY: build-coverage
build-coverage: ## Build the operator binary with coverage instrumentation.
$(GO) build $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) $(GO_LD_FLAGS) \
-cover -covermode=atomic -coverpkg=./... \
-o secrets-store-csi-driver-operator \
./cmd/secrets-store-csi-driver-operator
Comment on lines +80 to +85
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Test build-coverage target syntax by checking GO_BUILD_FLAGS expansion

# Extract GO_BUILD_FLAGS definition and test both FIPS and non-FIPS cases
echo "=== Non-FIPS case ==="
echo "GO_BUILD_FLAGS = -trimpath"
echo "Expanded: go build -trimpath,e2ecoverage"
echo "This would fail with: unknown flag: -trimpath,e2ecoverage"
echo ""
echo "=== FIPS case ==="
echo "GO_BUILD_FLAGS = -trimpath -tags strictfipsruntime,openssl"
echo "Expanded: go build -trimpath -tags strictfipsruntime,openssl,e2ecoverage"
echo "This works but is fragile (assumes GO_BUILD_FLAGS ends with -tags)"

Repository: openshift/secrets-store-csi-driver-operator

Length of output: 453


🏁 Script executed:

# Find the Makefile and examine the build-coverage target and GO_BUILD_FLAGS definition
find . -name "Makefile" -type f | head -20

Repository: openshift/secrets-store-csi-driver-operator

Length of output: 1152


🏁 Script executed:

# Check the actual build-coverage target and surrounding context
head -n 85 ./Makefile | tail -n 20

Repository: openshift/secrets-store-csi-driver-operator

Length of output: 574


🏁 Script executed:

# Find GO_BUILD_FLAGS definition in the Makefile
rg -n "GO_BUILD_FLAGS" ./Makefile -A 2 -B 2

Repository: openshift/secrets-store-csi-driver-operator

Length of output: 840


🏁 Script executed:

# Verify the actual behavior - check if build-coverage would fail with the current syntax
# Let's understand what the expansion would be in both cases

echo "=== Current state in non-FIPS mode ==="
echo "GO_BUILD_FLAGS=-trimpath"
echo "Expanded command: go build -trimpath,e2ecoverage"
echo ""
echo "=== Current state in FIPS mode ==="
echo "GO_BUILD_FLAGS=-trimpath -tags strictfipsruntime,openssl"
echo "Expanded command: go build -trimpath -tags strictfipsruntime,openssl,e2ecoverage"
echo ""
echo "=== Testing proposed fix in FIPS mode ==="
echo "Proposed: GO_BUILD_FLAGS -tags e2ecoverage"
echo "FIPS expansion: go build -trimpath -tags strictfipsruntime,openssl -tags e2ecoverage"
echo "(This has TWO -tags flags which is invalid)"

Repository: openshift/secrets-store-csi-driver-operator

Length of output: 589


Fix build-coverage flags syntax error—proposed fix is incorrect for FIPS mode.

The $(GO_BUILD_FLAGS),e2ecoverage concatenation produces invalid syntax in non-FIPS mode. When GO_BUILD_FLAGS is -trimpath, this expands to go build -trimpath,e2ecoverage, which is invalid Go build syntax.

However, the proposed fix ($(GO_BUILD_FLAGS) -tags e2ecoverage) breaks FIPS mode by creating duplicate -tags flags: go build -trimpath -tags strictfipsruntime,openssl -tags e2ecoverage. Go's build command does not accept multiple -tags flags.

The correct solution requires either:

  1. Appending to the tags list when -tags already exists in GO_BUILD_FLAGS, or
  2. Creating a separate tags variable for the coverage target that merges with existing FIPS tags
🤖 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 `@Makefile` around lines 80 - 85, The build-coverage target is generating
invalid Go flag syntax by naively concatenating "$(GO_BUILD_FLAGS),e2ecoverage"
and the suggested "-tags e2ecoverage" breaks FIPS by producing duplicate -tags;
fix by producing a single -tags value for the coverage build: detect if
GO_BUILD_FLAGS already contains a -tags clause and if so append ",e2ecoverage"
to that existing tag list, otherwise add a new "-tags e2ecoverage" flag;
implement this logic in the Makefile when constructing flags for the
build-coverage target (use GO_BUILD_FLAGS and create/compute a temporary
coverage tags variable or a modified FLAGS variable) so the final go build
invocation has at most one -tags flag and includes e2ecoverage.


COVERAGE_IMG ?= $(IMAGE_REGISTRY)/ocp/4.22:secrets-store-csi-driver-operator-e2e-coverage

.PHONY: docker-build-coverage
docker-build-coverage: ## Build coverage-instrumented Docker image.
$(IMAGE_BUILD_BUILDER) $(IMAGE_BUILD_DEFAULT_FLAGS) -t $(COVERAGE_IMG) -f Dockerfile.coverage .

.PHONY: docker-push-coverage
docker-push-coverage: ## Push coverage Docker image.
$(IMAGE_BUILD_BUILDER) push $(COVERAGE_IMG)

.PHONY: e2e-coverage-collect
e2e-coverage-collect: ## Collect e2e coverage data and optionally upload to Codecov.
ARTIFACT_DIR=$${ARTIFACT_DIR:-.} hack/e2e-coverage.sh collect
183 changes: 183 additions & 0 deletions hack/e2e-coverage.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
#!/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="openshift-cluster-csi-drivers"
DEPLOYMENT="secrets-store-csi-driver-operator"
POD_LABEL="app=secrets-store-csi-driver-operator"
GOCOVERDIR_PATH="/tmp/e2e-cover"
CODECOV_SECRET_PATH="/var/run/secrets/codecov/CODECOV_TOKEN"

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}"

echo "Discovering CSV from deployment ownerReference..."
local csv
csv=$(oc get deployment "${DEPLOYMENT}" -n "${NAMESPACE}" \
-o jsonpath='{.metadata.ownerReferences[?(@.kind=="ClusterServiceVersion")].name}')
if [[ -z "${csv}" ]]; then
echo "Error: no CSV found for ${DEPLOYMENT}"
exit 1
fi
echo "Found CSV: ${csv}"

echo "Patching CSV with coverage image..."
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}\"}
]"

local has_gocoverdir
has_gocoverdir=$(oc get csv "${csv}" -n "${NAMESPACE}" \
-o jsonpath='{.spec.install.spec.deployments[0].spec.template.spec.containers[0].env[?(@.name=="GOCOVERDIR")].name}' 2>/dev/null)
if [[ -z "${has_gocoverdir}" ]]; then
echo "Adding GOCOVERDIR env var to CSV..."
oc patch csv "${csv}" -n "${NAMESPACE}" --type=json -p "[
{\"op\": \"add\", \"path\": \"/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-\", \"value\": {\"name\": \"GOCOVERDIR\", \"value\": \"${GOCOVERDIR_PATH}\"}}
]"
else
echo "GOCOVERDIR env var already present in CSV"
fi

echo "Waiting for operator rollout with coverage image..."
sleep 5
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 pods -n "${NAMESPACE}" -l "${POD_LABEL}" \
-o jsonpath='{.items[0].metadata.name}' 2>/dev/null)
if [[ -z "${pod}" ]]; then
echo "Error: no operator pod found with label ${POD_LABEL}"
echo "Coverage collection requires the operator pod to be running."
exit 1
fi
echo "Found operator pod: ${pod}"

echo "Sending SIGTERM to flush coverage data (container will restart)..."
oc exec -n "${NAMESPACE}" "${pod}" -- /bin/sh -c 'kill -TERM 1' 2>/dev/null || true

echo "Waiting for container to restart and become ready..."
sleep 5
oc wait pod "${pod}" -n "${NAMESPACE}" --for=condition=Ready --timeout=120s

echo "Copying coverage data from the restarted container..."
mkdir -p "${coverage_dir}"
oc cp "${NAMESPACE}/${pod}:${GOCOVERDIR_PATH}/." "${coverage_dir}"

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_bin="${artifact_dir}/codecov"
curl -sS -o "${codecov_bin}" https://uploader.codecov.io/latest/linux/codecov
curl -sS -o "${codecov_bin}.SHA256SUM" https://uploader.codecov.io/latest/linux/codecov.SHA256SUM
curl -sS -o "${codecov_bin}.SHA256SUM.sig" https://uploader.codecov.io/latest/linux/codecov.SHA256SUM.sig

if command -v gpg >/dev/null 2>&1 && command -v gpgv >/dev/null 2>&1; then
curl -sS https://keybase.io/codecovsecurity/pgp_keys.asc \
| gpg --no-default-keyring --keyring trustedkeys.gpg --import 2>/dev/null || true
if gpgv "${codecov_bin}.SHA256SUM.sig" "${codecov_bin}.SHA256SUM" 2>/dev/null; then
echo "PGP signature verified"
else
echo "Warning: PGP signature verification failed (continuing with SHA256 check)"
fi
fi
cd "$(dirname "${codecov_bin}")" && sha256sum -c "$(basename "${codecov_bin}").SHA256SUM" && cd - >/dev/null
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" "${codecov_bin}.SHA256SUM.sig"
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 the process did not exit 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