Skip to content
Draft
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion .ci-operator.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
build_root_image:
name: release
namespace: openshift
tag: rhel-9-release-golang-1.26-openshift-5.0
tag: rhel-9-release-golang-1.25-openshift-4.22
8 changes: 4 additions & 4 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@ jobs:

steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Initialize CodeQL
uses: github/codeql-action/init@b8d3b6e8af63cde30bdc382c0bc28114f4346c88 # v2
uses: github/codeql-action/init@v2
with:
languages: ${{ matrix.language }}
queries: +security-and-quality

- name: Autobuild
uses: github/codeql-action/autobuild@b8d3b6e8af63cde30bdc382c0bc28114f4346c88 # v2
uses: github/codeql-action/autobuild@v2

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@b8d3b6e8af63cde30bdc382c0bc28114f4346c88 # v2
uses: github/codeql-action/analyze@v2
Comment on lines +27 to +39

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Tracked workflow files:\n'
git ls-files .github/workflows | sed -n '1,120p'

printf '\nAction refs in workflows (uses line):\n'
rg -n 'uses:\s*[^#\s]+' .github/workflows || true

printf '\nWorkflow snippets with line numbers:\n'
for f in .github/workflows/codeql.yml .github/workflows/image-push-master.yml .github/workflows/image-push-release.yml .github/workflows/kind-e2e.yml .github/workflows/stale-issues-prs.yml; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    nl -ba "$f" | sed -n '1,180p'
  else
    echo "missing $f"
  fi
done

printf '\nClassify each uses ref as tag-only vs full commit-ish vs local:\n'
python3 - <<'PY'
import os, re
root=".github/workflows"
for fn in sorted(os.listdir(root)):
    p=os.path.join(root,fn)
    if not os.path.isfile(p): continue
    with open(p, encoding="utf-8") as f:
        for i,line in enumerate(f,1):
            m=re.search(r'uses:\s*([^\s#]+)', line)
            if not m: continue
            ref=m.group(1)
            if ref.startswith('./'):
                cat='local'
            else:
                parts=ref.rsplit('@',1)
                token=parts[1] if len(parts)==2 else ''
                if re.fullmatch(r'[0-9a-fA-F]{40}', token):
                    cat='full_sha'
                elif ':' in token or ref.startswith('git://')):
                    cat='url'
                else:
                    cat='tag_or_branch'
            print(f"{p}:{i}:{cat}:{ref}")
PY

printf '\nDiff summary focused on workflows:\n'
git diff --stat -- .github/workflows || true
git diff -- .github/workflows | sed -n '1,240p' || true

Repository: openshift/multus-cni

Length of output: 5363


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Classify workflow action refs:\n'
python3 - <<'PY'
import os, re
root=".github/workflows"
tag_or_branch_pat=re.compile(r'^[A-Za-z._0-9-]+$|^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$')
for fn in sorted(os.listdir(root)):
    p=os.path.join(root,fn)
    if not os.path.isfile(p): continue
    with open(p, encoding="utf-8", errors="replace") as f:
        for i,line in enumerate(f,1):
            m=re.search(r'uses:\s*([^,\s#\r\n]+)', line)
            if not m: continue
            ref=m.group(1).strip()
            if ref.startswith('./') or ref.startswith('.\\'):
                cat='local'
                ref='.'.join(ref.split('@',1))
            else:
                if ref.count('@') < 1:
                    cat='no_at_ref'
                else:
                    parts=ref.rsplit('@',1)
                    token=parts[1]
                    if re.fullmatch(r'[0-9a-fA-F]{40}', token):
                        cat='full_sha'
                    elif ':' in token or token.startswith('git://'):
                        cat='url'
                    elif re.fullmatch(r'aqua-security/trivy-action/v[\d.]+', token):
                        cat='semver_tag'
                    elif re.fullmatch(r'[\d.]+', token) or token.startswith('v') and set(token[1:]) <= set('0123456789.'):
                        cat='semver_like_tag'
                    elif tag_or_branch_pat.fullmatch(token):
                        cat='tag_or_branch'
                    else:
                        cat='other'
            print(f"{p}:{i}:{cat}:{ref}")
PY

printf '\nFocused workflow contents:\n'
for f in .github/workflows/codeql.yml \
         .github/workflows/image-push-master.yml \
         .github/workflows/image-push-release.yml \
         .github/workflows/kind-e2e.yml \
         .github/workflows/stale-issues-prs.yml \
         .github/workflows/image-build.yml \
         .github/workflows/test.yml \
         .github/workflows/release.yml \
         .github/workflows/build.yml; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    awk '{printf "%6d  %s\n", NR, $0}' "$f"
  fi
done

Repository: openshift/multus-cni

Length of output: 29395


Restore immutable GitHub Actions references across the workflows.

The workflow files run action tags (@v*, major version tags, and semver-like tags) instead of full commit SHAs, so CI/E2E/image-publishing actions can change without review. Pin every active uses: action reference to its full commit SHA.

🧰 Tools
🪛 actionlint (1.7.12)

[error] 30-30: the runner of "github/codeql-action/init@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


[error] 36-36: the runner of "github/codeql-action/autobuild@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


[error] 39-39: the runner of "github/codeql-action/analyze@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

📍 Affects 5 files
  • .github/workflows/codeql.yml#L27-L39 (this comment)
  • .github/workflows/image-push-master.yml#L14-L29
  • .github/workflows/image-push-master.yml#L46-L61
  • .github/workflows/image-push-master.yml#L75-L75
  • .github/workflows/image-push-release.yml#L14-L37
  • .github/workflows/image-push-release.yml#L54-L77
  • .github/workflows/image-push-release.yml#L91-L91
  • .github/workflows/kind-e2e.yml#L36-L42
  • .github/workflows/kind-e2e.yml#L52-L52
  • .github/workflows/kind-e2e.yml#L111-L111
  • .github/workflows/stale-issues-prs.yml#L10-L10
🤖 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 @.github/workflows/codeql.yml around lines 27 - 39, Pin every active uses:
action reference to an immutable full commit SHA, replacing tag-based references
while preserving each action and workflow behavior. Apply this across
.github/workflows/codeql.yml:27-39, image-push-master.yml:14-29, 46-61, 75-75,
image-push-release.yml:14-37, 54-77, 91-91, kind-e2e.yml:36-42, 52-52, 111-111,
and stale-issues-prs.yml:10-10; update all affected action invocations,
including Initialize CodeQL, Autobuild, and Perform CodeQL Analysis, without
changing their configuration.

Source: Path instructions

Comment on lines 29 to +39

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow excerpt =="
sed -n '1,120p' .github/workflows/codeql.yml | cat -n

echo
echo "== actionlint availability/name =="
command -v actionlint || true

echo
echo "== workflow/actionlint usage in repo =="
rg -n "actionlint|github/codeql-action|codeql" .github/workflows || true

Repository: openshift/multus-cni

Length of output: 1719


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== actionlint version =="
actionlint -version || true

echo
echo "== actionlint on codeql.yml =="
actionlint .github/workflows/codeql.yml

echo
echo "== all action pins in workflows =="
python3 - <<'PY'
from pathlib import Path
import re
for p in sorted(Path(".github/workflows").glob("*.yml")) + sorted(Path(".github/workflows").glob("*.yaml")):
    txt = p.read_text()
    for i,line in enumerate(txt.splitlines(), 1):
        m=re.search(r"uses:\s*([^\s]+)@([^\s#]+)", line)
        if m:
            print(f"{p}:{i}:{m.group(1)}@{m.group(2)}")
PY

echo
echo "== fetch public codeql-action v2 ref object =="
if command -v gh >/dev/null 2>&1; then
  gh api repos/github/codeql-action/git/refs/tags/v2 --jq '{ref, object}'
else
  echo "gh not available"
fi

Repository: openshift/multus-cni

Length of output: 1177


Upgrade the CodeQL action version.

github/codeql-action/init@v2, autobuild@v2, and analyze@v2 are too old for GitHub Actions runners and violate the CI/CD requirement to pin actions by full SHA. Move these steps to a supported major version and pin them to full commit SHAs.

🧰 Tools
🪛 actionlint (1.7.12)

[error] 30-30: the runner of "github/codeql-action/init@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


[error] 36-36: the runner of "github/codeql-action/autobuild@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


[error] 39-39: the runner of "github/codeql-action/analyze@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 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 @.github/workflows/codeql.yml around lines 29 - 39, Update the CodeQL
workflow steps Initialize CodeQL, Autobuild, and Perform CodeQL Analysis to a
supported github/codeql-action major version, and pin each action reference to
its complete commit SHA instead of the current `@v2` tags. Keep the existing
language and queries configuration unchanged.

Source: Linters/SAST tools

with:
category: "/language:${{ matrix.language }}"
18 changes: 9 additions & 9 deletions .github/workflows/image-push-master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Push container image for thick plugin
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand All @@ -43,22 +43,22 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Push thin container image
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand All @@ -72,7 +72,7 @@ jobs:

- name: Push thin container debug image
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand Down
22 changes: 11 additions & 11 deletions .github/workflows/image-push-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,30 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Docker meta
id: docker_meta
uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5
uses: docker/metadata-action@v5
with:
images: ghcr.io/${{ github.repository }}
flavor: |
latest=false

- name: Push container image for thick plugin
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand All @@ -51,30 +51,30 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Docker meta
id: docker_meta
uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5
uses: docker/metadata-action@v5
with:
images: ghcr.io/${{ github.repository }}
flavor: |
latest=false

- name: Push thin container image
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand All @@ -88,7 +88,7 @@ jobs:

- name: Push thin container debug image
if: ${{ github.repository_owner == env.image-push-owner }}
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand Down
14 changes: 5 additions & 9 deletions .github/workflows/kind-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ jobs:
github.event_name == 'pull_request' ) || (github.event_name == 'push' && github.event.commits != '[]' )
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
uses: actions/checkout@v4

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
uses: docker/setup-buildx-action@v3

- name: Setup python
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
uses: actions/setup-python@v5
with:
python-version: 3.x

Expand All @@ -49,7 +49,7 @@ jobs:
echo $(j2 --version)

- name: Build latest-amd64
uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5
uses: docker/build-push-action@v5
with:
context: .
load: true
Expand Down Expand Up @@ -85,10 +85,6 @@ jobs:
working-directory: ./e2e
run: ./test-default-route1.sh

- name: Test connection limit
working-directory: ./e2e
run: ./test-connection-limit.sh

# - name: Test DRA integration
# working-directory: ./e2e
# run: ./test-dra-integration.sh
Expand All @@ -112,7 +108,7 @@ jobs:

- name: Upload kind logs
if: always()
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
uses: actions/upload-artifact@v4
with:
name: kind-logs-${{ env.JOB_NAME }}-${{ github.run_id }}
path: /tmp/kind/logs
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/stale-issues-prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
stale:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@5bef64f19d7facfb25b37b414482c7164d639639 # v9
- uses: actions/stale@v9
with:
stale-issue-message: 'This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.'
stale-pr-message: 'This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.'
Expand Down
6 changes: 3 additions & 3 deletions Dockerfile.microshift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This dockerfile is specific to building Multus for MicroShift
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.26-openshift-5.0 AS builder
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS rhel9
ADD . /usr/src/multus-cni
WORKDIR /usr/src/multus-cni
ENV CGO_ENABLED=1
Expand All @@ -8,10 +8,10 @@ ENV VERSION=rhel9 COMMIT=unset
RUN ./hack/build-go.sh
WORKDIR /

FROM registry.ci.openshift.org/ocp/5.0:base-rhel9
FROM registry.ci.openshift.org/ocp/4.22:base-rhel9
RUN dnf install -y util-linux && dnf clean all && \
mkdir -p /usr/src/multus-cni/bin
COPY --from=builder \
COPY --from=rhel9 \
/usr/src/multus-cni/bin/thin_entrypoint \
/usr/src/multus-cni/bin/multus \
/usr/src/multus-cni/bin
Comment on lines +14 to 17

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files matching Dockerfile.microshift =="
git ls-files | grep -F 'Dockerfile.microshift' || true

echo
echo "== Dockerfile.microshift relevant lines =="
if [ -f Dockerfile.microshift ]; then
  nl -ba Dockerfile.microshift | sed -n '1,80p'
fi

