Skip to content

OU-1389: add OWNERS_ALIAS and feature based OWNERS files#1083

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
PeterYurkovich:ou-1389-14
Jul 24, 2026
Merged

OU-1389: add OWNERS_ALIAS and feature based OWNERS files#1083
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
PeterYurkovich:ou-1389-14

Conversation

@PeterYurkovich

@PeterYurkovich PeterYurkovich commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Add per-feature OWNERS files and specific shared and cypress OWNER's files.

Adds @DavidRajnoha as a reviewer and approver in the cypress folder

Summary by CodeRabbit

Summary by CodeRabbit

  • Chores

    • Updated ownership/approval configurations across the web app, including new feature-area OWNERS files and Cypress-specific ownership rules.
    • Added centralized ownership alias groups to streamline reviewer/approver assignments.
  • New Features

    • Introduced a linting rule that validates required feature ownership data and reports clear diagnostics when missing.
  • Bug Fixes

    • Improved lint feedback by tying missing ownership reports to the specific affected feature.
  • Tests

    • Added rule test coverage for valid/invalid cases and a regression scenario to prevent stale results.

@openshift-ci-robot

openshift-ci-robot commented Jul 22, 2026

Copy link
Copy Markdown

@PeterYurkovich: This pull request references OU-1389 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 22, 2026
@openshift-ci
openshift-ci Bot requested review from etmurasaki and jgbernalp July 22, 2026 22:09
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Walkthrough

The change adds ownership aliases and OWNERS files across repository and web paths, and introduces an enabled ESLint rule that requires each src/features directory to contain an OWNERS file.

Changes

Feature ownership enforcement

Layer / File(s) Summary
Ownership aliases and assignments
OWNERS, OWNERS_ALIASES, web/cypress/OWNERS, web/src/features/*/OWNERS, web/src/shared/OWNERS
Defines reviewer and approver aliases and assigns ownership for repository, Cypress, feature, and shared web paths.
Feature OWNERS ESLint rule
web/eslint-rules/require-feature-owners.ts, web/eslint.config.ts, web/eslint-rules/require-feature-owners.spec.ts
Adds and enables an ESLint rule that checks feature-directory OWNERS files, reports missing ownership, and tests present, absent, and repeated-invocation cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ESLint
  participant requireFeatureOwners
  participant FileSystem
  ESLint->>requireFeatureOwners: lint file with feature path
  requireFeatureOwners->>FileSystem: check feature/OWNERS
  FileSystem-->>requireFeatureOwners: return existence result
  requireFeatureOwners-->>ESLint: accept file or report missingOwners
Loading

Suggested reviewers: jgbernalp, etmurasaki

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding OWNERS aliasing and feature-based OWNERS files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 Added test titles are static strings; no dynamic identifiers, timestamps, UUIDs, or other changing data appear in any It/Describe/Context/When names.
Test Structure And Quality ✅ Passed No Ginkgo or cluster-style tests are in this PR; the new test is an ESLint RuleTester spec, so the rubric is not applicable.
Microshift Test Compatibility ✅ Passed PR only adds OWNERS files and ESLint rule/unit tests; no new Ginkgo e2e tests or MicroShift-sensitive APIs were introduced.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only test change is a Jest RuleTester unit spec, so SNO assumptions are not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed PR only changes OWNERS files and ESLint rule/config code; no deployment manifests, operators, or controllers were modified.
Ote Binary Stdout Contract ✅ Passed No process-level stdout/logging writes were added; the diff is limited to ESLint rules/config and OWNERS files, with no main/init/suite code changes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only test file is an ESLint RuleTester unit test and it contains no IPv4 or external-network assumptions.
No-Weak-Crypto ✅ Passed PR only changes ESLint ownership rules/config; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons found.
Container-Privileges ✅ Passed Only ESLint rule/test/config files changed; no container/K8s manifests or privilege settings were introduced.
No-Sensitive-Data-In-Logs ✅ Passed The PR adds no logging statements and no sensitive data patterns (passwords, tokens, emails, etc.) appear in the changed files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/eslint-rules/require-feature-owners.ts`:
- Around line 41-43: Update the owners existence check in the feature validation
logic to require ownersPath to be a regular file rather than merely an existing
path, while preserving the cache behavior. Add a test covering a directory named
OWNERS and verify it does not satisfy the rule.
- Line 12: Scope or remove the module-level cache in the require-feature-owners
rule so OWNERS checks are not reused across ESLint invocations; preserve caching
only within one invocation if retained. In
web/eslint-rules/require-feature-owners.ts, update the cache associated with the
rule implementation, and in web/eslint-rules/require-feature-owners.spec.ts
lines 8-45 add a regression test that reuses the same rule instance while
existsSync changes from true to false and verifies the second run reflects the
change.

In `@web/eslint.config.ts`:
- Line 187: Replace the `as any` cast on the `local-rules` plugin registration
with a type-checked plugin object using `satisfies Linter.Plugin` or the
project’s equivalent typed form. Preserve the existing `file-naming` and
`require-feature-owners` rule registrations while ensuring the object remains
validated by ESLint types.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 68b6fdd8-7cdb-4904-94ce-4f734df85704

📥 Commits

Reviewing files that changed from the base of the PR and between f3168d1 and 1c86ce3.

📒 Files selected for processing (13)
  • OWNERS
  • OWNERS_ALIASES
  • web/cypress/OWNERS
  • web/eslint-rules/require-feature-owners.spec.ts
  • web/eslint-rules/require-feature-owners.ts
  • web/eslint.config.ts
  • web/src/features/alerts/OWNERS
  • web/src/features/incidents/OWNERS
  • web/src/features/legacy-dashboards/OWNERS
  • web/src/features/metrics/OWNERS
  • web/src/features/perses-dashboards/OWNERS
  • web/src/features/targets/OWNERS
  • web/src/shared/OWNERS

Comment thread web/eslint-rules/require-feature-owners.ts Outdated
Comment thread web/eslint-rules/require-feature-owners.ts Outdated
Comment thread web/eslint.config.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/eslint-rules/require-feature-owners.ts`:
- Line 37: Update the OWNERS-path validation around existsSync to require that
ownersPath resolves to a regular file, not merely an existing path; use
statSync(...).isFile() or the rule’s existing injectable predicate while
preserving current missing-file behavior. Add a regression test covering an
OWNERS directory and verify it is rejected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 41c199ca-cb8d-4630-b3b4-34c9a6cfb0bd

📥 Commits

Reviewing files that changed from the base of the PR and between 1c86ce3 and a2918d3.

📒 Files selected for processing (3)
  • web/eslint-rules/require-feature-owners.spec.ts
  • web/eslint-rules/require-feature-owners.ts
  • web/eslint.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/eslint-rules/require-feature-owners.spec.ts
  • web/eslint.config.ts

Comment thread web/eslint-rules/require-feature-owners.ts
@PeterYurkovich

Copy link
Copy Markdown
Contributor Author

/label qe-approved
No code changes

@openshift-ci openshift-ci Bot added the qe-approved Signifies that QE has signed off on this PR label Jul 23, 2026
@jgbernalp

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2026
@openshift-ci

openshift-ci Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jgbernalp, PeterYurkovich

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [PeterYurkovich,jgbernalp]

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

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 4c2373d and 2 for PR HEAD a2918d3 in total

@PeterYurkovich

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@PeterYurkovich: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 6b2451b into openshift:main Jul 24, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants