test/e2e/registry.Dockerfile (1)

1-1: Pin the base image by digest for reproducible and safer e2e builds.

registry:2.8.3 is valid, but tag-only pinning is mutable. Prefer immutable digest pinning to avoid unexpected upstream changes in CI/e2e environments.

🔒 Suggested hardening change

-FROM registry:2.8.3
+FROM registry:2.8.3@sha256:<resolved_digest>

You can verify and resolve the exact digest for registry:2.8.3 from your container registry metadata before updating.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/registry.Dockerfile` at line 1, Replace the mutable base image tag
in the Dockerfile by pinning the registry image to its immutable digest: locate
the FROM instruction in test/e2e/registry.Dockerfile (the line containing "FROM
registry:2.8.3"), look up the corresponding sha256 digest for registry:2.8.3
from the registry metadata or Docker Hub, and update the FROM to use the
image@sha256:<digest> form so CI/e2e builds use an immutable, reproducible base.

test/e2e/mcp_header_proxy.py (2)

87-91: Broad exception handling is acceptable here but could be narrowed.

The except Exception catch is flagged by Ruff (BLE001). While acceptable for a test utility's best-effort error path, you could narrow this to specific exceptions for cleaner diagnostics:

♻️ Optional: Narrow exception types

-        except Exception as exc:  # pragma: no cover - best effort diagnostic path
+        except (OSError, http.client.HTTPException) as exc:  # pragma: no cover - best effort diagnostic path
             self.send_error(502, f"upstream request failed: {exc}")
             return

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/mcp_header_proxy.py` around lines 87 - 91, The broad except
Exception in the upstream request handler should be narrowed to the relevant
error types to satisfy Ruff BLE001 and provide clearer diagnostics: replace the
generic except with specific exceptions (e.g., requests.RequestException /
urllib3 exceptions, socket.error or OSError, and TimeoutError as appropriate for
the client you're using) around the block that calls the upstream and calls
self.send_error and conn.close; ensure the same error message is passed to
self.send_error and keep the finally conn.close() behavior intact so resources
are still released.

---

76-77: Consider adding a timeout parameter for upstream connections.

The hardcoded 30-second timeout is reasonable for e2e tests, but making it configurable via CLI args would improve flexibility for debugging slow environments.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/mcp_header_proxy.py` around lines 76 - 77, The code currently
hardcodes a 30s timeout when creating the upstream connection (conn_class and
conn); make this timeout configurable via a CLI/test runner argument: add a new
parsed argument (e.g., --upstream-timeout or timeout_seconds) with a default of
30 and help text, read it into the same config/variable used in the module, and
replace the literal 30 in the conn = conn_class(..., timeout=30) call with that
variable so conn_class and conn use the CLI-provided timeout at runtime.

test/e2e/kind.sh (1)

896-948: Consider extracting duplicated Python post() function.

The post() function is defined identically in three separate embedded Python scripts (lines 908-921, 985-998, 1071-1084). This duplication makes maintenance harder and increases the risk of drift.

Consider extracting to a shared Python module in test/e2e/ that can be imported by the inline scripts, or consolidating the validation logic into a single Python script invoked with parameters.

Also applies to: 973-1018, 1059-1103

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/kind.sh` around lines 896 - 948, The inline Python helper post() is
duplicated across multiple embedded scripts (the post function used around the
initialize/notifications/ tools/call sequences); extract post() into a shared
helper module (e.g., test/e2e/mcp_client.py) and change the embedded Python
blocks to import and call post() (or consolidate the three validation blocks
into a single script that accepts parameters) so that the logic in post() is
defined once and reused to avoid drift.

internal/cli/setup.go (1)

1720-1726: Regex-based imagePullPolicy replacement is fragile.

The regex (?m)^(\s*imagePullPolicy:\s*)\S+ replaces all imagePullPolicy fields in the manifest. Per the context snippet from config/manager/manager.yaml, there's currently only one container, but if init containers or sidecars are added later, this could incorrectly modify unintended policies.

Consider targeting only the manager container's imagePullPolicy, or using YAML parsing for more precise modification.

♻️ Suggested improvement using targeted replacement

 func injectOperatorImagePullPolicy(yamlContent, pullPolicy string) string {
 	if strings.TrimSpace(pullPolicy) == "" {
 		return yamlContent
 	}
-	pullPolicyPattern := regexp.MustCompile(`(?m)^(\s*imagePullPolicy:\s*)\S+`)
-	return pullPolicyPattern.ReplaceAllString(yamlContent, "${1}"+pullPolicy)
+	// Replace only the first occurrence to target the manager container
+	pullPolicyPattern := regexp.MustCompile(`(?m)^(\s*imagePullPolicy:\s*)\S+`)
+	return pullPolicyPattern.ReplaceAllStringFunc(yamlContent, func(match string) string {
+		return pullPolicyPattern.ReplaceAllString(match, "${1}"+pullPolicy)
+	})
 }

Or use strings.Replace with n=1 for first occurrence only:

func injectOperatorImagePullPolicy(yamlContent, pullPolicy string) string {
	if strings.TrimSpace(pullPolicy) == "" {
		return yamlContent
	}
	return regexp.MustCompile(`(?m)^(\s*imagePullPolicy:\s*)\S+`).ReplaceAllStringFunc(yamlContent, func(match string) string {
		// Only replace once
		return strings.Replace(yamlContent, match, strings.TrimSuffix(match, strings.Fields(match)[1])+pullPolicy, 1)
	})
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/cli/setup.go` around lines 1720 - 1726, The current
injectOperatorImagePullPolicy function uses a global regex that will replace
every imagePullPolicy in the manifest; instead parse the YAML and set only the
manager container's imagePullPolicy (or init/sidecars named "manager" if
applicable). Update injectOperatorImagePullPolicy to unmarshal the YAML
(yaml.v3), locate .spec.template.spec.containers (and .initContainers), find the
container with name "manager", set its imagePullPolicy to the given value, then
marshal the document(s) back to YAML; if you prefer a minimal change, restrict
the replacement to the first match only by replacing the first regex match
rather than all matches, but the recommended fix is explicit YAML parsing and
targeted modification of the manager container.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes