diff --git a/.claude/agents/go-lint-enforcer.md b/.claude/agents/go-lint-enforcer.md new file mode 100644 index 00000000..55db98c6 --- /dev/null +++ b/.claude/agents/go-lint-enforcer.md @@ -0,0 +1,95 @@ +--- +name: go-lint-enforcer +description: Use this agent when you need comprehensive Go code quality enforcement through the project's go-dev-tools MCP server. This agent never invokes golangci-lint directly; instead it relies exclusively on the go-dev-tools provided lint(), fix(), and module-analysis tools. Designed for strict Go code review with quality gates, automated remediation, and module-level lint orchestration. +model: sonnet +color: purple +--- + +You are a Go Code Quality Enforcer, an expert specializing in rigorous Go code analysis and automated remediation. +**You are strictly forbidden from invoking golangci-lint directly.** +Instead, you must rely **only** on the go-dev-tools MCP server functions (`lint(module)`, `fix()`, module discovery methods, etc.), which internally handle linting. + +Your mission is to ensure every Go module meets the highest standards of code quality, idiomaticity, and maintainability using the provided wrapper tools and a fail-fast workflow. + +**Core Responsibilities:** + +1. **Automatic Module Discovery** + - Use project-aware tools such as `find_modules()`, `find_staged_modules()`, `get_info_about_module()`, and `get_module_from_pkg()` to identify module boundaries and understand architecture. + - Never bypass these tools for custom discovery or manual linting. + +2. **Strict Linting Enforcement (Without Direct golangci-lint Use)** + - Use **only** `lint(module)` to obtain lint results. + - **You may not run golangci-lint or any CLI command.** + - Treat all issues—warnings or errors—as blocking failures. + +3. **Fail-Fast Workflow** + - Immediately stop processing when any `status: "error"` lint report appears. + - Do not proceed to other modules until the current module is fully clean. + +4. **Systematic Remediation** + - Use `fix(module, preview=True)` and `fix(module)` for auto-fixable issues. + - Perform manual remediation for any issue not resolved automatically. + - Never attempt to run external tools; rely only on provided functions. + +5. **Validation Gates** + - After each remediation cycle, run `make test`. + - Re-run `lint(module)` to confirm zero remaining issues. + +**Workflow Protocol:** + +1. **Discovery Phase** + - Use `find_staged_modules()` for incremental workflows. + - Use `find_modules()` for full audits. + - Obtain structural information via `get_info_about_module()`. + +2. **Initial Assessment** + - Run `lint(module)` to gather current issues. + - Never call golangci-lint directly; rely solely on `lint()`. + +3. **Fail-Fast Gate** + - Immediately address the first failing module. + +4. **Preview Mode** + - Use `fix(module, preview=True)` to inspect auto-fixes. + +5. **Automated Remediation** + - Use `fix(module)` when safe. + +6. **Manual Remediation** + - Address non-auto-fixable issues: + - Code duplication + - Naming or style + - Error handling + - Performance + - Test quality + +7. **Validation Cycle** + - Run `make test`. + - Re-run `lint(module)` until zero issues remain. + +8. **Module Completion** + - A module is complete only after passing lint and tests with no remaining issues. + +**Quality Standards:** +- Zero tolerance for issues. +- Strict idiomatic Go practices. +- Security-aware remediation. +- Maintainable, clean architecture. + +**Technical Approach:** +- Use **only** the go-dev-tools MCP server functions. + **Never execute or reference golangci-lint directly.** +- Follow a strict per-module lint → fix → test cycle. + +**Communication Style:** +- Provide structured, precise reasoning. +- Justify each remediation step. +- Clearly highlight severity and file locations. + +**Success Criteria:** +- `lint(module)` returns zero issues. +- All `make test` executions pass. +- Code is idiomatic, secure, and maintainable. + +You operate with unwavering commitment to code quality, but must always honor the constraint: +**No direct execution, invocation, or referencing of golangci-lint. All linting must occur through the provided lint() API only.** diff --git a/.claude/agents/go-lint-enforcer.md.license b/.claude/agents/go-lint-enforcer.md.license new file mode 100644 index 00000000..e60f9757 --- /dev/null +++ b/.claude/agents/go-lint-enforcer.md.license @@ -0,0 +1,3 @@ +Copyright 2025 Deutsche Telekom IT GmbH + +SPDX-License-Identifier: Apache-2.0 diff --git a/.github/workflows/reusable-go-ci.yaml b/.github/workflows/reusable-go-ci.yaml index 93066b8d..60a9a4e0 100644 --- a/.github/workflows/reusable-go-ci.yaml +++ b/.github/workflows/reusable-go-ci.yaml @@ -123,7 +123,7 @@ jobs: with: version: v2.1 working-directory: ${{ inputs.module }} - args: --timeout 5m --issues-exit-code=0 --config ../.golangci.yml + args: --timeout 5m --config ../.golangci.yml tests: name: "Tests & Coverage for ${{ inputs.module }}" diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 00000000..e76a81a1 --- /dev/null +++ b/.mcp.json @@ -0,0 +1,25 @@ +{ + "mcpServers": { + "trivy": { + "type": "stdio", + "command": "trivy", + "args": [ + "mcp" + ], + "env": {} + }, + "go-dev-tools": { + "type": "stdio", + "command": "uv", + "args": [ + "run", + "--with", + "fastmcp", + "fastmcp", + "run", + ".mcp/go_dev_tools.py" + ], + "env": {} + } + } +} \ No newline at end of file diff --git a/.mcp/.gitignore b/.mcp/.gitignore new file mode 100644 index 00000000..89a16680 --- /dev/null +++ b/.mcp/.gitignore @@ -0,0 +1,5 @@ +# Copyright 2025 Deutsche Telekom IT GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +__pycache__ \ No newline at end of file diff --git a/.mcp/README.md b/.mcp/README.md new file mode 100644 index 00000000..f2aa3924 --- /dev/null +++ b/.mcp/README.md @@ -0,0 +1,21 @@ + + +# MCP + + +## Go Dev Tools + + +```bash + +# Claude Code +fastmcp install claude-code go_dev_tools.py + +# Gemini +fastmcp install gemini-cli go_dev_tools.py + +``` \ No newline at end of file diff --git a/.mcp/go_dev_tools.py b/.mcp/go_dev_tools.py new file mode 100644 index 00000000..f7b698f4 --- /dev/null +++ b/.mcp/go_dev_tools.py @@ -0,0 +1,940 @@ +# Copyright 2025 Deutsche Telekom IT GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +import json +import os +import subprocess +from pathlib import Path +from typing import Dict, List, Any + +from fastmcp import FastMCP + +mcp = FastMCP("go-dev-tools") + +# ============================================================================= +# GO DEVELOPMENT TOOLS WORKFLOW +# ============================================================================= +""" +This MCP server provides comprehensive Go development tools including linting, +code quality analysis, coverage reporting, and module management: + +DEVELOPMENT WORKFLOW: +1. 🔍 MODULE DISCOVERY + - Use find_modules() to discover all available Go modules in the project + - Use find_staged_modules() to identify only modules with staged changes + - Use get_info_about_module() to understand module structure and purpose + - Use get_module_from_pkg() to map package paths to their containing modules + +2. 🚨 ISSUE IDENTIFICATION (FAIL-FAST APPROACH) + - Use lint(module) to execute golangci-lint on specific modules + - Review structured JSON reports with file locations, severity, and linter details + - **CRITICAL**: Stop immediately if errors are found (status: "error") + - **IMPORTANT**: Fix critical errors before continuing with other modules + +3. 🔧 ISSUE RESOLUTION (AUTO-FIX + MANUAL STRATEGY) + - Address ERRORS first: Go version mismatches, build failures, critical issues + - NEW: Use fix(module, preview=True) to preview auto-fixable issues + - NEW: Use fix(module) to automatically fix formatting, imports, comment spacing + - Then manually fix remaining issues: code duplications, logic errors, test assertions + - Apply best practices for Go code quality standards + +4. ✅ VALIDATION + - Execute "make test" to run the full test suite after each fix + - Re-run lint(module) to verify issues are resolved + - Only proceed to next module when current module is clean + - Ensure no regressions in code quality or functionality + +INCREMENTAL WORKFLOW (STAGED MODULES ONLY): +``` +# Target only staged modules for efficient CI/development workflows +staged_modules = find_staged_modules() +if not staged_modules: + print("No modules staged - skipping lint") + exit(0) + +# Check only staged modules +for module in staged_modules: + lint(module) + # ... rest of workflow +``` + +COMPREHENSIVE WORKFLOW (ALL MODULES): +``` +# Discover all modules +modules = find_modules() + +# Check modules one by one - STOP ON ERRORS +for module in modules: + results = lint(module) + + # Parse JSON response + if results.status == "error": + print(f"CRITICAL: {module} has errors - STOP AND FIX FIRST") + # Fix the error (Go version, build issues, etc.) + # Re-run lint(module) until status != "error" + break # Don't continue until fixed + + elif results.status == "completed_with_issues": + print(f"Issues found in {module}") + + # NEW: Auto-fix simple issues first + preview = fix(module, preview=True) + if preview.estimated_fixable_issues > 0: + print(f"Auto-fixing {preview.estimated_fixable_issues} issues...") + fix_result = fix(module) + + # Re-lint to see remaining issues + results = lint(module) + + # Manual fix remaining complex issues + if results.status == "completed_with_issues": + print(f"Manual fixes needed for remaining issues") + # Fix remaining issues, run make test, re-lint + # Only proceed when clean or acceptable + + else: # status == "success" + print(f"{module} is clean ✅") + +# Only run make test after ALL critical errors are resolved +``` + +ERROR HANDLING PRIORITY: +- "error" status: STOP EVERYTHING - fix immediately +- "completed_with_issues": Fix before moving to next module +- "success": Continue to next module + +LINTER CONFIG: Uses project's .golangci.yml +OUTPUT FORMAT: Structured JSON with file paths, line numbers, and issue details +""" + +# Project root directory (parent of .mcp directory) +PROJECT_ROOT = Path(__file__).parent.parent.absolute() +GOLANGCI_CONFIG = PROJECT_ROOT / ".golangci.yml" + + +@mcp.tool +def lint(module: str) -> str: + """ + Lints the provided Go module and returns any issues found. + + Args: + module (str): The Go module name/directory to be linted (e.g., 'rover', 'identity', 'file-manager'). + + Returns: + str: A structured JSON report of linting issues found in the module. + """ + try: + # Resolve module directory + module_dir = PROJECT_ROOT / module + + if not module_dir.exists(): + return json.dumps({ + "error": f"Module directory '{module}' not found in project root", + "available_modules": _get_available_modules() + }, indent=2) + + # Check if go.mod exists in the module directory + go_mod_path = module_dir / "go.mod" + if not go_mod_path.exists(): + return json.dumps({ + "error": f"No go.mod found in module directory '{module}'", + "path": str(module_dir) + }, indent=2) + + # Execute golangci-lint + return _execute_golangci_lint(module_dir, module) + + except Exception as e: + return json.dumps({ + "error": f"Failed to lint module '{module}': {str(e)}" + }, indent=2) + + +@mcp.tool +def find_modules() -> list[str]: + """ + Finds and returns a list of Go module names available for linting. + + Returns: + list[str]: A list of Go module names. + """ + return _get_available_modules() + + +def _get_available_modules() -> List[str]: + """Get list of available Go modules in the project.""" + modules = [] + + # Check for direct modules in project root + for item in PROJECT_ROOT.iterdir(): + if item.is_dir() and (item / "go.mod").exists(): + modules.append(item.name) + + # Check for nested modules (like api modules) + for go_mod_path in PROJECT_ROOT.rglob("go.mod"): + if go_mod_path.parent != PROJECT_ROOT: + rel_path = go_mod_path.parent.relative_to(PROJECT_ROOT) + modules.append(str(rel_path)) + + return sorted(list(set(modules))) + + +@mcp.tool +def find_staged_modules() -> list[str]: + """ + Identifies Go modules that have staged changes based on Git diffs. + + Returns: + list[str]: A list of Go module names with staged changes. + """ + try: + # Get the list of changed files from git + git_cmd = ["git", "diff", "--name-only", "--cached"] + result = subprocess.run( + git_cmd, + cwd=PROJECT_ROOT, + capture_output=True, + text=True, + timeout=30 + ) + + if result.returncode != 0: + return [] + + output = result.stdout.strip() + if not output: + return [] + changed_files = output.split('\n') + + changed_modules = set() + + all_modules = _get_available_modules() + # Sort modules by path depth, descending, to match the most specific path first + # (e.g., 'a/b' before 'a'). + sorted_modules = sorted(all_modules, key=lambda p: p.count('/'), reverse=True) + + for file_path_str in changed_files: + if not file_path_str: + continue + + abs_file_path = (PROJECT_ROOT / file_path_str).resolve() + + # Find the most specific module for the changed file. + for module_name in sorted_modules: + module_path = PROJECT_ROOT / module_name + try: + if abs_file_path.is_relative_to(module_path.resolve()): + changed_modules.add(module_name) + break # Module found, proceed to the next file + except ValueError: + continue + + return sorted(list(changed_modules)) + + except Exception: + return [] + +@mcp.tool +def get_module_from_pkg(pkg: str) -> str: + """ + Determines the Go module that contains the specified package. + + Args: + pkg (str): The Go package name/path. + + Returns: + str: The Go module name/directory that contains the package. + """ + try: + # Get all modules first + modules = _get_available_modules() + + # Check if the package path directly matches a module + if pkg in modules: + return pkg + + # For package paths like "github.com/telekom/controlplane//...", + # extract the module part + if pkg.startswith("github.com/telekom/controlplane/"): + pkg_parts = pkg.replace("github.com/telekom/controlplane/", "").split("/") + + # Try progressively longer paths to find matching modules + for i in range(1, len(pkg_parts) + 1): + potential_module = "/".join(pkg_parts[:i]) + if potential_module in modules: + return potential_module + + # Check if any package exists as a subdirectory within any module + for module in modules: + module_dir = PROJECT_ROOT / module + if module_dir.exists(): + # Check if package path exists within this module + for potential_path in [pkg, pkg.replace("github.com/telekom/controlplane/", "")]: + if (module_dir / potential_path).exists(): + return module + + return json.dumps({ + "error": f"Package '{pkg}' not found in any module", + "available_modules": modules + }, indent=2) + + except Exception as e: + return json.dumps({ + "error": f"Failed to determine module for package '{pkg}': {str(e)}" + }, indent=2) + +@mcp.tool +def get_info_about_module(module: str) -> str: + """ + Provides information about the specified Go module. + + Args: + module (str): The Go module name/directory. + + Returns: + str: A readable string containing information about the module. + """ + try: + # Resolve module directory + module_dir = PROJECT_ROOT / module + + if not module_dir.exists(): + return json.dumps({ + "error": f"Module directory '{module}' not found", + "available_modules": _get_available_modules() + }, indent=2) + + # Collect module information + module_info = { + "module": module, + "path": str(module_dir), + "go_mod_info": {}, + "readme_content": "", + "structure": {}, + "has_tests": False + } + + # Read go.mod file + go_mod_path = module_dir / "go.mod" + if go_mod_path.exists(): + try: + with open(go_mod_path, 'r') as f: + go_mod_content = f.read() + + # Extract module name and Go version + for line in go_mod_content.split('\n'): + line = line.strip() + if line.startswith('module '): + module_info["go_mod_info"]["module_name"] = line.replace('module ', '') + elif line.startswith('go '): + module_info["go_mod_info"]["go_version"] = line.replace('go ', '') + + # Count dependencies + require_section = False + dep_count = 0 + for line in go_mod_content.split('\n'): + line = line.strip() + if line.startswith('require ('): + require_section = True + continue + elif line == ')' and require_section: + require_section = False + continue + elif require_section and line and not line.startswith('//'): + dep_count += 1 + elif line.startswith('require ') and not line.endswith('('): + dep_count += 1 + + module_info["go_mod_info"]["dependencies_count"] = dep_count + + except Exception as e: + module_info["go_mod_info"]["error"] = f"Failed to parse go.mod: {str(e)}" + + # Read README file + readme_files = ['README.md', 'README.rst', 'README.txt', 'README', 'readme.md', 'readme.txt'] + for readme_name in readme_files: + readme_path = module_dir / readme_name + if readme_path.exists(): + try: + with open(readme_path, 'r', encoding='utf-8') as f: + readme_content = f.read() + # Limit README content to first 2000 characters for readability + if len(readme_content) > 2000: + module_info["readme_content"] = readme_content[:2000] + "\n\n... (truncated)" + else: + module_info["readme_content"] = readme_content + break + except Exception as e: + module_info["readme_content"] = f"Error reading README: {str(e)}" + + # Analyze directory structure + try: + dirs = [] + files = [] + test_files = 0 + + for item in module_dir.iterdir(): + if item.is_dir(): + dirs.append(item.name) + elif item.is_file(): + files.append(item.name) + if item.name.endswith('_test.go'): + test_files += 1 + + module_info["structure"] = { + "directories": sorted(dirs), + "files_count": len(files), + "test_files_count": test_files + } + module_info["has_tests"] = test_files > 0 + + except Exception as e: + module_info["structure"]["error"] = f"Failed to analyze structure: {str(e)}" + + # Format output as readable text + output_lines = [] + output_lines.append(f"# Module: {module}") + output_lines.append(f"Path: {module_info['path']}") + output_lines.append("") + + # Go module information + if module_info["go_mod_info"]: + output_lines.append("## Go Module Information") + if "module_name" in module_info["go_mod_info"]: + output_lines.append(f"Module Name: {module_info['go_mod_info']['module_name']}") + if "go_version" in module_info["go_mod_info"]: + output_lines.append(f"Go Version: {module_info['go_mod_info']['go_version']}") + if "dependencies_count" in module_info["go_mod_info"]: + output_lines.append(f"Dependencies: {module_info['go_mod_info']['dependencies_count']}") + output_lines.append("") + + # Structure information + if module_info["structure"]: + output_lines.append("## Structure") + if "files_count" in module_info["structure"]: + output_lines.append(f"Files: {module_info['structure']['files_count']}") + if "test_files_count" in module_info["structure"]: + output_lines.append(f"Test Files: {module_info['structure']['test_files_count']}") + if "directories" in module_info["structure"] and module_info["structure"]["directories"]: + output_lines.append(f"Directories: {', '.join(module_info['structure']['directories'])}") + output_lines.append(f"Has Tests: {'Yes' if module_info['has_tests'] else 'No'}") + output_lines.append("") + + # README content + if module_info["readme_content"]: + output_lines.append("## README") + output_lines.append(module_info["readme_content"]) + else: + output_lines.append("## README") + output_lines.append("No README file found") + + return "\n".join(output_lines) + + except Exception as e: + return json.dumps({ + "error": f"Failed to get info for module '{module}': {str(e)}" + }, indent=2) + + +@mcp.tool +def fix(module: str, preview: bool = False) -> str: + """ + Automatically fixes simple linting issues in the provided Go module. + + Fixes formatting, imports, and simple style issues using golangci-lint --fix. + Complex issues like code duplication and logic errors require manual fixes. + + Args: + module (str): The Go module name/directory to auto-fix. + preview (bool): If True, shows current issues without applying fixes. + + Returns: + str: JSON report of fixes applied and remaining issues. + """ + try: + # Resolve module directory + module_dir = PROJECT_ROOT / module + + if not module_dir.exists(): + return json.dumps({ + "error": f"Module directory '{module}' not found in project root", + "available_modules": _get_available_modules() + }, indent=2) + + # Check if go.mod exists + go_mod_path = module_dir / "go.mod" + if not go_mod_path.exists(): + return json.dumps({ + "error": f"No go.mod found in module directory '{module}'", + "path": str(module_dir) + }, indent=2) + + # Execute fix operation + return _execute_golangci_fix(module_dir, module, preview) + + except Exception as e: + return json.dumps({ + "error": f"Failed to fix module '{module}': {str(e)}" + }, indent=2) + + +@mcp.tool +def analyze_code_coverage(module: str) -> str: + """ + Run the Go test coverage analysis for the specified module and return coverage statistics. + + Args: + module (str): The Go module name/directory to analyze. + + Returns: + str: A report of code coverage statistics. Which includes total coverage percentage, + uncovered files, and suggestions for improvement. + """ + try: + # Resolve module directory + module_dir = PROJECT_ROOT / module + + if not module_dir.exists(): + return json.dumps({ + "error": f"Module directory '{module}' not found in project root", + "available_modules": _get_available_modules() + }, indent=2) + + # Check if go.mod exists + go_mod_path = module_dir / "go.mod" + if not go_mod_path.exists(): + return json.dumps({ + "error": f"No go.mod found in module directory '{module}'", + "path": str(module_dir) + }, indent=2) + + # Execute coverage analysis + return _execute_coverage_analysis(module_dir, module) + + except Exception as e: + return json.dumps({ + "error": f"Failed to analyze coverage for module '{module}': {str(e)}" + }, indent=2) + + +def _execute_coverage_analysis(module_dir: Path, module_name: str) -> str: + """Execute Go test coverage analysis and return structured results.""" + try: + coverage_file = module_dir / "coverage.out" + + # First, run tests with coverage + test_cmd = ["go", "test", f"-coverprofile={coverage_file}", "./..."] + test_result = subprocess.run( + test_cmd, + cwd=module_dir, + capture_output=True, + text=True, + timeout=300 + ) + + if test_result.returncode != 0: + return json.dumps({ + "module": module_name, + "status": "test_failure", + "error_message": "Tests failed, cannot generate coverage report.", + "test_output": (test_result.stderr or test_result.stdout)[:5000] + }, indent=2) + + if not coverage_file.exists(): + return json.dumps({ + "module": module_name, + "status": "no_coverage_data", + "message": "No coverage data generated. Module may not have any tests." + }, indent=2) + + # Get overall and per-function coverage + coverage_cmd = ["go", "tool", "cover", f"-func={coverage_file}"] + coverage_result = subprocess.run( + coverage_cmd, + cwd=module_dir, + capture_output=True, + text=True, + timeout=60 + ) + + # Clean up coverage file immediately + try: + coverage_file.unlink() + except OSError: + pass + + if coverage_result.returncode != 0: + return json.dumps({ + "module": module_name, + "status": "coverage_analysis_failed", + "error_message": "Failed to analyze coverage data.", + "coverage_output": coverage_result.stderr + }, indent=2) + + coverage_output = coverage_result.stdout + coverage_summary = _parse_coverage_output(coverage_output) + file_summary, low_coverage_files = _summarize_file_coverage(coverage_output) + + report = { + "module": module_name, + "status": "success", + "coverage_summary": coverage_summary, + "file_summary": file_summary, + "low_coverage_files": low_coverage_files, + "coverage_analysis": _analyze_coverage_quality(coverage_summary.get("total_coverage", 0.0)), + "suggestions": _generate_coverage_suggestions(coverage_summary, low_coverage_files) + } + + return json.dumps(report, indent=2) + + except subprocess.TimeoutExpired: + return json.dumps({ + "module": module_name, + "status": "timeout", + "error_message": "Coverage analysis timed out." + }, indent=2) + except Exception as e: + return json.dumps({ + "module": module_name, + "status": "error", + "error_message": f"An unexpected error occurred: {str(e)}" + }, indent=2) + + +def _parse_coverage_output(coverage_output: str) -> Dict[str, Any]: + """Parse go tool cover output to extract overall coverage statistics.""" + lines = coverage_output.strip().split('\n') + total_line = next((line for line in reversed(lines) if "total:" in line.lower()), None) + + if not total_line: + return {"total_coverage": 0.0, "error": "Could not find total coverage line."} + + try: + percentage_str = total_line.split()[-1].rstrip('%') + total_coverage = float(percentage_str) + return { + "total_coverage": round(total_coverage, 2), + "total_line": total_line.strip() + } + except (ValueError, IndexError): + return {"total_coverage": 0.0, "error": "Could not parse total coverage percentage."} + + +def _summarize_file_coverage(coverage_output: str, low_coverage_threshold: float = 50.0): + """ + Parses file-level coverage, summarizes it by file, and identifies low-coverage files. + """ + lines = coverage_output.strip().split('\n') + function_coverage = [] + for line in lines: + if not line or line.startswith("total:"): + continue + + parts = line.strip().split('\t') + if len(parts) < 2: + continue + + full_func_name = parts[0] + percentage_str = parts[-1] + + last_colon_index = full_func_name.rfind(':') + if last_colon_index == -1: + continue + + file_path = full_func_name[:last_colon_index] + # Clean up file path from any line numbers if they exist + file_path_parts = file_path.split(':') + if len(file_path_parts) > 1 and file_path_parts[-1].isdigit(): + file_path = ':'.join(file_path_parts[:-1]) + + try: + coverage_pct = float(percentage_str.rstrip('%')) + function_coverage.append({"file": file_path, "coverage": coverage_pct, "is_covered": coverage_pct > 0}) + except ValueError: + continue + + file_stats = {} + for func in function_coverage: + if func["file"] not in file_stats: + file_stats[func["file"]] = {"total": 0, "covered": 0} + file_stats[func["file"]]["total"] += 1 + if func["is_covered"]: + file_stats[func["file"]]["covered"] += 1 + + summary_list = [ + { + "file": file, + "coverage": round((data["covered"] / data["total"]) * 100, 2) if data["total"] > 0 else 0.0, + "functions_total": data["total"], + "functions_covered": data["covered"], + } + for file, data in file_stats.items() + ] + summary_list = sorted(summary_list, key=lambda x: x["coverage"]) + + low_coverage_files = [f for f in summary_list if f["coverage"] < low_coverage_threshold] + + return summary_list, low_coverage_files + + +def _analyze_coverage_quality(total_coverage: float) -> Dict[str, Any]: + """Analyze coverage quality and provide assessment.""" + if total_coverage >= 80: + quality = "excellent" + assessment = "Great coverage! Your code is well-tested." + elif total_coverage >= 70: + quality = "good" + assessment = "Good coverage. Consider adding a few more tests for critical paths." + elif total_coverage >= 50: + quality = "moderate" + assessment = "Moderate coverage. Adding more tests would improve code reliability." + else: + quality = "low" + assessment = "Low coverage. Significant testing improvements are needed." + + return { + "quality_rating": quality, + "assessment": assessment + } + + +def _generate_coverage_suggestions( + coverage_summary: Dict[str, Any], + low_coverage_files: List[Dict[str, Any]] +) -> List[str]: + """Generate suggestions for improving code coverage.""" + suggestions = [] + total_coverage = coverage_summary.get("total_coverage", 0.0) + + if total_coverage < 50: + suggestions.append("🎯 Priority: Add basic unit tests for core functionality.") + elif total_coverage < 70: + suggestions.append("✅ Add tests for error handling and edge cases.") + else: + suggestions.append("🏆 Focus on covering remaining complex logic and edge cases.") + + if low_coverage_files: + suggestions.append(f"⚠️ Focus on improving test coverage for these {len(low_coverage_files)} files:") + for file_info in low_coverage_files[:3]: # Suggest up to 3 files + suggestions.append(f" - {file_info['file']} ({file_info['coverage']}% coverage)") + + suggestions.append("💡 Use table-driven tests for comprehensive case coverage.") + suggestions.append("🔄 Run tests with the -race flag to detect data races in concurrent code.") + return suggestions + + +def _execute_golangci_fix(module_dir: Path, module_name: str, preview: bool) -> str: + """Execute golangci-lint with --fix flag and return results.""" + try: + if preview: + # Preview mode: just show current issues + current_issues_result = _execute_golangci_lint(module_dir, module_name) + try: + current_data = json.loads(current_issues_result) + fixable_count = 0 + fixable_types = [] + + if "issues" in current_data: + for issue in current_data["issues"]: + linter = issue.get("linter", "") + # Count issues from auto-fixable linters + if linter in ["gofmt", "goimports", "revive"]: + fixable_count += 1 + if linter not in fixable_types: + fixable_types.append(linter) + + return json.dumps({ + "module": module_name, + "preview_mode": True, + "current_total_issues": current_data.get("summary", {}).get("total_issues", 0), + "estimated_fixable_issues": fixable_count, + "fixable_linters": fixable_types, + "message": f"Estimated {fixable_count} issues can be auto-fixed. Use fix('{module_name}') to apply fixes.", + "auto_fixable_types": ["formatting (gofmt)", "imports (goimports)", "comment spacing (revive)"], + "manual_fix_required": ["code duplication", "logic errors", "test assertions", "unused parameters"] + }, indent=2) + + except json.JSONDecodeError: + return json.dumps({ + "module": module_name, + "preview_mode": True, + "error": "Could not parse current lint results", + "raw_result": current_issues_result + }, indent=2) + + # Apply fixes mode + cmd = [ + "golangci-lint", + "run", + "--config", str(GOLANGCI_CONFIG), + "--fix", + "./..." + ] + + # Execute fix command + result = subprocess.run( + cmd, + cwd=module_dir, + capture_output=True, + text=True, + timeout=300 + ) + + # After fixing, run lint again to see what remains + remaining_issues_result = _execute_golangci_lint(module_dir, module_name) + + return json.dumps({ + "module": module_name, + "status": "fix_completed", + "fix_result": { + "return_code": result.returncode, + "stdout": result.stdout, + "stderr": result.stderr if result.stderr else "" + }, + "remaining_issues_summary": _extract_summary_from_lint_result(remaining_issues_result), + "message": f"Auto-fix completed for {module_name}. Run lint('{module_name}') for detailed remaining issues." + }, indent=2) + + except subprocess.TimeoutExpired: + return json.dumps({ + "module": module_name, + "status": "timeout", + "error_message": "Fix process timed out after 5 minutes" + }, indent=2) + except Exception as e: + return json.dumps({ + "module": module_name, + "status": "error", + "error_message": str(e) + }, indent=2) + + +def _extract_summary_from_lint_result(lint_result: str) -> Dict[str, Any]: + """Extract summary information from lint result JSON.""" + try: + data = json.loads(lint_result) + if "summary" in data: + return data["summary"] + else: + return {"message": "No issues found"} + except json.JSONDecodeError: + return {"error": "Could not parse lint results"} + + +def _execute_golangci_lint(module_dir: Path, module_name: str) -> str: + """Execute golangci-lint and return structured results.""" + try: + # Build golangci-lint command + cmd = [ + "golangci-lint", + "run", + "--config", str(GOLANGCI_CONFIG), + "--output.json.path", "stdout", + "./..." + ] + + # Execute in module directory + result = subprocess.run( + cmd, + cwd=module_dir, + capture_output=True, + text=True, + timeout=300 # 5 minute timeout + ) + + # Parse results + if result.returncode == 0: + # No issues found + return json.dumps({ + "module": module_name, + "status": "success", + "issues": [], + "summary": { + "total_issues": 0, + "severity_breakdown": {}, + "linter_breakdown": {} + } + }, indent=2) + + # Parse JSON output from golangci-lint + if result.stdout: + try: + lint_data = json.loads(result.stdout) + return _format_lint_results(module_name, lint_data) + except json.JSONDecodeError: + # Fallback to text output if JSON parsing fails + return json.dumps({ + "module": module_name, + "status": "completed_with_issues", + "raw_output": result.stdout, + "stderr": result.stderr if result.stderr else "" + }, indent=2) + + # Handle stderr or other errors + return json.dumps({ + "module": module_name, + "status": "error", + "error_message": result.stderr or "Unknown error occurred", + "return_code": result.returncode + }, indent=2) + + except subprocess.TimeoutExpired: + return json.dumps({ + "module": module_name, + "status": "timeout", + "error_message": "Linting process timed out after 5 minutes" + }, indent=2) + except FileNotFoundError: + return json.dumps({ + "module": module_name, + "status": "error", + "error_message": "golangci-lint not found. Please ensure it's installed and in PATH." + }, indent=2) + + +def _format_lint_results(module_name: str, lint_data: Dict[str, Any]) -> str: + """Format golangci-lint JSON results into structured output.""" + issues = lint_data.get("Issues", []) + + # Group issues by severity and linter + severity_breakdown = {} + linter_breakdown = {} + + formatted_issues = [] + for issue in issues: + severity = issue.get("Severity", "unknown") + linter = issue.get("FromLinter", "unknown") + + # Count by severity + severity_breakdown[severity] = severity_breakdown.get(severity, 0) + 1 + + # Count by linter + linter_breakdown[linter] = linter_breakdown.get(linter, 0) + 1 + + # Format issue + formatted_issue = { + "file": issue.get("Pos", {}).get("Filename", "unknown"), + "line": issue.get("Pos", {}).get("Line", 0), + "column": issue.get("Pos", {}).get("Column", 0), + "severity": severity, + "message": issue.get("Text", ""), + "linter": linter, + "rule": issue.get("Rule", "") + } + formatted_issues.append(formatted_issue) + + return json.dumps({ + "module": module_name, + "status": "completed_with_issues" if issues else "success", + "issues": formatted_issues, + "summary": { + "total_issues": len(issues), + "severity_breakdown": severity_breakdown, + "linter_breakdown": linter_breakdown + } + }, indent=2) + + +if __name__ == "__main__": + mcp.run(transport="stdio") \ No newline at end of file diff --git a/.mcp/requirements.txt b/.mcp/requirements.txt new file mode 100644 index 00000000..33ebf43c --- /dev/null +++ b/.mcp/requirements.txt @@ -0,0 +1,5 @@ +# Copyright 2025 Deutsche Telekom IT GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +fastmcp \ No newline at end of file diff --git a/REUSE.toml b/REUSE.toml index d350f6bb..7d8a4590 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -56,4 +56,10 @@ SPDX-License-Identifier = "Apache-2.0" path = "**/mocks/data/**" precedence = "aggregate" SPDX-FileCopyrightText = "Copyright 2025 Deutsche Telekom IT GmbH" -SPDX-License-Identifier = "Apache-2.0" \ No newline at end of file +SPDX-License-Identifier = "Apache-2.0" + +[[annotations]] +path = ".mcp.json" +precedence = "aggregate" +SPDX-FileCopyrightText = "Copyright 2025 Deutsche Telekom IT GmbH" +SPDX-License-Identifier = "CC0-1.0" diff --git a/common/pkg/errors/ctrlerrors/suite_test.go b/common/pkg/errors/ctrlerrors/suite_test.go index e66633d4..e8e32fb3 100644 --- a/common/pkg/errors/ctrlerrors/suite_test.go +++ b/common/pkg/errors/ctrlerrors/suite_test.go @@ -189,7 +189,7 @@ var _ = Describe("Test Suite", func() { Expect(condition.Message).To(Equal("This is a blocked error")) events := recorder.GetEvents(obj) - Expect(len(events)).To(Equal(1)) + Expect(events).To(HaveLen(1)) Expect(events[0].EventType).To(Equal("Warning")) Expect(events[0].Reason).To(Equal("Blocked")) Expect(events[0].Message).To(Equal("This is a blocked error")) diff --git a/common/pkg/errors/validation_test.go b/common/pkg/errors/validation_test.go index d65b187e..c459eefb 100644 --- a/common/pkg/errors/validation_test.go +++ b/common/pkg/errors/validation_test.go @@ -59,7 +59,7 @@ var _ = Describe("ValidationError", func() { It("should initialize with empty errors and warnings", func() { Expect(valErr.HasErrors()).To(BeFalse()) Expect(valErr.BuildWarnings()).To(BeNil()) - Expect(valErr.BuildError()).To(BeNil()) + Expect(valErr.BuildError()).To(Succeed()) // Verify internal state Expect(valErr.Errors).To(BeEmpty()) @@ -100,7 +100,7 @@ var _ = Describe("ValidationError", func() { Context("BuildError", func() { It("should return nil when there are no errors", func() { - Expect(valErr.BuildError()).To(BeNil()) + Expect(valErr.BuildError()).To(Succeed()) }) It("should build a properly formatted StatusError when there are errors", func() { @@ -110,7 +110,7 @@ var _ = Describe("ValidationError", func() { // Build the error statusErr := valErr.BuildError().(*apierrors.StatusError) - Expect(statusErr).NotTo(BeNil()) + Expect(statusErr).To(HaveOccurred()) // Verify error details Expect(statusErr.ErrStatus.Status).To(Equal(metav1.StatusFailure)) @@ -141,7 +141,7 @@ var _ = Describe("ValidationError", func() { // Build the error statusErr := valErr.BuildError().(*apierrors.StatusError) - Expect(statusErr).NotTo(BeNil()) + Expect(statusErr).To(HaveOccurred()) // Verify types are preserved Expect(statusErr.ErrStatus.Details.Causes).To(HaveLen(2)) @@ -213,7 +213,7 @@ var _ = Describe("ValidationError", func() { // Build error and verify statusErr := valErr.BuildError().(*apierrors.StatusError) - Expect(statusErr).NotTo(BeNil()) + Expect(statusErr).To(HaveOccurred()) Expect(statusErr.ErrStatus.Details.Name).To(Equal("test-object")) Expect(statusErr.ErrStatus.Details.Kind).To(Equal("TestResource")) diff --git a/common/pkg/util/labelutil/labelutil_suite_test.go b/common/pkg/util/labelutil/labelutil_suite_test.go index d9cdde23..b115fab3 100644 --- a/common/pkg/util/labelutil/labelutil_suite_test.go +++ b/common/pkg/util/labelutil/labelutil_suite_test.go @@ -44,7 +44,7 @@ var _ = Describe("Labelutil", func() { It("should not shorten names", func() { value := "This is a very long value/with some_unwanted\\characters that needs to be normalized and shortened" shortenedValue := NormalizeNameValue(value) - Expect(len(shortenedValue)).To(Equal(len(value))) + Expect(shortenedValue).To(HaveLen(len(value))) Expect(shortenedValue).To(Equal("this-is-a-very-long-value-with-some-unwanted-characters-that-needs-to-be-normalized-and-shortened")) }) diff --git a/gateway/internal/controller/consumer_controller.go b/gateway/internal/controller/consumer_controller.go index 4973bf1f..a053984b 100644 --- a/gateway/internal/controller/consumer_controller.go +++ b/gateway/internal/controller/consumer_controller.go @@ -19,7 +19,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/telekom/controlplane/common/pkg/types" gatewayv1 "github.com/telekom/controlplane/gateway/api/v1" consumer_handler "github.com/telekom/controlplane/gateway/internal/handler/consumer" ) @@ -60,33 +59,5 @@ func (r *ConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *ConsumerReconciler) mapRealmToConsumer(ctx context.Context, obj client.Object) []reconcile.Request { - // ensure its actually a Realm - realm, ok := obj.(*gatewayv1.Realm) - if !ok { - return nil - } - if realm.Labels == nil { - return nil - } - - listOpts := []client.ListOption{ - client.MatchingFields{ - IndexFieldSpecRealm: types.ObjectRefFromObject(realm).String(), - }, - client.MatchingLabels{ - cconfig.EnvironmentLabelKey: realm.Labels[cconfig.EnvironmentLabelKey], - }, - } - - list := gatewayv1.ConsumerList{} - if err := r.List(ctx, &list, listOpts...); err != nil { - return nil - } - - requests := make([]reconcile.Request, len(list.Items)) - for i, item := range list.Items { - requests[i] = reconcile.Request{NamespacedName: client.ObjectKey{Name: item.Name, Namespace: item.Namespace}} - } - - return requests + return mapRealmToObjects(ctx, r.Client, obj, &gatewayv1.ConsumerList{}) } diff --git a/gateway/internal/controller/route_controller.go b/gateway/internal/controller/route_controller.go index 0eb29c8b..e07f3d9d 100644 --- a/gateway/internal/controller/route_controller.go +++ b/gateway/internal/controller/route_controller.go @@ -9,7 +9,6 @@ import ( cconfig "github.com/telekom/controlplane/common/pkg/config" cc "github.com/telekom/controlplane/common/pkg/controller" - "github.com/telekom/controlplane/common/pkg/types" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -79,33 +78,5 @@ func (r *RouteReconciler) mapConsumeRouteToRoute(ctx context.Context, obj client } func (r *RouteReconciler) mapRealmToRoute(ctx context.Context, obj client.Object) []reconcile.Request { - // ensure its actually a Realm - realm, ok := obj.(*gatewayv1.Realm) - if !ok { - return nil - } - if realm.Labels == nil { - return nil - } - - listOpts := []client.ListOption{ - client.MatchingFields{ - IndexFieldSpecRealm: types.ObjectRefFromObject(realm).String(), - }, - client.MatchingLabels{ - cconfig.EnvironmentLabelKey: realm.Labels[cconfig.EnvironmentLabelKey], - }, - } - - list := gatewayv1.RouteList{} - if err := r.List(ctx, &list, listOpts...); err != nil { - return nil - } - - requests := make([]reconcile.Request, len(list.Items)) - for i, item := range list.Items { - requests[i] = reconcile.Request{NamespacedName: client.ObjectKey{Name: item.Name, Namespace: item.Namespace}} - } - - return requests + return mapRealmToObjects(ctx, r.Client, obj, &gatewayv1.RouteList{}) } diff --git a/gateway/internal/controller/util.go b/gateway/internal/controller/util.go new file mode 100644 index 00000000..826a058f --- /dev/null +++ b/gateway/internal/controller/util.go @@ -0,0 +1,61 @@ +// Copyright 2025 Deutsche Telekom IT GmbH +// +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + + cconfig "github.com/telekom/controlplane/common/pkg/config" + "github.com/telekom/controlplane/common/pkg/types" + gatewayv1 "github.com/telekom/controlplane/gateway/api/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// mapRealmToObjects is a generic helper function that maps a Realm object to reconcile requests +// for objects that reference that realm. The listType parameter must be a pointer to a list type +// (like &gatewayv1.ConsumerList{} or &gatewayv1.RouteList{}). +func mapRealmToObjects(ctx context.Context, c client.Client, obj client.Object, listType client.ObjectList) []reconcile.Request { + // ensure its actually a Realm + realm, ok := obj.(*gatewayv1.Realm) + if !ok { + return nil + } + if realm.Labels == nil { + return nil + } + + listOpts := []client.ListOption{ + client.MatchingFields{ + IndexFieldSpecRealm: types.ObjectRefFromObject(realm).String(), + }, + client.MatchingLabels{ + cconfig.EnvironmentLabelKey: realm.Labels[cconfig.EnvironmentLabelKey], + }, + } + + if err := c.List(ctx, listType, listOpts...); err != nil { + return nil + } + + // Extract items from the list using reflection-like approach + // We need to handle this differently since we can't use generics easily here + switch list := listType.(type) { + case *gatewayv1.ConsumerList: + requests := make([]reconcile.Request, len(list.Items)) + for i, item := range list.Items { + requests[i] = reconcile.Request{NamespacedName: client.ObjectKey{Name: item.Name, Namespace: item.Namespace}} + } + return requests + case *gatewayv1.RouteList: + requests := make([]reconcile.Request, len(list.Items)) + for i, item := range list.Items { + requests[i] = reconcile.Request{NamespacedName: client.ObjectKey{Name: item.Name, Namespace: item.Namespace}} + } + return requests + default: + return nil + } +} diff --git a/gateway/internal/features/builder_header_transformation_test.go b/gateway/internal/features/builder_header_transformation_test.go index 021f858a..07e22c69 100644 --- a/gateway/internal/features/builder_header_transformation_test.go +++ b/gateway/internal/features/builder_header_transformation_test.go @@ -58,7 +58,7 @@ var _ = Describe("FeatureBuilder HeaderTransformation", Ordered, func() { Expect(ok).To(BeTrue()) By("checking the request-transformer plugin config") - Expect(rtPlugin.Config.Remove.Headers.Contains("X-Remove-Header")) + Expect(rtPlugin.Config.Remove.Headers.Contains("X-Remove-Header")).To(BeTrue()) }) diff --git a/gateway/internal/features/builder_ratelimiting_test.go b/gateway/internal/features/builder_ratelimiting_test.go index 1ed4c54a..1a0ebd8e 100644 --- a/gateway/internal/features/builder_ratelimiting_test.go +++ b/gateway/internal/features/builder_ratelimiting_test.go @@ -189,7 +189,7 @@ var _ = Describe("FeatureBuilder RateLimiting", Ordered, func() { }) }) -func buildRateLimitFeature(ctx context.Context, route *gatewayv1.Route, isProxyRoute bool, consumeRoutes []*gatewayv1.ConsumeRoute, gateway *gatewayv1.Gateway, realm *gatewayv1.Realm) *features.Builder { +func buildRateLimitFeature(ctx context.Context, route *gatewayv1.Route, _ bool, consumeRoutes []*gatewayv1.ConsumeRoute, gateway *gatewayv1.Gateway, realm *gatewayv1.Realm) *features.Builder { builder := features.NewFeatureBuilder(mockKc, route, nil, realm, gateway) diff --git a/gateway/internal/features/feature/basic_auth.go b/gateway/internal/features/feature/basic_auth.go index 46437ee5..261a9a0e 100644 --- a/gateway/internal/features/feature/basic_auth.go +++ b/gateway/internal/features/feature/basic_auth.go @@ -84,7 +84,7 @@ func (b *BasicAuthFeature) Apply(ctx context.Context, builder features.FeaturesB if err != nil { return errors.Wrapf(err, "cannot get basic auth password for route %s", route.GetName()) } - jumperConfig.BasicAuth[plugin.ConsumerId(DefaultProviderKey)] = plugin.BasicAuthCredentials{ + jumperConfig.BasicAuth[DefaultProviderKey] = plugin.BasicAuthCredentials{ Username: security.M2M.Basic.Username, Password: passwordValue, } diff --git a/gateway/internal/features/feature/basic_auth_test.go b/gateway/internal/features/feature/basic_auth_test.go index 4b1b425e..93a21931 100644 --- a/gateway/internal/features/feature/basic_auth_test.go +++ b/gateway/internal/features/feature/basic_auth_test.go @@ -189,7 +189,7 @@ var _ = Describe("BasicAuthFeature", func() { // Verify Expect(err).ToNot(HaveOccurred()) - defaultCreds := jumperConfig.BasicAuth[plugin.ConsumerId(feature.DefaultProviderKey)] + defaultCreds := jumperConfig.BasicAuth[feature.DefaultProviderKey] Expect(defaultCreds.Username).To(Equal("testuser")) Expect(defaultCreds.Password).To(Equal("testpass")) }) @@ -224,7 +224,7 @@ var _ = Describe("BasicAuthFeature", func() { // Verify Expect(err).ToNot(HaveOccurred()) - defaultCreds := jumperConfig.BasicAuth[plugin.ConsumerId(feature.DefaultProviderKey)] + defaultCreds := jumperConfig.BasicAuth[feature.DefaultProviderKey] Expect(defaultCreds.Username).To(Equal("failoveruser")) Expect(defaultCreds.Password).To(Equal("failoverpass")) }) @@ -282,7 +282,7 @@ var _ = Describe("BasicAuthFeature", func() { Expect(err).ToNot(HaveOccurred()) // Check default provider creds - defaultCreds := jumperConfig.BasicAuth[plugin.ConsumerId(feature.DefaultProviderKey)] + defaultCreds := jumperConfig.BasicAuth[feature.DefaultProviderKey] Expect(defaultCreds.Username).To(Equal("routeuser")) Expect(defaultCreds.Password).To(Equal("routepass")) @@ -336,7 +336,7 @@ var _ = Describe("BasicAuthFeature", func() { Expect(consumerCreds.Password).To(Equal("consumerpass")) // Default provider should not have creds - _, hasDefaultCreds := jumperConfig.BasicAuth[plugin.ConsumerId(feature.DefaultProviderKey)] + _, hasDefaultCreds := jumperConfig.BasicAuth[feature.DefaultProviderKey] Expect(hasDefaultCreds).To(BeFalse()) }) @@ -375,7 +375,7 @@ var _ = Describe("BasicAuthFeature", func() { Expect(err).ToNot(HaveOccurred()) // Check default provider creds - defaultCreds := jumperConfig.BasicAuth[plugin.ConsumerId(feature.DefaultProviderKey)] + defaultCreds := jumperConfig.BasicAuth[feature.DefaultProviderKey] Expect(defaultCreds.Username).To(Equal("routeuser")) Expect(defaultCreds.Password).To(Equal("routepass")) diff --git a/gateway/internal/features/feature/circuit_breaker.go b/gateway/internal/features/feature/circuit_breaker.go index 2febcdd3..984d2d03 100644 --- a/gateway/internal/features/feature/circuit_breaker.go +++ b/gateway/internal/features/feature/circuit_breaker.go @@ -7,6 +7,7 @@ package feature import ( "context" "fmt" + "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/telekom/controlplane/common/pkg/util/contextutil" @@ -101,7 +102,7 @@ func handleDeletion(ctx context.Context, builder features.FeaturesBuilder, route func isDeleteScenario(route *gatewayv1.Route) bool { // completely removed or turned to false - if (route.Spec.Traffic.CircuitBreaker != nil && route.Spec.Traffic.CircuitBreaker.Enabled == false) && route.GetUpstreamId() != "" { + if (route.Spec.Traffic.CircuitBreaker != nil && !route.Spec.Traffic.CircuitBreaker.Enabled) && route.GetUpstreamId() != "" { return true } else { return false diff --git a/gateway/internal/features/feature/circuit_breaker_test.go b/gateway/internal/features/feature/circuit_breaker_test.go index 746abe61..2bddea13 100644 --- a/gateway/internal/features/feature/circuit_breaker_test.go +++ b/gateway/internal/features/feature/circuit_breaker_test.go @@ -6,12 +6,13 @@ package feature_test import ( "context" + "net/http" + "github.com/telekom/controlplane/common/pkg/util/contextutil" "github.com/telekom/controlplane/gateway/internal/features/feature/config" kong "github.com/telekom/controlplane/gateway/pkg/kong/api" "github.com/telekom/controlplane/gateway/pkg/kong/client/mock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "net/http" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" diff --git a/gateway/internal/features/feature/custom_scopes.go b/gateway/internal/features/feature/custom_scopes.go index 7a43134b..ffadd06f 100644 --- a/gateway/internal/features/feature/custom_scopes.go +++ b/gateway/internal/features/feature/custom_scopes.go @@ -60,7 +60,7 @@ func (f *CustomScopesFeature) Apply(ctx context.Context, builder features.Featur if route.Spec.Security != nil && route.Spec.Security.M2M != nil { if len(route.Spec.Security.M2M.Scopes) > 0 { // Join scopes with a space, as Kong expects a single string with space-separated scopes - jumperConfig.OAuth[plugin.ConsumerId(DefaultProviderKey)] = plugin.OauthCredentials{ + jumperConfig.OAuth[DefaultProviderKey] = plugin.OauthCredentials{ Scopes: strings.Join(route.Spec.Security.M2M.Scopes, " "), } } diff --git a/gateway/internal/features/feature/external_idp_test.go b/gateway/internal/features/feature/external_idp_test.go index ef45c747..005c3d8d 100644 --- a/gateway/internal/features/feature/external_idp_test.go +++ b/gateway/internal/features/feature/external_idp_test.go @@ -58,7 +58,7 @@ var _ = Describe("ExternalIDPFeature", func() { Host: "example.com", Port: 0, Path: "/api", - IssuerUrl: "example.com/issuer", //has IssuerUrl == Proxy Route + IssuerUrl: "example.com/issuer", // has IssuerUrl == Proxy Route }, }, }, diff --git a/gateway/internal/features/feature/failover_test.go b/gateway/internal/features/feature/failover_test.go index 98246db2..838aa5a1 100644 --- a/gateway/internal/features/feature/failover_test.go +++ b/gateway/internal/features/feature/failover_test.go @@ -20,6 +20,47 @@ import ( "go.uber.org/mock/gomock" ) +// FailoverUpstreamExpectations defines expected values for failover upstream configuration +type FailoverUpstreamExpectations struct { + TargetZoneName string + Issuer string + RemoteApiUrl string +} + +// createMockFailoverPlugin creates a mock plugin handler for failover feature tests +func createMockFailoverPlugin(normalUpstream, failoverUpstream FailoverUpstreamExpectations) func(context.Context, client.CustomPlugin) (*kong.Plugin, error) { + return func(ctx context.Context, customPlugin client.CustomPlugin) (*kong.Plugin, error) { + switch p := customPlugin.(type) { + case *plugin.RequestTransformerPlugin: + Expect(p.GetName()).To(Equal("request-transformer")) + b64str := p.Config.Append.Headers.Get(plugin.RoutingConfigKey) + routingCfg, err := plugin.FromBase64[plugin.RoutingConfigs](b64str) + if err != nil { + Fail("Failed to decode routing config: " + err.Error()) + } + Expect(routingCfg).ToNot(BeNil()) + Expect(routingCfg.Len()).To(Equal(2)) + + // Validate normal upstream + normalUpstreamCfg := routingCfg.Get(0) + Expect(normalUpstreamCfg.TargetZoneName).To(Equal(normalUpstream.TargetZoneName)) + Expect(normalUpstreamCfg.Issuer).To(Equal(normalUpstream.Issuer)) + Expect(normalUpstreamCfg.RemoteApiUrl).To(Equal(normalUpstream.RemoteApiUrl)) + + // Validate failover upstream + failoverUpstreamCfg := routingCfg.Get(1) + Expect(failoverUpstreamCfg.TargetZoneName).To(Equal(failoverUpstream.TargetZoneName)) + Expect(failoverUpstreamCfg.Issuer).To(Equal(failoverUpstream.Issuer)) + Expect(failoverUpstreamCfg.RemoteApiUrl).To(Equal(failoverUpstream.RemoteApiUrl)) + + default: + Fail("Unexpected plugin type: " + p.GetName()) + } + + return nil, nil + } +} + var _ = Describe("Failover", func() { It("should have the correct priority", func() { Expect(feature.InstanceFailoverFeature.Priority()).To(Equal(feature.InstanceCircuitBreakerFeature.Priority() - 1)) @@ -84,34 +125,17 @@ var _ = Describe("Failover", func() { } mockKc.EXPECT().CreateOrReplaceRoute(ctx, gomock.Any(), gomock.Any()).DoAndReturn(mockCreateOrReplaceRoute).Times(1) - mockCreateOrReplacePlugin := func(ctx context.Context, customPlugin client.CustomPlugin) (kongPlugin *kong.Plugin, err error) { - switch p := customPlugin.(type) { - case *plugin.RequestTransformerPlugin: - Expect(p.GetName()).To(Equal("request-transformer")) - b64str := p.Config.Append.Headers.Get(plugin.RoutingConfigKey) - routingCfg, err := plugin.FromBase64[plugin.RoutingConfigs](b64str) - if err != nil { - Fail("Failed to decode routing config: " + err.Error()) - } - Expect(routingCfg).ToNot(BeNil()) - Expect(routingCfg.Len()).To(Equal(2)) - - normalUpstream := routingCfg.Get(0) - Expect(normalUpstream.TargetZoneName).To(Equal("zone1")) - Expect(normalUpstream.Issuer).To(Equal("https://issuer2.example.com")) - Expect(normalUpstream.RemoteApiUrl).To(Equal("http://gateway2.example.com:80")) - - failoverUpstream := routingCfg.Get(1) - Expect(failoverUpstream.TargetZoneName).To(Equal("")) - Expect(failoverUpstream.Issuer).To(Equal("")) - Expect(failoverUpstream.RemoteApiUrl).To(Equal("http://upstream1:80/")) - - default: - Fail("Unexpected plugin type: " + p.GetName()) - } - - return nil, nil + normalUpstreamExpectations := FailoverUpstreamExpectations{ + TargetZoneName: "zone1", + Issuer: "https://issuer2.example.com", + RemoteApiUrl: "http://gateway2.example.com:80", } + failoverUpstreamExpectations := FailoverUpstreamExpectations{ + TargetZoneName: "", + Issuer: "", + RemoteApiUrl: "http://upstream1:80/", + } + mockCreateOrReplacePlugin := createMockFailoverPlugin(normalUpstreamExpectations, failoverUpstreamExpectations) mockKc.EXPECT().CreateOrReplacePlugin(ctx, gomock.Any()).DoAndReturn(mockCreateOrReplacePlugin).Times(1) mockKc.EXPECT().CleanupPlugins(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) @@ -305,34 +329,17 @@ var _ = Describe("Failover", func() { } mockKc.EXPECT().CreateOrReplaceRoute(ctx, gomock.Any(), gomock.Any()).DoAndReturn(mockCreateOrReplaceRoute).Times(1) - mockCreateOrReplacePlugin := func(ctx context.Context, customPlugin client.CustomPlugin) (kongPlugin *kong.Plugin, err error) { - switch p := customPlugin.(type) { - case *plugin.RequestTransformerPlugin: - Expect(p.GetName()).To(Equal("request-transformer")) - b64str := p.Config.Append.Headers.Get(plugin.RoutingConfigKey) - routingCfg, err := plugin.FromBase64[plugin.RoutingConfigs](b64str) - if err != nil { - Fail("Failed to decode routing config: " + err.Error()) - } - Expect(routingCfg).ToNot(BeNil()) - Expect(routingCfg.Len()).To(Equal(2)) - - normalUpstream := routingCfg.Get(0) - Expect(normalUpstream.TargetZoneName).To(Equal("zone1")) - Expect(normalUpstream.Issuer).To(Equal("https://issuer2.example.com")) - Expect(normalUpstream.RemoteApiUrl).To(Equal("http://gateway2.example.com:80")) - - failoverUpstream := routingCfg.Get(1) - Expect(failoverUpstream.TargetZoneName).To(Equal("")) - Expect(failoverUpstream.Issuer).To(Equal("https://issuer2.example.com")) - Expect(failoverUpstream.RemoteApiUrl).To(Equal("http://upstream1:80/")) - - default: - Fail("Unexpected plugin type: " + p.GetName()) - } - - return nil, nil + normalUpstreamExpectations := FailoverUpstreamExpectations{ + TargetZoneName: "zone1", + Issuer: "https://issuer2.example.com", + RemoteApiUrl: "http://gateway2.example.com:80", + } + failoverUpstreamExpectations := FailoverUpstreamExpectations{ + TargetZoneName: "", + Issuer: "https://issuer2.example.com", + RemoteApiUrl: "http://upstream1:80/", } + mockCreateOrReplacePlugin := createMockFailoverPlugin(normalUpstreamExpectations, failoverUpstreamExpectations) mockKc.EXPECT().CreateOrReplacePlugin(ctx, gomock.Any()).DoAndReturn(mockCreateOrReplacePlugin).Times(1) mockKc.EXPECT().CleanupPlugins(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1) diff --git a/gateway/internal/features/feature/iprestriction_test.go b/gateway/internal/features/feature/iprestriction_test.go index d5bf21d6..db0e08f2 100644 --- a/gateway/internal/features/feature/iprestriction_test.go +++ b/gateway/internal/features/feature/iprestriction_test.go @@ -179,76 +179,54 @@ var _ = Describe("IpRestrictionFeature", func() { ctx = context.Background() }) - It("should apply IP restriction feature for a consumer with allow list", func() { - mockKc := mock.NewMockKongClient(mockCtrl) - - consumer := NewConsumer() - consumer.Spec.Security = &gatewayv1.ConsumerSecurity{ - IpRestrictions: &gatewayv1.IpRestrictions{ - Allow: []string{"192.168.1.1", "10.0.0.0/24"}, - }, - } - - // Mock CreateOrReplaceConsumer which will be called by the builder - mockKc.EXPECT().CreateOrReplaceConsumer(ctx, gomock.Any()).Return(&kong.Consumer{Id: stringPtr("test-consumer-id")}, nil).Times(1) - - // Mock CreateOrReplacePlugin for the IP restriction plugin - mockKc.EXPECT().CreateOrReplacePlugin(ctx, gomock.Any()).DoAndReturn( - func(ctx context.Context, customPlugin client.CustomPlugin) (*kong.Plugin, error) { - ipPlugin, ok := customPlugin.(*plugin.IpRestrictionPlugin) - Expect(ok).To(BeTrue()) - config := ipPlugin.GetConfig() - allowSet, ok := config["allow"].(*hashset.Set) - Expect(ok).To(BeTrue()) - Expect(allowSet.Contains("192.168.1.1")).To(BeTrue()) - Expect(allowSet.Contains("10.0.0.0/24")).To(BeTrue()) - return &kong.Plugin{Id: stringPtr("test-plugin-id")}, nil - }).Times(1) - - // Mock CleanupPlugins which will be called by the builder - mockKc.EXPECT().CleanupPlugins(ctx, nil, gomock.Any(), gomock.Any()).Return(nil).Times(1) - - builder := features.NewFeatureBuilder(mockKc, nil, consumer, nil, nil) - builder.EnableFeature(feature.InstanceIpRestrictionFeature) - - err := builder.BuildForConsumer(ctx) - Expect(err).ToNot(HaveOccurred()) - }) + DescribeTable("should apply IP restriction feature for a consumer", + func(configType string, ipList []string) { + mockKc := mock.NewMockKongClient(mockCtrl) - It("should apply IP restriction feature for a consumer with deny list", func() { - mockKc := mock.NewMockKongClient(mockCtrl) - - consumer := NewConsumer() - consumer.Spec.Security = &gatewayv1.ConsumerSecurity{ - IpRestrictions: &gatewayv1.IpRestrictions{ - Deny: []string{"192.168.1.2", "10.0.1.0/24"}, - }, - } - - // Mock CreateOrReplaceConsumer which will be called by the builder - mockKc.EXPECT().CreateOrReplaceConsumer(ctx, gomock.Any()).Return(&kong.Consumer{Id: stringPtr("test-consumer-id")}, nil).Times(1) - - // Mock CreateOrReplacePlugin for the IP restriction plugin - mockKc.EXPECT().CreateOrReplacePlugin(ctx, gomock.Any()).DoAndReturn( - func(ctx context.Context, customPlugin client.CustomPlugin) (*kong.Plugin, error) { - ipPlugin, ok := customPlugin.(*plugin.IpRestrictionPlugin) - Expect(ok).To(BeTrue()) - config := ipPlugin.GetConfig() - denySet, ok := config["deny"].(*hashset.Set) - Expect(ok).To(BeTrue()) - Expect(denySet.Contains("192.168.1.2")).To(BeTrue()) - Expect(denySet.Contains("10.0.1.0/24")).To(BeTrue()) - return &kong.Plugin{Id: stringPtr("test-plugin-id")}, nil - }).Times(1) - - // Mock CleanupPlugins which will be called by the builder - mockKc.EXPECT().CleanupPlugins(ctx, nil, gomock.Any(), gomock.Any()).Return(nil).Times(1) - - builder := features.NewFeatureBuilder(mockKc, nil, consumer, nil, nil) - builder.EnableFeature(feature.InstanceIpRestrictionFeature) - - err := builder.BuildForConsumer(ctx) - Expect(err).ToNot(HaveOccurred()) - }) + consumer := NewConsumer() + consumer.Spec.Security = &gatewayv1.ConsumerSecurity{ + IpRestrictions: &gatewayv1.IpRestrictions{}, + } + + // Set the appropriate IP list based on config type + if configType == "allow" { + consumer.Spec.Security.IpRestrictions.Allow = ipList + } else { + consumer.Spec.Security.IpRestrictions.Deny = ipList + } + + // Mock CreateOrReplaceConsumer which will be called by the builder + mockKc.EXPECT().CreateOrReplaceConsumer(ctx, gomock.Any()).Return(&kong.Consumer{Id: stringPtr("test-consumer-id")}, nil).Times(1) + + // Mock CreateOrReplacePlugin for the IP restriction plugin + mockKc.EXPECT().CreateOrReplacePlugin(ctx, gomock.Any()).DoAndReturn( + func(ctx context.Context, customPlugin client.CustomPlugin) (*kong.Plugin, error) { + ipPlugin, ok := customPlugin.(*plugin.IpRestrictionPlugin) + Expect(ok).To(BeTrue()) + config := ipPlugin.GetConfig() + + ipSet, ok := config[configType].(*hashset.Set) + Expect(ok).To(BeTrue()) + + // Verify all expected IPs are in the set + for _, ip := range ipList { + Expect(ipSet.Contains(ip)).To(BeTrue()) + } + + return &kong.Plugin{Id: stringPtr("test-plugin-id")}, nil + }).Times(1) + + // Mock CleanupPlugins which will be called by the builder + mockKc.EXPECT().CleanupPlugins(ctx, nil, gomock.Any(), gomock.Any()).Return(nil).Times(1) + + builder := features.NewFeatureBuilder(mockKc, nil, consumer, nil, nil) + builder.EnableFeature(feature.InstanceIpRestrictionFeature) + + err := builder.BuildForConsumer(ctx) + Expect(err).ToNot(HaveOccurred()) + }, + Entry("with allow list", "allow", []string{"192.168.1.1", "10.0.0.0/24"}), + Entry("with deny list", "deny", []string{"192.168.1.2", "10.0.1.0/24"}), + ) }) }) diff --git a/gateway/internal/features/feature/ratelimit.go b/gateway/internal/features/feature/ratelimit.go index 20ec64ee..1cbbd8fa 100644 --- a/gateway/internal/features/feature/ratelimit.go +++ b/gateway/internal/features/feature/ratelimit.go @@ -73,7 +73,7 @@ func (f *RateLimitFeature) Apply(ctx context.Context, builder features.FeaturesB Hour: route.Spec.Traffic.RateLimit.Limits.Hour, }, } - rateLimitPlugin = setOptions(rateLimitPlugin, &route.Spec.Traffic.RateLimit.Options) + setOptions(rateLimitPlugin, &route.Spec.Traffic.RateLimit.Options) } for _, allowedConsumer := range builder.GetAllowedConsumers() { @@ -105,11 +105,11 @@ func (f *RateLimitFeature) Apply(ctx context.Context, builder features.FeaturesB Hour: route.Spec.Traffic.RateLimit.Limits.Hour, } - rateLimitPlugin = setOptions(rateLimitPlugin, + setOptions(rateLimitPlugin, &route.Spec.Traffic.RateLimit.Options, ) } else if route.HasRateLimit() { - rateLimitPlugin = setOptions(rateLimitPlugin, &route.Spec.Traffic.RateLimit.Options) + setOptions(rateLimitPlugin, &route.Spec.Traffic.RateLimit.Options) } } } @@ -135,7 +135,7 @@ func setCommonConfigs(ctx context.Context, rateLimitPlugin *plugin.RateLimitPlug // setOptions applies additional options to the rate limit plugin configuration. // Options should be in order of precedence, with the last one having the highest priority. -func setOptions(rateLimitPlugin *plugin.RateLimitPlugin, options ...*gatewayv1.RateLimitOptions) *plugin.RateLimitPlugin { +func setOptions(rateLimitPlugin *plugin.RateLimitPlugin, options ...*gatewayv1.RateLimitOptions) { for _, option := range options { if option == nil { continue @@ -143,5 +143,4 @@ func setOptions(rateLimitPlugin *plugin.RateLimitPlugin, options ...*gatewayv1.R rateLimitPlugin.Config.HideClientHeaders = option.HideClientHeaders rateLimitPlugin.Config.FaultTolerant = option.FaultTolerant } - return rateLimitPlugin } diff --git a/gateway/pkg/kong/client/client.go b/gateway/pkg/kong/client/client.go index 97761277..fce4d834 100644 --- a/gateway/pkg/kong/client/client.go +++ b/gateway/pkg/kong/client/client.go @@ -71,7 +71,7 @@ type KongAdminApi interface { var _ KongClient = &kongClient{} type kongClient struct { - //client kong.ClientWithResponsesInterface + // client kong.ClientWithResponsesInterface client KongAdminApi commonTags []string } diff --git a/gateway/pkg/kong/client/plugin/iprestriction.go b/gateway/pkg/kong/client/plugin/iprestriction.go index bcd3f3e5..cc00c1ce 100644 --- a/gateway/pkg/kong/client/plugin/iprestriction.go +++ b/gateway/pkg/kong/client/plugin/iprestriction.go @@ -30,7 +30,6 @@ func (c *IpRestrictionPluginConfig) AddDeny(deny string) { type IpRestrictionPlugin struct { Id string `json:"id,omitempty"` Config IpRestrictionPluginConfig `json:"config,omitempty"` - route *gatewayv1.Route consumer *gatewayv1.Consumer } diff --git a/gateway/test/utils/utils.go b/gateway/test/utils/utils.go index fcf601ea..0f503c3a 100644 --- a/gateway/test/utils/utils.go +++ b/gateway/test/utils/utils.go @@ -10,7 +10,7 @@ import ( "os/exec" "strings" - . "github.com/onsi/ginkgo/v2" //nolint:golint,revive + "github.com/onsi/ginkgo/v2" ) const ( @@ -24,7 +24,7 @@ const ( ) func warnError(err error) { - _, _ = fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "warning: %v\n", err) } // InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics. @@ -41,12 +41,12 @@ func Run(cmd *exec.Cmd) ([]byte, error) { cmd.Dir = dir if err := os.Chdir(cmd.Dir); err != nil { - _, _ = fmt.Fprintf(GinkgoWriter, "chdir dir: %s\n", err) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "chdir dir: %s\n", err) } cmd.Env = append(os.Environ(), "GO111MODULE=on") command := strings.Join(cmd.Args, " ") - _, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "running: %s\n", command) output, err := cmd.CombinedOutput() if err != nil { return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output)) @@ -123,6 +123,6 @@ func GetProjectDir() (string, error) { if err != nil { return wd, err } - wd = strings.Replace(wd, "/test/e2e", "", -1) + wd = strings.ReplaceAll(wd, "/test/e2e", "") return wd, nil } diff --git a/identity/internal/controller/client_controller.go b/identity/internal/controller/client_controller.go index 96be7261..0deeaa6a 100644 --- a/identity/internal/controller/client_controller.go +++ b/identity/internal/controller/client_controller.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 -package controller +package controller // nolint: dupl import ( "context" @@ -71,7 +71,7 @@ func (r *ClientReconciler) mapRealmObjToIdentityClient(ctx context.Context, obj } list := &identityv1.ClientList{} - err := r.Client.List(ctx, list, client.MatchingLabels{ + err := r.List(ctx, list, client.MatchingLabels{ cconfig.EnvironmentLabelKey: realm.Labels[cconfig.EnvironmentLabelKey], }) if err != nil { diff --git a/identity/internal/controller/realm_controller.go b/identity/internal/controller/realm_controller.go index f4f1762a..94b7a131 100644 --- a/identity/internal/controller/realm_controller.go +++ b/identity/internal/controller/realm_controller.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 -package controller +package controller // nolint: dupl import ( "context" @@ -71,7 +71,7 @@ func (r *RealmReconciler) mapIdpObjToRealm(ctx context.Context, obj client.Objec } list := &identityv1.RealmList{} - err := r.Client.List(ctx, list, client.MatchingLabels{ + err := r.List(ctx, list, client.MatchingLabels{ cconfig.EnvironmentLabelKey: idp.Labels[cconfig.EnvironmentLabelKey], }) if err != nil { diff --git a/identity/internal/model/client/default_test.go b/identity/internal/model/client/default_test.go index e3697dc9..84d17d54 100644 --- a/identity/internal/model/client/default_test.go +++ b/identity/internal/model/client/default_test.go @@ -44,9 +44,9 @@ func TestNewClientIsCreatedCorrectly(t *testing.T) { client := NewClient(name, namespace, environment, realmName) assert.NotNil(t, client) - assert.Equal(t, name, client.ObjectMeta.Name) - assert.Equal(t, namespace, client.ObjectMeta.Namespace) - assert.Equal(t, environment, client.ObjectMeta.Labels[config.EnvironmentLabelKey]) + assert.Equal(t, name, client.Name) + assert.Equal(t, namespace, client.Namespace) + assert.Equal(t, environment, client.Labels[config.EnvironmentLabelKey]) assert.Equal(t, realmName, client.Spec.Realm.Name) assert.Equal(t, namespace, client.Spec.Realm.Namespace) assert.Equal(t, clientId, client.Spec.ClientId) diff --git a/identity/internal/model/identityprovider/default_test.go b/identity/internal/model/identityprovider/default_test.go index 4587ff47..1519c1f1 100644 --- a/identity/internal/model/identityprovider/default_test.go +++ b/identity/internal/model/identityprovider/default_test.go @@ -40,9 +40,9 @@ func TestIdentityProviderIsCreatedCorrectly(t *testing.T) { provider := NewIdentityProvider(name, namespace, environment) assert.NotNil(t, provider) - assert.Equal(t, name, provider.ObjectMeta.Name) - assert.Equal(t, namespace, provider.ObjectMeta.Namespace) - assert.Equal(t, environment, provider.ObjectMeta.Labels[config.EnvironmentLabelKey]) + assert.Equal(t, name, provider.Name) + assert.Equal(t, namespace, provider.Namespace) + assert.Equal(t, environment, provider.Labels[config.EnvironmentLabelKey]) assert.Equal(t, "https://iris-distcp1-dataplane1.dev.dhei.telekom.de/auth/admin/realms/", provider.Spec.AdminUrl) assert.Equal(t, "admin-cli", provider.Spec.AdminClientId) assert.Equal(t, "admin", provider.Spec.AdminUserName) diff --git a/identity/internal/model/realm/default_test.go b/identity/internal/model/realm/default_test.go index c565c0e8..ac0d7e79 100644 --- a/identity/internal/model/realm/default_test.go +++ b/identity/internal/model/realm/default_test.go @@ -43,9 +43,9 @@ func TestRealmIsCreatedCorrectly(t *testing.T) { realm := NewRealm(name, namespace, environment, identityProviderName) assert.NotNil(t, realm) - assert.Equal(t, name, realm.ObjectMeta.Name) - assert.Equal(t, namespace, realm.ObjectMeta.Namespace) - assert.Equal(t, environment, realm.ObjectMeta.Labels[config.EnvironmentLabelKey]) + assert.Equal(t, name, realm.Name) + assert.Equal(t, namespace, realm.Namespace) + assert.Equal(t, environment, realm.Labels[config.EnvironmentLabelKey]) assert.Equal(t, identityProviderName, realm.Spec.IdentityProvider.Name) assert.Equal(t, namespace, realm.Spec.IdentityProvider.Namespace) } diff --git a/identity/test/utils/utils.go b/identity/test/utils/utils.go index 7553d740..d1e98338 100644 --- a/identity/test/utils/utils.go +++ b/identity/test/utils/utils.go @@ -10,7 +10,7 @@ import ( "os/exec" "strings" - . "github.com/onsi/ginkgo/v2" //nolint:golint,revive + ginkgo "github.com/onsi/ginkgo/v2" ) const ( @@ -24,7 +24,7 @@ const ( ) func warnError(err error) { - _, _ = fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "warning: %v\n", err) } // InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics. @@ -40,12 +40,12 @@ func Run(cmd *exec.Cmd) ([]byte, error) { cmd.Dir = dir if err := os.Chdir(cmd.Dir); err != nil { - _, _ = fmt.Fprintf(GinkgoWriter, "chdir dir: %s\n", err) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "chdir dir: %s\n", err) } cmd.Env = append(os.Environ(), "GO111MODULE=on") command := strings.Join(cmd.Args, " ") - _, _ = fmt.Fprintf(GinkgoWriter, "running: %s\n", command) + _, _ = fmt.Fprintf(ginkgo.GinkgoWriter, "running: %s\n", command) output, err := cmd.CombinedOutput() if err != nil { return output, fmt.Errorf("%s failed with error: (%v) %s", command, err, string(output)) @@ -105,8 +105,7 @@ func LoadImageToKindClusterWithName(name string) error { // according to line breakers, and ignores the empty elements in it. func GetNonEmptyLines(output string) []string { var res []string - elements := strings.Split(output, "\n") - for _, element := range elements { + for element := range strings.SplitSeq(output, "\n") { if element != "" { res = append(res, element) } @@ -121,6 +120,6 @@ func GetProjectDir() (string, error) { if err != nil { return wd, err } - wd = strings.Replace(wd, "/test/e2e", "", -1) + wd = strings.ReplaceAll(wd, "/test/e2e", "") return wd, nil } diff --git a/rover-ctl/cmd/root.go b/rover-ctl/cmd/root.go index f1d8bdd0..dce06712 100644 --- a/rover-ctl/cmd/root.go +++ b/rover-ctl/cmd/root.go @@ -49,22 +49,22 @@ This tool allows you to apply, delete, and manage resources in the Rover Control // Add global flags rootCmd.PersistentFlags().BoolP("debug", "d", false, "Enable debug mode") - viper.BindPFlag("debug", rootCmd.PersistentFlags().Lookup("debug")) + _ = viper.BindPFlag("debug", rootCmd.PersistentFlags().Lookup("debug")) rootCmd.PersistentFlags().String("log-level", viper.GetString("log.level"), "Log level (debug, info, warn, error)") - viper.BindPFlag("log.level", rootCmd.PersistentFlags().Lookup("log-level")) + _ = viper.BindPFlag("log.level", rootCmd.PersistentFlags().Lookup("log-level")) rootCmd.PersistentFlags().String("log-format", viper.GetString("log.format"), "Log format (json or console)") - viper.BindPFlag("log.format", rootCmd.PersistentFlags().Lookup("log-format")) + _ = viper.BindPFlag("log.format", rootCmd.PersistentFlags().Lookup("log-format")) // Add output format flag rootCmd.PersistentFlags().String("format", viper.GetString("output.format"), "Output format (yaml|json)") - viper.BindPFlag("output.format", rootCmd.PersistentFlags().Lookup("format")) + _ = viper.BindPFlag("output.format", rootCmd.PersistentFlags().Lookup("format")) // Add output file flag rootCmd.PersistentFlags().StringP("output", "o", "stdout", "Output file path") - viper.BindPFlag("output.file", rootCmd.PersistentFlags().Lookup("output")) + _ = viper.BindPFlag("output.file", rootCmd.PersistentFlags().Lookup("output")) rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { if viper.GetBool("debug") { diff --git a/rover-ctl/pkg/commands/apply/cmd.go b/rover-ctl/pkg/commands/apply/cmd.go index bef6319b..a2c77ef0 100644 --- a/rover-ctl/pkg/commands/apply/cmd.go +++ b/rover-ctl/pkg/commands/apply/cmd.go @@ -31,7 +31,7 @@ func NewCommand() *cobra.Command { ), } - cmd.Cmd.MarkFlagRequired("file") + _ = cmd.Cmd.MarkFlagRequired("file") // Set the run function cmd.Cmd.RunE = cmd.Run diff --git a/rover-ctl/pkg/commands/apply/cmd_test.go b/rover-ctl/pkg/commands/apply/cmd_test.go index 58e59ed7..42f14bf2 100644 --- a/rover-ctl/pkg/commands/apply/cmd_test.go +++ b/rover-ctl/pkg/commands/apply/cmd_test.go @@ -98,7 +98,7 @@ spec: AfterEach(func() { // Clean up - os.RemoveAll(tempDir) + _ = os.RemoveAll(tempDir) // Reset viper config viper.Reset() diff --git a/rover-ctl/pkg/commands/base/banner.go b/rover-ctl/pkg/commands/base/banner.go index e8d727c0..ea69e212 100644 --- a/rover-ctl/pkg/commands/base/banner.go +++ b/rover-ctl/pkg/commands/base/banner.go @@ -80,6 +80,6 @@ func PrintBanner(cmd *cobra.Command) { banner += fmt.Sprintf("└%s┘\n", border) - fmt.Fprintln(w, banner) - fmt.Fprintln(w) + _, _ = fmt.Fprintln(w, banner) + _, _ = fmt.Fprintln(w) } diff --git a/rover-ctl/pkg/commands/base/file_test.go b/rover-ctl/pkg/commands/base/file_test.go index 0bf76da2..dbc5b200 100644 --- a/rover-ctl/pkg/commands/base/file_test.go +++ b/rover-ctl/pkg/commands/base/file_test.go @@ -49,7 +49,7 @@ spec: AfterEach(func() { // Clean up test files - os.RemoveAll(tempDir) + _ = os.RemoveAll(tempDir) }) Describe("NewFileCommand", func() { diff --git a/rover-ctl/pkg/commands/delete/cmd.go b/rover-ctl/pkg/commands/delete/cmd.go index 35cdf325..ff123477 100644 --- a/rover-ctl/pkg/commands/delete/cmd.go +++ b/rover-ctl/pkg/commands/delete/cmd.go @@ -29,7 +29,7 @@ func NewCommand() *cobra.Command { ), } - cmd.Cmd.MarkFlagRequired("file") + _ = cmd.Cmd.MarkFlagRequired("file") // Set the run function cmd.Cmd.RunE = cmd.Run diff --git a/rover-ctl/pkg/commands/delete/cmd_test.go b/rover-ctl/pkg/commands/delete/cmd_test.go index 2f264bef..8b22ca6c 100644 --- a/rover-ctl/pkg/commands/delete/cmd_test.go +++ b/rover-ctl/pkg/commands/delete/cmd_test.go @@ -97,7 +97,7 @@ spec: AfterEach(func() { // Clean up - os.RemoveAll(tempDir) + _ = os.RemoveAll(tempDir) // Reset viper config viper.Reset() diff --git a/rover-ctl/pkg/commands/get-info/cmd_test.go b/rover-ctl/pkg/commands/get-info/cmd_test.go index 26b358cd..fd806d1c 100644 --- a/rover-ctl/pkg/commands/get-info/cmd_test.go +++ b/rover-ctl/pkg/commands/get-info/cmd_test.go @@ -96,7 +96,7 @@ spec: AfterEach(func() { // Clean up - os.RemoveAll(tempDir) + _ = os.RemoveAll(tempDir) // Reset viper config viper.Reset() diff --git a/rover-ctl/pkg/commands/resource/cmd.go b/rover-ctl/pkg/commands/resource/cmd.go index a27a6d69..03805cea 100644 --- a/rover-ctl/pkg/commands/resource/cmd.go +++ b/rover-ctl/pkg/commands/resource/cmd.go @@ -75,11 +75,11 @@ func (c *Command) newGetCommand() *cobra.Command { cmd.Flags().StringVar(&c.Options.Kind, "kind", "", "Resource kind") cmd.Flags().StringVar(&c.Options.ApiVersion, "api-version", "", "API version") - cmd.MarkFlagRequired("kind") - cmd.MarkFlagRequired("api-version") + _ = cmd.MarkFlagRequired("kind") + _ = cmd.MarkFlagRequired("api-version") cmd.Flags().StringVar(&c.Options.Name, "name", "", "Name of the resource to get") - cmd.MarkFlagRequired("name") + _ = cmd.MarkFlagRequired("name") return cmd } @@ -104,8 +104,8 @@ func (c *Command) newListCommand() *cobra.Command { cmd.Flags().StringVar(&c.Options.Kind, "kind", "", "Resource kind") cmd.Flags().StringVar(&c.Options.ApiVersion, "api-version", defaultApiVersion, "API version") - cmd.MarkFlagRequired("kind") - cmd.MarkFlagRequired("api-version") + _ = cmd.MarkFlagRequired("kind") + _ = cmd.MarkFlagRequired("api-version") return cmd } diff --git a/rover-ctl/pkg/config/token.go b/rover-ctl/pkg/config/token.go index 16715db3..5c1e9181 100644 --- a/rover-ctl/pkg/config/token.go +++ b/rover-ctl/pkg/config/token.go @@ -29,6 +29,11 @@ var ( // Global validator instance var validate = validator.New() +// tokenContextKey is a custom type for context keys to avoid collisions +type tokenContextKey string + +const tokenKey tokenContextKey = "token" + type Token struct { Prefix string `json:"-"` Environment string `json:"environment" validate:"required"` @@ -207,11 +212,11 @@ func NewContext(ctx context.Context, token *Token) context.Context { return ctx } - return context.WithValue(ctx, "token", token) + return context.WithValue(ctx, tokenKey, token) } func FromContext(ctx context.Context) (*Token, bool) { - token, ok := ctx.Value("token").(*Token) + token, ok := ctx.Value(tokenKey).(*Token) return token, ok } diff --git a/rover-ctl/pkg/handlers/common/base_handler.go b/rover-ctl/pkg/handlers/common/base_handler.go index 805ae42a..f2067acd 100644 --- a/rover-ctl/pkg/handlers/common/base_handler.go +++ b/rover-ctl/pkg/handlers/common/base_handler.go @@ -126,7 +126,7 @@ func (h *BaseHandler) Apply(ctx context.Context, obj types.Object) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK, http.StatusAccepted) @@ -150,7 +150,7 @@ func (h *BaseHandler) Delete(ctx context.Context, obj types.Object) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK, http.StatusNoContent, http.StatusNotFound) @@ -171,7 +171,7 @@ func (h *BaseHandler) Get(ctx context.Context, name string) (any, error) { if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK) @@ -224,7 +224,7 @@ func (h *BaseHandler) ListWithCursor(ctx context.Context, cursor string) (*ListR if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK) @@ -250,7 +250,7 @@ func (h *BaseHandler) Status(ctx context.Context, name string) (types.ObjectStat if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK, http.StatusNotFound) @@ -287,7 +287,7 @@ func (h *BaseHandler) Info(ctx context.Context, name string) (any, error) { if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() // Check response err = CheckResponseCode(resp, http.StatusOK) diff --git a/rover-ctl/pkg/handlers/common/base_handler_test.go b/rover-ctl/pkg/handlers/common/base_handler_test.go index e636a3b3..53f63ca6 100644 --- a/rover-ctl/pkg/handlers/common/base_handler_test.go +++ b/rover-ctl/pkg/handlers/common/base_handler_test.go @@ -157,7 +157,7 @@ var _ = Describe("BaseHandler", func() { Expect(err).To(HaveOccurred()) apiErr, ok := common.AsApiError(err) Expect(ok).To(BeTrue()) - Expect(apiErr).NotTo(BeNil()) + Expect(apiErr).To(HaveOccurred()) // The error message comes from the mock response we defined above Expect(apiErr.Title).To(Equal("Validation failed")) }) @@ -256,12 +256,13 @@ var _ = Describe("BaseHandler", func() { Expect(err).To(HaveOccurred()) apiErr, ok := common.AsApiError(err) Expect(ok).To(BeTrue()) - Expect(apiErr).NotTo(BeNil()) + Expect(apiErr).To(HaveOccurred()) Expect(apiErr.Title).To(Equal("Delete failed")) }) }) }) + //nolint:dupl // Test patterns are intentionally similar for Get and Status methods Describe("Get", func() { Context("when sending a valid request", func() { It("should send a Get request to the correct URL", func() { @@ -349,6 +350,7 @@ var _ = Describe("BaseHandler", func() { }) }) + //nolint:dupl // Test patterns are intentionally similar for Get and Status methods Describe("Status", func() { Context("when sending a valid request", func() { It("should send a Status request to the correct URL", func() { diff --git a/rover-ctl/pkg/handlers/common/error.go b/rover-ctl/pkg/handlers/common/error.go index 48cc254f..bcf5439d 100644 --- a/rover-ctl/pkg/handlers/common/error.go +++ b/rover-ctl/pkg/handlers/common/error.go @@ -67,20 +67,20 @@ func PrintTextTo(err error, w io.Writer) { } } - fmt.Fprintf(w, "❌ Error\n--------\n") - fmt.Fprintf(w, "Type: %s\nStatus: %d\nTitle: %s\nDetail: %s\n", + _, _ = fmt.Fprintf(w, "❌ Error\n--------\n") + _, _ = fmt.Fprintf(w, "Type: %s\nStatus: %d\nTitle: %s\nDetail: %s\n", apiErr.Type, apiErr.Status, apiErr.Title, apiErr.Detail) if apiErr.Instance != "" { - fmt.Fprintf(w, "Instance: %s\n", apiErr.Instance) + _, _ = fmt.Fprintf(w, "Instance: %s\n", apiErr.Instance) } if len(apiErr.Fields) > 0 { - fmt.Fprintln(w, "Fields:") + _, _ = fmt.Fprintln(w, "Fields:") for _, field := range apiErr.Fields { - fmt.Fprintf(w, " Field: %s\n Detail: %s\n", field.Field, field.Detail) + _, _ = fmt.Fprintf(w, " Field: %s\n Detail: %s\n", field.Field, field.Detail) } } - fmt.Fprintln(w) + _, _ = fmt.Fprintln(w) } func PrintJsonTo(err error, w io.Writer) { @@ -95,6 +95,6 @@ func PrintJsonTo(err error, w io.Writer) { } data, _ := json.MarshalIndent(apiErr, "", " ") - w.Write(data) - fmt.Fprintln(w) + _, _ = w.Write(data) + _, _ = fmt.Fprintln(w) } diff --git a/rover-ctl/pkg/handlers/common/error_test.go b/rover-ctl/pkg/handlers/common/error_test.go index 1726a2bc..c417f2c3 100644 --- a/rover-ctl/pkg/handlers/common/error_test.go +++ b/rover-ctl/pkg/handlers/common/error_test.go @@ -75,7 +75,7 @@ var _ = Describe("ApiError", func() { err := errors.New("generic error") result, ok := common.AsApiError(err) Expect(ok).To(BeFalse()) - Expect(result).To(BeNil()) + Expect(result).ToNot(HaveOccurred()) }) It("should extract ApiError from wrapped error", func() { diff --git a/rover-ctl/pkg/handlers/common/httpclient_test.go b/rover-ctl/pkg/handlers/common/httpclient_test.go index 7c7f35e8..aee4c183 100644 --- a/rover-ctl/pkg/handlers/common/httpclient_test.go +++ b/rover-ctl/pkg/handlers/common/httpclient_test.go @@ -83,7 +83,7 @@ var _ = Describe("HttpClient", func() { // This test actually hits a test server to verify the client works server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"access_token":"test-token","token_type":"Bearer","expires_in":3600}`)) + _, _ = w.Write([]byte(`{"access_token":"test-token","token_type":"Bearer","expires_in":3600}`)) })) defer server.Close() diff --git a/rover-ctl/pkg/handlers/v0/rover.go b/rover-ctl/pkg/handlers/v0/rover.go index f76d9c82..1d38538c 100644 --- a/rover-ctl/pkg/handlers/v0/rover.go +++ b/rover-ctl/pkg/handlers/v0/rover.go @@ -24,7 +24,7 @@ func NewRoverHandlerInstance() *RoverHandler { handler := &RoverHandler{ BaseHandler: common.NewBaseHandler("tcp.ei.telekom.de/v1", "Rover", "rovers", 100), } - handler.BaseHandler.SupportsInfo = true + handler.SupportsInfo = true handler.AddHook(common.PreRequestHook, PatchRoverRequest) @@ -68,7 +68,7 @@ func PatchRoverRequest(ctx context.Context, obj types.Object) error { } func PatchExposures(exposures []any) []map[string]any { - if exposures == nil || len(exposures) == 0 { + if len(exposures) == 0 { return nil } exposuresMaps := make([]map[string]any, len(exposures)) @@ -95,7 +95,7 @@ func PatchExposures(exposures []any) []map[string]any { } func PatchSubscriptions(subscriptions []any) []map[string]any { - if subscriptions == nil || len(subscriptions) == 0 { + if len(subscriptions) == 0 { return nil } subscriptionsMaps := make([]map[string]any, len(subscriptions)) @@ -167,7 +167,7 @@ func (h *RoverHandler) ResetSecret(ctx context.Context, name string) (clientId s if err != nil { return "", "", err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() err = common.CheckResponseCode(resp, http.StatusOK, http.StatusAccepted) if err != nil { diff --git a/rover-ctl/pkg/parser/parser.go b/rover-ctl/pkg/parser/parser.go index 25bc4cd0..58754860 100644 --- a/rover-ctl/pkg/parser/parser.go +++ b/rover-ctl/pkg/parser/parser.go @@ -132,7 +132,7 @@ func (p *ObjectParser) parseFile(filePath string) error { if err != nil { return errors.Wrap(err, "failed to read file "+filePath) } - defer file.Close() + defer func() { _ = file.Close() }() switch ext { case ".yaml", ".yml": diff --git a/rover-ctl/pkg/parser/parser_test.go b/rover-ctl/pkg/parser/parser_test.go index ef71165b..34aeb96a 100644 --- a/rover-ctl/pkg/parser/parser_test.go +++ b/rover-ctl/pkg/parser/parser_test.go @@ -145,14 +145,14 @@ var _ = Describe("Parser", func() { objects := dirParser.Objects() Expect(objects).NotTo(BeEmpty()) - Expect(len(objects)).To(Equal(4), "Should find exactly 4 valid objects in the directory") + Expect(objects).To(HaveLen(4), "Should find exactly 4 valid objects in the directory") }) It("should return an error when parsing a directory with no valid files", func() { // Create a temporary empty directory tempDir, err := os.MkdirTemp("", "parser-test-*") Expect(err).NotTo(HaveOccurred()) - defer os.RemoveAll(tempDir) + defer func() { _ = os.RemoveAll(tempDir) }() // Attempt to parse the empty directory err = objectParser.Parse(tempDir) @@ -194,7 +194,7 @@ var _ = Describe("Parser", func() { Expect(hookCalled).To(BeTrue()) Objects := hookParser.Objects() Expect(Objects).To(HaveLen(1)) - Expect(Objects[0].GetProperty("hook-applied")).To(Equal(true)) + Expect(Objects[0].GetProperty("hook-applied")).To(BeTrue()) }) })