Skip to content

feat(oac): exempt username-scoped cluster roles and add Render releaseNamespace#3333

Merged
eball merged 7 commits into
mainfrom
module-oac
Jun 10, 2026
Merged

feat(oac): exempt username-scoped cluster roles and add Render releaseNamespace#3333
eball merged 7 commits into
mainfrom
module-oac

Conversation

@hysyeah

@hysyeah hysyeah commented Jun 10, 2026

Copy link
Copy Markdown
Member
  • Background
    Allow cluster-scoped fixed-name lint to pass when allowMultipleInstall=false
    and the resource name is templated on {{ .Values.bfl.username }}, detected
    via paired username dry-runs. Add ClusterScopedFixedNameOpts and wire
    username probes in checkAllowMultipleInstall.

    Extend helmrender.Render with an optional releaseNamespace argument: empty
    uses RenderNamespace; non-empty overrides instAction.Namespace. Username
    probes pass each probe username as the release namespace so
    {{ .Release.Namespace }} renders consistently during comparison.

  • Target Version for Merge
    v1.12.6,v1.12.7

  • Related Issues
    None

  • PRs Involving Sub-Systems
    None

  • Other information:


Note

Medium Risk
Changes OAC lint behavior for cluster-scoped charts and manifest resource validation rules; incorrect probe pairing could miss real collisions or accept bad manifests.

Overview
OAC lint now treats cluster-scoped RBAC whose metadata.name follows {{ .Values.bfl.username }} as acceptable when options.allowMultipleInstall is false. For that case, checkAllowMultipleInstall runs extra Helm dry-runs with two usernames (namespace set to each probe user) and CheckClusterScopedFixedNames skips “fixed name” hits when names differ across those probes. The cluster-scoped error hint now points authors at {{ .Release.Namespace }} instead of release name.

helmrender.Render takes a fourth argument, releaseNamespace (empty keeps RenderNamespace); username probes use it so namespace-templated charts compare correctly.

Manifest validation threads options.templateOnly into resource quantity checks: template-only apps may use -1 on CPU/memory/GPU (and modern spec.resources[] entries) for install-time auto resolution, but disk quantities must stay explicit; non-template manifests still reject -1 on legacy flat fields.

Reviewed by Cursor Bugbot for commit 6e9ce7d. Bugbot is set up for automated code reviews on this repo. Configure here.

dkeven and others added 5 commits June 9, 2026 21:18
…eNamespace

Allow cluster-scoped fixed-name lint to pass when allowMultipleInstall=false
and the resource name is templated on {{ .Values.bfl.username }}, detected
via paired username dry-runs. Add ClusterScopedFixedNameOpts and wire
username probes in checkAllowMultipleInstall.

Extend helmrender.Render with an optional releaseNamespace argument: empty
uses RenderNamespace; non-empty overrides instAction.Namespace. Username
probes pass each probe username as the release namespace so
{{ .Release.Namespace }} renders consistently during comparison.
@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
olares Ready Ready Preview, Comment Jun 10, 2026 2:45pm
olares-docs Ready Ready Preview, Comment Jun 10, 2026 2:45pm

Request Review

Comment thread framework/oac/chart.go
}
//if err := resources.CheckWorkloadReplicas(list, replicas); err != nil {
// errs = append(errs, err)
//}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Workload replica lint disabled

High Severity

checkManifestWorkloadRefs no longer calls resources.CheckWorkloadReplicas because that block is commented out. Lint still runs on the mandatory path via lintRenderedScenario, so manifests with workloadReplicas can pass when rendered Deployment/StatefulSet names do not match the declared keys.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 82cec78. Configure here.

if strings.HasPrefix(e.Name(), ".") {
continue
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

V2 fleet skip commented out

Medium Severity

scanFleet no longer skips v2 manifest directories because the isV2ManifestDir guard is commented out, while the function comment still says v2 apps are excluded. Fleet report tests will run full Lint on v2 layouts that the harness was designed to skip.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 82cec78. Configure here.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 4 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6e9ce7d. Configure here.

if !k8sQuantity.MatchString(p.value) {
errs = append(errs, fmt.Errorf("%s%s must be a valid Kubernetes quantity (got %q)", prefix, p.field, p.value))
if err := validateResourceQuantity(p.value, prefix+p.field, templateOnly, true); err != nil {
errs = append(errs, err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto disk skip breaks validation

Medium Severity

In validateQuantities, entries equal to the auto-resolve sentinel are skipped before validateResourceQuantity runs, so template-only spec.resources[] rows can keep -1 on requiredDisk or limitedDisk even though the new rules and tests expect a rejection there.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6e9ce7d. Configure here.

}
remaining = append(remaining, key)
}
return remaining

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Username probe index mis-pairs roles

High Severity

filterUsernameScopedClusterKeys aligns username dry-runs to release-probe cluster resources by sorted index. When a fixed cluster-scoped name and a username-templated one sort differently across probes, exemptions and violations can be applied to the wrong resources.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6e9ce7d. Configure here.

@eball eball merged commit 85c4f9f into main Jun 10, 2026
4 checks passed
eball added a commit that referenced this pull request Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants