Skip to content

Duplication scanner still reports clones in the EKS validator tests #2777

Description

@devantler

🤖 Generated by the Daily AI Engineer

Evidence

After #2776 the duplication scanner drops from 19 findings to 13 in scripts/validate-eks-ci-role-policy/main_test.go, measured locally with the same settings the lint job uses (jscpd --min-lines 5 --min-tokens 50 --threshold 0). The remaining 13 fall into two groups, and they want different treatment:

  • Shared manifest shapes. Several test fixtures repeat the same ClusterRoleBinding → roleRef → subjects block with different names — for example the same-name role shadow case and the binding-to-unavailable-privileged-role case. These are genuine copy-paste and should be built from a shared renderer, exactly as fix(lint): scope the lint gate to the paths it actually governs #2776 did for the six identity-binding fixtures.
  • Table-driven test boilerplate. Others are the standard closing of a case table followed by for _, tt := range tests { t.Run(...) }, repeated once per test function. This is idiomatic Go rather than copy-paste debt, and abstracting it away naively would make the tests harder to read, not easier. The fix here is a shared case runner, not an inlined clever helper.

Why it matters

Duplication is held at 0% deliberately and is never thresholded or ignored, so every remaining finding keeps the lint job red — and a red lint job on every pull request is what made the original defects invisible in the first place.

Acceptance criteria

  • jscpd reports 0 clones for scripts/validate-eks-ci-role-policy/, with the threshold left at 0.0%.
  • The test inventory is unchanged: same test and subtest names, same count.
  • Any fixture rendered by a helper is proved byte-identical to the literal it replaced before that literal is deleted. These tests assert a manifest is rejected, so a helper emitting subtly wrong YAML still produces the expected rejection — the assertions go vacuous without that proof. fix(lint): scope the lint gate to the paths it actually governs #2776 has the pattern.

Activity

  1. moved this to 📥 Backlog in 🌊 Project Boardon Jul 21, 2026
  2. coderabbitai commented on Jul 21, 2026

    @coderabbitai
    Contributor
    🔗 Related PRs

    #2273 - ci: re-enable in-process Helm render in validate (revert --skip-helm-render) [merged]
    #2370 - fix(autoscaling): break the DaemonSet-VPA node ratchet; keep Longhorn storage off autoscale nodes [merged]
    #2507 - fix(apps): make actual-budget openid schema offline-resolvable [merged]
    #2580 - fix(policy): refresh replica-floor exemptions as skips [merged]
    #2633 - fix(deploy): verify incoming image on Talos nodes [merged]


    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  3. devantler commented on Jul 21, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Daily AI Engineer

    A finding from self-review on #2776 that belongs to this issue rather than that PR, so it is not lost.

    awsIdentityBinding there is deliberately cut at the boundary of one test's six fixtures, not at "an RBAC binding fixture in this file". It hardcodes metadata.name: aws-shadow and the leading ---, and derives roleRef from scope. So the other standalone binding documents in main_test.go — around lines 114, 391, 492 — cannot adopt it as it stands.

    That was left alone on purpose: adding fields no current fixture uses would be speculative, and this issue is where the rest of the file gets deduplicated anyway. Worth knowing when picking it up:

    • Widening it is additive, not a rewrite. Give it explicit roleKind/roleName/name fields and keep scope selecting the defaults the six existing routes rely on, and those routes do not change.
    • One combination is missing and is the interesting one. A RoleBinding referencing a ClusterRole is how a cluster role is granted namespace-scoped, and the fixture at line 114 already uses that shape. It is a plausible escalation route to aws/aws that the table does not currently cover. An earlier version of the helper's comment wrongly claimed Kubernetes forbids it — corrected in fix(lint): scope the lint gate to the paths it actually governs #2776, but flagging it here since the gap in coverage is real and is worth a case of its own.
    • The byte-identical proof is not optional. Every one of these tests asserts a manifest is rejected, so a helper emitting subtly wrong YAML still yields the expected rejection and the refactor looks correct while asserting nothing. Pin each build against the literal it replaces, perturb the proof to confirm it fails on a wrong build, then delete both proof and literal.
  4. added a commit that references this issue on Oct 3, 2026
  5. moved this from 📥 Backlog to 👀 In Review in 🌊 Project Boardon Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    No fields configured for Refactor.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions