Merge pull request #1528 from schsebabot/devtask-b25fc0a0-create-coderabbityaml-for-multus-cni

Create .coderabbit.yaml for multus-cni

.coderabbit.yaml

@@ -0,0 +1,304 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+language: en-US
+early_access: false
+enable_free_tier: true
+
+reviews:
+  profile: chill
+  request_changes_workflow: false
+  high_level_summary: true
+  poem: false
+  in_progress_fortune: false
+  review_status: true
+  collapse_walkthrough: false
+
+  path_filters:
+    - "!**/vendor/**"
+    - "!vendor/**"
+    - "!go.sum"
+
+  auto_review:
+    enabled: true
+    drafts: true
+
+  # ── Path instructions ────────────────────────────────────────
+  path_instructions:
+
+    # ── Go code — general ────────────────────────────────────────
+    - path: "**/*.go"
+      instructions: |
+        Go security and best practices:
+        - Never ignore error returns; wrap errors with context using fmt.Errorf("...: %w", err)
+        - Use stdlib crypto/* and golang.org/x/crypto; avoid third-party crypto libraries
+        - Integer overflow: bounds-check user-supplied sizes
+        - context.Context for cancellation and timeouts on all network/gRPC operations
+        - Nil pointer checks on CNI args, NetConf, and DelegateNetConf structs
+          before accessing fields — CNI runtime data can be incomplete
+        - Use structured logging (klog v1/v2) with appropriate verbosity levels
+        - Network namespace operations require careful error handling —
+          always restore the original namespace on error paths
+        - CNI handler functions (cmdAdd, cmdDel, cmdCheck) may return
+          contextual or wrapped Go errors; the skel.PluginMainFuncs
+          dispatcher handles CNI JSON error serialization
+
+    # ── Core multus logic ────────────────────────────────────────
+    - path: "pkg/multus/**/*.go"
+      instructions: |
+        Core CNI plugin logic review:
+        - Verify CNI spec compliance across supported versions (0.2.0, 0.4.0, 1.0.0)
+        - Delegate plugin invocation must handle failures gracefully —
+          rollback already-added interfaces if a later delegate fails in cmdAdd
+        - Network selection from annotations must validate the annotation format
+          and handle malformed NetworkSelectionElement entries
+        - Network namespace operations: always defer namespace restoration
+          to prevent leaving the process in the wrong namespace on error
+        - Check that the default network delegate is always invoked first
+        - Verify proper handling of prevResult for chained CNI plugins
+        - Resource (device) allocation tracking via checkpoint must be atomic
+
+    # ── Thick plugin server ──────────────────────────────────────
+    - path: "pkg/server/**/*.go"
+      instructions: |
+        Thick plugin daemon and gRPC server review:
+        - gRPC server must handle concurrent requests safely
+        - Socket file cleanup on shutdown: verify the Unix domain socket
+          is removed on graceful and ungraceful termination
+        - Configuration file watching (fsnotify): verify events are debounced
+          and do not cause config reload storms
+        - TLS/certificate handling for metrics endpoint: verify proper rotation
+        - Server configuration must be validated at startup, not at request time
+        - API compatibility between shim and daemon must be maintained —
+          check that request/response types match across versions
+        - Health check and readiness endpoints should reflect actual daemon state
+
+    # ── Kubernetes client ────────────────────────────────────────
+    - path: "pkg/k8sclient/**/*.go"
+      instructions: |
+        Kubernetes client review:
+        - Kubeconfig file handling: verify secure file permissions (0600)
+          and no secrets logged
+        - Network Attachment Definition (NAD) lookups must handle not-found
+          errors distinctly from other API errors
+        - Client caching: verify informer/lister patterns are used correctly
+          to avoid excessive API server load
+        - Namespace scoping: ensure NAD lookups respect the pod's namespace
+          and the default network namespace
+
+    # ── Types and configuration ──────────────────────────────────
+    - path: "pkg/types/**/*.go"
+      instructions: |
+        Type definitions and configuration review:
+        - NetConf changes must maintain backward compatibility with existing
+          CNI configuration files — new fields should have sensible zero values
+        - NetworkSelectionElement: verify JSON tag naming follows CNI conventions
+        - DelegateNetConf: ensure all delegate plugin configuration fields
+          are properly forwarded without loss
+        - Validate that type changes are reflected in all serialization paths
+          (JSON marshaling/unmarshaling)
+
+    # ── Network utilities ────────────────────────────────────────
+    - path: "pkg/netutils/**/*.go"
+      instructions: |
+        Network utility review:
+        - Network namespace operations are privileged — verify proper
+          error handling and namespace restoration on all code paths
+        - Interface naming must comply with Linux kernel limits (15 chars max)
+        - Device operations (link set, address set) via netlink must handle
+          EBUSY and other transient errors with appropriate retries
+        - Verify no file descriptor leaks when opening network namespaces
+
+    # ── DRA and kubelet client ───────────────────────────────────
+    - path: "pkg/{draclient,kubeletclient}/**/*.go"
+      instructions: |
+        Dynamic Resource Allocation and kubelet client review:
+        - gRPC connections to kubelet must use proper timeouts and retry logic
+        - PodResources API responses may be incomplete — handle nil/empty
+          device lists gracefully
+        - DRA resource claims must be validated before use
+        - Connection errors should be distinguished from data errors
+          for proper error reporting
+
+    # ── Command binaries ─────────────────────────────────────────
+    - path: "cmd/**/*.go"
+      instructions: |
+        Binary entry point review:
+        - CNI plugin binaries (multus, multus-shim, passthru-cni) must follow
+          the CNI specification for stdin/stdout/stderr handling
+        - Daemon binary (multus-daemon) must handle signals (SIGTERM, SIGINT)
+          for graceful shutdown including socket cleanup
+        - Installer binary must validate target paths and handle permission
+          errors when copying binaries to host CNI directories
+        - Entry point binaries must not panic — recover and return proper
+          error codes/messages
+
+    # ── Kubernetes/OpenShift manifests ────────────────────────────
+    - path: "**/*.{yaml,yml}"
+      instructions: |
+        If this is a Kubernetes/OpenShift manifest:
+        - securityContext: runAsNonRoot, readOnlyRootFilesystem,
+          allowPrivilegeEscalation: false where applicable
+        - Drop ALL capabilities, add only what is required
+        - Resource limits (cpu, memory) on every container
+        - RBAC: least privilege; no cluster-admin for workloads
+        - Liveness + readiness probes defined
+        - automountServiceAccountToken: false unless needed
+
+        Multus-specific manifest review:
+        - DaemonSet manifests: verify volume mounts for CNI bin directory
+          (/opt/cni/bin) and CNI config directory (/etc/cni/net.d) are correct
+        - Thick plugin DaemonSet: verify Unix socket mount for daemon communication
+        - Host path volumes require proper mount propagation settings
+        - Init containers for CNI binary installation must complete before
+          the main container starts
+        - CRI-O specific manifests: verify socket path differences from containerd
+
+    # ── Deployment manifests ─────────────────────────────────────
+    - path: "deployments/**/*.{yaml,yml}"
+      instructions: |
+        Deployment manifest review:
+        - Three deployment modes: thin (multus-daemonset.yml),
+          thick (multus-daemonset-thick.yml), CRI-O (multus-daemonset-crio.yml)
+        - Verify consistency across all three manifests for shared configuration
+        - DaemonSet update strategy should be RollingUpdate with appropriate
+          maxUnavailable settings for CNI plugins (node disruption impact)
+        - ServiceAccount and RBAC must allow reading NetworkAttachmentDefinitions
+          and pods but nothing more
+        - Tolerations should include control-plane/master taints for full coverage
+
+    # ── Dockerfiles / container images ───────────────────────────
+    - path: "images/{Dockerfile,Dockerfile.*}"
+      instructions: |
+        Container image security:
+        - Multi-stage builds preferred; no build tools in final image
+        - USER non-root where possible (note: CNI plugins may need root for
+          network namespace operations)
+        - COPY specific files, not entire context
+        - No secrets in ENV, ARG, or COPY
+        - No package manager cache in final layer
+        - This project has 4 container images:
+          Dockerfile (thin plugin, distroless base),
+          Dockerfile.thick (thick plugin, Debian slim),
+          Dockerfile.debug (debug, distroless debug base),
+          Dockerfile.openshift (OpenShift/OKD build)
+        - Verify ENTRYPOINT matches the intended binary for each image variant
+
+    # ── E2E tests ────────────────────────────────────────────────
+    - path: "e2e/**/*.sh"
+      instructions: |
+        E2E test scripts review (shell-based, Kind clusters):
+        - Scripts must use 'set -e' for fail-fast behavior
+        - Cleanup of Kind clusters and resources on test failure (trap handlers)
+        - kubectl commands should use explicit timeouts and --wait flags
+        - Network connectivity tests should retry with backoff for pod readiness
+        - Verify tests cover both thin and thick plugin deployment modes
+        - Template generation (j2cli/sed) must produce valid YAML
+        - Check for hardcoded image tags that should be parameterized
+
+    # ── Unit tests ───────────────────────────────────────────────
+    - path: "{cmd,pkg}/**/*_test.go"
+      instructions: |
+        Unit test quality (Ginkgo v2 + Gomega):
+        - Test names must be stable and deterministic — no dynamic values
+          in It/Describe/Context titles
+        - CNI version-specific tests (020, 040, 100): verify coverage
+          across all supported CNI spec versions
+        - Mock external dependencies (Kubernetes API, network namespace ops)
+          rather than requiring real cluster or root access
+        - Test error paths, not just happy paths
+        - Network namespace tests may require sudo — verify CI compatibility
+        - Assertions should include meaningful failure messages
+        - Verify test cleanup of any created network namespaces or interfaces
+
+    # ── Hack/build scripts ───────────────────────────────────────
+    - path: "hack/**/*.sh"
+      instructions: |
+        Build and test scripts review:
+        - Scripts must be POSIX-compatible or explicitly require bash
+        - build-go.sh: verify cross-compilation settings (GOOS, GOARCH)
+          produce correct binaries for all supported platforms
+          (386, amd64, arm, arm64, ppc64le, s390x)
+        - test-go.sh: verify race detector and coverage flags are correct
+        - Scripts should exit with appropriate error codes on failure
+
+    # ── Supply chain & dependencies ──────────────────────────────
+    - path: "**/go.mod"
+      instructions: |
+        Supply chain security:
+        - New dependencies: justify the need, check license compatibility
+        - Pin exact versions; verify no unnecessary indirect dependencies added
+        - Flag known CVEs (cross-ref osv.dev)
+        - Check for replace directives pointing to forks — ensure they are
+          intentional and documented
+        - This project uses gopkg.in import path (gopkg.in/k8snetworkplumbingwg/multus-cni.v4)
+          — verify import path consistency
+        - Key dependencies: containernetworking/cni, containernetworking/plugins,
+          k8snetworkplumbingwg/network-attachment-definition-client,
+          vishvananda/netlink, k8s.io/api, k8s.io/client-go, k8s.io/kubelet
+
+    # ── CI/CD & GitHub Actions ───────────────────────────────────
+    - path: ".github/workflows/**/*"
+      instructions: |
+        CI/CD security:
+        - Pin actions by full SHA, not tag
+        - No secrets in logs; mask sensitive outputs
+        - Least privilege: minimize GITHUB_TOKEN permissions
+        - No pull_request_target with checkout of PR head
+        - Verify test.yml includes Revive lint, go fmt, go vet, and unit tests
+        - Build workflow must cover all architectures:
+          386, amd64, arm, arm64, ppc64le, s390x
+        - Image build workflow: verify Trivy scan is included
+        - Kind E2E workflow: verify both thin and thick plugin modes are tested
+
+    # ── Cryptography files ───────────────────────────────────────
+    - path: "**/*{crypt,cipher,sign,hash,tls,ssl,cert,key,token}*"
+      instructions: |
+        Cryptographic security:
+        - Banned: MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode
+        - Symmetric: AES-256-GCM or ChaCha20-Poly1305
+        - Signing: Ed25519 or ECDSA P-256+
+        - Constant-time comparison for all secret/token data
+        - No custom crypto; use vetted libraries only
+        - Certificate generation and approval (cert-approver, kubeconfig_generator):
+          verify minimum TLS version (1.2+) and proper certificate rotation
+
+    # ── GoReleaser configuration ─────────────────────────────────
+    - path: ".goreleaser.yml"
+      instructions: |
+        GoReleaser configuration review:
+        - Verify build targets match the supported architectures
+        - Check that binary names match the expected CNI plugin names
+        - Verify release artifacts include all necessary binaries
+
+  # ── Security scanners ────────────────────────────────────────
+  tools:
+    gitleaks:
+      enabled: true
+    semgrep:
+      enabled: true
+    checkov:
+      enabled: true
+    hadolint:
+      enabled: true
+    trivy:
+      enabled: true
+    osvScanner:
+      enabled: true
+    actionlint:
+      enabled: true
+    ast-grep:
+      essential_rules: true
+
+# ── Knowledge base ───────────────────────────────────────────
+knowledge_base:
+  code_guidelines:
+    enabled: true
+    filePatterns:
+      - "**/CONTRIBUTING.md"
+  issues:
+    scope: "auto"
+  pull_requests:
+    scope: "auto"
+
+chat:
+  auto_reply: true
+  art: false