echo
echo "== any multi-source COPY instructions in Dockerfile.microshift =="
rg -n "^(FROM|COPY|ARG|USER|WORKDIR|HEALTHCHECK|RUN)" Dockerfile.microshift || true

echo
echo "== compare COPY behavior for two sources with and without trailing slash in bash cp =="
tmp=$(mktemp -d)
mkdir -p "$tmp/src" "$tmp/dest"
touch "$tmp/src/a" "$tmp/src/b"
echo "with leading slash:"
cp "$tmp/src/a" "$tmp/src/b" "$tmp/src/bin" && ls -la "$tmp/src/bin"
rm -rf "$tmp/src/bin"
mkdir -p "$tmp/src/bin"
echo "with trailing slash:"
cp "$tmp/src/a" "$tmp/src/b" "$tmp/src/bin/" && ls -la "$tmp/src/bin"
rm -rf "$tmp"

Repository: openshift/multus-cni

Length of output: 313


🌐 Web query:

Docker COPY multiple sources destination must be directory ending with / Dockerfile COPY semantics

💡 Result:

In Docker, when the COPY instruction specifies multiple source files or directories (either directly or through the use of wildcards), the destination path must be a directory, and it must explicitly end with a trailing slash (/) [1][2][3]. This requirement ensures that Docker treats the destination as a directory to house the multiple items being copied [4]. If the destination does not end with a slash in this multi-source scenario, the build will fail [1][4]. Key points regarding COPY semantics with multiple sources: - Mandatory Trailing Slash: If multiple sources are provided, the destination argument must end with / to be interpreted as a directory [1][2]. - Behavior with Wildcards: The same rule applies when wildcards (e.g., *.txt) result in multiple files being matched [1][2]. - Single Source Behavior: If you specify only a single source file, the trailing slash is not strictly required, though it is still common practice to include it if you intend the destination to be a directory [2]. If the source is a file and the destination does not end in a slash, Docker will copy the file to the destination path as a regular file (renaming it if the path name differs) [2]. - Missing Directories: If the specified destination directory does not exist, Docker will create it along with any necessary parent directories [2][3].

Citations:


Make the multi-source COPY destination explicit.

This instruction copies two source files, so Docker expects the destination to be an existing directory. Add the trailing slash to avoid reliance on any build-time existing directory at /usr/src/multus-cni/bin.

Proposed fix
 COPY --from=rhel9 \
   /usr/src/multus-cni/bin/thin_entrypoint \
   /usr/src/multus-cni/bin/multus \
-  /usr/src/multus-cni/bin
+  /usr/src/multus-cni/bin/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
COPY --from=rhel9 \
/usr/src/multus-cni/bin/thin_entrypoint \
/usr/src/multus-cni/bin/multus \
/usr/src/multus-cni/bin
COPY --from=rhel9 \
/usr/src/multus-cni/bin/thin_entrypoint \
/usr/src/multus-cni/bin/multus \
/usr/src/multus-cni/bin/
🧰 Tools
🪛 Hadolint (2.14.0)

[error] 14-14: COPY with more than 2 arguments requires the last argument to end with /

(DL3021)

🤖 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 `@Dockerfile.microshift` around lines 14 - 17, Update the multi-source COPY
instruction in Dockerfile.microshift to use an explicit trailing slash on the
destination directory `/usr/src/multus-cni/bin/`, preserving both source files
unchanged.

Source: Linters/SAST tools

Expand Down
6 changes: 3 additions & 3 deletions Dockerfile.openshift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This dockerfile is specific to building Multus for OpenShift
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.26-openshift-5.0 AS builder
FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS rhel9
ADD . /usr/src/multus-cni
WORKDIR /usr/src/multus-cni
ENV CGO_ENABLED=1
Expand All @@ -9,11 +9,11 @@ RUN ./hack/build-go.sh && \
cd /usr/src/multus-cni/bin
WORKDIR /

FROM registry.ci.openshift.org/ocp/5.0:base-rhel9
FROM registry.ci.openshift.org/ocp/4.22:base-rhel9
RUN dnf install -y util-linux && dnf clean all && \
mkdir -p /usr/src/multus-cni/images && \
mkdir -p /usr/src/multus-cni/bin
COPY --from=builder /usr/src/multus-cni/bin /usr/src/multus-cni/bin
COPY --from=rhel9 /usr/src/multus-cni/bin /usr/src/multus-cni/bin
ADD ./images/entrypoint.sh /

LABEL io.k8s.display-name="Multus CNI" \
Expand Down
2 changes: 1 addition & 1 deletion cmd/cert-approver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ func (c *CertController) denyCSR(ctx context.Context, csr *certificatesv1.Certif
},
)

c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by %s: %s", csr.Name, ControllerName, message)
c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by: %s", csr.Name, ControllerName, message)

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Format string has 2 verbs but 3 args — message will render as a %!(EXTRA ...) artifact.

🐛 Proposed fix
-	c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by: %s", csr.Name, ControllerName, message)
+	c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by %s: %s", csr.Name, ControllerName, message)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by: %s", csr.Name, ControllerName, message)
c.recorder.Eventf(csr, corev1.EventTypeWarning, "CSRDenied", "The CSR %q has been denied by %s: %s", csr.Name, ControllerName, message)
🤖 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 `@cmd/cert-approver/main.go` at line 341, Fix the Eventf call in the CSR denial
handling by making its format string match the supplied arguments, ensuring the
denial message is intentionally included and no extra-argument formatting
artifact is produced. Update the call containing "CSRDenied" while preserving
the existing event type and context.

_, err := c.clientset.CertificatesV1().CertificateSigningRequests().Update(ctx, csr, metav1.UpdateOptions{})
return err
}
Expand Down
36 changes: 3 additions & 33 deletions cmd/multus-daemon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"io"
"net/http"
"net/http/pprof"
"os"
"os/signal"
"os/user"
Expand All @@ -31,7 +30,6 @@ import (
"syscall"
"time"

"golang.org/x/net/netutil"
utilwait "k8s.io/apimachinery/pkg/util/wait"

"gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/logging"
Expand Down Expand Up @@ -171,46 +169,18 @@ func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf,
}

if daemonConfig.MetricsPort != nil {
mux := http.NewServeMux()
mux.Handle("/metrics", promhttp.Handler())
if daemonConfig.EnablePprof != nil && *daemonConfig.EnablePprof {
mux.HandleFunc("/debug/pprof/", pprof.Index)
mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
logging.Verbosef("pprof endpoints enabled on metrics port %d", *daemonConfig.MetricsPort)
}
metricsSrv := &http.Server{
Addr: fmt.Sprintf(":%d", *daemonConfig.MetricsPort),
Handler: mux,
ReadHeaderTimeout: 10 * time.Second,
}
logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
go utilwait.UntilWithContext(ctx, func(_ context.Context) {
if err := metricsSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
logging.Debugf("metrics server error: %v", err)
}
http.Handle("/metrics", promhttp.Handler())
logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
logging.Debugf("metrics: %s", http.ListenAndServe(fmt.Sprintf(":%d", *daemonConfig.MetricsPort), nil))
}, 0)
go func() {
<-ctx.Done()
metricsSrv.Shutdown(context.Background())
}()
}
Comment on lines 171 to 177

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.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

http.Handle inside a retried loop will panic if the metrics listener ever restarts.

http.Handle registers /metrics on the global http.DefaultServeMux, which panics on a duplicate pattern registration. utilwait.UntilWithContext(ctx, f, 0) re-invokes f (and thus re-calls http.Handle) every time http.ListenAndServe returns an error (e.g. port already in use, transient bind failure). The second invocation will panic on "http: multiple registrations for /metrics" instead of retrying gracefully. This also drops the previous graceful-shutdown wiring (ctx.Done()Shutdown()) for the metrics server.

🐛 Proposed fix: register once, use a dedicated mux, and support shutdown
 	if daemonConfig.MetricsPort != nil {
-		go utilwait.UntilWithContext(ctx, func(_ context.Context) {
-			http.Handle("/metrics", promhttp.Handler())
-			logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
-			logging.Debugf("metrics: %s", http.ListenAndServe(fmt.Sprintf(":%d", *daemonConfig.MetricsPort), nil))
-		}, 0)
+		metricsMux := http.NewServeMux()
+		metricsMux.Handle("/metrics", promhttp.Handler())
+		metricsSrv := &http.Server{Addr: fmt.Sprintf(":%d", *daemonConfig.MetricsPort), Handler: metricsMux}
+		go func() {
+			logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
+			if err := metricsSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+				logging.Errorf("metrics server error: %v", err)
+			}
+		}()
+		go func() {
+			<-ctx.Done()
+			_ = metricsSrv.Shutdown(context.Background())
+		}()
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if daemonConfig.MetricsPort != nil {
mux := http.NewServeMux()
mux.Handle("/metrics", promhttp.Handler())
if daemonConfig.EnablePprof != nil && *daemonConfig.EnablePprof {
mux.HandleFunc("/debug/pprof/", pprof.Index)
mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
logging.Verbosef("pprof endpoints enabled on metrics port %d", *daemonConfig.MetricsPort)
}
metricsSrv := &http.Server{
Addr: fmt.Sprintf(":%d", *daemonConfig.MetricsPort),
Handler: mux,
ReadHeaderTimeout: 10 * time.Second,
}
logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
go utilwait.UntilWithContext(ctx, func(_ context.Context) {
if err := metricsSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
logging.Debugf("metrics server error: %v", err)
}
http.Handle("/metrics", promhttp.Handler())
logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
logging.Debugf("metrics: %s", http.ListenAndServe(fmt.Sprintf(":%d", *daemonConfig.MetricsPort), nil))
}, 0)
go func() {
<-ctx.Done()
metricsSrv.Shutdown(context.Background())
}()
}
if daemonConfig.MetricsPort != nil {
metricsMux := http.NewServeMux()
metricsMux.Handle("/metrics", promhttp.Handler())
metricsSrv := &http.Server{Addr: fmt.Sprintf(":%d", *daemonConfig.MetricsPort), Handler: metricsMux}
go func() {
logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
if err := metricsSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
logging.Errorf("metrics server error: %v", err)
}
}()
go func() {
<-ctx.Done()
_ = metricsSrv.Shutdown(context.Background())
}()
}
🤖 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 `@cmd/multus-daemon/main.go` around lines 171 - 177, Update the metrics-serving
block around utilwait.UntilWithContext so the /metrics handler is registered
only once on a dedicated ServeMux, avoiding repeated global http.Handle
registration across retries. Preserve retry behavior while wiring the metrics
server to ctx cancellation so shutdown is graceful via the server’s Shutdown
method.


l, err := srv.GetListener(api.SocketPath(daemonConfig.SocketDir))
if err != nil {
return fmt.Errorf("failed to start the CNI server using socket %s. Reason: %+v", api.SocketPath(daemonConfig.SocketDir), err)
}

if limit := daemonConfig.ConnectionLimit; limit != nil {
if *limit <= 0 {
return fmt.Errorf("connection limit must be greater than 0, got %d", *limit)
}
logging.Debugf("connection limit: %d", *limit)
l = netutil.LimitListener(l, *limit)
}

server.Start(ctx, l)

go func() {
Expand Down
14 changes: 1 addition & 13 deletions deployments/multus-daemonset-crio.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ spec:
singular: network-attachment-definition
kind: NetworkAttachmentDefinition
shortNames:
- nad
- net-attach-def
- net-attach-def
versions:
- name: v1
served: true
Expand Down Expand Up @@ -70,18 +69,7 @@ rules:
- pods/status
verbs:
- get
- list
- update
- watch
- apiGroups:
- "resource.k8s.io"
resources:
- resourceclaims
- resourceclaims/status
- resourceslices
verbs:
- get
- list
- apiGroups:
- ""
- events.k8s.io
Expand Down
10 changes: 0 additions & 10 deletions deployments/multus-daemonset-thick.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ spec:
singular: network-attachment-definition
kind: NetworkAttachmentDefinition
shortNames:
- nad
- net-attach-def
versions:
- name: v1
Expand Down Expand Up @@ -73,15 +72,6 @@ rules:
- list
- update
- watch
- apiGroups:
- "resource.k8s.io"
resources:
- resourceclaims
- resourceclaims/status
- resourceslices
verbs:
- get
- list
- apiGroups:
- ""
- events.k8s.io
Expand Down
Loading