-
Notifications
You must be signed in to change notification settings - Fork 567
NO-JIRA: Add API dependency import restriction tool #8364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
openshift-merge-bot
merged 4 commits into
openshift:main
from
JoelSpeed:api-import-restrictions
Apr 29, 2026
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4f36e92
feat: Add API dependency import restriction tool
JoelSpeed 480294d
feat: Use an .imports_allowed file to move allowlist under API OWNERS…
JoelSpeed 8ffd5a2
fix: Find repo root so that the tool can run from any dir
JoelSpeed 043607a
fix: Update API agents to ensure it understands imports
JoelSpeed File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Allowed direct dependencies for the HyperShift API module | ||
| # | ||
| # This file defines the restricted list of dependencies that the API module | ||
| # is permitted to directly import. Any changes to this file MUST be reviewed | ||
| # and approved by API reviewers. | ||
| # | ||
| # Format: one module path per line, comments start with # | ||
|
|
||
| # Core Kubernetes API dependencies | ||
| k8s.io/api | ||
| k8s.io/apimachinery | ||
| k8s.io/utils | ||
|
|
||
| # OpenShift API dependencies | ||
| github.com/openshift/api |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| # API Dependencies Verification Tool | ||
|
|
||
| This tool enforces strict dependency restrictions on the HyperShift API module (`api/`) to maintain API stability, compatibility, and a minimal dependency footprint. | ||
|
|
||
| ## Purpose | ||
|
|
||
| The HyperShift API module is a separate Go module with its own `go.mod` file. It should only have these **direct** dependencies: | ||
|
|
||
| - Core Kubernetes APIs (`k8s.io/api`, `k8s.io/apimachinery`, `k8s.io/utils`) | ||
| - OpenShift API definitions (`github.com/openshift/api`) | ||
|
|
||
| ## Allowlist Configuration | ||
|
|
||
| The allowed dependencies are defined in `api/.imports_allowed` - a simple text file with one module path per line. This file: | ||
|
|
||
| - **Lives in the API module** alongside the `go.mod` file | ||
| - **Falls under API reviewer control** via the OWNERS file | ||
| - **Requires API reviewer approval** for any changes | ||
| - **Supports comments** (lines starting with `#`) | ||
| - **One dependency per line** format | ||
|
|
||
| ## How It Works | ||
|
|
||
| 1. **Finds** the repository root by walking up directories to locate the `.git` directory | ||
| 2. **Locates** the API module at `<repo-root>/api` | ||
| 3. **Loads** the allowed dependencies from `api/.imports_allowed` file | ||
| 4. **Reads** the `api/go.mod` file | ||
| 5. **Parses** the required dependencies (ignoring indirect dependencies) | ||
| 6. **Validates** each dependency against the allowlist | ||
| 7. **Fails** with a detailed error message if unauthorized dependencies are found | ||
|
|
||
| ## Usage | ||
|
|
||
| ```bash | ||
| # Run as part of verification | ||
| make verify | ||
|
|
||
| # Run standalone | ||
| make verify-api-deps | ||
|
|
||
| # Build and run directly (works from any directory within the repo) | ||
| cd hack/tools/verify-api-deps | ||
| go run main.go | ||
| ``` | ||
|
|
||
| ## Adding New Dependencies | ||
|
|
||
| If you need to add a new dependency to the API module: | ||
|
|
||
| 1. **Consult API reviewers first** - discuss alternatives and necessity | ||
| 2. **Ensure the dependency is essential** for API type definitions | ||
| 3. **Verify compatibility** and that it doesn't introduce breaking changes | ||
| 4. **After approval**, add the module path to `api/.imports_allowed` | ||
| 5. **Update this documentation** if the reasoning changes | ||
|
|
||
| ## Error Messages | ||
|
|
||
| When the tool detects unauthorized dependencies, it provides: | ||
|
|
||
| - ❌ Clear list of violating dependencies | ||
| - 📋 Instructions for the review process | ||
| - 📁 Location to update the allowlist after approval | ||
| - 👥 Guidance to contact API reviewers | ||
|
|
||
| ## Integration | ||
|
|
||
| This tool runs automatically as part of: | ||
|
|
||
| - `make verify` (full verification suite) | ||
| - `make verify-parallel` (parallel verification tasks) | ||
| - Pre-commit hooks | ||
| - CI/CD pipelines | ||
|
|
||
| ## Rationale | ||
|
|
||
| The strict direct dependency restrictions for the API module ensure: | ||
|
|
||
| - **Stability**: Minimal direct dependencies mean fewer potential breaking changes | ||
| - **Compatibility**: Reduced version conflict risks with consumer projects | ||
| - **Performance**: Faster builds and smaller dependency trees | ||
| - **Security**: Smaller attack surface with fewer third-party dependencies | ||
| - **Maintainability**: Clear separation between API definitions and implementations | ||
| - **Simplicity**: Only essential APIs are directly imported, everything else is transitive |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "bufio" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "golang.org/x/mod/modfile" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
| ) | ||
|
|
||
|
|
||
| func main() { | ||
| if err := verifyAPIDependencies(); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| fmt.Println("✅ API dependencies verification passed") | ||
| } | ||
|
|
||
| func verifyAPIDependencies() error { | ||
| // Find the repository root and locate the API module | ||
| repoRoot, err := findRepoRoot() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to find repository root: %w", err) | ||
| } | ||
|
|
||
| apiModPath := filepath.Join(repoRoot, "api") | ||
|
|
||
| // Load allowed dependencies from the .imports_allowed file | ||
| allowedAPIModules, err := loadAllowedImports(apiModPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to load allowed imports: %w", err) | ||
| } | ||
|
|
||
| // Read the go.mod file | ||
| goModPath := filepath.Join(apiModPath, "go.mod") | ||
| data, err := os.ReadFile(goModPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read %s: %w", goModPath, err) | ||
| } | ||
|
|
||
| // Parse the go.mod file | ||
| modFile, err := modfile.Parse(goModPath, data, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse %s: %w", goModPath, err) | ||
| } | ||
|
|
||
| // Check required dependencies | ||
| var violations []string | ||
| for _, req := range modFile.Require { | ||
| if req.Indirect { | ||
| // Skip indirect dependencies as they're managed transitively | ||
| continue | ||
| } | ||
|
|
||
| modulePath := req.Mod.Path | ||
| if !allowedAPIModules.Has(modulePath) { | ||
| violations = append(violations, modulePath) | ||
| } | ||
| } | ||
|
|
||
| if len(violations) > 0 { | ||
| return fmt.Errorf(`❌ Unauthorized API dependencies detected: | ||
|
|
||
| %s | ||
|
|
||
| The HyperShift API module has strict dependency restrictions to maintain: | ||
| - API stability and compatibility | ||
| - Minimal dependency footprint | ||
| - Clear separation between API and implementation | ||
|
|
||
| Before adding any new dependencies to the API module, you must: | ||
|
|
||
| 1. Consult with API reviewers to discuss alternatives | ||
| 2. Ensure the dependency is absolutely necessary for the API layer | ||
| 3. Verify it doesn't introduce breaking changes or version conflicts | ||
| 4. Update the allowlist in api/.imports_allowed after approval | ||
|
|
||
| If this dependency is approved by API reviewers, add it to the allowlist in: | ||
| api/.imports_allowed | ||
|
|
||
| For questions, reach out to the HyperShift API review team.`, | ||
| formatViolations(violations)) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func formatViolations(violations []string) string { | ||
| var formatted []string | ||
| for _, v := range violations { | ||
| formatted = append(formatted, fmt.Sprintf(" • %s", v)) | ||
| } | ||
| return strings.Join(formatted, "\n") | ||
| } | ||
|
|
||
| func loadAllowedImports(apiModPath string) (sets.Set[string], error) { | ||
| allowedImportsPath := filepath.Join(apiModPath, ".imports_allowed") | ||
|
|
||
| file, err := os.Open(allowedImportsPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to open %s: %w", allowedImportsPath, err) | ||
| } | ||
| defer file.Close() | ||
|
|
||
| allowedModules := sets.New[string]() | ||
| scanner := bufio.NewScanner(file) | ||
|
|
||
| for scanner.Scan() { | ||
| line := strings.TrimSpace(scanner.Text()) | ||
|
|
||
| // Skip empty lines and comments | ||
| if line == "" || strings.HasPrefix(line, "#") { | ||
| continue | ||
| } | ||
|
|
||
| allowedModules.Insert(line) | ||
| } | ||
|
|
||
| if err := scanner.Err(); err != nil { | ||
| return nil, fmt.Errorf("failed to read %s: %w", allowedImportsPath, err) | ||
| } | ||
|
|
||
| return allowedModules, nil | ||
| } | ||
|
|
||
| func findRepoRoot() (string, error) { | ||
| // Start from current working directory and walk up to find .git directory | ||
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get working directory: %w", err) | ||
| } | ||
|
|
||
| dir := cwd | ||
| for { | ||
| // Check if .git directory exists | ||
| if fileExists(filepath.Join(dir, ".git")) { | ||
| return dir, nil | ||
| } | ||
|
|
||
| // Check if we've reached the root | ||
| parent := filepath.Dir(dir) | ||
| if parent == dir { | ||
| break | ||
| } | ||
| dir = parent | ||
| } | ||
|
|
||
| return "", fmt.Errorf("could not find repository root (no .git directory found)") | ||
| } | ||
|
|
||
| func fileExists(path string) bool { | ||
| _, err := os.Stat(path) | ||
| return err == nil | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.