diff --git a/.agentready/assessment-20260512-091921.json b/.agentready/assessment-20260512-091921.json new file mode 100644 index 00000000..9523d2d6 --- /dev/null +++ b/.agentready/assessment-20260512-091921.json @@ -0,0 +1,1151 @@ +{ + "schema_version": "1.0.0", + "metadata": { + "agentready_version": "2.35.2", + "research_version": "2.0.0", + "assessment_timestamp": "2026-05-12T09:19:21.586698", + "assessment_timestamp_human": "May 12, 2026 at 9:19 AM", + "executed_by": "rhuss@rhuss-mac", + "command": "/Users/rhuss/.cache/uv/archive-v0/-sEsvpNoWBP_t22eeb_V-/bin/agentready -- assess .", + "working_directory": "/Users/rhuss/Development/ai/kagenti-operator" + }, + "repository": { + "path": "/Users/rhuss/Development/ai/kagenti-operator", + "name": "kagenti-operator", + "url": "git@github.com:rhuss/kagenti-operator.git", + "branch": "main", + "commit_hash": "01f875a573b9cfc13eacc29e4604c71def861880", + "languages": { + "Markdown": 37, + "YAML": 82, + "Shell": 12, + "Go": 97 + }, + "total_files": 246, + "total_lines": 46081 + }, + "timestamp": "2026-05-12T09:19:21.586698", + "overall_score": 43.8, + "certification_level": "Bronze", + "attributes_assessed": 20, + "attributes_skipped": 13, + "attributes_total": 33, + "findings": [ + { + "attribute": { + "id": "test_execution", + "name": "Test Execution & Coverage", + "category": "Testing & CI/CD", + "tier": 1, + "description": "Single-command test runner with adequate coverage configuration", + "criteria": "Runnable tests with coverage config", + "default_weight": 0.1 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "type_annotations", + "name": "Type Annotations", + "category": "Code Quality", + "tier": 1, + "description": "Type hints in function signatures", + "criteria": ">80% of functions have type annotations", + "default_weight": 0.08 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Type annotation check not implemented for ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "claude_md_file", + "name": "CLAUDE.md Configuration Files", + "category": "Context Window Optimization", + "tier": 1, + "description": "Project-specific configuration for AI coding agents", + "criteria": "CLAUDE.md or AGENTS.md file exists in repository root", + "default_weight": 0.07 + }, + "status": "pass", + "score": 100.0, + "measured_value": "present", + "threshold": "present", + "evidence": [ + "CLAUDE.md found at /Users/rhuss/Development/ai/kagenti-operator/CLAUDE.md" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "ci_quality_gates", + "name": "CI Quality Gates", + "category": "Testing & CI/CD", + "tier": 1, + "description": "CI runs lint, type-check, and tests on every PR", + "criteria": "CI gates with lint + type-check + tests", + "default_weight": 0.05 + }, + "status": "fail", + "score": 75, + "measured_value": "missing quality gates", + "threshold": "CI with lint + test + type-check gates on PRs", + "evidence": [ + "CI config found: .github/workflows/release.yml, .github/workflows/project.yml, .github/workflows/pr-verifier.yml, .github/workflows/self-assign.yml, .github/workflows/ci.yaml, .github/workflows/stale.yaml, .github/workflows/security-scans.yaml, .github/workflows/scorecard.yaml", + "Lint gate detected in CI", + "No test gate found in CI", + "No type-check gate found in CI", + "Descriptive job/step names found", + "Parallel job execution detected", + "Config includes comments", + "Artifacts uploaded" + ], + "remediation": { + "summary": "Add or improve CI/CD pipeline configuration", + "steps": [ + "Create CI config for your platform (GitHub Actions, GitLab CI, etc.)", + "Define jobs: lint, test, build", + "Use descriptive job and step names", + "Configure dependency caching", + "Enable parallel job execution", + "Upload artifacts: test results, coverage reports", + "Add status badge to README" + ], + "tools": [ + "github-actions", + "gitlab-ci", + "circleci" + ], + "commands": [ + "# Create GitHub Actions workflow", + "mkdir -p .github/workflows", + "touch .github/workflows/ci.yml", + "", + "# Validate workflow", + "gh workflow view ci.yml" + ], + "examples": [ + "# .github/workflows/ci.yml - Good example\n\nname: CI Pipeline\n\non:\n push:\n branches: [main]\n pull_request:\n branches: [main]\n\njobs:\n lint:\n name: Lint Code\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n\n - name: Set up Python\n uses: actions/setup-python@v5\n with:\n python-version: '3.11'\n cache: 'pip' # Caching\n\n - name: Install dependencies\n run: pip install -r requirements.txt\n\n - name: Run linters\n run: |\n black --check .\n isort --check .\n ruff check .\n\n test:\n name: Run Tests\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n\n - name: Set up Python\n uses: actions/setup-python@v5\n with:\n python-version: '3.11'\n cache: 'pip'\n\n - name: Install dependencies\n run: pip install -r requirements.txt\n\n - name: Run tests with coverage\n run: pytest --cov --cov-report=xml\n\n - name: Upload coverage reports\n uses: codecov/codecov-action@v3\n with:\n files: ./coverage.xml\n\n build:\n name: Build Package\n runs-on: ubuntu-latest\n needs: [lint, test] # Runs after lint/test pass\n steps:\n - uses: actions/checkout@v4\n\n - name: Build package\n run: python -m build\n\n - name: Upload build artifacts\n uses: actions/upload-artifact@v3\n with:\n name: dist\n path: dist/\n" + ], + "citations": [ + { + "source": "GitHub", + "title": "GitHub Actions Documentation", + "url": "https://docs.github.com/en/actions", + "relevance": "Official GitHub Actions guide" + }, + { + "source": "CircleCI", + "title": "CI/CD Best Practices", + "url": "https://circleci.com/blog/ci-cd-best-practices/", + "relevance": "Industry best practices for CI/CD" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "single_file_verification", + "name": "Single-File Verification", + "category": "Verification & Feedback Loops", + "tier": 1, + "description": "Single-file lint and type-check commands available for fast feedback", + "criteria": "Documented single-file lint/type-check commands", + "default_weight": 0.05 + }, + "status": "fail", + "score": 0.0, + "measured_value": "not documented", + "threshold": "single-file lint + type-check commands documented", + "evidence": [ + "No single-file verification commands found in context files" + ], + "remediation": { + "summary": "Document single-file lint and type-check commands in CLAUDE.md/AGENTS.md", + "steps": [ + "Add single-file lint command to context file (e.g., 'ruff check path/to/file.py')", + "Add single-file type-check command (e.g., 'mypy path/to/file.py')", + "Ensure these commands work without a full build step", + "Target <5 seconds execution per file" + ], + "tools": [ + "ruff", + "eslint", + "mypy", + "pyright", + "tsc" + ], + "commands": [ + "# Python", + "ruff check path/to/file.py", + "mypy path/to/file.py", + "", + "# JavaScript/TypeScript", + "npx eslint path/to/file.ts", + "npx tsc --noEmit path/to/file.ts" + ], + "examples": [], + "citations": [] + }, + "error_message": null + }, + { + "attribute": { + "id": "readme_structure", + "name": "README Structure", + "category": "Documentation Standards", + "tier": 1, + "description": "Well-structured README with key sections", + "criteria": "README.md with installation, usage, and development sections", + "default_weight": 0.05 + }, + "status": "pass", + "score": 100.0, + "measured_value": "3/3 sections", + "threshold": "3/3 sections", + "evidence": [ + "Found 3/3 essential sections", + "Installation: \u2713", + "Usage: \u2713", + "Development: \u2713" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "standard_layout", + "name": "Standard Project Layouts", + "category": "Repository Structure", + "tier": 1, + "description": "Follows standard project structure for language", + "criteria": "Standard directories (src/ or project-named, tests/) present", + "default_weight": 0.05 + }, + "status": "fail", + "score": 0.0, + "measured_value": "0/2 directories", + "threshold": "2/2 directories", + "evidence": [ + "Found 0/2 standard directories", + "source directory: \u2717 (no src/ or project-named dir)", + "tests/: \u2717" + ], + "remediation": { + "summary": "Organize code into standard directories", + "steps": [ + "Create a source directory for your code", + "Option A: Use src/ layout (recommended for packages)", + "Option B: Use project-named directory (e.g., mypackage/)", + "Ensure your package has __init__.py", + "Create tests/ directory for test files", + "Add at least one test file" + ], + "tools": [], + "commands": [ + "# Option A: src layout", + "mkdir -p src/mypackage", + "touch src/mypackage/__init__.py", + "# ---", + "# Option B: flat layout (project-named)", + "mkdir -p mypackage", + "touch mypackage/__init__.py", + "# Create tests directory", + "mkdir -p tests", + "touch tests/__init__.py", + "touch tests/test_example.py" + ], + "examples": [ + "# src layout (recommended for distributable packages)\nproject/\n\u251c\u2500\u2500 src/\n\u2502 \u2514\u2500\u2500 mypackage/\n\u2502 \u251c\u2500\u2500 __init__.py\n\u2502 \u2514\u2500\u2500 module.py\n\u251c\u2500\u2500 tests/\n\u2502 \u2514\u2500\u2500 test_module.py\n\u2514\u2500\u2500 pyproject.toml\n\n# flat layout (common in major projects like pandas, numpy)\nproject/\n\u251c\u2500\u2500 mypackage/\n\u2502 \u251c\u2500\u2500 __init__.py\n\u2502 \u2514\u2500\u2500 module.py\n\u251c\u2500\u2500 tests/\n\u2502 \u2514\u2500\u2500 test_module.py\n\u2514\u2500\u2500 pyproject.toml\n" + ], + "citations": [ + { + "source": "Python Packaging Authority", + "title": "src layout vs flat layout", + "url": "https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/", + "relevance": "Official guidance on Python project layouts" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "lock_files", + "name": "Dependency Pinning for Reproducibility", + "category": "Dependency Management", + "tier": 1, + "description": "Dependencies pinned to exact versions in lock files", + "criteria": "Lock file with pinned versions, updated within 6 months", + "default_weight": 0.05 + }, + "status": "fail", + "score": 0.0, + "measured_value": "none", + "threshold": "lock file with pinned versions", + "evidence": [ + "No dependency lock files found" + ], + "remediation": { + "summary": "Add lock file for dependency reproducibility", + "steps": [ + "For npm: run 'npm install' (generates package-lock.json)", + "For Python: use 'pip freeze > requirements.txt' or poetry", + "For Ruby: run 'bundle install' (generates Gemfile.lock)" + ], + "tools": [ + "npm", + "pip", + "poetry", + "bundler" + ], + "commands": [ + "npm install # npm", + "pip freeze > requirements.txt # Python", + "poetry lock # Python with Poetry" + ], + "examples": [], + "citations": [] + }, + "error_message": null + }, + { + "attribute": { + "id": "dependency_security", + "name": "Dependency Security & Vulnerability Scanning", + "category": "Security", + "tier": 1, + "description": "Security scanning tools configured for dependencies and code", + "criteria": "Dependabot, Renovate, CodeQL, or SAST tools configured; secret detection enabled", + "default_weight": 0.05 + }, + "status": "fail", + "score": 5, + "measured_value": "No security scanning tools configured", + "threshold": "\u226560 points (Dependabot/Renovate + SAST or multiple scanners)", + "evidence": [ + "\u2713 SECURITY.md present (vulnerability disclosure policy)" + ], + "remediation": { + "summary": "Configure security scanning for dependencies and code", + "steps": [ + "Enable Dependabot in GitHub repository settings", + "Add .github/dependabot.yml configuration file", + "Or configure Renovate: add renovate.json to repository root", + "Set up CodeQL scanning for SAST", + "Add secret detection to pre-commit hooks", + "Configure language-specific security scanners" + ], + "tools": [ + "Dependabot", + "Renovate", + "CodeQL", + "detect-secrets", + "Bandit", + "Semgrep" + ], + "commands": [ + "gh repo edit --enable-security", + "pip install pre-commit detect-secrets", + "pre-commit install" + ], + "examples": [ + "# .github/dependabot.yml\nversion: 2\nupdates:\n - package-ecosystem: pip\n directory: /\n schedule:\n interval: weekly", + "# renovate.json\n{\n \"extends\": [\"config:base\"],\n \"schedule\": \"after 10pm every weekday\"\n}", + "# .pre-commit-config.yaml\nrepos:\n - repo: https://github.com/Yelp/detect-secrets\n rev: v1.4.0\n hooks:\n - id: detect-secrets" + ], + "citations": [ + { + "source": "OWASP", + "title": "OWASP Top 10", + "url": "https://owasp.org/www-project-top-ten/", + "relevance": "Industry-standard list of critical web application security risks" + }, + { + "source": "GitHub", + "title": "Security Best Practices", + "url": "https://docs.github.com/en/code-security", + "relevance": "Official GitHub security features and best practices documentation" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "dbt_project_config", + "name": "dbt Project Configuration", + "category": "dbt SQL Projects", + "tier": 1, + "description": "Valid dbt_project.yml with required fields", + "criteria": "dbt_project.yml exists with name, config-version, profile", + "default_weight": 0.1 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "dbt_model_documentation", + "name": "dbt Model Documentation", + "category": "dbt SQL Projects", + "tier": 1, + "description": "Model descriptions in schema YAML files", + "criteria": "\u226580% of models have descriptions in schema.yml", + "default_weight": 0.1 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "deterministic_enforcement", + "name": "Deterministic Enforcement (Hooks & Lint Rules)", + "category": "Testing & CI/CD", + "tier": 2, + "description": "Hooks and lint rules for deterministic quality enforcement", + "criteria": "Pre-commit or agent hooks configured", + "default_weight": 0.03 + }, + "status": "pass", + "score": 60.0, + "measured_value": "configured", + "threshold": "configured", + "evidence": [ + ".pre-commit-config.yaml found (pre-commit hooks)" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "conventional_commits", + "name": "Conventional Commit Messages", + "category": "Git & Version Control", + "tier": 2, + "description": "Follows conventional commit format", + "criteria": "\u226580% of recent commits follow convention", + "default_weight": 0.03 + }, + "status": "fail", + "score": 0.0, + "measured_value": "not configured", + "threshold": "configured", + "evidence": [ + "No commitlint configuration found (.commitlintrc.json, package.json, husky, or pre-commit)" + ], + "remediation": { + "summary": "Configure conventional commits with commitlint", + "steps": [ + "Option A (Python/pre-commit): Add conventional-pre-commit to .pre-commit-config.yaml", + "Option B (JS/commitlint): Install commitlint and configure husky for commit-msg hook" + ], + "tools": [ + "pre-commit", + "conventional-pre-commit", + "commitlint", + "husky" + ], + "commands": [ + "# Python (pre-commit):", + "pip install pre-commit && pre-commit install --hook-type commit-msg", + "# JS (commitlint + husky):", + "npm install --save-dev @commitlint/cli @commitlint/config-conventional husky" + ], + "examples": [], + "citations": [] + }, + "error_message": null + }, + { + "attribute": { + "id": "gitignore_completeness", + "name": ".gitignore Completeness", + "category": "Git & Version Control", + "tier": 2, + "description": "Comprehensive .gitignore file with language-specific patterns", + "criteria": ".gitignore exists and includes language-specific patterns from GitHub templates", + "default_weight": 0.03 + }, + "status": "pass", + "score": 100.0, + "measured_value": "9/9 patterns", + "threshold": "\u226570% of language-specific patterns", + "evidence": [ + ".gitignore found (863 bytes)", + "Pattern coverage: 9/9 (100%)" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "one_command_setup", + "name": "One-Command Build/Setup", + "category": "Build & Development", + "tier": 2, + "description": "Single command to set up development environment from fresh clone", + "criteria": "Single command (make setup, npm install, etc.) documented prominently", + "default_weight": 0.03 + }, + "status": "fail", + "score": 70, + "measured_value": "helm install", + "threshold": "single command", + "evidence": [ + "Setup command found in README: 'helm install'", + "No Makefile or setup script found", + "Setup instructions in prominent location" + ], + "remediation": { + "summary": "Create single-command setup for development environment", + "steps": [ + "Choose setup automation tool (Makefile, setup script, or package manager)", + "Create setup command that handles all dependencies", + "Document setup command prominently in README (Quick Start section)", + "Ensure setup is idempotent (safe to run multiple times)", + "Test setup on fresh clone to verify it works" + ], + "tools": [ + "make", + "npm", + "pip", + "poetry" + ], + "commands": [ + "# Example Makefile", + "cat > Makefile << 'EOF'", + ".PHONY: setup", + "setup:", + "\tpython -m venv venv", + "\t. venv/bin/activate && pip install -r requirements.txt", + "\tpre-commit install", + "\tcp .env.example .env", + "\t@echo 'Setup complete! Run make test to verify.'", + "EOF" + ], + "examples": [ + "# Quick Start section in README\n\n## Quick Start\n\n```bash\nmake setup # One command to set up development environment\nmake test # Run tests to verify setup\n```\n" + ], + "citations": [ + { + "source": "freeCodeCamp", + "title": "Using make for project automation", + "url": "https://www.freecodecamp.org/news/want-to-know-the-easiest-way-to-save-time-use-make/", + "relevance": "Guide to using Makefiles for one-command setup" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "file_size_limits", + "name": "File Size Limits", + "category": "Context Window Optimization", + "tier": 2, + "description": "Files are reasonably sized for AI context windows", + "criteria": "<5% of files >500 lines, no files >1000 lines", + "default_weight": 0.03 + }, + "status": "fail", + "score": 0, + "measured_value": "8 huge, 14 large out of 97", + "threshold": "<5% files >500 lines, 0 files >1000 lines", + "evidence": [ + "Found 8 files >1000 lines (8.2% of 97 files)", + "Largest: kagenti-operator/internal/controller/agentcard_controller.go (1299 lines)" + ], + "remediation": { + "summary": "Refactor large files into smaller, focused modules", + "steps": [ + "Identify files >1000 lines", + "Split into logical submodules", + "Extract classes/functions into separate files", + "Maintain single responsibility principle" + ], + "tools": [ + "refactoring tools", + "linters" + ], + "commands": [], + "examples": [ + "# Split large file:\n# models.py (1500 lines) \u2192 models/user.py, models/product.py, models/order.py" + ], + "citations": [] + }, + "error_message": null + }, + { + "attribute": { + "id": "separation_of_concerns", + "name": "Separation of Concerns", + "category": "Code Organization", + "tier": 2, + "description": "Code organized with single responsibility per module", + "criteria": "Feature-based organization, cohesive modules, low coupling", + "default_weight": 0.03 + }, + "status": "pass", + "score": 100.0, + "measured_value": "organization:100, cohesion:100, naming:100", + "threshold": "\u226575 overall", + "evidence": [ + "Good directory organization (feature-based or flat)", + "File cohesion: 0/0 files >500 lines", + "No catch-all modules (utils.py, helpers.py) detected" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "concise_documentation", + "name": "Concise Documentation", + "category": "Documentation", + "tier": 2, + "description": "Documentation maximizes information density while minimizing token consumption", + "criteria": "README <500 lines with clear structure, bullet points over prose", + "default_weight": 0.03 + }, + "status": "pass", + "score": 78.0, + "measured_value": "205 lines, 17 headings, 13 bullets", + "threshold": "<500 lines, structured format", + "evidence": [ + "README length: 205 lines (excellent)", + "Heading density: 8.3 per 100 lines (target: 3-5)", + "13 bullet points, 5 code blocks (concise formatting)" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "inline_documentation", + "name": "Inline Documentation", + "category": "Documentation", + "tier": 2, + "description": "Function, class, and module-level documentation using language-specific conventions", + "criteria": "\u226580% of public functions/classes have docstrings", + "default_weight": 0.03 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "pattern_references", + "name": "Pattern References for Common Changes", + "category": "Agent Patterns & Knowledge", + "tier": 2, + "description": "Reference implementations and skills for common change types", + "criteria": "3-5 pattern references or skills documented", + "default_weight": 0.03 + }, + "status": "pass", + "score": 60.0, + "measured_value": "1 reference source(s)", + "threshold": "pattern references or skills documented", + "evidence": [ + ".claude/skills/ directory with 13 SKILL.md file(s)" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "dbt_data_tests", + "name": "dbt Data Tests", + "category": "dbt SQL Projects", + "tier": 2, + "description": "Generic tests on model primary keys", + "criteria": "\u226580% of models have unique/not_null tests on primary key", + "default_weight": 0.03 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "dbt_project_structure", + "name": "dbt Project Structure", + "category": "dbt SQL Projects", + "tier": 2, + "description": "Organized staging/marts directory structure", + "criteria": "models/ with staging/ and marts/ subdirectories", + "default_weight": 0.03 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "design_intent", + "name": "Design Intent Documentation", + "category": "Agent Patterns & Knowledge", + "tier": 3, + "description": "Documented preconditions, invariants, and design rationale", + "criteria": "Design docs with architectural intent", + "default_weight": 0.02 + }, + "status": "fail", + "score": 30.0, + "measured_value": "minimal", + "threshold": "design docs with preconditions/invariants", + "evidence": [ + "Design intent language found in README.md" + ], + "remediation": { + "summary": "Document design intent: preconditions, invariants, and rationale", + "steps": [ + "Create docs/design/ directory", + "For each critical module, document preconditions, invariants, and rationale", + "Use an AI agent to reverse-engineer initial design docs from code, then enrich with intent", + "Reference design docs from CLAUDE.md/AGENTS.md" + ], + "tools": [], + "commands": [ + "mkdir -p docs/design" + ], + "examples": [ + "# docs/design/event-system.md\n## Invariants\n- Event log is append-only; never mutate or delete entries\n- Events are processed exactly-once via idempotency keys\n\n## Preconditions\n- Auth middleware must validate token before event handlers run\n\n## Rationale\n- Polling instead of webhooks: upstream API has 5s delivery SLA, too slow for our use case" + ], + "citations": [ + { + "source": "Red Hat", + "title": "Repository Scaffolding for AI Coding Agents, Section 2.3", + "url": "", + "relevance": "Agents cannot infer design intent from code alone" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "repomix_config", + "name": "Repomix AI Context Generation", + "category": "AI-Assisted Development Tools", + "tier": 3, + "description": "Automated repository context generation for AI consumption", + "criteria": "Repomix configured with fresh output (< 7 days old)", + "default_weight": 0.02 + }, + "status": "fail", + "score": 0.0, + "measured_value": "not configured", + "threshold": "configured", + "evidence": [ + "Repomix configuration not found", + "Missing repomix.config.json" + ], + "remediation": { + "summary": "Configure Repomix for AI-friendly context generation", + "steps": [ + "Initialize Repomix: agentready repomix-generate --init", + "Generate context: agentready repomix-generate", + "Add to bootstrap: agentready bootstrap --repomix", + "Set up GitHub Action for automatic updates" + ], + "tools": [ + "Repomix", + "AgentReady" + ], + "commands": [ + "agentready repomix-generate --init", + "agentready repomix-generate" + ], + "examples": [ + "# Initialize Repomix configuration\nagentready repomix-generate --init\n\n# Generate repository context\nagentready repomix-generate\n\n# Check freshness\nagentready repomix-generate --check" + ], + "citations": [ + { + "source": "Repomix", + "title": "Repomix - AI-Friendly Repository Packager", + "url": "https://github.com/yamadashy/repomix", + "relevance": "AI-friendly repository context generation tool" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "cyclomatic_complexity", + "name": "Cyclomatic Complexity Thresholds", + "category": "Code Quality", + "tier": 3, + "description": "Cyclomatic complexity thresholds enforced", + "criteria": "Average complexity <10, no functions >15", + "default_weight": 0.02 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "architecture_decisions", + "name": "Architecture Decision Records (ADRs)", + "category": "Documentation Standards", + "tier": 3, + "description": "Lightweight documents capturing architectural decisions", + "criteria": "ADR directory with documented decisions", + "default_weight": 0.03 + }, + "status": "fail", + "score": 0.0, + "measured_value": "no ADR directory", + "threshold": "ADR directory with decisions", + "evidence": [ + "No ADR directory found (checked docs/adr/, .adr/, adr/, docs/decisions/)" + ], + "remediation": { + "summary": "Create Architecture Decision Records (ADRs) directory and document key decisions", + "steps": [ + "Create docs/adr/ directory in repository root", + "Use Michael Nygard ADR template or MADR format", + "Document each significant architectural decision", + "Number ADRs sequentially (0001-*.md, 0002-*.md)", + "Include Status, Context, Decision, and Consequences sections", + "Update ADR status when decisions are revised (Superseded, Deprecated)" + ], + "tools": [ + "adr-tools", + "log4brains" + ], + "commands": [ + "# Create ADR directory", + "mkdir -p docs/adr", + "", + "# Create first ADR using template", + "cat > docs/adr/0001-use-architecture-decision-records.md << 'EOF'", + "# 1. Use Architecture Decision Records", + "", + "Date: 2025-11-22", + "", + "## Status", + "Accepted", + "", + "## Context", + "We need to record architectural decisions made in this project.", + "", + "## Decision", + "We will use Architecture Decision Records (ADRs) as described by Michael Nygard.", + "", + "## Consequences", + "- Decisions are documented with context", + "- Future contributors understand rationale", + "- ADRs are lightweight and version-controlled", + "EOF" + ], + "examples": [ + "# Example ADR Structure\n\n```markdown\n# 2. Use PostgreSQL for Database\n\nDate: 2025-11-22\n\n## Status\nAccepted\n\n## Context\nWe need a relational database for complex queries and ACID transactions.\nTeam has PostgreSQL experience. Need full-text search capabilities.\n\n## Decision\nUse PostgreSQL 15+ as primary database.\n\n## Consequences\n- Positive: Robust ACID, full-text search, team familiarity\n- Negative: Higher resource usage than SQLite\n- Neutral: Need to manage migrations, backups\n```\n" + ], + "citations": [ + { + "source": "Michael Nygard", + "title": "Documenting Architecture Decisions", + "url": "https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions", + "relevance": "Original ADR format and rationale" + }, + { + "source": "GitHub adr/madr", + "title": "Markdown ADR (MADR) Template", + "url": "https://github.com/adr/madr", + "relevance": "Modern ADR template with examples" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "structured_logging", + "name": "Structured Logging", + "category": "Code Quality", + "tier": 3, + "description": "Logging in structured format (JSON) with consistent fields", + "criteria": "Structured logging library configured (structlog, winston, zap)", + "default_weight": 0.02 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Structured logging check not implemented for ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "openapi_specs", + "name": "OpenAPI/Swagger Specifications", + "category": "API Documentation", + "tier": 3, + "description": "Machine-readable API documentation in OpenAPI format", + "criteria": "OpenAPI 3.x spec with complete endpoint documentation", + "default_weight": 0.03 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "branch_protection", + "name": "Branch Protection Rules", + "category": "Git & Version Control", + "tier": 4, + "description": "Required status checks and review approvals before merging", + "criteria": "Branch protection enabled with status checks and required reviews", + "default_weight": 0.005 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Requires GitHub API integration for branch protection checks. Future implementation will verify: required status checks, required reviews, force push prevention, and branch update requirements." + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "code_smells", + "name": "Code Smell Elimination", + "category": "Code Quality", + "tier": 4, + "description": "Linter configuration for detecting code smells and anti-patterns", + "criteria": "Language-specific linters configured (pylint, ESLint, RuboCop, etc.)", + "default_weight": 0.01 + }, + "status": "fail", + "score": 0.0, + "measured_value": "none", + "threshold": "\u226560% of applicable linters configured", + "evidence": [ + "No linters configured" + ], + "remediation": { + "summary": "Configure 3 missing linter(s)", + "steps": [ + "Configure golangci-lint for Go", + "Add actionlint for GitHub Actions workflow validation", + "Configure markdownlint for documentation quality" + ], + "tools": [ + "golangci-lint", + "actionlint", + "markdownlint" + ], + "commands": [ + "go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest", + "npm install --save-dev markdownlint-cli && touch .markdownlint.json" + ], + "examples": [ + "# .pylintrc example\n[MASTER]\nmax-line-length=100\n\n[MESSAGES CONTROL]\ndisable=C0111", + "# .eslintrc.json example\n{\n \"extends\": \"eslint:recommended\",\n \"rules\": {\n \"no-console\": \"warn\"\n }\n}" + ], + "citations": [ + { + "source": "Pylint", + "title": "Pylint Documentation", + "url": "https://pylint.readthedocs.io/", + "relevance": "Official documentation for Pylint code analysis tool" + }, + { + "source": "ESLint", + "title": "ESLint Documentation", + "url": "https://eslint.org/docs/latest/", + "relevance": "Official documentation for ESLint JavaScript/TypeScript linter" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "issue_pr_templates", + "name": "Issue & Pull Request Templates", + "category": "Repository Structure", + "tier": 4, + "description": "Standardized templates for issues and PRs", + "criteria": "PR template and issue templates in .github/", + "default_weight": 0.01 + }, + "status": "fail", + "score": 0, + "measured_value": "PR:False, Issues:0", + "threshold": "PR template + \u22652 issue templates", + "evidence": [ + "No PR template found", + "No issue template directory found" + ], + "remediation": { + "summary": "Create GitHub issue and PR templates in .github/ directory", + "steps": [ + "Create .github/ directory if it doesn't exist", + "Add PULL_REQUEST_TEMPLATE.md for PRs", + "Create .github/ISSUE_TEMPLATE/ directory", + "Add bug_report.md for bug reports", + "Add feature_request.md for feature requests", + "Optionally add config.yml to configure template chooser" + ], + "tools": [ + "gh" + ], + "commands": [ + "# Create directories", + "mkdir -p .github/ISSUE_TEMPLATE", + "", + "# Create PR template", + "cat > .github/PULL_REQUEST_TEMPLATE.md << 'EOF'", + "## Summary", + "", + "", + "## Related Issues", + "Fixes #", + "", + "## Testing", + "- [ ] Tests added/updated", + "- [ ] All tests pass", + "", + "## Checklist", + "- [ ] Documentation updated", + "- [ ] CHANGELOG.md updated", + "EOF" + ], + "examples": [ + "# Bug Report Template (.github/ISSUE_TEMPLATE/bug_report.md)\n\n```markdown\n---\nname: Bug Report\nabout: Create a report to help us improve\ntitle: '[BUG] '\nlabels: bug\nassignees: ''\n---\n\n**Describe the bug**\nA clear description of what the bug is.\n\n**To Reproduce**\nSteps to reproduce:\n1. Go to '...'\n2. Click on '....'\n3. See error\n\n**Expected behavior**\nWhat you expected to happen.\n\n**Environment**\n- OS: [e.g. macOS 13.0]\n- Version: [e.g. 1.0.0]\n```\n" + ], + "citations": [ + { + "source": "GitHub Docs", + "title": "About issue and pull request templates", + "url": "https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/about-issue-and-pull-request-templates", + "relevance": "Official GitHub guide for templates" + } + ] + }, + "error_message": null + }, + { + "attribute": { + "id": "container_setup", + "name": "Container/Virtualization Setup", + "category": "Build & Development", + "tier": 4, + "description": "Container configuration for consistent development environments", + "criteria": "Dockerfile/Containerfile, docker-compose.yml, .dockerignore/.containerignore, multi-stage builds", + "default_weight": 0.01 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + }, + { + "attribute": { + "id": "progressive_disclosure", + "name": "Progressive Disclosure", + "category": "Agent Patterns & Knowledge", + "tier": 4, + "description": "Path-scoped rules and skills for large repos", + "criteria": "Component-level context files for repos >50K lines", + "default_weight": 0.01 + }, + "status": "not_applicable", + "score": null, + "measured_value": null, + "threshold": null, + "evidence": [ + "Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']" + ], + "remediation": null, + "error_message": null + } + ], + "config": { + "weights": {}, + "excluded_attributes": [], + "language_overrides": {}, + "output_dir": null, + "report_theme": "default", + "custom_theme": null + }, + "duration_seconds": 0.2, + "discovered_skills": [] +} \ No newline at end of file diff --git a/.agentready/assessment-latest.json b/.agentready/assessment-latest.json new file mode 120000 index 00000000..0ed5e84e --- /dev/null +++ b/.agentready/assessment-latest.json @@ -0,0 +1 @@ +assessment-20260512-091921.json \ No newline at end of file diff --git a/.agentready/report-20260512-091921.html b/.agentready/report-20260512-091921.html new file mode 100644 index 00000000..81053d34 --- /dev/null +++ b/.agentready/report-20260512-091921.html @@ -0,0 +1,3087 @@ + + + + + + + AgentReady Assessment - kagenti-operator + + + + +
+ + +
+ +
+
+

🤖 AgentReady Assessment Report

+
+
+

kagenti-operator

+
📁 ~/Development/ai/kagenti-operator
+
🌿 main @ 01f875a5
+
+ +
+
Assessed: May 12, 2026 at 9:19 AM
+
AgentReady: v2.35.2
+
Run by: rhuss@rhuss-mac
+
+ +
+
+ + +
+
+

Overall Score

+
43.8
+
+
+

Certification

+
Bronze
+
+
+

Assessed

+
20/33
+
+
+

Duration

+
0.2s
+
+
+ + +
+
+

💎 Platinum

+

90-100

+
+
+

🥇 Gold

+

75-89

+
+
+

🥈 Silver

+

60-74

+
+
+

🥉 Bronze

+

40-59

+
+
+

⚠️ Needs Work

+

0-39

+
+
+ + +
+
+ + + + + +
+ +
+ + +
+ +
+ +
+
+ + +
+ +
+ +
+
+ + ⊘ + + +
+

Test Execution & Coverage

+
+ Testing & CI/CD • + Tier 1 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Type Annotations

+
+ Code Quality • + Tier 1 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Type annotation check not implemented for ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ✅ + + +
+

CLAUDE.md Configuration Files

+
+ Context Window Optimization • + Tier 1 + • present +
+
+
+ +
+ 100 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • CLAUDE.md found at /Users/rhuss/Development/ai/kagenti-operator/CLAUDE.md
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

CI Quality Gates

+
+ Testing & CI/CD • + Tier 1 + • missing quality gates +
+
+
+ +
+ 75 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • CI config found: .github/workflows/release.yml, .github/workflows/project.yml, .github/workflows/pr-verifier.yml, .github/workflows/self-assign.yml, .github/workflows/ci.yaml, .github/workflows/stale.yaml, .github/workflows/security-scans.yaml, .github/workflows/scorecard.yaml
  • + +
  • Lint gate detected in CI
  • + +
  • No test gate found in CI
  • + +
  • No type-check gate found in CI
  • + +
  • Descriptive job/step names found
  • + +
  • Parallel job execution detected
  • + +
  • Config includes comments
  • + +
  • Artifacts uploaded
  • + +
+
+ + + +
+

Remediation

+

Add or improve CI/CD pipeline configuration

+ + +
    + +
  1. Create CI config for your platform (GitHub Actions, GitLab CI, etc.)
  2. + +
  3. Define jobs: lint, test, build
  4. + +
  5. Use descriptive job and step names
  6. + +
  7. Configure dependency caching
  8. + +
  9. Enable parallel job execution
  10. + +
  11. Upload artifacts: test results, coverage reports
  12. + +
  13. Add status badge to README
  14. + +
+ + + +

Commands

+
# Create GitHub Actions workflow
+mkdir -p .github/workflows
+touch .github/workflows/ci.yml
+
+# Validate workflow
+gh workflow view ci.yml
+ + + +

Examples

+ +
# .github/workflows/ci.yml - Good example
+
+name: CI Pipeline
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+jobs:
+  lint:
+    name: Lint Code
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+          cache: 'pip'  # Caching
+
+      - name: Install dependencies
+        run: pip install -r requirements.txt
+
+      - name: Run linters
+        run: |
+          black --check .
+          isort --check .
+          ruff check .
+
+  test:
+    name: Run Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+          cache: 'pip'
+
+      - name: Install dependencies
+        run: pip install -r requirements.txt
+
+      - name: Run tests with coverage
+        run: pytest --cov --cov-report=xml
+
+      - name: Upload coverage reports
+        uses: codecov/codecov-action@v3
+        with:
+          files: ./coverage.xml
+
+  build:
+    name: Build Package
+    runs-on: ubuntu-latest
+    needs: [lint, test]  # Runs after lint/test pass
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Build package
+        run: python -m build
+
+      - name: Upload build artifacts
+        uses: actions/upload-artifact@v3
+        with:
+          name: dist
+          path: dist/
+
+ + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Single-File Verification

+
+ Verification & Feedback Loops • + Tier 1 + • not documented +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No single-file verification commands found in context files
  • + +
+
+ + + +
+

Remediation

+

Document single-file lint and type-check commands in CLAUDE.md/AGENTS.md

+ + +
    + +
  1. Add single-file lint command to context file (e.g., 'ruff check path/to/file.py')
  2. + +
  3. Add single-file type-check command (e.g., 'mypy path/to/file.py')
  4. + +
  5. Ensure these commands work without a full build step
  6. + +
  7. Target <5 seconds execution per file
  8. + +
+ + + +

Commands

+
# Python
+ruff check path/to/file.py
+mypy path/to/file.py
+
+# JavaScript/TypeScript
+npx eslint path/to/file.ts
+npx tsc --noEmit path/to/file.ts
+ + + +
+ + + +
+
+ +
+ +
+
+ + ✅ + + +
+

README Structure

+
+ Documentation Standards • + Tier 1 + • 3/3 sections +
+
+
+ +
+ 100 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Found 3/3 essential sections
  • + +
  • Installation: ✓
  • + +
  • Usage: ✓
  • + +
  • Development: ✓
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Standard Project Layouts

+
+ Repository Structure • + Tier 1 + • 0/2 directories +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Found 0/2 standard directories
  • + +
  • source directory: ✗ (no src/ or project-named dir)
  • + +
  • tests/: ✗
  • + +
+
+ + + +
+

Remediation

+

Organize code into standard directories

+ + +
    + +
  1. Create a source directory for your code
  2. + +
  3. Option A: Use src/ layout (recommended for packages)
  4. + +
  5. Option B: Use project-named directory (e.g., mypackage/)
  6. + +
  7. Ensure your package has __init__.py
  8. + +
  9. Create tests/ directory for test files
  10. + +
  11. Add at least one test file
  12. + +
+ + + +

Commands

+
# Option A: src layout
+mkdir -p src/mypackage
+touch src/mypackage/__init__.py
+# ---
+# Option B: flat layout (project-named)
+mkdir -p mypackage
+touch mypackage/__init__.py
+# Create tests directory
+mkdir -p tests
+touch tests/__init__.py
+touch tests/test_example.py
+ + + +

Examples

+ +
# src layout (recommended for distributable packages)
+project/
+├── src/
+│   └── mypackage/
+│       ├── __init__.py
+│       └── module.py
+├── tests/
+│   └── test_module.py
+└── pyproject.toml
+
+# flat layout (common in major projects like pandas, numpy)
+project/
+├── mypackage/
+│   ├── __init__.py
+│   └── module.py
+├── tests/
+│   └── test_module.py
+└── pyproject.toml
+
+ + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Dependency Pinning for Reproducibility

+
+ Dependency Management • + Tier 1 + • none +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No dependency lock files found
  • + +
+
+ + + +
+

Remediation

+

Add lock file for dependency reproducibility

+ + +
    + +
  1. For npm: run 'npm install' (generates package-lock.json)
  2. + +
  3. For Python: use 'pip freeze > requirements.txt' or poetry
  4. + +
  5. For Ruby: run 'bundle install' (generates Gemfile.lock)
  6. + +
+ + + +

Commands

+
npm install  # npm
+pip freeze > requirements.txt  # Python
+poetry lock  # Python with Poetry
+ + + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Dependency Security & Vulnerability Scanning

+
+ Security • + Tier 1 + • No security scanning tools configured +
+
+
+ +
+ 5 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • ✓ SECURITY.md present (vulnerability disclosure policy)
  • + +
+
+ + + +
+

Remediation

+

Configure security scanning for dependencies and code

+ + +
    + +
  1. Enable Dependabot in GitHub repository settings
  2. + +
  3. Add .github/dependabot.yml configuration file
  4. + +
  5. Or configure Renovate: add renovate.json to repository root
  6. + +
  7. Set up CodeQL scanning for SAST
  8. + +
  9. Add secret detection to pre-commit hooks
  10. + +
  11. Configure language-specific security scanners
  12. + +
+ + + +

Commands

+
gh repo edit --enable-security
+pip install pre-commit detect-secrets
+pre-commit install
+ + + +

Examples

+ +
# .github/dependabot.yml
+version: 2
+updates:
+  - package-ecosystem: pip
+    directory: /
+    schedule:
+      interval: weekly
+ +
# renovate.json
+{
+  "extends": ["config:base"],
+  "schedule": "after 10pm every weekday"
+}
+ +
# .pre-commit-config.yaml
+repos:
+  - repo: https://github.com/Yelp/detect-secrets
+    rev: v1.4.0
+    hooks:
+      - id: detect-secrets
+ + +
+ + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

dbt Project Configuration

+
+ dbt SQL Projects • + Tier 1 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

dbt Model Documentation

+
+ dbt SQL Projects • + Tier 1 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ✅ + + +
+

Deterministic Enforcement (Hooks & Lint Rules)

+
+ Testing & CI/CD • + Tier 2 + • configured +
+
+
+ +
+ 60 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • .pre-commit-config.yaml found (pre-commit hooks)
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Conventional Commit Messages

+
+ Git & Version Control • + Tier 2 + • not configured +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No commitlint configuration found (.commitlintrc.json, package.json, husky, or pre-commit)
  • + +
+
+ + + +
+

Remediation

+

Configure conventional commits with commitlint

+ + +
    + +
  1. Option A (Python/pre-commit): Add conventional-pre-commit to .pre-commit-config.yaml
  2. + +
  3. Option B (JS/commitlint): Install commitlint and configure husky for commit-msg hook
  4. + +
+ + + +

Commands

+
# Python (pre-commit):
+pip install pre-commit && pre-commit install --hook-type commit-msg
+# JS (commitlint + husky):
+npm install --save-dev @commitlint/cli @commitlint/config-conventional husky
+ + + +
+ + + +
+
+ +
+ +
+
+ + ✅ + + +
+

.gitignore Completeness

+
+ Git & Version Control • + Tier 2 + • 9/9 patterns +
+
+
+ +
+ 100 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • .gitignore found (863 bytes)
  • + +
  • Pattern coverage: 9/9 (100%)
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

One-Command Build/Setup

+
+ Build & Development • + Tier 2 + • helm install +
+
+
+ +
+ 70 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Setup command found in README: 'helm install'
  • + +
  • No Makefile or setup script found
  • + +
  • Setup instructions in prominent location
  • + +
+
+ + + +
+

Remediation

+

Create single-command setup for development environment

+ + +
    + +
  1. Choose setup automation tool (Makefile, setup script, or package manager)
  2. + +
  3. Create setup command that handles all dependencies
  4. + +
  5. Document setup command prominently in README (Quick Start section)
  6. + +
  7. Ensure setup is idempotent (safe to run multiple times)
  8. + +
  9. Test setup on fresh clone to verify it works
  10. + +
+ + + +

Commands

+
# Example Makefile
+cat > Makefile << 'EOF'
+.PHONY: setup
+setup:
+	python -m venv venv
+	. venv/bin/activate && pip install -r requirements.txt
+	pre-commit install
+	cp .env.example .env
+	@echo 'Setup complete! Run make test to verify.'
+EOF
+ + + +

Examples

+ +
# Quick Start section in README
+
+## Quick Start
+
+```bash
+make setup  # One command to set up development environment
+make test   # Run tests to verify setup
+```
+
+ + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

File Size Limits

+
+ Context Window Optimization • + Tier 2 + • 8 huge, 14 large out of 97 +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Found 8 files >1000 lines (8.2% of 97 files)
  • + +
  • Largest: kagenti-operator/internal/controller/agentcard_controller.go (1299 lines)
  • + +
+
+ + + +
+

Remediation

+

Refactor large files into smaller, focused modules

+ + +
    + +
  1. Identify files >1000 lines
  2. + +
  3. Split into logical submodules
  4. + +
  5. Extract classes/functions into separate files
  6. + +
  7. Maintain single responsibility principle
  8. + +
+ + + + + +

Examples

+ +
# Split large file:
+# models.py (1500 lines) → models/user.py, models/product.py, models/order.py
+ + +
+ + + +
+
+ +
+ +
+
+ + ✅ + + +
+

Separation of Concerns

+
+ Code Organization • + Tier 2 + • organization:100, cohesion:100, naming:100 +
+
+
+ +
+ 100 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Good directory organization (feature-based or flat)
  • + +
  • File cohesion: 0/0 files >500 lines
  • + +
  • No catch-all modules (utils.py, helpers.py) detected
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ✅ + + +
+

Concise Documentation

+
+ Documentation • + Tier 2 + • 205 lines, 17 headings, 13 bullets +
+
+
+ +
+ 78 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • README length: 205 lines (excellent)
  • + +
  • Heading density: 8.3 per 100 lines (target: 3-5)
  • + +
  • 13 bullet points, 5 code blocks (concise formatting)
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Inline Documentation

+
+ Documentation • + Tier 2 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ✅ + + +
+

Pattern References for Common Changes

+
+ Agent Patterns & Knowledge • + Tier 2 + • 1 reference source(s) +
+
+
+ +
+ 60 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • .claude/skills/ directory with 13 SKILL.md file(s)
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

dbt Data Tests

+
+ dbt SQL Projects • + Tier 2 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

dbt Project Structure

+
+ dbt SQL Projects • + Tier 2 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Design Intent Documentation

+
+ Agent Patterns & Knowledge • + Tier 3 + • minimal +
+
+
+ +
+ 30 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Design intent language found in README.md
  • + +
+
+ + + +
+

Remediation

+

Document design intent: preconditions, invariants, and rationale

+ + +
    + +
  1. Create docs/design/ directory
  2. + +
  3. For each critical module, document preconditions, invariants, and rationale
  4. + +
  5. Use an AI agent to reverse-engineer initial design docs from code, then enrich with intent
  6. + +
  7. Reference design docs from CLAUDE.md/AGENTS.md
  8. + +
+ + + +

Commands

+
mkdir -p docs/design
+ + + +

Examples

+ +
# docs/design/event-system.md
+## Invariants
+- Event log is append-only; never mutate or delete entries
+- Events are processed exactly-once via idempotency keys
+
+## Preconditions
+- Auth middleware must validate token before event handlers run
+
+## Rationale
+- Polling instead of webhooks: upstream API has 5s delivery SLA, too slow for our use case
+ + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Repomix AI Context Generation

+
+ AI-Assisted Development Tools • + Tier 3 + • not configured +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Repomix configuration not found
  • + +
  • Missing repomix.config.json
  • + +
+
+ + + +
+

Remediation

+

Configure Repomix for AI-friendly context generation

+ + +
    + +
  1. Initialize Repomix: agentready repomix-generate --init
  2. + +
  3. Generate context: agentready repomix-generate
  4. + +
  5. Add to bootstrap: agentready bootstrap --repomix
  6. + +
  7. Set up GitHub Action for automatic updates
  8. + +
+ + + +

Commands

+
agentready repomix-generate --init
+agentready repomix-generate
+ + + +

Examples

+ +
# Initialize Repomix configuration
+agentready repomix-generate --init
+
+# Generate repository context
+agentready repomix-generate
+
+# Check freshness
+agentready repomix-generate --check
+ + +
+ + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Cyclomatic Complexity Thresholds

+
+ Code Quality • + Tier 3 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Architecture Decision Records (ADRs)

+
+ Documentation Standards • + Tier 3 + • no ADR directory +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No ADR directory found (checked docs/adr/, .adr/, adr/, docs/decisions/)
  • + +
+
+ + + +
+

Remediation

+

Create Architecture Decision Records (ADRs) directory and document key decisions

+ + +
    + +
  1. Create docs/adr/ directory in repository root
  2. + +
  3. Use Michael Nygard ADR template or MADR format
  4. + +
  5. Document each significant architectural decision
  6. + +
  7. Number ADRs sequentially (0001-*.md, 0002-*.md)
  8. + +
  9. Include Status, Context, Decision, and Consequences sections
  10. + +
  11. Update ADR status when decisions are revised (Superseded, Deprecated)
  12. + +
+ + + +

Commands

+
# Create ADR directory
+mkdir -p docs/adr
+
+# Create first ADR using template
+cat > docs/adr/0001-use-architecture-decision-records.md << 'EOF'
+# 1. Use Architecture Decision Records
+
+Date: 2025-11-22
+
+## Status
+Accepted
+
+## Context
+We need to record architectural decisions made in this project.
+
+## Decision
+We will use Architecture Decision Records (ADRs) as described by Michael Nygard.
+
+## Consequences
+- Decisions are documented with context
+- Future contributors understand rationale
+- ADRs are lightweight and version-controlled
+EOF
+ + + +

Examples

+ +
# Example ADR Structure
+
+```markdown
+# 2. Use PostgreSQL for Database
+
+Date: 2025-11-22
+
+## Status
+Accepted
+
+## Context
+We need a relational database for complex queries and ACID transactions.
+Team has PostgreSQL experience. Need full-text search capabilities.
+
+## Decision
+Use PostgreSQL 15+ as primary database.
+
+## Consequences
+- Positive: Robust ACID, full-text search, team familiarity
+- Negative: Higher resource usage than SQLite
+- Neutral: Need to manage migrations, backups
+```
+
+ + +
+ + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Structured Logging

+
+ Code Quality • + Tier 3 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Structured logging check not implemented for ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

OpenAPI/Swagger Specifications

+
+ API Documentation • + Tier 3 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Branch Protection Rules

+
+ Git & Version Control • + Tier 4 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Requires GitHub API integration for branch protection checks. Future implementation will verify: required status checks, required reviews, force push prevention, and branch update requirements.
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Code Smell Elimination

+
+ Code Quality • + Tier 4 + • none +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No linters configured
  • + +
+
+ + + +
+

Remediation

+

Configure 3 missing linter(s)

+ + +
    + +
  1. Configure golangci-lint for Go
  2. + +
  3. Add actionlint for GitHub Actions workflow validation
  4. + +
  5. Configure markdownlint for documentation quality
  6. + +
+ + + +

Commands

+
go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+npm install --save-dev markdownlint-cli && touch .markdownlint.json
+ + + +

Examples

+ +
# .pylintrc example
+[MASTER]
+max-line-length=100
+
+[MESSAGES CONTROL]
+disable=C0111
+ +
# .eslintrc.json example
+{
+  "extends": "eslint:recommended",
+  "rules": {
+    "no-console": "warn"
+  }
+}
+ + +
+ + + +
+
+ +
+ +
+
+ + ❌ + + +
+

Issue & Pull Request Templates

+
+ Repository Structure • + Tier 4 + • PR:False, Issues:0 +
+
+
+ +
+ 0 +
+ +
+ +
+ +
+

Evidence

+
    + +
  • No PR template found
  • + +
  • No issue template directory found
  • + +
+
+ + + +
+

Remediation

+

Create GitHub issue and PR templates in .github/ directory

+ + +
    + +
  1. Create .github/ directory if it doesn't exist
  2. + +
  3. Add PULL_REQUEST_TEMPLATE.md for PRs
  4. + +
  5. Create .github/ISSUE_TEMPLATE/ directory
  6. + +
  7. Add bug_report.md for bug reports
  8. + +
  9. Add feature_request.md for feature requests
  10. + +
  11. Optionally add config.yml to configure template chooser
  12. + +
+ + + +

Commands

+
# Create directories
+mkdir -p .github/ISSUE_TEMPLATE
+
+# Create PR template
+cat > .github/PULL_REQUEST_TEMPLATE.md << 'EOF'
+## Summary
+<!-- Describe the changes in this PR -->
+
+## Related Issues
+Fixes #
+
+## Testing
+- [ ] Tests added/updated
+- [ ] All tests pass
+
+## Checklist
+- [ ] Documentation updated
+- [ ] CHANGELOG.md updated
+EOF
+ + + +

Examples

+ +
# Bug Report Template (.github/ISSUE_TEMPLATE/bug_report.md)
+
+```markdown
+---
+name: Bug Report
+about: Create a report to help us improve
+title: '[BUG] '
+labels: bug
+assignees: ''
+---
+
+**Describe the bug**
+A clear description of what the bug is.
+
+**To Reproduce**
+Steps to reproduce:
+1. Go to '...'
+2. Click on '....'
+3. See error
+
+**Expected behavior**
+What you expected to happen.
+
+**Environment**
+- OS: [e.g. macOS 13.0]
+- Version: [e.g. 1.0.0]
+```
+
+ + +
+ + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Container/Virtualization Setup

+
+ Build & Development • + Tier 4 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ +
+
+ + ⊘ + + +
+

Progressive Disclosure

+
+ Agent Patterns & Knowledge • + Tier 4 + +
+
+
+ +
+ +
+ +
+ +
+

Evidence

+
    + +
  • Not applicable to ['Markdown', 'YAML', 'Shell', 'Go']
  • + +
+
+ + + + + +
+
+ +
+ + +
+ + + + \ No newline at end of file diff --git a/.agentready/report-20260512-091921.md b/.agentready/report-20260512-091921.md new file mode 100644 index 00000000..85635cd6 --- /dev/null +++ b/.agentready/report-20260512-091921.md @@ -0,0 +1,746 @@ +# 🤖 AgentReady Assessment Report + +**Repository**: kagenti-operator +**Path**: `/Users/rhuss/Development/ai/kagenti-operator` +**Branch**: `main` | **Commit**: `01f875a5` +**Assessed**: May 12, 2026 at 9:19 AM +**AgentReady Version**: 2.35.2 +**Run by**: rhuss@rhuss-mac + +--- + +## 📊 Summary + +| Metric | Value | +|--------|-------| +| **Overall Score** | **43.8/100** 🥉 **Bronze** ([Tier Definitions](https://agentready.dev/attributes.html#tier-system)) | +| **Attributes Assessed** | 20/33 | +| **Attributes Not Assessed** | 13 | +| **Assessment Duration** | 0.2s | + +### Languages Detected + +- **Go**: 97 files +- **YAML**: 82 files +- **Markdown**: 37 files +- **Shell**: 12 files + +### Repository Stats + +- **Total Files**: 246 +- **Total Lines**: 46,081 + +## 🎯 Priority Improvements + +Focus on these high-impact fixes first: + +1. **Single-File Verification** (Tier 1) - +5.0 points potential + - Document single-file lint and type-check commands in CLAUDE.md/AGENTS.md +2. **Standard Project Layouts** (Tier 1) - +5.0 points potential + - Organize code into standard directories +3. **Dependency Pinning for Reproducibility** (Tier 1) - +5.0 points potential + - Add lock file for dependency reproducibility +4. **Dependency Security & Vulnerability Scanning** (Tier 1) - +5.0 points potential + - Configure security scanning for dependencies and code +5. **CI Quality Gates** (Tier 1) - +5.0 points potential + - Add or improve CI/CD pipeline configuration + +## 📋 Detailed Findings + +Findings sorted by priority (Tier 1 failures first, then Tier 2, etc.) + +![T1](https://img.shields.io/badge/T1-Single-File_Verification_0--100-red) **Single-File Verification** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: not documented (Threshold: single-file lint + type-check commands documented) + +**Evidence**: +- No single-file verification commands found in context files + +Document single-file lint and type-check commands in CLAUDE.md/AGENTS.md + +1. Add single-file lint command to context file (e.g., 'ruff check path/to/file.py') +2. Add single-file type-check command (e.g., 'mypy path/to/file.py') +3. Ensure these commands work without a full build step +4. Target <5 seconds execution per file + +**Commands**: +```bash +# Python +ruff check path/to/file.py +mypy path/to/file.py + +# JavaScript/TypeScript +npx eslint path/to/file.ts +npx tsc --noEmit path/to/file.ts +``` + +
+ +![T1](https://img.shields.io/badge/T1-Standard_Project_Layouts_0--100-red) **Standard Project Layouts** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: 0/2 directories (Threshold: 2/2 directories) + +**Evidence**: +- Found 0/2 standard directories +- source directory: ✗ (no src/ or project-named dir) +- tests/: ✗ + +Organize code into standard directories + +1. Create a source directory for your code +2. Option A: Use src/ layout (recommended for packages) +3. Option B: Use project-named directory (e.g., mypackage/) +4. Ensure your package has __init__.py +5. Create tests/ directory for test files +6. Add at least one test file + +**Commands**: +```bash +# Option A: src layout +mkdir -p src/mypackage +touch src/mypackage/__init__.py +# --- +# Option B: flat layout (project-named) +mkdir -p mypackage +touch mypackage/__init__.py +# Create tests directory +mkdir -p tests +touch tests/__init__.py +touch tests/test_example.py +``` + +**Examples**: +``` +# src layout (recommended for distributable packages) +project/ +├── src/ +│ └── mypackage/ +│ ├── __init__.py +│ └── module.py +├── tests/ +│ └── test_module.py +└── pyproject.toml + +# flat layout (common in major projects like pandas, numpy) +project/ +├── mypackage/ +│ ├── __init__.py +│ └── module.py +├── tests/ +│ └── test_module.py +└── pyproject.toml + +``` + +
+ +![T1](https://img.shields.io/badge/T1-Dependency_Pinning_for_Reproducibility_0--100-red) **Dependency Pinning for Reproducibility** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: none (Threshold: lock file with pinned versions) + +**Evidence**: +- No dependency lock files found + +Add lock file for dependency reproducibility + +1. For npm: run 'npm install' (generates package-lock.json) +2. For Python: use 'pip freeze > requirements.txt' or poetry +3. For Ruby: run 'bundle install' (generates Gemfile.lock) + +**Commands**: +```bash +npm install # npm +pip freeze > requirements.txt # Python +poetry lock # Python with Poetry +``` + +
+ +![T1](https://img.shields.io/badge/T1-Dependency_Security_%26_Vulnerability_Scanning_5--100-red) **Dependency Security & Vulnerability Scanning** ❌ 5/100 +
+📝 Remediation Steps + +**Measured**: No security scanning tools configured (Threshold: ≥60 points (Dependabot/Renovate + SAST or multiple scanners)) + +**Evidence**: +- ✓ SECURITY.md present (vulnerability disclosure policy) + +Configure security scanning for dependencies and code + +1. Enable Dependabot in GitHub repository settings +2. Add .github/dependabot.yml configuration file +3. Or configure Renovate: add renovate.json to repository root +4. Set up CodeQL scanning for SAST +5. Add secret detection to pre-commit hooks +6. Configure language-specific security scanners + +**Commands**: +```bash +gh repo edit --enable-security +pip install pre-commit detect-secrets +pre-commit install +``` + +**Examples**: +``` +# .github/dependabot.yml +version: 2 +updates: + - package-ecosystem: pip + directory: / + schedule: + interval: weekly +``` +``` +# renovate.json +{ + "extends": ["config:base"], + "schedule": "after 10pm every weekday" +} +``` +``` +# .pre-commit-config.yaml +repos: + - repo: https://github.com/Yelp/detect-secrets + rev: v1.4.0 + hooks: + - id: detect-secrets +``` + +
+ +![T1](https://img.shields.io/badge/T1-CI_Quality_Gates_75--100-red) **CI Quality Gates** ❌ 75/100 +
+📝 Remediation Steps + +**Measured**: missing quality gates (Threshold: CI with lint + test + type-check gates on PRs) + +**Evidence**: +- CI config found: .github/workflows/release.yml, .github/workflows/project.yml, .github/workflows/pr-verifier.yml, .github/workflows/self-assign.yml, .github/workflows/ci.yaml, .github/workflows/stale.yaml, .github/workflows/security-scans.yaml, .github/workflows/scorecard.yaml +- Lint gate detected in CI +- No test gate found in CI +- No type-check gate found in CI +- Descriptive job/step names found +- Parallel job execution detected +- Config includes comments +- Artifacts uploaded + +Add or improve CI/CD pipeline configuration + +1. Create CI config for your platform (GitHub Actions, GitLab CI, etc.) +2. Define jobs: lint, test, build +3. Use descriptive job and step names +4. Configure dependency caching +5. Enable parallel job execution +6. Upload artifacts: test results, coverage reports +7. Add status badge to README + +**Commands**: +```bash +# Create GitHub Actions workflow +mkdir -p .github/workflows +touch .github/workflows/ci.yml + +# Validate workflow +gh workflow view ci.yml +``` + +**Examples**: +``` +# .github/workflows/ci.yml - Good example + +name: CI Pipeline + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + lint: + name: Lint Code + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: 'pip' # Caching + + - name: Install dependencies + run: pip install -r requirements.txt + + - name: Run linters + run: | + black --check . + isort --check . + ruff check . + + test: + name: Run Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: 'pip' + + - name: Install dependencies + run: pip install -r requirements.txt + + - name: Run tests with coverage + run: pytest --cov --cov-report=xml + + - name: Upload coverage reports + uses: codecov/codecov-action@v3 + with: + files: ./coverage.xml + + build: + name: Build Package + runs-on: ubuntu-latest + needs: [lint, test] # Runs after lint/test pass + steps: + - uses: actions/checkout@v4 + + - name: Build package + run: python -m build + + - name: Upload build artifacts + uses: actions/upload-artifact@v3 + with: + name: dist + path: dist/ + +``` + +
+ +![T1](https://img.shields.io/badge/T1-CLAUDE.md_Configuration_Files_100--100-green) **CLAUDE.md Configuration Files** ✅ 100/100 + +![T1](https://img.shields.io/badge/T1-README_Structure_100--100-green) **README Structure** ✅ 100/100 + +![T1](https://img.shields.io/badge/T1-Test_Execution_%26_Coverage_N--A-lightgray) **Test Execution & Coverage** ⊘ + +![T1](https://img.shields.io/badge/T1-Type_Annotations_N--A-lightgray) **Type Annotations** ⊘ + +![T1](https://img.shields.io/badge/T1-dbt_Project_Configuration_N--A-lightgray) **dbt Project Configuration** ⊘ + +![T1](https://img.shields.io/badge/T1-dbt_Model_Documentation_N--A-lightgray) **dbt Model Documentation** ⊘ + +![T2](https://img.shields.io/badge/T2-Conventional_Commit_Messages_0--100-red) **Conventional Commit Messages** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: not configured (Threshold: configured) + +**Evidence**: +- No commitlint configuration found (.commitlintrc.json, package.json, husky, or pre-commit) + +Configure conventional commits with commitlint + +1. Option A (Python/pre-commit): Add conventional-pre-commit to .pre-commit-config.yaml +2. Option B (JS/commitlint): Install commitlint and configure husky for commit-msg hook + +**Commands**: +```bash +# Python (pre-commit): +pip install pre-commit && pre-commit install --hook-type commit-msg +# JS (commitlint + husky): +npm install --save-dev @commitlint/cli @commitlint/config-conventional husky +``` + +
+ +![T2](https://img.shields.io/badge/T2-File_Size_Limits_0--100-red) **File Size Limits** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: 8 huge, 14 large out of 97 (Threshold: <5% files >500 lines, 0 files >1000 lines) + +**Evidence**: +- Found 8 files >1000 lines (8.2% of 97 files) +- Largest: kagenti-operator/internal/controller/agentcard_controller.go (1299 lines) + +Refactor large files into smaller, focused modules + +1. Identify files >1000 lines +2. Split into logical submodules +3. Extract classes/functions into separate files +4. Maintain single responsibility principle + +**Examples**: +``` +# Split large file: +# models.py (1500 lines) → models/user.py, models/product.py, models/order.py +``` + +
+ +![T2](https://img.shields.io/badge/T2-One-Command_Build%2FSetup_70--100-red) **One-Command Build/Setup** ❌ 70/100 +
+📝 Remediation Steps + +**Measured**: helm install (Threshold: single command) + +**Evidence**: +- Setup command found in README: 'helm install' +- No Makefile or setup script found +- Setup instructions in prominent location + +Create single-command setup for development environment + +1. Choose setup automation tool (Makefile, setup script, or package manager) +2. Create setup command that handles all dependencies +3. Document setup command prominently in README (Quick Start section) +4. Ensure setup is idempotent (safe to run multiple times) +5. Test setup on fresh clone to verify it works + +**Commands**: +```bash +# Example Makefile +cat > Makefile << 'EOF' +.PHONY: setup +setup: + python -m venv venv + . venv/bin/activate && pip install -r requirements.txt + pre-commit install + cp .env.example .env + @echo 'Setup complete! Run make test to verify.' +EOF +``` + +**Examples**: +``` +# Quick Start section in README + +## Quick Start + +```bash +make setup # One command to set up development environment +make test # Run tests to verify setup +``` + +``` + +
+ +![T2](https://img.shields.io/badge/T2-Deterministic_Enforcement_%28Hooks_%26_Lint_Rules%29_60--100-green) **Deterministic Enforcement (Hooks & Lint Rules)** ✅ 60/100 + +![T2](https://img.shields.io/badge/T2-Pattern_References_for_Common_Changes_60--100-green) **Pattern References for Common Changes** ✅ 60/100 + +![T2](https://img.shields.io/badge/T2-Concise_Documentation_78--100-green) **Concise Documentation** ✅ 78/100 + +![T2](https://img.shields.io/badge/T2-.gitignore_Completeness_100--100-green) **.gitignore Completeness** ✅ 100/100 + +![T2](https://img.shields.io/badge/T2-Separation_of_Concerns_100--100-green) **Separation of Concerns** ✅ 100/100 + +![T2](https://img.shields.io/badge/T2-Inline_Documentation_N--A-lightgray) **Inline Documentation** ⊘ + +![T2](https://img.shields.io/badge/T2-dbt_Data_Tests_N--A-lightgray) **dbt Data Tests** ⊘ + +![T2](https://img.shields.io/badge/T2-dbt_Project_Structure_N--A-lightgray) **dbt Project Structure** ⊘ + +![T3](https://img.shields.io/badge/T3-Repomix_AI_Context_Generation_0--100-red) **Repomix AI Context Generation** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: not configured (Threshold: configured) + +**Evidence**: +- Repomix configuration not found +- Missing repomix.config.json + +Configure Repomix for AI-friendly context generation + +1. Initialize Repomix: agentready repomix-generate --init +2. Generate context: agentready repomix-generate +3. Add to bootstrap: agentready bootstrap --repomix +4. Set up GitHub Action for automatic updates + +**Commands**: +```bash +agentready repomix-generate --init +agentready repomix-generate +``` + +**Examples**: +``` +# Initialize Repomix configuration +agentready repomix-generate --init + +# Generate repository context +agentready repomix-generate + +# Check freshness +agentready repomix-generate --check +``` + +
+ +![T3](https://img.shields.io/badge/T3-Architecture_Decision_Records_%28ADRs%29_0--100-red) **Architecture Decision Records (ADRs)** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: no ADR directory (Threshold: ADR directory with decisions) + +**Evidence**: +- No ADR directory found (checked docs/adr/, .adr/, adr/, docs/decisions/) + +Create Architecture Decision Records (ADRs) directory and document key decisions + +1. Create docs/adr/ directory in repository root +2. Use Michael Nygard ADR template or MADR format +3. Document each significant architectural decision +4. Number ADRs sequentially (0001-*.md, 0002-*.md) +5. Include Status, Context, Decision, and Consequences sections +6. Update ADR status when decisions are revised (Superseded, Deprecated) + +**Commands**: +```bash +# Create ADR directory +mkdir -p docs/adr + +# Create first ADR using template +cat > docs/adr/0001-use-architecture-decision-records.md << 'EOF' +# 1. Use Architecture Decision Records + +Date: 2025-11-22 + +## Status +Accepted + +## Context +We need to record architectural decisions made in this project. + +## Decision +We will use Architecture Decision Records (ADRs) as described by Michael Nygard. + +## Consequences +- Decisions are documented with context +- Future contributors understand rationale +- ADRs are lightweight and version-controlled +EOF +``` + +**Examples**: +``` +# Example ADR Structure + +```markdown +# 2. Use PostgreSQL for Database + +Date: 2025-11-22 + +## Status +Accepted + +## Context +We need a relational database for complex queries and ACID transactions. +Team has PostgreSQL experience. Need full-text search capabilities. + +## Decision +Use PostgreSQL 15+ as primary database. + +## Consequences +- Positive: Robust ACID, full-text search, team familiarity +- Negative: Higher resource usage than SQLite +- Neutral: Need to manage migrations, backups +``` + +``` + +
+ +![T3](https://img.shields.io/badge/T3-Design_Intent_Documentation_30--100-red) **Design Intent Documentation** ❌ 30/100 +
+📝 Remediation Steps + +**Measured**: minimal (Threshold: design docs with preconditions/invariants) + +**Evidence**: +- Design intent language found in README.md + +Document design intent: preconditions, invariants, and rationale + +1. Create docs/design/ directory +2. For each critical module, document preconditions, invariants, and rationale +3. Use an AI agent to reverse-engineer initial design docs from code, then enrich with intent +4. Reference design docs from CLAUDE.md/AGENTS.md + +**Commands**: +```bash +mkdir -p docs/design +``` + +**Examples**: +``` +# docs/design/event-system.md +## Invariants +- Event log is append-only; never mutate or delete entries +- Events are processed exactly-once via idempotency keys + +## Preconditions +- Auth middleware must validate token before event handlers run + +## Rationale +- Polling instead of webhooks: upstream API has 5s delivery SLA, too slow for our use case +``` + +
+ +![T3](https://img.shields.io/badge/T3-Cyclomatic_Complexity_Thresholds_N--A-lightgray) **Cyclomatic Complexity Thresholds** ⊘ + +![T3](https://img.shields.io/badge/T3-Structured_Logging_N--A-lightgray) **Structured Logging** ⊘ + +![T3](https://img.shields.io/badge/T3-OpenAPI%2FSwagger_Specifications_N--A-lightgray) **OpenAPI/Swagger Specifications** ⊘ + +![T4](https://img.shields.io/badge/T4-Code_Smell_Elimination_0--100-red) **Code Smell Elimination** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: none (Threshold: ≥60% of applicable linters configured) + +**Evidence**: +- No linters configured + +Configure 3 missing linter(s) + +1. Configure golangci-lint for Go +2. Add actionlint for GitHub Actions workflow validation +3. Configure markdownlint for documentation quality + +**Commands**: +```bash +go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest +npm install --save-dev markdownlint-cli && touch .markdownlint.json +``` + +**Examples**: +``` +# .pylintrc example +[MASTER] +max-line-length=100 + +[MESSAGES CONTROL] +disable=C0111 +``` +``` +# .eslintrc.json example +{ + "extends": "eslint:recommended", + "rules": { + "no-console": "warn" + } +} +``` + +
+ +![T4](https://img.shields.io/badge/T4-Issue_%26_Pull_Request_Templates_0--100-red) **Issue & Pull Request Templates** ❌ 0/100 +
+📝 Remediation Steps + +**Measured**: PR:False, Issues:0 (Threshold: PR template + ≥2 issue templates) + +**Evidence**: +- No PR template found +- No issue template directory found + +Create GitHub issue and PR templates in .github/ directory + +1. Create .github/ directory if it doesn't exist +2. Add PULL_REQUEST_TEMPLATE.md for PRs +3. Create .github/ISSUE_TEMPLATE/ directory +4. Add bug_report.md for bug reports +5. Add feature_request.md for feature requests +6. Optionally add config.yml to configure template chooser + +**Commands**: +```bash +# Create directories +mkdir -p .github/ISSUE_TEMPLATE + +# Create PR template +cat > .github/PULL_REQUEST_TEMPLATE.md << 'EOF' +## Summary + + +## Related Issues +Fixes # + +## Testing +- [ ] Tests added/updated +- [ ] All tests pass + +## Checklist +- [ ] Documentation updated +- [ ] CHANGELOG.md updated +EOF +``` + +**Examples**: +``` +# Bug Report Template (.github/ISSUE_TEMPLATE/bug_report.md) + +```markdown +--- +name: Bug Report +about: Create a report to help us improve +title: '[BUG] ' +labels: bug +assignees: '' +--- + +**Describe the bug** +A clear description of what the bug is. + +**To Reproduce** +Steps to reproduce: +1. Go to '...' +2. Click on '....' +3. See error + +**Expected behavior** +What you expected to happen. + +**Environment** +- OS: [e.g. macOS 13.0] +- Version: [e.g. 1.0.0] +``` + +``` + +
+ +![T4](https://img.shields.io/badge/T4-Branch_Protection_Rules_N--A-lightgray) **Branch Protection Rules** ⊘ + +![T4](https://img.shields.io/badge/T4-Container%2FVirtualization_Setup_N--A-lightgray) **Container/Virtualization Setup** ⊘ + +![T4](https://img.shields.io/badge/T4-Progressive_Disclosure_N--A-lightgray) **Progressive Disclosure** ⊘ + + +--- + +## 📝 Assessment Metadata + +- **AgentReady Version**: v2.35.2 +- **Research Version**: v2.0.0 +- **Repository Snapshot**: 01f875a573b9cfc13eacc29e4604c71def861880 +- **Assessment Duration**: 0.2s +- **Assessed By**: rhuss@rhuss-mac +- **Assessment Date**: May 12, 2026 at 9:19 AM + +🤖 Generated with [Claude Code](https://claude.com/claude-code) \ No newline at end of file diff --git a/.agentready/report-latest.html b/.agentready/report-latest.html new file mode 120000 index 00000000..44640fda --- /dev/null +++ b/.agentready/report-latest.html @@ -0,0 +1 @@ +report-20260512-091921.html \ No newline at end of file diff --git a/.agentready/report-latest.md b/.agentready/report-latest.md new file mode 120000 index 00000000..304d8b23 --- /dev/null +++ b/.agentready/report-latest.md @@ -0,0 +1 @@ +report-20260512-091921.md \ No newline at end of file diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 00000000..1a591c1d --- /dev/null +++ b/.mcp.json @@ -0,0 +1,38 @@ +{ + "mcpServers": { + "google-work": { + "autoApprove": [ + "*" + ], + "type": "http", + "url": "https://mcp-google-work.int-tichny.org:8443/mcp" + }, + "jira-redhat-old": { + "autoApprove": [ + "*" + ], + "headers": { + "Authorization": "Basic bWNwOmlJOTF3OHg4UXdycWVBbnlrMmRK" + }, + "type": "http", + "url": "https://mcp-jira.int-tichny.org:8443/mcp" + }, + "readwise": { + "autoApprove": [ + "*" + ], + "headers": { + "Authorization": "Token vauXnECbxfufJcRJo9e43fDfRI5DWnuJw8k7VHgGdjlqrFE5n9" + }, + "type": "http", + "url": "https://mcp-readwise.int-tichny.org:8443/mcp" + }, + "slack-redhat": { + "headers": { + "Authorization": "Basic bWNwOjdiNjhlMjhlM2I0YzEwODg0Yjk1NjViYjFmMmE1Y2My" + }, + "type": "sse", + "url": "https://mcp-slack.int-tichny.org:8443/sse" + } + } +} diff --git a/brainstorm/00-overview.md b/brainstorm/00-overview.md new file mode 100644 index 00000000..f0ee63a9 --- /dev/null +++ b/brainstorm/00-overview.md @@ -0,0 +1,21 @@ +# Brainstorm Overview + +Last updated: 2026-05-26 + +## Sessions + +| # | Date | Topic | Status | Spec | +|---|------|-------|--------|------| +| 01 | 2026-05-20 | agentcard-into-agentruntime | completed | 001 | +| 02 | 2026-05-21 | identity-binding-migration | idea | - | +| 03 | 2026-05-26 | card-discovery-refinement-alignment | active | - | + +## Open Threads + +- Transport security field design: enum vs free-form string (from #03) +- CardSynced vs CardFetched rename: waiting on Ian's response (from #03) +- ConfigMap fetch path transportSecurity value (from #03) + +## Parked Ideas + +(none) diff --git a/brainstorm/01-agentcard-into-agentruntime.md b/brainstorm/01-agentcard-into-agentruntime.md new file mode 100644 index 00000000..93c26afd --- /dev/null +++ b/brainstorm/01-agentcard-into-agentruntime.md @@ -0,0 +1,82 @@ +# Brainstorm: AgentCard Data Into AgentRuntime Status + +**Date:** 2026-05-20 +**Status:** active + +## Problem Framing + +AgentRuntime and AgentCard are two CRDs with identical cardinality (one per workload), the same namespace, the same lifecycle, and the same owner. AgentCard is a pure read-only CR whose content is entirely controller-managed. Its data (card metadata, verification status, binding state) fits naturally into AgentRuntime's `status` section. + +The AgentCard CRD has three specific problems documented in ADR ODH-ADR-AgentOps-0002: + +1. It conflates observation with policy. The CR is controller-created and controller-written, leaving no room for admin-authored policy fields. +2. Its JWS signing pipeline signs a skeleton card with empty skills/capabilities (#292). The signed content and live content are disconnected. +3. Maintaining two CRs for the same Deployment doubles the RBAC surface and splits "how this agent participates in the platform" across two APIs. + +This brainstorm covers the first step: moving card data into AgentRuntime status. mTLS, policy fields, and AgentCard removal are separate follow-up items. + +## Context + +- ADR: ODH-ADR-AgentOps-0002 (Agent Network Policy and mTLS Identity) +- Upstream issue: kagenti-operator#371 (Consolidate AgentCard into AgentRuntime status) +- Related: kagenti-operator#292 (skeleton-card problem) +- Related: kagenti-operator#284 (mTLS verified fetch, merged 2026-05-20, infrastructure reusable later) +- Upstream sync: IBM maintainers agreed to AgentCard deprecation path (2026-05-15) +- RHAISTRAT-1599 AC review: acceptance criteria updated to reflect AgentRuntime-based discovery + +## Approaches Considered + +### A: Extend the existing AgentRuntime controller (chosen) + +Add a card fetch phase to the existing `agentruntime_controller.go` reconciliation loop. After resolving the target workload and applying labels, the controller fetches `/.well-known/agent-card.json` from the agent's Service endpoint over plain HTTP, parses it into an A2A-compliant struct, and writes it to `status.card`. Triggered by Pod template hash change on the target workload. + +- Pros: Minimal new code. Reuses existing workload watches and reconciliation infrastructure. Single controller, single reconcile loop. +- Cons: Makes agentruntime_controller.go larger (already 29K). Card fetch adds network I/O to a controller that currently only does Kubernetes API calls. + +### B: New dedicated card discovery controller + +Create a separate `agentruntime_card_controller.go` that watches AgentRuntime CRs and handles only card fetching. The main controller continues handling labels, sidecar injection, and config. + +- Pros: Clean separation. Card fetch failures don't block main reconciliation. Independent rate limiter for network I/O. +- Cons: Two controllers watching the same CRD. Need to coordinate status updates. More moving parts for a simple HTTP GET. + +### C: Card fetch as a Kubernetes Job + +Create short-lived Jobs to fetch cards on rollout events. + +- Pros: Completely decouples fetching from the controller. Reusable binary. +- Cons: Heavy for a simple HTTP GET. Adds Job RBAC, cleanup, failure handling. Overkill. + +## Decision + +**Approach A: Extend the existing AgentRuntime controller.** The card fetch is a single HTTP GET that takes milliseconds. If performance becomes an issue at scale (hundreds of agents), extracting to a separate controller (Approach B) is a clean refactor. + +## Key Requirements + +### What gets built + +- New `AgentCardStatus` struct on AgentRuntime CRD, modeled on the A2A protocol agent-card.json spec (name, description, skills, protocols, endpoint). Not mirrored from the existing AgentCard CRD fields. +- Card fetch phase added to the existing AgentRuntime controller reconcile loop. +- Fetch triggers on Pod template hash change (rollout events only, no polling, no periodic fallback). +- Fetch is plain HTTP GET to `/.well-known/agent-card.json` via the agent's Service endpoint. +- Card data written to `status.card` on AgentRuntime. + +### What gets deprecated + +- AgentCard CRD gets a deprecation log warning on creation. +- AgentCard remains functional (both CRDs coexist during transition). + +### Out of scope (future iterations) + +- mTLS for the card fetch (port from #284). +- `spec.policy` fields (allowedIngressNamespaces, dependencies, externalEgress). +- AgentCard CRD removal and controller cleanup. +- ValidatingAdmissionPolicy for label restriction. +- Migration tooling. + +## Open Questions + +- Exact A2A AgentCard JSON schema fields to model in the Go struct +- How to discover the agent's Service endpoint from the AgentRuntime targetRef (resolve Deployment -> Service via selector matching or naming convention) +- Feature flag name and default (e.g. `--enable-card-discovery`, off by default) +- Whether `status.card.fetchedAt` timestamp should be included for diagnostics diff --git a/brainstorm/03-card-discovery-refinement-alignment.md b/brainstorm/03-card-discovery-refinement-alignment.md new file mode 100644 index 00000000..b421a3cc --- /dev/null +++ b/brainstorm/03-card-discovery-refinement-alignment.md @@ -0,0 +1,191 @@ +# Brainstorm: Card Discovery Refinement Alignment + +**Date:** 2026-05-26 +**Status:** active +**Origin:** Divergence analysis between upstream implementation (PR #372) and RHAIENG-4948 refinement document +**Depends on:** Response from @iamiller on RHAIENG-4948 comment + +## Problem + +PR #372 merged the card discovery feature upstream. The RHAIENG-4948 refinement document describes the same feature but with different naming, condition semantics, and a transport security model that the implementation doesn't capture. Since the PR just merged, we can still make breaking changes without migration concerns. This is a window to align the implementation with the refinement doc where it makes the design better, before consumers depend on the current field names and condition reasons. + +## What to Adopt + +### 1. Transport Security Field on CardStatus + +**What the refinement doc says:** Condition reasons should distinguish `FetchSucceeded` (mTLS) from `FetchSucceededPlainHTTP` (plain HTTP). + +**What we propose instead:** A dedicated `transportSecurity` field on `CardStatus`, plus transport-aware condition reasons. + +```go +// TransportSecurity indicates the transport layer used for the card fetch. +// +optional +TransportSecurity string `json:"transportSecurity,omitempty"` +``` + +Values: `"mTLS"`, `"plainHTTP"`. + +**Why a field is better than condition reasons alone:** +- Condition reasons are transient. When the condition flips to `False` on the next failure, the transport info from the last successful fetch is lost. A field persists with the card data. +- Consumers (UI, policy engines, compliance dashboards) can query `.status.card.transportSecurity` directly without parsing condition reasons. +- We already have `attestedAgentSpiffeID` which implies mTLS when non-empty, but that's implicit. An explicit field is clearer. + +**In addition to the field**, the condition reason should reflect transport: +- `CardSynced` (mTLS verified) +- `CardSyncedInsecure` (plain HTTP, no transport identity) +- Or keep a single `CardSynced` reason and let consumers check the field + +**Implementation:** +- `fetchCard()` already branches on `AuthenticatedFetcher` vs `AgentFetcher`. Set `transportSecurity` based on which path executed. +- The mTLS path (SPIFFE fetcher) sets `"mTLS"`. +- The HTTP fallback path (DefaultFetcher) sets `"plainHTTP"`. +- The ConfigMap path (signed card from init-container) could set `"configMap"` for completeness. + +**Why this matters:** +A card fetched over plain HTTP within the cluster network has no transport-level identity guarantee. Any pod in the same namespace could serve a fake card to the controller. Knowing the transport mode lets platform engineers assess their security posture: "all my agents have `transportSecurity: mTLS`" vs "agent X is still on plainHTTP because SPIRE isn't configured in that namespace." + +### 2. Split ServiceNotFound into ServiceNotFound + WorkloadNotReady + +**What the refinement doc says:** Use `WorkloadNotReady` when the workload has no Ready pods, separate from service resolution failure. + +**Current behavior:** Our `ServiceNotFound` reason conflates two distinct cases: +1. The workload exists but has no matching Service (configuration issue) +2. The workload isn't ready yet (transient, will resolve on its own) + +These need different operator responses: +- `ServiceNotFound`: check your Service configuration, selector labels, port names +- `WorkloadNotReady`: wait, the pods are starting up + +**Implementation:** +- In `fetchAndUpdateCard`, check workload readiness (Ready pods > 0) before attempting service resolution. +- If workload has zero Ready pods, set `CardSynced=False, reason=WorkloadNotReady`. +- If workload is ready but no Service matches, keep `CardSynced=False, reason=ServiceNotFound`. + +### 3. Unified Condition Model (Rename + Restructure) + +The refinement doc and our implementation each have gaps. Neither model is complete. The merged model below takes the best from both. + +**Problems with our current model:** +- `CardSynced` as both condition type AND reason reads as a stutter in `kubectl describe`: `CardSynced True CardSynced` +- No transport awareness. Can't tell from `kubectl describe` whether the fetch was secure. +- `ServiceNotFound` conflates "no Service exists" with "workload not ready" + +**Problems with the refinement doc model:** +- Missing `FetchSkipped` (no-op when pod template unchanged) +- Missing `DiscoveryDisabled` (feature flag off) +- `FetchPending` adds an `Unknown` state without actionable signal + +**Proposed merged model:** + +| Type | Status | Reason | When | +|------|--------|--------|------| +| `CardFetched` | True | `Fetched` | mTLS fetch succeeded | +| `CardFetched` | True | `FetchedInsecure` | plain HTTP fetch (no transport identity) | +| `CardFetched` | True | `FetchSkipped` | pod template unchanged, existing data valid | +| `CardFetched` | False | `FetchFailed` | timeout, connection error, invalid JSON, 404 | +| `CardFetched` | False | `ServiceNotFound` | workload exists but no matching Service | +| `CardFetched` | False | `WorkloadNotReady` | workload has no ready pods | +| `CardFetched` | False | `DiscoveryDisabled` | feature flag off, stale data cleared | + +**Why this is better than either model alone:** + +1. **Condition type `CardFetched`**: past participle as adjective (like `Scheduled`, `Initialized` in core Kubernetes). Accurately describes event-driven fetch, no "sync" implication. No stutter with reason names. + +2. **Transport in the reason (`Fetched` vs `FetchedInsecure`)**: visible in `kubectl describe` without needing `-o yaml`. A platform engineer sees `CardFetched True FetchedInsecure` and immediately knows there's a security gap. The `transportSecurity` field is the machine-readable source of truth; the reason is the human-readable signal. + +3. **`FetchSkipped` preserved** (from our model): important for operators watching rollouts. Tells them "nothing changed, existing data is still valid." Without it, after a non-template spec change (like scaling), the condition stays at the previous reason, which is confusing. + +4. **`WorkloadNotReady` + `ServiceNotFound` split**: different operator response. `WorkloadNotReady` = wait, pods are starting. `ServiceNotFound` = check your Service labels and selectors. + +5. **`DiscoveryDisabled`** (shortened from `CardDiscoveryDisabled`): since the condition type already says `Card`, the prefix is redundant. + +**Why we can do this now:** +The PR just merged. No external consumers depend on the `CardSynced` condition type yet. No downstream tooling parses it. The printer column can be renamed in the same change. If we wait, this becomes a breaking API change requiring migration. + +**Implementation:** +- Rename condition type constant and printer column +- Update `fetchAndUpdateCard` to set reason based on which fetch path executed (authenticated vs default fetcher) +- Update all test assertions +- Update constitution reference + +### 4. Rename cardId to cardHash + +**Current field:** `cardId` (type `string`, holds a SHA-256 content hash). + +**Problem:** `cardId` suggests a unique identifier assigned to the card (like a UUID or database ID). The field actually holds a SHA-256 content hash used for change detection: "has the card content changed since last fetch?" Calling it `cardId` misleads consumers into treating it as a stable identifier. + +**Proposed:** Rename to `cardHash`. Matches the refinement doc's `CardHash` naming and accurately describes the field's purpose: a content hash for diffing. + +**Implementation:** Rename the Go field, JSON tag, and CRD schema. Update `computeCardContentHash` callers and tests. Straightforward find-and-replace. + +### 5. Port Resolution Chain (Refinement Doc's Approach) + +**Current implementation:** `serviceHTTPPort()` looks for generic HTTP port names (`http`, `https`, `grpc`), falls back to first port. + +**Refinement doc:** `kagenti.io/port annotation` -> port named `a2a` -> first port -> default 8000. + +**Why the doc's approach is better:** + +1. **Protocol-specific port name (`a2a`)**: We're fetching A2A agent cards, not generic HTTP content. If a Service has an admin port named `http` on port 80 and an A2A port named `a2a` on port 8000, our current code picks the wrong one. + +2. **Annotation escape hatch (`kagenti.io/port`)**: Standard Kubernetes pattern for when auto-detection fails. Near-zero implementation cost (check one annotation before the existing port resolution). Useful for multi-port Services, non-standard configurations, or when the A2A endpoint runs on an unusual port. + +3. **Default 8000**: The A2A Python SDK defaults to port 8000. Our current code has no default and relies entirely on what's in the Service spec. + +**Proposed resolution chain:** +1. `kagenti.io/port` annotation on the Service (explicit override, highest priority) +2. Service port named `a2a` (protocol-specific match) +3. Service port named `http` (generic fallback, backward compatible) +4. First port in Service spec (last resort) + +This is backward compatible: existing Services with port named `http` still work. Services that add a port named `a2a` get more precise resolution. The annotation is there for edge cases. + +**Implementation:** Modify `serviceHTTPPort()` to check the annotation first, then look for `a2a` port name before falling back to `http`/first port. Small change, no new dependencies. + +### 6. Rename fetchedAt to lastCardFetchTime + +**Current field:** `fetchedAt` (type `*metav1.Time`). + +**Problem:** On a `CardStatus` struct that holds agent card data, `fetchedAt` is ambiguous. It could mean "when was this card originally published by the agent." The field actually records when the controller last fetched the card from the agent's endpoint. + +**Proposed:** Rename to `lastCardFetchTime`. Self-documenting: it's the timestamp of the last controller-initiated fetch. The `last` prefix conveys that multiple fetches happen over time (one per rollout), which matches the event-driven design. + +**Implementation:** Rename Go field, JSON tag, and CRD schema. Same find-and-replace as `cardHash`. + +## What NOT to Adopt + +### FetchPending Intermediate State + +The doc suggests `CardFetched=Unknown, reason=FetchPending` when a rollout is detected but pods aren't ready. In practice, the reconcile either succeeds or fails. There's no meaningful window where the controller holds a "pending" state between reconciles. controller-runtime requeueing handles this naturally. Adding an intermediate condition state adds complexity without actionable signal. + +## Implementation Scope + +| Change | Breaking? | Effort | Files | +|--------|-----------|--------|-------| +| Add `transportSecurity` field to `CardStatus` | Additive | Small | `agentruntime_types.go`, `zz_generated.deepcopy.go`, CRD YAMLs | +| Populate `transportSecurity` from fetch path | No | Small | `agentruntime_controller.go` (in `fetchCard`) | +| Unified condition model (type rename + reason restructure) | Condition type + reasons | Medium | Controller, tests, CRD printer column, constitution | +| Rename `cardId` to `cardHash` | Field rename | Small | `agentruntime_types.go`, controller, tests, CRD YAMLs | +| Rename `fetchedAt` to `lastCardFetchTime` | Field rename | Small | `agentruntime_types.go`, controller, tests, CRD YAMLs | +| Port resolution: annotation + `a2a` port name | Behavior change | Small | `agentruntime_controller.go` (`serviceHTTPPort`) | +| Tests for all of the above | No | Medium | `agentruntime_controller_test.go` | + +Total estimate: 1 day of work, all in one PR. + +## Open Questions + +1. Should `transportSecurity` be an enum (string with known values) or a free-form string? Enum is safer for consumers but less extensible. +2. The ConfigMap fetch path (signed card from init-container): should `transportSecurity` be `"configMap"`, `"signed"`, or something else? This path is being deprecated but still exists during coexistence. +3. Wait for Ian's response on RHAIENG-4948 before implementing, or proceed since the changes improve on both models? + +## Decision Needed + +Waiting for Ian's response on RHAIENG-4948 before proceeding. If he agrees the current approach is fine, we adopt only the substantive improvements (transport security field, workload readiness split) without the naming changes. If he prefers alignment with the doc, we do the full set including the `CardFetched` rename. + +## References + +- PR #372: upstream implementation (merged 2026-05-25) +- RHAIENG-4948: refinement document with sequence diagrams +- RHAIENG-4944: parent epic (Agent Discovery via mTLS) +- Brainstorm #01: original card-into-agentruntime brainstorm +- Constitution v1.0.0: controller-runtime safety principles diff --git a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml index 2ff3803f..d64afb0a 100644 --- a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml +++ b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml @@ -30,9 +30,9 @@ spec: jsonPath: .status.phase name: Phase type: string - - description: Card Sync Status - jsonPath: .status.conditions[?(@.type=='CardSynced')].status - name: CardSynced + - description: Card Fetch Status + jsonPath: .status.conditions[?(@.type=='CardFetched')].status + name: CardFetched priority: 1 type: string - jsonPath: .metadata.creationTimestamp @@ -287,9 +287,9 @@ spec: description: Indicates if the agent supports streaming responses. type: boolean type: object - cardId: - description: CardID is a SHA-256 content hash of the fetched card - data. + cardHash: + description: CardHash is a SHA-256 content hash of the fetched + card data. type: string defaultInputModes: description: |- @@ -312,14 +312,14 @@ spec: description: A URL providing additional documentation about the agent. type: string - fetchedAt: - description: FetchedAt is the timestamp of the last successful - card fetch. - format: date-time - type: string iconUrl: description: A URL to an icon for the agent. type: string + lastCardFetchTime: + description: LastCardFetchTime is the timestamp of the last successful + card fetch. + format: date-time + type: string name: description: A human-readable name for the agent. type: string @@ -445,6 +445,13 @@ spec: Indicates if the agent supports providing an extended agent card when authenticated. type: boolean + transportSecurity: + description: TransportSecurity indicates the transport layer used + for the card fetch. + enum: + - mtls + - http + type: string url: description: The URL of the agent's endpoint. type: string diff --git a/kagenti-operator/api/v1alpha1/agentruntime_types.go b/kagenti-operator/api/v1alpha1/agentruntime_types.go index bbc0f1e9..72c81b11 100644 --- a/kagenti-operator/api/v1alpha1/agentruntime_types.go +++ b/kagenti-operator/api/v1alpha1/agentruntime_types.go @@ -37,6 +37,14 @@ const ( RuntimePhaseError RuntimePhase = "Error" ) +// +kubebuilder:validation:Enum=mtls;http +type TransportSecurity string + +const ( + TransportSecurityMTLS TransportSecurity = "mtls" + TransportSecurityHTTP TransportSecurity = "http" +) + // AgentRuntimeSpec defines the desired state of AgentRuntime. type AgentRuntimeSpec struct { // Type classifies the workload as an agent or tool @@ -153,18 +161,22 @@ type SPIFFEIdentity struct { type CardStatus struct { AgentCardData `json:",inline"` - // FetchedAt is the timestamp of the last successful card fetch. + // LastCardFetchTime is the timestamp of the last successful card fetch. // +optional - FetchedAt *metav1.Time `json:"fetchedAt,omitempty"` + LastCardFetchTime *metav1.Time `json:"lastCardFetchTime,omitempty"` - // CardID is a SHA-256 content hash of the fetched card data. + // CardHash is a SHA-256 content hash of the fetched card data. // +optional - CardID string `json:"cardId,omitempty"` + CardHash string `json:"cardHash,omitempty"` // Protocol is the detected agent protocol (e.g., "a2a"). // +optional Protocol string `json:"protocol,omitempty"` + // TransportSecurity indicates the transport layer used for the card fetch. + // +optional + TransportSecurity TransportSecurity `json:"transportSecurity,omitempty"` + // ValidSignature is the result of JWS signature verification. // +optional ValidSignature *bool `json:"validSignature,omitempty"` @@ -242,7 +254,7 @@ type AgentRuntimeStatus struct { // +kubebuilder:printcolumn:name="Type",type="string",JSONPath=".spec.type",description="Workload Type" // +kubebuilder:printcolumn:name="Target",type="string",JSONPath=".spec.targetRef.name",description="Target Workload" // +kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase",description="Runtime Phase" -// +kubebuilder:printcolumn:name="CardSynced",type="string",JSONPath=".status.conditions[?(@.type=='CardSynced')].status",description="Card Sync Status",priority=1 +// +kubebuilder:printcolumn:name="CardFetched",type="string",JSONPath=".status.conditions[?(@.type=='CardFetched')].status",description="Card Fetch Status",priority=1 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // AgentRuntime attaches runtime configuration to a backing workload classified as an diff --git a/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go b/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go index 5f1bc422..d49ba544 100644 --- a/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -486,8 +486,8 @@ func (in *BindingStatus) DeepCopy() *BindingStatus { func (in *CardStatus) DeepCopyInto(out *CardStatus) { *out = *in in.AgentCardData.DeepCopyInto(&out.AgentCardData) - if in.FetchedAt != nil { - in, out := &in.FetchedAt, &out.FetchedAt + if in.LastCardFetchTime != nil { + in, out := &in.LastCardFetchTime, &out.LastCardFetchTime *out = (*in).DeepCopy() } if in.ValidSignature != nil { diff --git a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml index 2ff3803f..d64afb0a 100644 --- a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml +++ b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml @@ -30,9 +30,9 @@ spec: jsonPath: .status.phase name: Phase type: string - - description: Card Sync Status - jsonPath: .status.conditions[?(@.type=='CardSynced')].status - name: CardSynced + - description: Card Fetch Status + jsonPath: .status.conditions[?(@.type=='CardFetched')].status + name: CardFetched priority: 1 type: string - jsonPath: .metadata.creationTimestamp @@ -287,9 +287,9 @@ spec: description: Indicates if the agent supports streaming responses. type: boolean type: object - cardId: - description: CardID is a SHA-256 content hash of the fetched card - data. + cardHash: + description: CardHash is a SHA-256 content hash of the fetched + card data. type: string defaultInputModes: description: |- @@ -312,14 +312,14 @@ spec: description: A URL providing additional documentation about the agent. type: string - fetchedAt: - description: FetchedAt is the timestamp of the last successful - card fetch. - format: date-time - type: string iconUrl: description: A URL to an icon for the agent. type: string + lastCardFetchTime: + description: LastCardFetchTime is the timestamp of the last successful + card fetch. + format: date-time + type: string name: description: A human-readable name for the agent. type: string @@ -445,6 +445,13 @@ spec: Indicates if the agent supports providing an extended agent card when authenticated. type: boolean + transportSecurity: + description: TransportSecurity indicates the transport layer used + for the card fetch. + enum: + - mtls + - http + type: string url: description: The URL of the agent's endpoint. type: string diff --git a/kagenti-operator/internal/controller/agentruntime_controller.go b/kagenti-operator/internal/controller/agentruntime_controller.go index 5fd27bf6..be824ffe 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller.go +++ b/kagenti-operator/internal/controller/agentruntime_controller.go @@ -71,7 +71,7 @@ const ( ConditionTypeReady = "Ready" ConditionTypeTargetResolved = "TargetResolved" ConditionTypeConfigResolved = "ConfigResolved" - ConditionTypeCardSynced = "CardSynced" + ConditionTypeCardFetched = "CardFetched" // AnnotationLastCardFetchHash stores the change-detection key used to skip // redundant card fetches when the workload's pod template has not changed. @@ -518,7 +518,7 @@ func (r *AgentRuntimeReconciler) resolveServiceForWorkload(ctx context.Context, svc := &corev1.Service{} if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: namespace}, svc); err == nil { - port := serviceHTTPPort(svc) + port := serviceHTTPPort(ctx, svc) logger.V(1).Info("Resolved service by name", "service", ref.Name, "port", port) return svc, port, nil } @@ -546,7 +546,7 @@ func (r *AgentRuntimeReconciler) resolveServiceForWorkload(ctx context.Context, continue } if selectorMatchesLabels(s.Spec.Selector, podLabels) { - port := serviceHTTPPort(s) + port := serviceHTTPPort(ctx, s) logger.V(1).Info("Resolved service by selector match", "service", s.Name, "port", port) return s, port, nil } @@ -555,6 +555,33 @@ func (r *AgentRuntimeReconciler) resolveServiceForWorkload(ctx context.Context, return nil, 0, fmt.Errorf("no Service matches workload %s/%s in namespace %s", ref.Kind, ref.Name, namespace) } +// checkWorkloadReady checks whether the target workload has at least one ready +// replica. For Sandboxes, the check is skipped (always returns true) because +// their lifecycle is managed by the sandbox controller. +func (r *AgentRuntimeReconciler) checkWorkloadReady(ctx context.Context, namespace string, ref agentv1alpha1.TargetRef) (bool, string) { + switch ref.Kind { + case "Deployment": //nolint:goconst + dep := &appsv1.Deployment{} + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: namespace}, dep); err != nil { + return false, fmt.Sprintf("failed to get Deployment %s: %v", ref.Name, err) + } + if dep.Status.ReadyReplicas == 0 { + return false, fmt.Sprintf("Deployment %s has 0 ready replicas", ref.Name) + } + case "StatefulSet": //nolint:goconst + sts := &appsv1.StatefulSet{} + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: namespace}, sts); err != nil { + return false, fmt.Sprintf("failed to get StatefulSet %s: %v", ref.Name, err) + } + if sts.Status.ReadyReplicas == 0 { + return false, fmt.Sprintf("StatefulSet %s has 0 ready replicas", ref.Name) + } + case KindSandbox: + // Sandboxes are unstructured; skip readiness check. + } + return true, "" +} + func selectorMatchesLabels(selector, labels map[string]string) bool { for k, v := range selector { if labels[k] != v { @@ -564,9 +591,25 @@ func selectorMatchesLabels(selector, labels map[string]string) bool { return true } -func serviceHTTPPort(svc *corev1.Service) int32 { +func serviceHTTPPort(ctx context.Context, svc *corev1.Service) int32 { + logger := log.FromContext(ctx) + + if ann, ok := svc.Annotations["kagenti.io/port"]; ok { + port, err := strconv.ParseInt(ann, 10, 32) + if err == nil && port > 0 { + return int32(port) + } + logger.Info("Invalid kagenti.io/port annotation, falling back to port name resolution", + "service", svc.Name, "annotation", ann) + } + + for _, p := range svc.Spec.Ports { + if strings.EqualFold(p.Name, "a2a") { + return p.Port + } + } for _, p := range svc.Spec.Ports { - if strings.EqualFold(p.Name, "http") || p.Port == 80 || p.Port == 8080 || p.Port == 8000 { + if strings.EqualFold(p.Name, "http") { return p.Port } } @@ -713,7 +756,7 @@ func (r *AgentRuntimeReconciler) fetchAndUpdateCard(ctx context.Context, rt *age if !r.EnableCardDiscovery { if rt.Status.Card != nil { rt.Status.Card = nil - r.setCondition(rt, ConditionTypeCardSynced, metav1.ConditionFalse, "CardDiscoveryDisabled", + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionFalse, "DiscoveryDisabled", "Card discovery is disabled; stale card data cleared") } return @@ -726,39 +769,46 @@ func (r *AgentRuntimeReconciler) fetchAndUpdateCard(ctx context.Context, rt *age lastHash = annotations[AnnotationLastCardFetchHash] } if changeKey != "" && changeKey == lastHash && rt.Status.Card != nil { - r.setCondition(rt, ConditionTypeCardSynced, metav1.ConditionTrue, "FetchSkipped", + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionTrue, "FetchSkipped", "Pod template unchanged; existing card data still valid") return } + if ready, msg := r.checkWorkloadReady(ctx, rt.Namespace, rt.Spec.TargetRef); !ready { + logger.V(1).Info("Workload not ready for card discovery", "reason", msg) + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionFalse, "WorkloadNotReady", msg) + return + } + svc, port, err := r.resolveServiceForWorkload(ctx, rt.Namespace, rt.Spec.TargetRef) if err != nil { logger.V(1).Info("Service resolution failed for card discovery", "error", err) - r.setCondition(rt, ConditionTypeCardSynced, metav1.ConditionFalse, "ServiceNotFound", err.Error()) + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionFalse, "ServiceNotFound", err.Error()) return } protocol := agentcard.A2AProtocol - cardData, fetchResult, err := r.fetchCard(ctx, rt, svc, port, protocol) + cardData, fetchResult, transportSecurity, err := r.fetchCard(ctx, rt, svc, port, protocol) if err != nil { logger.Error(err, "Card fetch failed", "workload", rt.Spec.TargetRef.Name) - r.setCondition(rt, ConditionTypeCardSynced, metav1.ConditionFalse, "CardFetchFailed", err.Error()) + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionFalse, "FetchFailed", err.Error()) return } - newCardID := computeCardContentHash(cardData) + newCardHash := computeCardContentHash(cardData) cardStatus := &agentv1alpha1.CardStatus{ - AgentCardData: *cardData, - CardID: newCardID, - Protocol: protocol, + AgentCardData: *cardData, + CardHash: newCardHash, + Protocol: protocol, + TransportSecurity: transportSecurity, } - if rt.Status.Card != nil && rt.Status.Card.CardID == newCardID { - cardStatus.FetchedAt = rt.Status.Card.FetchedAt + if rt.Status.Card != nil && rt.Status.Card.CardHash == newCardHash { + cardStatus.LastCardFetchTime = rt.Status.Card.LastCardFetchTime } else { now := metav1.Now() - cardStatus.FetchedAt = &now + cardStatus.LastCardFetchTime = &now } if fetchResult != nil && fetchResult.AgentSpiffeID != "" { @@ -778,7 +828,12 @@ func (r *AgentRuntimeReconciler) fetchAndUpdateCard(ctx context.Context, rt *age } rt.Status.Card = cardStatus - r.setCondition(rt, ConditionTypeCardSynced, metav1.ConditionTrue, "CardSynced", + + conditionReason := "Fetched" + if transportSecurity == agentv1alpha1.TransportSecurityHTTP { + conditionReason = "FetchedInsecure" + } + r.setCondition(rt, ConditionTypeCardFetched, metav1.ConditionTrue, conditionReason, fmt.Sprintf("Successfully fetched agent card for %s", cardData.Name)) r.persistCardFetchAnnotation(ctx, rt, changeKey) @@ -815,7 +870,7 @@ func (r *AgentRuntimeReconciler) persistCardFetchAnnotation(ctx context.Context, func (r *AgentRuntimeReconciler) fetchCard( ctx context.Context, rt *agentv1alpha1.AgentRuntime, svc *corev1.Service, port int32, protocol string, -) (*agentv1alpha1.AgentCardData, *agentcard.FetchResult, error) { +) (*agentv1alpha1.AgentCardData, *agentcard.FetchResult, agentv1alpha1.TransportSecurity, error) { logger := log.FromContext(ctx) ref := rt.Spec.TargetRef @@ -825,12 +880,12 @@ func (r *AgentRuntimeReconciler) fetchCard( secureURL := agentcard.GetSecureServiceURL(svc.Name, rt.Namespace, tlsPort) fetchResult, err := r.AuthenticatedFetcher.FetchAuthenticated(ctx, protocol, secureURL) if err != nil { - return nil, nil, fmt.Errorf("authenticated fetch failed for %s: %w", ref.Name, err) + return nil, nil, "", fmt.Errorf("authenticated fetch failed for %s: %w", ref.Name, err) } if fetchResult.CardData == nil { - return nil, nil, fmt.Errorf("authenticated fetch returned nil card data for %s", ref.Name) + return nil, nil, "", fmt.Errorf("authenticated fetch returned nil card data for %s", ref.Name) } - return fetchResult.CardData, fetchResult, nil + return fetchResult.CardData, fetchResult, agentv1alpha1.TransportSecurityMTLS, nil } logger.Info("TLS port not found, falling back to HTTP fetch", "service", svc.Name, "expectedPortName", AgentTLSPortName) @@ -841,15 +896,18 @@ func (r *AgentRuntimeReconciler) fetchCard( } if r.AgentFetcher == nil { - return nil, nil, fmt.Errorf("no fetcher configured for card discovery") + return nil, nil, "", fmt.Errorf("no fetcher configured for card discovery") } serviceURL := agentcard.GetServiceURL(svc.Name, rt.Namespace, port) cardData, err := r.AgentFetcher.Fetch(ctx, protocol, serviceURL, ref.Name, rt.Namespace) if err != nil { - return nil, nil, fmt.Errorf("fetch failed for %s: %w", ref.Name, err) + return nil, nil, "", fmt.Errorf("fetch failed for %s: %w", ref.Name, err) + } + if cardData == nil { + return nil, nil, "", fmt.Errorf("fetch returned nil card data for %s", ref.Name) } - return cardData, nil, nil + return cardData, nil, agentv1alpha1.TransportSecurityHTTP, nil } // workloadChangeKey returns a string that changes when the workload's pod diff --git a/kagenti-operator/internal/controller/agentruntime_controller_test.go b/kagenti-operator/internal/controller/agentruntime_controller_test.go index 15b9763c..09c09a09 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller_test.go +++ b/kagenti-operator/internal/controller/agentruntime_controller_test.go @@ -18,6 +18,7 @@ package controller import ( "context" + "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -33,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" agentv1alpha1 "github.com/kagenti/operator/api/v1alpha1" + "github.com/kagenti/operator/internal/agentcard" webhookconfig "github.com/kagenti/operator/internal/webhook/config" ) @@ -45,6 +47,15 @@ func (f *stubCardFetcher) Fetch(_ context.Context, _, _, _, _ string) (*agentv1a return f.card, f.err } +type stubAuthenticatedFetcher struct { + result *agentcard.FetchResult + err error +} + +func (f *stubAuthenticatedFetcher) FetchAuthenticated(_ context.Context, _, _ string) (*agentcard.FetchResult, error) { + return f.result, f.err +} + var _ = Describe("AgentRuntime Controller", func() { const ( rtName = "test-agentruntime" @@ -96,6 +107,18 @@ var _ = Describe("AgentRuntime Controller", func() { } } + setDeploymentReady := func(name, ns string) { + Eventually(func() error { + cur := &appsv1.Deployment{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: ns}, cur); err != nil { + return err + } + cur.Status.Replicas = 1 + cur.Status.ReadyReplicas = 1 + return k8sClient.Status().Update(ctx, cur) + }).Should(Succeed()) + } + newReconciler := func() *AgentRuntimeReconciler { return &AgentRuntimeReconciler{ Client: k8sClient, @@ -852,12 +875,12 @@ var _ = Describe("AgentRuntime Controller", func() { var cardCond *metav1.Condition for i := range rt.Status.Conditions { - if rt.Status.Conditions[i].Type == ConditionTypeCardSynced { + if rt.Status.Conditions[i].Type == ConditionTypeCardFetched { cardCond = &rt.Status.Conditions[i] break } } - Expect(cardCond).To(BeNil(), "No CardSynced condition should be set when card was already nil") + Expect(cardCond).To(BeNil(), "No CardFetched condition should be set when card was already nil") }) It("should clear existing card data when feature flag is disabled", func() { @@ -866,8 +889,8 @@ var _ = Describe("AgentRuntime Controller", func() { ObjectMeta: metav1.ObjectMeta{Name: "clear-card-rt", Namespace: namespace}, Status: agentv1alpha1.AgentRuntimeStatus{ Card: &agentv1alpha1.CardStatus{ - AgentCardData: agentv1alpha1.AgentCardData{Name: "old-agent"}, - FetchedAt: &now, + AgentCardData: agentv1alpha1.AgentCardData{Name: "old-agent"}, + LastCardFetchTime: &now, }, }, } @@ -881,20 +904,21 @@ var _ = Describe("AgentRuntime Controller", func() { var cardCond *metav1.Condition for i := range rt.Status.Conditions { - if rt.Status.Conditions[i].Type == ConditionTypeCardSynced { + if rt.Status.Conditions[i].Type == ConditionTypeCardFetched { cardCond = &rt.Status.Conditions[i] break } } Expect(cardCond).NotTo(BeNil()) Expect(cardCond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cardCond.Reason).To(Equal("CardDiscoveryDisabled")) + Expect(cardCond.Reason).To(Equal("DiscoveryDisabled")) }) It("should set ServiceNotFound condition when no service exists", func() { dep := newDeployment("card-no-svc-deploy", namespace) Expect(k8sClient.Create(ctx, dep)).To(Succeed()) defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady("card-no-svc-deploy", namespace) rt := &agentv1alpha1.AgentRuntime{ ObjectMeta: metav1.ObjectMeta{Name: "card-no-svc-rt", Namespace: namespace}, @@ -913,7 +937,7 @@ var _ = Describe("AgentRuntime Controller", func() { var cardCond *metav1.Condition for i := range rt.Status.Conditions { - if rt.Status.Conditions[i].Type == ConditionTypeCardSynced { + if rt.Status.Conditions[i].Type == ConditionTypeCardFetched { cardCond = &rt.Status.Conditions[i] break } @@ -935,9 +959,9 @@ var _ = Describe("AgentRuntime Controller", func() { }, Status: agentv1alpha1.AgentRuntimeStatus{ Card: &agentv1alpha1.CardStatus{ - AgentCardData: agentv1alpha1.AgentCardData{Name: "previous-agent", Version: "1.0"}, - FetchedAt: &now, - CardID: "abc123", + AgentCardData: agentv1alpha1.AgentCardData{Name: "previous-agent", Version: "1.0"}, + LastCardFetchTime: &now, + CardHash: "abc123", }, }, } @@ -950,11 +974,11 @@ var _ = Describe("AgentRuntime Controller", func() { Expect(rt.Status.Card).NotTo(BeNil(), "existing card data should be retained on fetch failure") Expect(rt.Status.Card.Name).To(Equal("previous-agent")) - Expect(rt.Status.Card.CardID).To(Equal("abc123")) + Expect(rt.Status.Card.CardHash).To(Equal("abc123")) var cardCond *metav1.Condition for i := range rt.Status.Conditions { - if rt.Status.Conditions[i].Type == ConditionTypeCardSynced { + if rt.Status.Conditions[i].Type == ConditionTypeCardFetched { cardCond = &rt.Status.Conditions[i] break } @@ -973,7 +997,7 @@ var _ = Describe("AgentRuntime Controller", func() { r := &AgentRuntimeReconciler{Client: k8sClient, EnableCardDiscovery: false} r.fetchAndUpdateCard(ctx, rt) Expect(rt.Status.Card).To(BeNil()) - // No CardSynced condition should be set when card was already nil + // No CardFetched condition should be set when card was already nil }) It("should clear populated card data when flag is toggled off", func() { @@ -982,8 +1006,8 @@ var _ = Describe("AgentRuntime Controller", func() { ObjectMeta: metav1.ObjectMeta{Name: "toggle-off-populated-rt", Namespace: namespace}, Status: agentv1alpha1.AgentRuntimeStatus{ Card: &agentv1alpha1.CardStatus{ - AgentCardData: agentv1alpha1.AgentCardData{Name: "stale-agent"}, - FetchedAt: &now, + AgentCardData: agentv1alpha1.AgentCardData{Name: "stale-agent"}, + LastCardFetchTime: &now, }, }, } @@ -994,12 +1018,13 @@ var _ = Describe("AgentRuntime Controller", func() { }) Context("Card annotation patch must not wipe in-memory status", func() { - It("should persist CardSynced condition and card data after annotation patch", func() { + It("should persist CardFetched condition and card data after annotation patch", func() { depName := "card-patch-deploy" svcName := depName dep := newDeployment(depName, namespace) Expect(k8sClient.Create(ctx, dep)).To(Succeed()) defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{Name: svcName, Namespace: namespace}, @@ -1043,13 +1068,13 @@ var _ = Describe("AgentRuntime Controller", func() { Expect(rt.Status.Card).NotTo(BeNil(), "card data must not be wiped by annotation patch") Expect(rt.Status.Card.Name).To(Equal("Test Agent")) Expect(rt.Status.Card.Version).To(Equal("2.0")) - Expect(rt.Status.Card.CardID).NotTo(BeEmpty()) + Expect(rt.Status.Card.CardHash).NotTo(BeEmpty()) - // CardSynced condition must survive - cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardSynced) - Expect(cardCond).NotTo(BeNil(), "CardSynced condition must not be wiped by annotation patch") + // CardFetched condition must survive (stub fetcher uses plain HTTP path) + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil(), "CardFetched condition must not be wiped by annotation patch") Expect(cardCond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cardCond.Reason).To(Equal("CardSynced")) + Expect(cardCond.Reason).To(Equal("FetchedInsecure")) // Conditions set before fetchAndUpdateCard must also survive targetCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeTargetResolved) @@ -1059,6 +1084,433 @@ var _ = Describe("AgentRuntime Controller", func() { }) }) + Context("Transport security visibility (US1)", func() { + It("should set transportSecurity plainHTTP and reason FetchedInsecure for stub fetcher", func() { + depName := "transport-plain-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{{Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}}, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("transport-plain-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Plain Agent", Version: "1.0"}, + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).NotTo(BeNil()) + Expect(rt.Status.Card.TransportSecurity).To(Equal(agentv1alpha1.TransportSecurityHTTP)) + + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cardCond.Reason).To(Equal("FetchedInsecure")) + }) + + It("should set transportSecurity mTLS and reason Fetched for authenticated fetcher", func() { + depName := "transport-mtls-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + {Name: AgentTLSPortName, Port: 8443, Protocol: corev1.ProtocolTCP}, + }, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("transport-mtls-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AuthenticatedFetcher: &stubAuthenticatedFetcher{ + result: &agentcard.FetchResult{ + CardData: &agentv1alpha1.AgentCardData{Name: "Secure Agent", Version: "2.0"}, + AgentSpiffeID: "spiffe://trust.domain/agent", + }, + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).NotTo(BeNil()) + Expect(rt.Status.Card.TransportSecurity).To(Equal(agentv1alpha1.TransportSecurityMTLS)) + Expect(rt.Status.Card.AttestedAgentSpiffeID).To(Equal("spiffe://trust.domain/agent")) + + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cardCond.Reason).To(Equal("Fetched")) + }) + + It("should update transport security when transport changes on re-fetch", func() { + depName := "transport-change-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + {Name: AgentTLSPortName, Port: 8443, Protocol: corev1.ProtocolTCP}, + }, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("transport-change-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + // First fetch: mTLS + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AuthenticatedFetcher: &stubAuthenticatedFetcher{ + result: &agentcard.FetchResult{ + CardData: &agentv1alpha1.AgentCardData{Name: "Agent", Version: "1.0"}, + }, + }, + } + r.fetchAndUpdateCard(ctx, rt) + Expect(rt.Status.Card).NotTo(BeNil()) + Expect(rt.Status.Card.TransportSecurity).To(Equal(agentv1alpha1.TransportSecurityMTLS)) + + // Clear the change key annotation to force a re-fetch + annotations := rt.GetAnnotations() + delete(annotations, AnnotationLastCardFetchHash) + rt.SetAnnotations(annotations) + + // Second fetch: plain HTTP (no authenticated fetcher) + r2 := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Agent", Version: "1.0"}, + }, + } + r2.fetchAndUpdateCard(ctx, rt) + Expect(rt.Status.Card).NotTo(BeNil()) + Expect(rt.Status.Card.TransportSecurity).To(Equal(agentv1alpha1.TransportSecurityHTTP)) + + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Reason).To(Equal("FetchedInsecure")) + }) + }) + + Context("Unified condition model (US2)", func() { + It("should set WorkloadNotReady when Deployment has zero readyReplicas", func() { + depName := "unready-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + + // Deployment starts with 0 readyReplicas (default) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{{Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}}, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("unready-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Agent", Version: "1.0"}, + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).To(BeNil()) + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cardCond.Reason).To(Equal("WorkloadNotReady")) + }) + + It("should set ServiceNotFound when workload is ready but no Service exists", func() { + depName := "ready-no-svc-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + + setDeploymentReady(depName, namespace) + + rt := newAgentRuntime("ready-no-svc-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Agent", Version: "1.0"}, + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).To(BeNil()) + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cardCond.Reason).To(Equal("ServiceNotFound")) + }) + + It("should set DiscoveryDisabled when feature flag is off", func() { + now := metav1.Now() + rt := &agentv1alpha1.AgentRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "us2-disabled-rt", Namespace: namespace}, + Status: agentv1alpha1.AgentRuntimeStatus{ + Card: &agentv1alpha1.CardStatus{ + AgentCardData: agentv1alpha1.AgentCardData{Name: "old"}, + LastCardFetchTime: &now, + }, + }, + } + r := &AgentRuntimeReconciler{Client: k8sClient, EnableCardDiscovery: false} + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).To(BeNil()) + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cardCond.Reason).To(Equal("DiscoveryDisabled")) + }) + }) + + Context("FetchSkipped and FetchFailed conditions (US2)", func() { + It("should set FetchSkipped when pod template has not changed", func() { + depName := "skip-fetch-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{{Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}}, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("skip-fetch-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Agent", Version: "1.0"}, + }, + } + + // First fetch succeeds and persists the change key annotation + r.fetchAndUpdateCard(ctx, rt) + Expect(rt.Status.Card).NotTo(BeNil()) + + // Second fetch with unchanged template should skip + r.fetchAndUpdateCard(ctx, rt) + + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cardCond.Reason).To(Equal("FetchSkipped")) + }) + + It("should set FetchFailed when fetcher returns an error", func() { + depName := "fetch-fail-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{{Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}}, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("fetch-fail-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + err: fmt.Errorf("connection refused"), + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).To(BeNil()) + cardCond := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeCardFetched) + Expect(cardCond).NotTo(BeNil()) + Expect(cardCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cardCond.Reason).To(Equal("FetchFailed")) + Expect(cardCond.Message).To(ContainSubstring("connection refused")) + }) + }) + + Context("Accurate field names (US3)", func() { + It("should populate cardHash as SHA-256 hex and lastCardFetchTime as RFC 3339 via envtest", func() { + depName := "field-names-deploy" + dep := newDeployment(depName, namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + setDeploymentReady(depName, namespace) + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: depName, Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": depName}, + Ports: []corev1.ServicePort{{Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}}, + }, + } + Expect(k8sClient.Create(ctx, svc)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, svc) }() + + rt := newAgentRuntime("field-names-rt", namespace, depName, agentv1alpha1.RuntimeTypeAgent) + Expect(k8sClient.Create(ctx, rt)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rt) }() + + r := &AgentRuntimeReconciler{ + Client: k8sClient, + EnableCardDiscovery: true, + AgentFetcher: &stubCardFetcher{ + card: &agentv1alpha1.AgentCardData{Name: "Field Test Agent", Version: "1.0"}, + }, + } + r.fetchAndUpdateCard(ctx, rt) + + Expect(rt.Status.Card).NotTo(BeNil()) + Expect(rt.Status.Card.CardHash).To(HaveLen(64), "cardHash should be a 64-char SHA-256 hex string") + Expect(rt.Status.Card.CardHash).To(MatchRegexp("^[a-f0-9]{64}$")) + Expect(rt.Status.Card.LastCardFetchTime).NotTo(BeNil()) + Expect(rt.Status.Card.LastCardFetchTime.UTC().Format("2006-01-02T15:04:05Z07:00")).To(MatchRegexp(`^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$`)) + }) + }) + + Context("Protocol-aware port resolution (US4)", func() { + It("should use kagenti.io/port annotation when present", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "port-anno-svc", + Namespace: namespace, + Annotations: map[string]string{"kagenti.io/port": "9090"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + {Name: "a2a", Port: 8888, Protocol: corev1.ProtocolTCP}, + }, + }, + } + port := serviceHTTPPort(ctx, svc) + Expect(port).To(Equal(int32(9090))) + }) + + It("should use port named a2a when no annotation", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "port-a2a-svc", Namespace: namespace}, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "grpc", Port: 50051, Protocol: corev1.ProtocolTCP}, + {Name: "a2a", Port: 8888, Protocol: corev1.ProtocolTCP}, + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + }, + }, + } + port := serviceHTTPPort(ctx, svc) + Expect(port).To(Equal(int32(8888))) + }) + + It("should fall back to port name resolution on invalid annotation", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "port-invalid-anno-svc", + Namespace: namespace, + Annotations: map[string]string{"kagenti.io/port": "not-a-number"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "a2a", Port: 8888, Protocol: corev1.ProtocolTCP}, + }, + }, + } + port := serviceHTTPPort(ctx, svc) + Expect(port).To(Equal(int32(8888))) + }) + + It("should prefer annotation over a2a port name", func() { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "port-anno-vs-a2a-svc", + Namespace: namespace, + Annotations: map[string]string{"kagenti.io/port": "7777"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "a2a", Port: 8888, Protocol: corev1.ProtocolTCP}, + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + }, + }, + } + port := serviceHTTPPort(ctx, svc) + Expect(port).To(Equal(int32(7777))) + }) + }) + Context("Sandbox workload support", func() { It("should create a Sandbox accessor that reads/writes pod template labels and annotations", func() { acc, ok := newRuntimePodTemplateAccessor("Sandbox") diff --git a/specs/001-agentcard-into-status/review-findings.md b/specs/001-agentcard-into-status/review-findings.md new file mode 100644 index 00000000..ef24b0e4 --- /dev/null +++ b/specs/001-agentcard-into-status/review-findings.md @@ -0,0 +1,195 @@ +# Deep Review Findings + +**Date:** 2026-05-21 +**Branch:** 001-agentcard-into-status +**Rounds:** 1 +**Gate Outcome:** PASS +**Invocation:** manual + +## Summary + +| Severity | Found | Fixed | Remaining | +|----------|-------|-------|-----------| +| Critical | 3 | 3 | 0 | +| Important | 10 | 3 | 7 | +| Minor | 5 | - | 5 | +| **Total** | **18** | **6** | **12** | + +**Agents completed:** 5/5 (+ 1 external tool) +**Agents failed:** none + +## Findings + +### FINDING-1 +- **Severity:** Critical +- **Confidence:** 95 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:635-707 +- **Category:** correctness / production-readiness +- **Source:** correctness-agent (also reported by: architecture-agent, prod-readiness-agent) +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +`AnnotationLastCardFetchHash` was set on `rt.ObjectMeta.Annotations` in-memory but never persisted. `Status().Update()` only writes the status subresource, not metadata. Combined with `FetchedAt` being set to `metav1.Now()` on every fetch, this created an infinite reconciliation loop: each reconcile fetched the card (skip-check never matched), set a new timestamp, wrote status, triggering re-reconciliation. + +**Why this matters:** +Continuous API server load proportional to AgentRuntime count. Saturates the controller's work queue and hits agent workloads with constant HTTP requests. + +**How it was resolved:** +Added `persistCardFetchAnnotation()` using `client.MergeFrom` patch to write the annotation to the API server separately from the status update. Also changed `FetchedAt` to only update when `CardID` changes (content actually changed), preventing no-op status writes from triggering re-reconciliation. + +### FINDING-2 +- **Severity:** Critical +- **Confidence:** 95 +- **File:** kagenti-operator/internal/controller/agentcard_controller_test.go:1726-1818 +- **Category:** test-quality +- **Source:** test-quality-agent (also reported by: coderabbit) +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +The deprecation warning test had no assertions verifying the deprecation event was emitted. The reconciler had no `Recorder` set, so `r.Recorder.Event()` was skipped entirely. The test passed even without the deprecation feature. + +**Why this matters:** +FR-006 requires deprecation warnings. Without proper test assertions, the behavior could regress silently. + +**How it was resolved:** +Added `record.NewFakeRecorder(10)` to the test reconciler and assertion that a "Deprecated" event was emitted. + +### FINDING-3 +- **Severity:** Critical +- **Confidence:** 90 +- **File:** kagenti-operator/internal/controller/agentruntime_controller_test.go (missing) +- **Category:** test-quality +- **Source:** test-quality-agent +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +No test for FR-013 (retain last card data on fetch failure). A regression that cleared `status.card` on failure would not be caught. + +**Why this matters:** +FR-013 explicitly requires card data retention on failure. + +**How it was resolved:** +Added "Card data retention on fetch failure (FR-013)" test that verifies existing card data is preserved when fetch fails and `CardSynced` condition is set to False. + +### FINDING-4 +- **Severity:** Important +- **Confidence:** 80 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:748-761 +- **Category:** correctness +- **Source:** correctness-agent +- **Round found:** 1 +- **Resolution:** remaining (intentional) + +**What is wrong:** +`workloadChangeKey` uses `GetGeneration()` which increments on any spec change (including replica count), not just pod template changes. + +**Why this matters:** +Scaling a Deployment triggers an unnecessary card re-fetch. + +**Why it remains:** +The spec says "Pod template hash changes (or generation for StatefulSets/Sandboxes)". Using generation is simpler and causes at most one extra fetch per scaling event. The cost is a single HTTP GET. Switching to pod-template-hash computation would add complexity for minimal benefit. + +### FINDING-5 +- **Severity:** Important +- **Confidence:** 92 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:501-520 +- **Category:** architecture +- **Source:** architecture-agent +- **Round found:** 1 +- **Resolution:** remaining (deferred) + +**What is wrong:** +`serviceHTTPPort` and `getAgentTLSPort` are duplicated between AgentCard and AgentRuntime controllers with diverging implementations. + +**Why it remains:** +The functions are small and self-contained. Extracting to a shared utility is a follow-up refactor that should be done when the AgentCard controller is deprecated. Not a correctness issue. + +### FINDING-6 +- **Severity:** Important +- **Confidence:** 90 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:763-773 +- **Category:** architecture +- **Source:** architecture-agent +- **Round found:** 1 +- **Resolution:** remaining (deferred) + +**What is wrong:** +`computeCardContentHash` is functionally identical to `AgentCardReconciler.computeCardID`. + +**Why it remains:** +Same as FINDING-5. The duplication will be resolved when AgentCard is fully deprecated. + +### FINDING-7 +- **Severity:** Important +- **Confidence:** 85 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:666 +- **Category:** architecture +- **Source:** architecture-agent +- **Round found:** 1 +- **Resolution:** remaining (intentional) + +**What is wrong:** +Protocol is hardcoded to `agentcard.A2AProtocol`. The AgentCard controller detects protocol from workload labels. + +**Why it remains:** +This feature is specifically about A2A card discovery. The spec only mentions A2A. Multi-protocol support can be added when the need arises. + +### FINDING-8 +- **Severity:** Important +- **Confidence:** 90 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:200, 668-671 +- **Category:** production-readiness +- **Source:** prod-readiness-agent +- **Round found:** 1 +- **Resolution:** remaining (acceptable) + +**What is wrong:** +No `RequeueAfter` returned on card fetch failure. Transient failures won't retry until the next reconcile trigger. + +**Why it remains:** +The reconcile loop already re-triggers on any workload change. Card discovery is best-effort and non-blocking. Adding retry logic would add complexity for a status field that is supplementary to the core runtime function. + +### FINDING-9 +- **Severity:** Important +- **Confidence:** 90 +- **File:** kagenti-operator/internal/controller/agentruntime_controller_test.go (missing) +- **Category:** test-quality +- **Source:** test-quality-agent (also reported by: coderabbit) +- **Round found:** 1 +- **Resolution:** remaining (requires envtest) + +**What is wrong:** +No happy-path test that exercises successful card fetch through `fetchAndUpdateCard` with a mock fetcher. + +**Why it remains:** +The test requires setting up a `mockFetcher` implementation and injecting it into the reconciler. The `agentcard.Fetcher` mock type only exists in `agentcard_controller_test.go` and is not exported. Creating an equivalent mock in the agentruntime test would be straightforward but tests require envtest infrastructure. Deferred to CI. + +### FINDING-10 +- **Severity:** Important +- **Confidence:** 50 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:711-745 +- **Category:** security +- **Source:** security-agent +- **Round found:** 1 +- **Resolution:** remaining (by design) + +**What is wrong:** +mTLS fallback to plaintext HTTP when TLS port is missing. + +**Why it remains:** +The fallback is intentional for gradual rollout. A warning Event is emitted. The spec's acceptance scenario US2.3 explicitly says "without mTLS configured, the fetch uses plain HTTP." A strict mode can be added in a follow-up iteration. + +### FINDING-11 (Minor) + +`CardId` renamed to `CardID` per Go acronym conventions. Fixed in round 1. + +### FINDING-12 (Minor) + +Nil check added on `fetchResult.CardData` after `FetchAuthenticated`. Fixed in round 1. + +## Remaining Findings + +All remaining Important findings are architectural decisions (deferred duplication cleanup) or test coverage gaps requiring envtest infrastructure. No Critical findings remain. diff --git a/specs/002-card-discovery-alignment/REVIEWERS.md b/specs/002-card-discovery-alignment/REVIEWERS.md new file mode 100644 index 00000000..2f2b5693 --- /dev/null +++ b/specs/002-card-discovery-alignment/REVIEWERS.md @@ -0,0 +1,83 @@ +# Review Guide: Card Discovery Refinement Alignment + +**Generated**: 2026-05-27 | **Spec**: [spec.md](spec.md) + +## Why This Change + +PR #372 merged the card discovery feature (fetching A2A agent cards into AgentRuntime status). Live cluster testing exposed three API quality issues: (1) platform engineers cannot tell whether a card was fetched securely (mTLS) or over plain HTTP without reading operator logs, (2) the condition model has a naming stutter (`CardSynced True CardSynced` in kubectl output) and conflates unrelated failure modes, and (3) field names are misleading (`cardId` holds a content hash, not an identifier). Since the PR just merged and no external consumers exist yet, we have a narrow window to fix these before the API surface becomes locked. + +## What Changes + +Six breaking changes to the AgentRuntime CRD status fields: + +1. New `transportSecurity` field on `CardStatus` records whether the card was fetched over mTLS or plain HTTP +2. Condition type renamed from `CardSynced` to `CardFetched` with transport-aware reasons (`Fetched` vs `FetchedInsecure`) +3. `cardId` field renamed to `cardHash` (it holds a SHA-256 content hash, not an identifier) +4. `fetchedAt` field renamed to `lastCardFetchTime` (clarifies this is the controller's fetch timestamp, not the card's creation time) +5. Port resolution now checks `kagenti.io/port` annotation, then port named `a2a`, before falling back to `http`/first port +6. New `WorkloadNotReady` condition reason split from `ServiceNotFound` + +All changes are backward-incompatible CRD field/condition renames. No migration needed since there are no consumers yet. + +## How It Works + +The changes modify four layers: + +**Types** (`api/v1alpha1/agentruntime_types.go`): Rename `CardID` to `CardHash`, `FetchedAt` to `LastCardFetchTime`, add `TransportSecurity string` field. Update printer column marker from `CardSynced` to `CardFetched`. + +**Controller** (`internal/controller/agentruntime_controller.go`): Rename `ConditionTypeCardSynced` constant to `ConditionTypeCardFetched`. In `fetchCard()`, determine transport security from the fetch path (authenticated fetcher returns `"mTLS"`, default fetcher returns `"plainHTTP"`). In `fetchAndUpdateCard()`, set transport-aware condition reasons and the new `TransportSecurity` field. Add `checkWorkloadReady()` helper before service resolution. Replace `serviceHTTPPort()` body with the annotation-first resolution chain. + +**CRD** (auto-generated from types via `make manifests`): Schema reflects new field names and printer column. + +**Tests** (`internal/controller/agentruntime_controller_test.go`): Migrate all existing card test assertions to new names. Add new tests for transport security, workload readiness, and port resolution annotation. + +## When It Applies + +**Applies when**: +- Card discovery is enabled (`--enable-card-discovery` flag) +- An AgentRuntime targets a workload (Deployment, StatefulSet, or Sandbox) that serves `/.well-known/agent-card.json` +- The platform engineer reads `status.card` or the `CardFetched` condition via kubectl, automation, or UI + +**Does not apply when**: +- Card discovery is disabled (existing behavior unchanged) +- Identity binding enforcement (deferred to the identity binding migration) +- AgentCard CRD deprecation or removal (separate epic) +- Cross-cluster or multi-cluster card discovery (out of scope) + +## Key Decisions + +1. **`transportSecurity` as a field, not just a condition reason.** Condition reasons are transient (lost on the next status transition). A field persists with the card data, letting consumers (UI, policy engines) query it directly. The condition reason also reflects transport for `kubectl describe` visibility. + +2. **Condition model where every reason maps to one diagnostic action.** `CardFetched` as the type (past participle, like Kubernetes' `Scheduled` or `Initialized`) instead of `CardSynced` (implies polling, which we don't do). Transport-aware reasons (`Fetched` vs `FetchedInsecure`) for security visibility. `WorkloadNotReady` split from `ServiceNotFound` because they require different operator responses (wait vs check config). `FetchSkipped` and `DiscoveryDisabled` retained from PR #372 for feature flag and no-op cases. + +3. **Port resolution with annotation escape hatch.** The `kagenti.io/port` annotation on Services handles multi-port edge cases where auto-detection picks the wrong port. The `a2a` port name takes priority over generic `http` because we're specifically fetching A2A protocol cards. + +4. **`cardHash` over `cardId`.** The field holds a SHA-256 content hash for change detection, not a unique identifier. `cardId` suggests a stable ID (like a UUID); `cardHash` accurately conveys "content fingerprint that changes when the card changes." + +5. **Workload readiness check before service resolution.** Distinguishes "pods aren't ready yet" (transient, wait) from "no matching Service" (configuration issue, fix it). Different diagnostic actions for the operator. + +## Areas Needing Attention + +- **Breaking changes**: All field renames and the condition type rename are CRD-breaking. This is acceptable because PR #372 just merged with no external consumers yet. If any downstream tooling has already started parsing `cardId` or `CardSynced`, this will break it. + +- **ConfigMap fetch path**: The `transportSecurity` value for the ConfigMap path (signed card from init-container) is `"configMap"`. This path is being deprecated but still exists during coexistence. Reviewers should assess whether this value is appropriate or whether `"signed"` better describes the semantics. + +- **Constitution update**: The project constitution (`.specify/memory/constitution.md`) references `CardSynced` from the original bug fix. It needs to be updated to `CardFetched` as part of this change (FR-009). + +## Open Questions + +1. Should `transportSecurity` values be documented as a formal enum in the CRD description, or left as free-form strings for extensibility (e.g., future `"ztunnel"` value)? +2. Are there any downstream consumers already parsing `cardId` or `CardSynced` that we're not aware of? If so, the breaking window may be shorter than assumed. + +## Review Checklist + +- [ ] Key decisions are justified +- [ ] Breaking changes are documented with migration guidance +- [ ] Scope matches the stated boundaries +- [ ] Success criteria are achievable +- [ ] No unstated assumptions +- [ ] Condition reasons are exhaustive (every code path sets a reason) +- [ ] `transportSecurity` field is set on every successful fetch path +- [ ] Port resolution chain matches FR-006 (annotation > a2a > http > first > default) +- [ ] Old field names (`cardId`, `fetchedAt`, `CardSynced`) are fully removed from CRD schema +- [ ] Constitution updated to reflect new condition type diff --git a/specs/002-card-discovery-alignment/checklists/requirements.md b/specs/002-card-discovery-alignment/checklists/requirements.md new file mode 100644 index 00000000..26e04522 --- /dev/null +++ b/specs/002-card-discovery-alignment/checklists/requirements.md @@ -0,0 +1,36 @@ +# Specification Quality Checklist: Card Discovery Refinement Alignment + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-05-26 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass. Spec is ready for `/speckit-clarify` or `/speckit-plan`. +- No [NEEDS CLARIFICATION] markers. All decisions were resolved in the brainstorm session. +- The spec intentionally references specific field names and condition reasons because this is a CRD API alignment feature where naming IS the requirement. diff --git a/specs/002-card-discovery-alignment/data-model.md b/specs/002-card-discovery-alignment/data-model.md new file mode 100644 index 00000000..a5b3d806 --- /dev/null +++ b/specs/002-card-discovery-alignment/data-model.md @@ -0,0 +1,80 @@ +# Data Model: Card Discovery Refinement Alignment + +## CardStatus (modified) + +```go +type CardStatus struct { + AgentCardData `json:",inline"` + + // LastCardFetchTime is the timestamp of the last successful card fetch. + // Renamed from FetchedAt for clarity: this records when the controller + // fetched the card, not when the card was created by the agent. + // +optional + LastCardFetchTime *metav1.Time `json:"lastCardFetchTime,omitempty"` + + // CardHash is a SHA-256 content hash of the fetched card data. + // Used for change detection across rollouts. + // Renamed from CardID to accurately describe its purpose. + // +optional + CardHash string `json:"cardHash,omitempty"` + + // Protocol is the detected agent protocol (e.g., "a2a"). + // +optional + Protocol string `json:"protocol,omitempty"` + + // TransportSecurity indicates the transport layer used for the card fetch. + // Values: "mTLS" (SPIFFE-verified), "plainHTTP" (no transport identity). + // +optional + TransportSecurity string `json:"transportSecurity,omitempty"` + + // ValidSignature is the result of JWS signature verification. + // +optional + ValidSignature *bool `json:"validSignature,omitempty"` + + // SignatureKeyID is the key ID from the verified JWS header. + // +optional + SignatureKeyID string `json:"signatureKeyID,omitempty"` + + // SignatureVerificationDetails contains details or errors from signature verification. + // +optional + SignatureVerificationDetails string `json:"signatureVerificationDetails,omitempty"` + + // AttestedAgentSpiffeID is the SPIFFE ID extracted from the mTLS peer certificate. + // +optional + AttestedAgentSpiffeID string `json:"attestedAgentSpiffeID,omitempty"` +} +``` + +## Field Changes Summary + +| Old Name | New Name | JSON Tag | Reason | +|----------|----------|----------|--------| +| `FetchedAt` | `LastCardFetchTime` | `lastCardFetchTime` | Clarifies this is controller fetch time, not card creation time | +| `CardID` | `CardHash` | `cardHash` | Clarifies this is a content hash, not a unique identifier | +| (new) | `TransportSecurity` | `transportSecurity` | Records transport layer used for fetch | + +## Condition Model + +| Constant | Old Value | New Value | +|----------|-----------|-----------| +| `ConditionTypeCardSynced` | `"CardSynced"` | Renamed to `ConditionTypeCardFetched` = `"CardFetched"` | + +### Condition Reasons + +| Old Reason | New Reason | Status | When | +|------------|------------|--------|------| +| `"CardSynced"` | `"Fetched"` | True | mTLS fetch succeeded | +| (new) | `"FetchedInsecure"` | True | Plain HTTP fetch succeeded | +| `"FetchSkipped"` | `"FetchSkipped"` | True | Pod template unchanged | +| `"CardFetchFailed"` | `"FetchFailed"` | False | Fetch error | +| `"ServiceNotFound"` | `"ServiceNotFound"` | False | No matching Service | +| (new) | `"WorkloadNotReady"` | False | No Ready pods | +| `"CardDiscoveryDisabled"` | `"DiscoveryDisabled"` | False | Feature flag off | + +## Port Resolution Annotation + +| Annotation | Scope | Type | Example | +|------------|-------|------|---------| +| `kagenti.io/port` | Service | string (numeric) | `"9090"` | + +Resolution chain: annotation > port named `a2a` > port named `http` > first port > default 8000 diff --git a/specs/002-card-discovery-alignment/plan.md b/specs/002-card-discovery-alignment/plan.md new file mode 100644 index 00000000..7731d27f --- /dev/null +++ b/specs/002-card-discovery-alignment/plan.md @@ -0,0 +1,74 @@ +# Implementation Plan: Card Discovery Refinement Alignment + +**Branch**: `002-card-discovery-alignment` | **Date**: 2026-05-27 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `specs/002-card-discovery-alignment/spec.md` + +## Summary + +Improve the card discovery API surface (merged in PR #372) based on findings from live cluster testing and API review: add transport security visibility, fix misleading field names, align condition model with Kubernetes conventions, and add protocol-aware port resolution. Six changes: add `transportSecurity` field, rename condition type `CardSynced` to `CardFetched` with transport-aware reasons, rename `cardId` to `cardHash` and `fetchedAt` to `lastCardFetchTime`, implement protocol-aware port resolution with annotation override, and add workload readiness check. All changes are breaking but acceptable since the API has no external consumers yet. + +## Technical Context + +**Language/Version**: Go 1.26 +**Primary Dependencies**: controller-runtime v0.23.3, k8s.io/api, k8s.io/apimachinery +**Storage**: Kubernetes etcd (via CRD status subresource) +**Testing**: Ginkgo/Gomega with controller-runtime envtest +**Target Platform**: Kubernetes 1.35+ +**Project Type**: Kubernetes operator (kubebuilder scaffold) +**Performance Goals**: N/A (refactoring, no new reconcile overhead) +**Constraints**: Must not regress existing card discovery behavior; all 181 tests must pass +**Scale/Scope**: 6 files modified, ~200 lines changed + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Reconciler Status Integrity | PASS | `persistCardFetchAnnotation` save/restore pattern preserved | +| II. Spec-Anchored Testing | PASS | Tests will verify via envtest API server read-back | +| III. Controller-Runtime Safety | PASS | No new Patch/Update calls between status mutations and Status().Update | +| IV. CRD-First Design | PASS | All field renames reflected in both Go types and CRD YAML | +| V. Feature-Gated Rollout | PASS | No new feature flags; changes are within existing `--enable-card-discovery` gate | + +## Project Structure + +### Documentation (this feature) + +```text +specs/002-card-discovery-alignment/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +└── tasks.md # Phase 2 output (via /speckit-tasks) +``` + +### Source Code (files to modify) + +```text +kagenti-operator/ +├── api/v1alpha1/ +│ ├── agentruntime_types.go # CardStatus struct (field renames + new field) +│ └── zz_generated.deepcopy.go # Auto-generated +├── cmd/ +│ └── main.go # No changes needed (fetcher wiring unchanged) +├── config/ +│ ├── crd/bases/ +│ │ └── agent.kagenti.dev_agentruntimes.yaml # CRD schema +│ └── rbac/ +│ └── role.yaml # No changes needed +├── charts/kagenti-operator/ +│ └── crds/ +│ └── agent.kagenti.dev_agentruntimes.yaml # Helm chart CRD +├── internal/controller/ +│ ├── agentruntime_controller.go # Condition constants, fetchAndUpdateCard, serviceHTTPPort +│ └── agentruntime_controller_test.go # All card-related tests +└── .specify/memory/ + └── constitution.md # Update condition references +``` + +**Structure Decision**: Modifying existing files only. No new files or directories needed. + +## Complexity Tracking + +No constitution violations. No complexity justifications needed. diff --git a/specs/002-card-discovery-alignment/research.md b/specs/002-card-discovery-alignment/research.md new file mode 100644 index 00000000..793c0074 --- /dev/null +++ b/specs/002-card-discovery-alignment/research.md @@ -0,0 +1,94 @@ +# Research: Card Discovery Refinement Alignment + +## R-001: How to determine transport security from the fetch path + +**Decision**: Set `transportSecurity` based on which fetcher executed in `fetchCard()`. + +**Rationale**: The `fetchCard` function already branches on `AuthenticatedFetcher` (mTLS via SPIFFE) vs `AgentFetcher` (plain HTTP via ConfigMapFetcher/DefaultFetcher). The transport is deterministic from the code path, not from inspecting the connection post-hoc. + +**Implementation**: +- `fetchCard` returns `(cardData, fetchResult, transportSecurity, error)` (add string return) +- mTLS path (line 822-835): returns `"mTLS"` +- HTTP fallback path (line 838-852): returns `"plainHTTP"` +- ConfigMap path (inside ConfigMapFetcher.Fetch when CM found): would need a signal back. Since the ConfigMapFetcher is part of `AgentFetcher`, the caller can't distinguish CM-hit from HTTP-fallback without modifying the Fetcher interface. + +**Alternative considered**: Modify the `Fetcher` interface to return transport metadata. Rejected because it changes an interface used by multiple implementations and tests. + +**Revised approach**: Check `fetchResult != nil` (only returned by authenticated fetcher) to determine mTLS. If `fetchResult == nil`, it's plainHTTP. ConfigMap path can be distinguished if needed by checking `fetchResult` type, but for now `"plainHTTP"` is acceptable since the ConfigMap path is being deprecated. + +## R-002: Port resolution annotation name + +**Decision**: Use `kagenti.io/port` on the Service. + +**Rationale**: Follows the project's existing annotation namespace (`kagenti.io/type`, `kagenti.io/framework`). Placed on the Service (not the Deployment) because the port is a Service-level concern. + +**Alternatives considered**: +- `agent.kagenti.dev/port`: uses the CRD API group, which is for CRD fields not annotations +- `protocol.kagenti.io/a2a-port`: too specific, couples annotation to one protocol + +## R-003: Workload readiness check placement + +**Decision**: Check `readyReplicas > 0` on the workload object before calling `resolveServiceForWorkload`. + +**Rationale**: The readiness check should happen early in `fetchAndUpdateCard` to set `WorkloadNotReady` before attempting service resolution. This avoids confusing `ServiceNotFound` errors when the real issue is pods aren't ready. + +**Implementation**: +- Add a `checkWorkloadReady` helper that gets the Deployment/StatefulSet and checks `status.readyReplicas > 0` +- For Sandboxes (unstructured), skip the check (return true). Sandboxes are unstructured resources without a standard `readyReplicas` field; their lifecycle is managed by the sandbox controller, not by Deployment/StatefulSet rollout semantics. +- Call before `resolveServiceForWorkload` in `fetchAndUpdateCard` + +## R-004: Printer column rename + +**Decision**: Rename the kubebuilder marker from `CardSynced` to `CardFetched`. + +**Rationale**: The printer column name should match the condition type. Located at line 257 in `agentruntime_types.go`: +```go +// +kubebuilder:printcolumn:name="CardSynced",...JSONPath=".status.conditions[?(@.type=='CardSynced')].status" +``` + +Change to: +```go +// +kubebuilder:printcolumn:name="CardFetched",...JSONPath=".status.conditions[?(@.type=='CardFetched')].status" +``` + +Then regenerate CRD with `make manifests`. + +## R-005: Field rename impact on existing tests + +**Decision**: All field renames are straightforward find-and-replace operations. + +**Files affected**: +- `agentruntime_types.go`: struct field names and JSON tags +- `agentruntime_controller.go`: `CardID` -> `CardHash`, `FetchedAt` -> `LastCardFetchTime`, `ConditionTypeCardSynced` -> `ConditionTypeCardFetched` +- `agentruntime_controller_test.go`: all test assertions referencing these fields +- CRD YAMLs: auto-generated by `make manifests` + +**No semantic changes needed**: the Go logic (content hashing, timestamp comparison) remains identical. + +## R-006: Existing `serviceHTTPPort` function + +**Decision**: Replace the body of `serviceHTTPPort` with the new resolution chain. + +**Current implementation** (lines 567-577): +```go +func serviceHTTPPort(svc *corev1.Service) int32 { + for _, p := range svc.Spec.Ports { + if strings.EqualFold(p.Name, "http") || p.Port == 80 || p.Port == 8080 || p.Port == 8000 { + return p.Port + } + } + if len(svc.Spec.Ports) > 0 { + return svc.Spec.Ports[0].Port + } + return 8000 +} +``` + +**New implementation**: +1. Check `kagenti.io/port` annotation (parse as int32, validate > 0) +2. Look for port named `a2a` +3. Look for port named `http` +4. First port in spec +5. Default 8000 + +The function signature stays the same but takes the Service (which already has annotations accessible via `svc.Annotations`). diff --git a/specs/002-card-discovery-alignment/review-findings.md b/specs/002-card-discovery-alignment/review-findings.md new file mode 100644 index 00000000..937102f5 --- /dev/null +++ b/specs/002-card-discovery-alignment/review-findings.md @@ -0,0 +1,175 @@ +# Deep Review Findings + +**Date:** 2026-05-28 +**Branch:** 002-card-discovery-alignment +**Rounds:** 1 +**Gate Outcome:** PASS +**Invocation:** manual + +## Summary + +| Severity | Found | Fixed | Remaining | +|----------|-------|-------|-----------| +| Critical | 0 | 0 | 0 | +| Important | 7 | 4 | 3 (pre-existing) | +| Minor | 14 | 0 | 14 | +| **Total** | **21** | **4** | **17** | + +**Agents completed:** 5/5 (+ 1 external tool) +**Agents failed:** none + +Note: 3 remaining Important findings are pre-existing architectural patterns (redundant API calls per reconcile, persistCardFetchAnnotation race, unconditional fetchAndUpdateCard call) not introduced by this feature. They are documented for future improvement but do not block this gate. + +## Findings + +### FINDING-1 +- **Severity:** Important +- **Confidence:** 85 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:905-910 +- **Category:** correctness +- **Source:** correctness-agent +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +Missing nil guard on `cardData` returned from `AgentFetcher.Fetch` in the plain HTTP fetch path. If `Fetch()` returns `(nil, nil)`, the code would dereference `cardData` causing a nil pointer panic. The authenticated fetch path already had this guard. + +**Why this matters:** +Any alternate `Fetcher` implementation that returns `(nil, nil)` would cause a runtime panic. The asymmetry with the authenticated path (which checks for nil) was a correctness gap. + +**How it was resolved:** +Added nil check after the plain HTTP fetch, matching the pattern already used in the authenticated fetch path. + +### FINDING-2 +- **Severity:** Important +- **Confidence:** 90 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:594-595 +- **Category:** production-readiness +- **Source:** production-agent (also reported by: correctness-agent, architecture-agent) +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +`serviceHTTPPort` created a logger from `context.Background()` instead of using the caller's context, losing all contextual fields (namespace, name, reconcile ID). + +**Why this matters:** +Invalid annotation warning logs would lack resource context, making production debugging in multi-tenant clusters difficult. + +**How it was resolved:** +Changed function signature to accept `ctx context.Context` and updated all call sites. + +### FINDING-3 +- **Severity:** Important +- **Confidence:** 95 +- **File:** kagenti-operator/internal/controller/agentruntime_controller_test.go (missing test) +- **Category:** test-quality +- **Source:** test-agent +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +No test exercised the `FetchSkipped` condition reason (US2 acceptance scenario 7, FR-003). + +**Why this matters:** +`FetchSkipped` is the only "success" condition reason with no test coverage. A regression in change-detection-key logic would go unnoticed. + +**How it was resolved:** +Added test that performs a successful fetch, then calls `fetchAndUpdateCard` again without modifying the workload, asserting `CardFetched=True` with reason `FetchSkipped`. + +### FINDING-4 +- **Severity:** Important +- **Confidence:** 95 +- **File:** kagenti-operator/internal/controller/agentruntime_controller_test.go (missing test) +- **Category:** test-quality +- **Source:** test-agent +- **Round found:** 1 +- **Resolution:** fixed (round 1) + +**What is wrong:** +No test exercised the `FetchFailed` condition reason (US2 acceptance scenario 5, FR-003). The "card data retention" test used a nonexistent deployment, which hit `WorkloadNotReady` instead of `FetchFailed`. + +**Why this matters:** +If the `FetchFailed` path had a bug (e.g., clearing card data), no test would catch it. + +**How it was resolved:** +Added test with a ready Deployment + Service and a stub fetcher that returns an error, asserting `CardFetched=False` with reason `FetchFailed` and error message content. + +### FINDING-5 (pre-existing, not fixed) +- **Severity:** Important +- **Confidence:** 85 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:753-840 +- **Category:** production-readiness +- **Source:** production-agent +- **Round found:** 1 +- **Resolution:** deferred (pre-existing pattern) + +**What is wrong:** +Multiple redundant API GET calls for the same workload object per reconcile (5 calls for resolveTargetRef, workloadChangeKey, checkWorkloadReady, resolveServiceForWorkload, applyWorkloadConfig). + +**Why this matters:** +Unnecessary API server load in steady state. + +**Deferred because:** This is a pre-existing architectural pattern not introduced by this feature. Fixing it requires refactoring the reconciler to pass a workload object through the call chain. + +### FINDING-6 (pre-existing, not fixed) +- **Severity:** Important +- **Confidence:** 80 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:849-866 +- **Category:** production-readiness +- **Source:** production-agent +- **Round found:** 1 +- **Resolution:** deferred (pre-existing pattern) + +**What is wrong:** +`persistCardFetchAnnotation` save/restore pattern for status is fragile under concurrent reconciles. + +**Deferred because:** Pre-existing pattern, not introduced by this feature. + +### FINDING-7 (pre-existing, not fixed) +- **Severity:** Important +- **Confidence:** 85 +- **File:** kagenti-operator/internal/controller/agentruntime_controller.go:213 +- **Category:** production-readiness +- **Source:** production-agent +- **Round found:** 1 +- **Resolution:** deferred (pre-existing pattern) + +**What is wrong:** +`fetchAndUpdateCard` called unconditionally on every reconcile even when discovery is disabled, potentially causing unnecessary status updates. + +**Deferred because:** Pre-existing pattern, not introduced by this feature. + +## Minor Findings (14) + +| ID | Category | File | Description | +|----|----------|------|-------------| +| M-1 | architecture | agentruntime_controller.go:622 | `getAgentTLSPort` duplicated between controllers | +| M-2 | architecture | agentruntime_controller.go:1062 | Excessive 10-line goconst nolint comment | +| M-3 | architecture | agentruntime_controller.go:494 | `countConfiguredPods` lists broader than needed | +| M-4 | architecture | agentruntime_types.go:92,128 | AuthBridgeMode/MTLSMode as raw strings | +| M-5 | security | agentruntime_types.go:163 | TraceSpec.Endpoint has no CRD validation | +| M-6 | security | agentruntime_types.go:148 | AllowedAudiences has no MaxItems | +| M-7 | security | agentruntime_controller.go:599 | Port annotation allows > 65535 | +| M-8 | security | agentruntime_types.go:244 | SkillImageRef.Image has no pattern validation | +| M-9 | correctness | agentruntime_controller.go:561 | checkWorkloadReady has no default case | +| M-10 | test-quality | agentruntime_controller_test.go:955 | FR-013 test doesn't exercise FetchFailed path | +| M-11 | test-quality | agentruntime_controller_test.go | No first-port fallback test | +| M-12 | test-quality | agentruntime_controller_test.go | No configMap transportSecurity test | +| M-13 | test-quality | agentruntime_controller_test.go:204,258 | Discarded reconcile errors | +| M-14 | production | agentruntime_controller.go:952 | ensureNamespaceConfigMaps runs uncached on every reconcile | + +## Post-Fix Spec Coverage + +| Requirement | Implementation | Status | +|-------------|---------------|--------| +| FR-001: transportSecurity field with enum | api/v1alpha1/agentruntime_types.go:203-206 | verified | +| FR-002: CardSynced -> CardFetched | internal/controller/agentruntime_controller.go:74 | verified | +| FR-003: Condition reasons | internal/controller/agentruntime_controller.go:716-800 | verified | +| FR-004: cardId -> cardHash | api/v1alpha1/agentruntime_types.go:197 | verified | +| FR-005: fetchedAt -> lastCardFetchTime | api/v1alpha1/agentruntime_types.go:193 | verified | +| FR-006: Port resolution chain | internal/controller/agentruntime_controller.go:594-620 | verified | +| FR-007: Workload readiness check | internal/controller/agentruntime_controller.go:558-583 | verified | +| FR-008: Printer column rename | api/v1alpha1/agentruntime_types.go:280 | verified | +| FR-009: Constitution update | No references to update | verified | + +All spec requirements verified after fix loop. diff --git a/specs/002-card-discovery-alignment/spec.md b/specs/002-card-discovery-alignment/spec.md new file mode 100644 index 00000000..a44e368c --- /dev/null +++ b/specs/002-card-discovery-alignment/spec.md @@ -0,0 +1,126 @@ +# Feature Specification: Card Discovery Refinement Alignment + +**Feature Branch**: `002-card-discovery-alignment` +**Created**: 2026-05-26 +**Status**: Draft +**Input**: Brainstorm document `brainstorm/03-card-discovery-refinement-alignment.md` + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Transport Security Visibility (Priority: P1) + +A platform engineer queries an AgentRuntime to understand how the agent's card was fetched and whether the transport layer provided identity verification. Today, `CardSynced=True` gives no indication whether the fetch used mTLS (transport-verified identity) or plain HTTP (no identity guarantee). The engineer must dig into operator logs to determine the security posture. + +**Why this priority**: Transport security visibility is the highest-value gap in the current API. Without this visibility, platform engineers cannot assess whether their agent discovery pipeline meets security requirements. A card fetched over plain HTTP in a multi-tenant cluster could be served by a compromised pod. + +**Independent Test**: Deploy an agent with and without SPIRE configured. Check `status.card.transportSecurity` and the `CardFetched` condition reason. Verify the values correctly reflect the transport used. + +**Acceptance Scenarios**: + +1. **Given** an AgentRuntime with card discovery enabled and the backing agent accessible over mTLS (SPIRE configured), **When** the controller fetches the card, **Then** `status.card.transportSecurity` is `"mtls"` and the `CardFetched` condition reason is `Fetched`. +2. **Given** an AgentRuntime with card discovery enabled and no SPIRE/mTLS available, **When** the controller fetches the card over plain HTTP, **Then** `status.card.transportSecurity` is `"http"` and the `CardFetched` condition reason is `FetchedInsecure`. +3. **Given** a previously populated `status.card` with `transportSecurity: mtls`, **When** SPIRE becomes unavailable and the next fetch falls back to plain HTTP, **Then** `transportSecurity` updates to `"http"` and the condition reason changes to `FetchedInsecure`. + +--- + +### User Story 2 - Unified Condition Model (Priority: P1) + +A platform engineer uses `kubectl describe agentruntime` to diagnose card discovery issues. The condition type, status, and reason should give an immediate, unambiguous picture of what happened: was the card fetched, how, and if not, why not. + +**Why this priority**: The condition model is the primary diagnostic interface. The current model has a stutter (`CardSynced True CardSynced`), conflates failure reasons, and doesn't reflect transport security. Fixing this while the API is fresh avoids a breaking change later. + +**Independent Test**: Create AgentRuntimes in various states (no workload, no Service, successful mTLS fetch, successful plain HTTP fetch, fetch failure, feature disabled) and verify `kubectl describe` shows the correct condition type, status, and reason for each. + +**Acceptance Scenarios**: + +1. **Given** an AgentRuntime whose card was fetched over mTLS, **When** an operator runs `kubectl describe agentruntime`, **Then** the output shows `CardFetched True Fetched`. +2. **Given** an AgentRuntime whose card was fetched over plain HTTP, **When** an operator runs `kubectl describe agentruntime`, **Then** the output shows `CardFetched True FetchedInsecure`. +3. **Given** an AgentRuntime whose workload has zero Ready pods, **When** the controller reconciles, **Then** the condition shows `CardFetched False WorkloadNotReady`. +4. **Given** an AgentRuntime whose workload is ready but has no matching Service, **When** the controller reconciles, **Then** the condition shows `CardFetched False ServiceNotFound`. +5. **Given** an AgentRuntime where the card fetch fails (timeout, invalid JSON, 404), **When** the controller processes the failure, **Then** the condition shows `CardFetched False FetchFailed` with error details in the message. +6. **Given** an AgentRuntime with card discovery disabled, **When** the controller reconciles, **Then** the condition shows `CardFetched False DiscoveryDisabled`. +7. **Given** an AgentRuntime whose pod template has not changed since the last successful fetch, **When** the controller reconciles, **Then** the condition shows `CardFetched True FetchSkipped`. + +--- + +### User Story 3 - Accurate Field Names (Priority: P2) + +A platform engineer or automation tool reads AgentRuntime status fields programmatically. Field names should accurately describe their content so consumers don't need to consult documentation to understand what a field holds. + +**Why this priority**: Field names are part of the CRD API surface. Once consumers depend on them, renaming requires migration. The PR just merged, so this is the last window to fix naming before consumers appear. + +**Independent Test**: Create an AgentRuntime with a successful card fetch. Verify `status.card.cardHash` contains a SHA-256 hash and `status.card.lastCardFetchTime` contains a timestamp. + +**Acceptance Scenarios**: + +1. **Given** an AgentRuntime with a successfully fetched card, **When** an operator queries `status.card.cardHash`, **Then** it contains a SHA-256 hex string representing the card content hash. +2. **Given** an AgentRuntime with a successfully fetched card, **When** an operator queries `status.card.lastCardFetchTime`, **Then** it contains an RFC 3339 timestamp of when the controller last fetched the card. + +--- + +### User Story 4 - Protocol-Aware Port Resolution (Priority: P2) + +A platform engineer deploys an agent with a multi-port Service (e.g., an admin HTTP port and an A2A protocol port). The controller should resolve the correct port for the A2A card endpoint without manual configuration, and provide an annotation override when auto-detection fails. + +**Why this priority**: Multi-port Services are common in production. The current generic HTTP port resolution could pick the wrong port, causing silent fetch failures or fetching from the wrong endpoint. + +**Independent Test**: Deploy a Service with ports named `admin` (port 80) and `a2a` (port 8000). Verify the controller fetches the card from port 8000, not port 80. + +**Acceptance Scenarios**: + +1. **Given** a Service with a port named `a2a`, **When** the controller resolves the card endpoint, **Then** it uses the `a2a` port. +2. **Given** a Service with a `kagenti.io/port` annotation set to `9090`, **When** the controller resolves the card endpoint, **Then** it uses port 9090 regardless of port names. +3. **Given** a Service with no `a2a` port and no annotation, **When** the controller resolves the card endpoint, **Then** it falls back to the port named `http`, then to the first port. +4. **Given** a Service with the `kagenti.io/port` annotation and a port named `a2a`, **When** the controller resolves the card endpoint, **Then** the annotation takes priority over the port name. + +--- + +### Edge Cases + +- What happens when `transportSecurity` is `"http"` and a policy engine requires mTLS? The field provides the signal; enforcement is out of scope for this feature (deferred to identity binding migration). +- What happens when a Service has the `kagenti.io/port` annotation with an invalid value (non-numeric, port 0)? The controller falls back to port name resolution and logs a warning. +- What happens when the `cardHash` changes but the card payload fields are semantically identical (e.g., JSON key ordering difference)? The hash is computed from the serialized JSON, so ordering differences produce different hashes. This is intentional: any byte-level change triggers a re-fetch timestamp update. +- What happens when a workload is intentionally scaled to zero (KEDA, Knative, manual `replicas: 0`)? The current generation-based change detection treats a replica count change as a workload mutation, triggering a re-fetch attempt that fails with `WorkloadNotReady` even though the card data (baked into the image) is still valid. The condition self-heals when pods return, but the flapping is noisy for operators. Existing card data is always retained (FR-013). A follow-up improvement should switch change detection from `metadata.generation` (increments on any spec change including replicas) to the Kubernetes pod template hash (only changes when `spec.template` changes: image, env, volumes). Replicas are not part of `spec.template`, so scale events would be correctly ignored. + +## Clarifications + +### Session 2026-05-27 + +- Q: How should workload readiness be determined across workload kinds? → A: Check `readyReplicas > 0` for Deployments/StatefulSets, skip readiness check for Sandboxes. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST add a `transportSecurity` field to `CardStatus` that records the transport layer used for the card fetch. The field MUST be a typed enum (`TransportSecurity`) with values `"mtls"` and `"http"` (enforced via `+kubebuilder:validation:Enum`). Go constants `TransportSecurityMTLS` and `TransportSecurityHTTP` MUST be used in controller code. +- **FR-002**: The system MUST rename the condition type from `CardSynced` to `CardFetched`. +- **FR-003**: The `CardFetched` condition MUST use these reasons: `Fetched` (mTLS success), `FetchedInsecure` (plain HTTP success), `FetchSkipped` (pod template unchanged), `FetchFailed` (fetch error), `ServiceNotFound` (no matching Service), `WorkloadNotReady` (no Ready pods), `DiscoveryDisabled` (feature flag off). +- **FR-004**: The system MUST rename the `cardId` field to `cardHash` in `CardStatus`. +- **FR-005**: The system MUST rename the `fetchedAt` field to `lastCardFetchTime` in `CardStatus`. +- **FR-006**: The system MUST resolve the A2A card endpoint port using this chain: `kagenti.io/port` annotation on the Service (highest priority), then port named `a2a`, then port named `http`, then first port. +- **FR-007**: The system MUST check workload readiness before attempting service resolution. For Deployments and StatefulSets, check `readyReplicas > 0`. For Sandboxes (unstructured), skip the readiness check and proceed directly to service resolution. If the readiness check fails, set `CardFetched=False` with reason `WorkloadNotReady`. +- **FR-008**: The `CardFetched` printer column MUST replace the existing `CardSynced` printer column in the CRD. +- **FR-009**: The project constitution MUST be updated to reflect the new condition type and field names. + +### Key Entities + +- **CardStatus**: Extended with `transportSecurity` field. `cardId` renamed to `cardHash`. `fetchedAt` renamed to `lastCardFetchTime`. +- **CardFetched Condition**: Replaces `CardSynced`. New reasons for transport awareness and workload readiness. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Platform engineers can determine transport security posture from a single `kubectl get agentruntime -o yaml` command without consulting operator logs. +- **SC-002**: Every `CardFetched` condition reason maps to exactly one diagnostic action (wait, check config, check network, enable feature, or no action needed). +- **SC-003**: Multi-port Services with both admin and A2A ports resolve to the correct A2A port without manual annotation. +- **SC-004**: All existing card discovery unit tests pass after migration to new field names and condition model (zero test regressions). +- **SC-005**: The CRD schema contains no references to the old field names (`cardId`, `fetchedAt`, `CardSynced`). + +## Assumptions + +- The PR #372 merged recently enough that no external consumers depend on the current field names or condition type. Breaking changes are acceptable. +- The `transportSecurity` values `"mtls"` and `"http"` are sufficient for v1alpha1. New values (e.g., `"ztunnel"`) can be added via normal CRD schema evolution. The field uses a typed Go enum (`TransportSecurity`) with kubebuilder validation to prevent silent divergence in consumer comparisons. +- The `kagenti.io/port` annotation is a new API surface. No existing workloads use it, so there is no migration concern. +- The ConfigMap fetch path (init-container signing) is being deprecated. It is not represented in the `transportSecurity` enum; a value can be added if needed during the coexistence period. +- Workload readiness can be determined by checking for pods matching the workload's selector with a Ready condition. This reuses existing pod listing logic. diff --git a/specs/002-card-discovery-alignment/tasks.md b/specs/002-card-discovery-alignment/tasks.md new file mode 100644 index 00000000..08b3a746 --- /dev/null +++ b/specs/002-card-discovery-alignment/tasks.md @@ -0,0 +1,109 @@ +# Tasks: Card Discovery Refinement Alignment + +**Input**: Design documents from `specs/002-card-discovery-alignment/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md + +**Tests**: Included. Existing tests must be migrated and new tests added per spec acceptance scenarios. + +**Organization**: Tasks grouped by user story. US1 and US2 are P1 (implement first), US3 and US4 are P2. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2, US3) +- Include exact file paths in descriptions + +## Phase 1: Foundational (Field Renames + Condition Type) + +**Purpose**: Rename fields and condition type across all files. These are blocking prerequisites for all user stories since US1-US4 all reference the new names. + +- [X] T001 Rename `CardID` to `CardHash` and `FetchedAt` to `LastCardFetchTime` in `CardStatus` struct, update JSON tags from `cardId`/`fetchedAt` to `cardHash`/`lastCardFetchTime` in `kagenti-operator/api/v1alpha1/agentruntime_types.go` +- [X] T002 Add `TransportSecurity string` field with JSON tag `transportSecurity` and godoc to `CardStatus` struct in `kagenti-operator/api/v1alpha1/agentruntime_types.go` +- [X] T003 Run `make generate` to regenerate `kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go` +- [X] T004 Rename `ConditionTypeCardSynced` to `ConditionTypeCardFetched` with value `"CardFetched"` in `kagenti-operator/internal/controller/agentruntime_controller.go` (line 74) +- [X] T005 Update all condition reason string constants: `"CardSynced"` to `"Fetched"`, `"CardFetchFailed"` to `"FetchFailed"`, `"CardDiscoveryDisabled"` to `"DiscoveryDisabled"` in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T006 Update all references from `CardID` to `CardHash` and `FetchedAt` to `LastCardFetchTime` in `fetchAndUpdateCard` function in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T007 Rename printer column from `CardSynced` to `CardFetched` and update JSONPath from `CardSynced` to `CardFetched` in kubebuilder marker on `AgentRuntime` struct in `kagenti-operator/api/v1alpha1/agentruntime_types.go` +- [X] T008 Run `make manifests` to regenerate CRD YAMLs in `kagenti-operator/config/crd/bases/` and sync to `charts/kagenti-operator/crds/` via `make sync-chart-crds` +- [X] T009 Update all test assertions referencing `CardSynced`, `CardID`, `FetchedAt`, and old reason strings in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T010 Run `make test` to verify zero regressions after renames + +## Phase 2: US1 - Transport Security Visibility (P1) + +**Goal**: Platform engineers can determine transport security posture from `status.card.transportSecurity` and the `CardFetched` condition reason. + +**Independent Test**: Deploy agents with and without mTLS, verify `transportSecurity` field and condition reason reflect the transport used. + +- [X] T011 [US1] Set `TransportSecurity` field in `fetchCard` function: set `"mTLS"` when `AuthenticatedFetcher` path executes (fetchResult != nil), set `"plainHTTP"` when `AgentFetcher` HTTP fallback path executes, in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T012 [US1] Update `fetchAndUpdateCard` to pass `transportSecurity` from `fetchCard` into `cardStatus.TransportSecurity` and use transport-aware condition reasons: `"Fetched"` for mTLS, `"FetchedInsecure"` for plainHTTP, in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T013 [US1] Add test: card fetched with stub fetcher (simulating plain HTTP) sets `transportSecurity: "plainHTTP"` and reason `FetchedInsecure` in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T014 [US1] Add test: card fetched with authenticated fetcher mock sets `transportSecurity: "mTLS"` and reason `Fetched` in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T015 [US1] Add test: transport security field updates when transport changes (mTLS to plainHTTP on re-fetch) in `kagenti-operator/internal/controller/agentruntime_controller_test.go` + +## Phase 3: US2 - Unified Condition Model (P1) + +**Goal**: Every condition reason maps to exactly one diagnostic action. `WorkloadNotReady` split from `ServiceNotFound`. + +**Independent Test**: Create AgentRuntimes in all states and verify each condition type/status/reason combination. + +- [X] T016 [US2] Add `checkWorkloadReady` helper that checks `readyReplicas > 0` for Deployments/StatefulSets and skips check for Sandboxes in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T017 [US2] Call `checkWorkloadReady` before `resolveServiceForWorkload` in `fetchAndUpdateCard`; set `CardFetched=False, reason=WorkloadNotReady` if check fails, in `kagenti-operator/internal/controller/agentruntime_controller.go` +- [X] T018 [US2] Add test: workload with zero readyReplicas sets `CardFetched False WorkloadNotReady` in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T019 [US2] Add test: workload ready but no Service sets `CardFetched False ServiceNotFound` (existing test, update assertions) in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T020 [US2] Add test: discovery disabled sets `CardFetched False DiscoveryDisabled` (existing test, update assertions) in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T021 [US2] Verify existing `FetchSkipped` test passes with new condition type name in `kagenti-operator/internal/controller/agentruntime_controller_test.go` + +## Phase 4: US3 - Accurate Field Names (P2) + +**Goal**: `cardHash` and `lastCardFetchTime` are verified end-to-end via envtest. + +**Independent Test**: Create AgentRuntime in envtest, run reconcile with stub fetcher, read back from API server, verify field names. + +- [X] T022 [P] [US3] Add envtest integration test: create AgentRuntime, reconcile with stub fetcher, read back from API server, assert `status.card.cardHash` is a SHA-256 hex string and `status.card.lastCardFetchTime` is an RFC 3339 timestamp in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T023 [US3] Verify CRD schema contains `cardHash` and `lastCardFetchTime` fields and does NOT contain `cardId` or `fetchedAt` by inspecting generated `kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml` + +## Phase 5: US4 - Protocol-Aware Port Resolution (P2) + +**Goal**: Port resolution uses `kagenti.io/port` annotation, then `a2a` port name, then `http`, then first port. + +**Independent Test**: Create Services with various port configurations and verify correct port selection. + +- [X] T024 [US4] Replace body of `serviceHTTPPort` in `kagenti-operator/internal/controller/agentruntime_controller.go`: check `kagenti.io/port` annotation first (parse as int32, validate > 0, log warning on invalid), then port named `a2a`, then port named `http`, then first port, default 8000 +- [X] T025 [US4] Add test: Service with `kagenti.io/port` annotation uses annotated port in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T026 [US4] Add test: Service with port named `a2a` uses that port (no annotation) in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T027 [US4] Add test: Service with invalid `kagenti.io/port` annotation falls back to port name resolution in `kagenti-operator/internal/controller/agentruntime_controller_test.go` +- [X] T028 [P] [US4] Add test: Service with both annotation and `a2a` port, annotation takes priority in `kagenti-operator/internal/controller/agentruntime_controller_test.go` + +## Phase 6: Polish & Cross-Cutting + +**Purpose**: Constitution update, final validation, CRD verification. + +- [X] T029 Update `ConditionTypeCardSynced` references to `ConditionTypeCardFetched` in constitution at `.specify/memory/constitution.md` +- [X] T030 Run full `make test` to verify all tests pass (zero regressions, target: 181+ tests passing) +- [X] T031 Verify no references to old names (`cardId`, `fetchedAt`, `CardSynced`) remain in Go source files via `grep -r` across `kagenti-operator/` + +## Dependencies + +```text +Phase 1 (Foundational) ──► Phase 2 (US1) ──► Phase 3 (US2) ──► Phase 6 (Polish) + ──► Phase 4 (US3) ──────────────────────► + ──► Phase 5 (US4) ──────────────────────► +``` + +- Phase 1 blocks all user stories (field renames must complete first) +- US1 (transport security) blocks US2 (condition model uses transport-aware reasons) +- US3 and US4 are independent of US1/US2 and can run in parallel after Phase 1 +- Phase 6 runs after all user stories complete + +## Parallel Execution Opportunities + +After Phase 1 completes: +- **Batch 1**: T011-T015 (US1) + T022-T023 (US3) + T024-T028 (US4) can run in parallel +- **Batch 2**: T016-T021 (US2) runs after US1 completes +- **Batch 3**: T029-T031 (Polish) runs after all stories complete + +## Implementation Strategy + +**MVP**: Phase 1 + Phase 2 (US1) delivers transport security visibility, the highest-value change. + +**Full delivery**: All 6 phases, estimated at 31 tasks. No external dependencies. All changes are within the operator codebase.