Skip to content

ROSAENG-62490 | refactor: Add lint rules to enforce environmental boundaries - #3449

Open
markirish wants to merge 1 commit into
openshift:masterfrom
markirish:ROSAENG-62485-add-achitecture-enforcement-checks
Open

ROSAENG-62490 | refactor: Add lint rules to enforce environmental boundaries#3449
markirish wants to merge 1 commit into
openshift:masterfrom
markirish:ROSAENG-62485-add-achitecture-enforcement-checks

Conversation

@markirish

@markirish markirish commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Adds CI-enforced checks to prevent disallowed code patterns (e.g. calling os.Exit from /pkg), and prevent disallowed dependency directions (core layer -> CLI). Enforces architectural patterns that have been laid out in guidelines/ARCHITECTURE.md and guidelines/refactor/pkg-architecture.md

Detailed Description of the Issue

  • Add depguard deny list to prohibit imports defined in ARCHITECTURE.md (reporter, output, interactive, arguments, color, debug, commands, rosa, options, aws/profile, aws/region, aws/commandbuilder)
  • Add nolint:depguard annotations to pre-existing internal import violations in files already within whole-files lint scope
  • Add nolint:forbidigo annotations to pre-existing fmt.Print and os.Exit violations in files already within whole-files lint scope

As files are touched, //nolint:forbidgo and //nolint:depguard will have to be added until files are refactored as part of this initiative.

Related Issues and PRs

Type of Change

  • feat - adds a new user-facing capability.
  • fix - resolves an incorrect behavior or bug.
  • docs - updates documentation only.
  • style - formatting or naming changes with no logic impact.
  • refactor - code restructuring with no behavior change.
  • test - adds or updates tests only.
  • chore - maintenance work (tooling, housekeeping, non-product code).
  • build - changes build system, packaging, or dependencies for build output.
  • ci - changes CI pipelines, jobs, or automation workflows.
  • perf - improves performance without changing intended behavior.

Previous Behavior

Bad dependency imports and disallowed code patterns were allowed

Behavior After This Change

Dependency imports as laid out in ARCHITECTURE.md and refactor/pkg-architecture.md are enforced.

How to Test (Step-by-Step)

Preconditions

Test Steps

  1. Add an incorrect dependency import (e.g. importing /cmd in /pkg) or incorrect code smell (e.g. /pkg calling os.Exit or writing to stdout)
  2. Try to run the linter or push the code

Expected Results

Linter should catch the architectural violations

Proof of the Fix

  • Screenshots:
  • Videos:
  • Logs/CLI output:
  • Other artifacts:

Breaking Changes

  • No breaking changes
  • Yes, this PR introduces a breaking change (describe impact and migration plan below)

Developer Verification Checklist

  • Commit subject/title follows [JIRA-TICKET] | [TYPE]: <MESSAGE>.
  • PR description clearly explains both what changed and why.
  • Relevant Jira/GitHub issues and related PRs are linked.
  • make install-hooks has been run in this clone.
  • Tests were added/updated where appropriate.
  • I manually tested the change.
  • make test passes.
  • make lint passes.
  • make rosa passes.
  • Documentation or repo-local agent guidance was added/updated where appropriate.
  • Any risk, limitation, or follow-up work is documented.

Summary by CodeRabbit

  • Chores
    • Strengthened static analysis rules to detect prohibited dependencies, direct console output, and process exits.
    • Added targeted exceptions where existing command-line and interactive behaviors require these patterns.
  • Refactor
    • Standardized lint annotations across AWS, cluster, machine pool, input, and helper components.
    • No user-facing functionality, public APIs, or runtime behavior changed.

… boundaries

- Add depguard deny list to prohibit imports defined in ARCHITECTURE.md (reporter, output, interactive, arguments, color, debug, commands, rosa, options, aws/profile, aws/region, aws/commandbuilder)
- Add nolint:depguard annotations to pre-existing internal import violations in files already within whole-files lint scope
- Add nolint:forbidigo annotations to pre-existing fmt.Print and os.Exit violations in files already within whole-files lint scope
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The pull request enables depguard and forbidigo in .golangci.yml. It restricts core pkg/ imports and direct fmt.Print* and os.Exit usage. It adds path-based exclusions and targeted nolint annotations for existing permitted imports, output calls, and process exits. Runtime behavior and public APIs remain unchanged.

Possibly related PRs

Suggested reviewers: olucasfreitas

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The PR changes no test files, and added lines contain no Ginkgo title APIs; it introduces no test names that can be unstable or dynamic.
Test Structure And Quality ✅ Passed The pull request changes no Ginkgo test files or test code; it only updates linter configuration and annotations in production Go files, so these test-quality requirements are not applicable.
Microshift Test Compatibility ✅ Passed The patch adds no test files, changes no *_test.go files, and adds no Ginkgo declarations; it only changes lint configuration and existing package files.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The patch adds no new Ginkgo e2e tests or test files; it only changes .golangci.yml and lint annotations in existing pkg code.
Topology-Aware Scheduling Compatibility ✅ Passed The diff changes only .golangci.yml and lint annotations in CLI/core Go files; no deployment manifests, controllers, workload objects, or scheduling constraints were added or modified.
Ote Binary Stdout Contract ✅ Passed PR does not modify process-level code (cmd/ entry points). forbidigo rule applies to pkg/ library code only, excluded from cmd/ paths. cmd/rosa/main.go correctly uses stderr for output.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The diff changes only .golangci.yml and existing non-test Go files; no new Ginkgo tests or IPv4/external-connectivity test logic was added.
No-Weak-Crypto ✅ Passed The patch only adds linter configuration and suppression comments; no weak crypto primitives, custom crypto, or secret-value comparisons were introduced.
Container-Privileges ✅ Passed The PR changes no container or Kubernetes manifest and adds no privileged, host namespace, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed PR adds linter config and annotations only; no new logging introduced. Existing annotated logging statements do not expose passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.
Title check ✅ Passed The title clearly identifies the refactor that adds lint rules to enforce environmental boundaries.
Description check ✅ Passed The description explains the problem, changes, validation steps, issue links, change type, and verification results.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: markirish
Once this PR has been reviewed and has the lgtm label, please assign davidleerh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markirish markirish changed the title ROSAENG-62490 | refactor: Add CI integration to enforce environmental boundaries ROSAENG-62490 | refactor: Add lint rules to enforce environmental boundaries Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant