Skip to content

fix: fan out cluster-report RBAC into skill scripts tree - #159

Merged
r2dedios merged 2 commits into
RHEcosystemAppEng:mainfrom
rhartuv:fix/symlinks
Sep 15, 2026
Merged

r2dedios merged 2 commits into
RHEcosystemAppEng:mainfrom
rhartuv:fix/symlinks

Conversation

@rhartuv

@rhartuv rhartuv commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Issue

cluster-report keeps shared files under pack scripts/cluster-report/ and symlinks only the .py files into skills/cluster-report/scripts/. build-kubeconfig.py loads cluster-reporter-rbac.yaml as a sibling of itself. After publish/install, only the skill tree is present, so the YAML is missing. Skill docs also tell the agent to run python3 ocp-admin/scripts/..., which is an authoring-repo path, not a skill-local path.
Jira

Summary

  • Add skills/cluster-report/scripts/cluster-reporter-rbac.yaml symlink so build-kubeconfig.py can apply RBAC when only the skill tree is installed (publish/install no longer ships pack scripts/cluster-report/ alone).
  • Update skills/cluster-report/references/multi-cluster-auth.md to use skill-local commands (python3 scripts/build-kubeconfig.py, oc apply -f scripts/cluster-reporter-rbac.yaml) instead of authoring-repo ocp-admin/scripts/... paths.
  • Update ocp-admin/README.md quick start to use plugin-root python3 scripts/cluster-report/... for human operators.
  • Document the skill-local scripts rule in SKILL_DESIGN_PRINCIPLES.md and enforce it in validate_compass_manifests.py (all non-test files in a linked scripts/<group>/ must be symlinked under the skill; no <pack>/scripts/ in skill markdown).

Test plan

  • uv run python scripts/validate_compass_manifests.py
  • uv run python -m unittest scripts.test_validate_compass_layout
  • From ocp-admin/skills/cluster-report, confirm python3 scripts/build-kubeconfig.py setup --help runs and scripts/cluster-reporter-rbac.yaml resolves via symlink

@rhartuv
rhartuv requested a review from RedTanny September 14, 2026 07:57
@rhartuv rhartuv self-assigned this Sep 14, 2026
@rhartuv rhartuv added the enhancement New feature or request label Sep 14, 2026
@rhartuv
rhartuv requested a review from TomerFi as a code owner September 14, 2026 07:57
@rhartuv rhartuv added the skills label Sep 14, 2026
Comment on lines +99 to +117
def test_missing_shared_script_symlink_flagged(self) -> None:
pack_dir = self.fixture_root
group_dir = pack_dir / "scripts" / "demo-group"
group_dir.mkdir(parents=True)
(group_dir / "run.py").write_text("# run\n", encoding="utf-8")
(group_dir / "config.yaml").write_text("key: value\n", encoding="utf-8")

scripts_dir = pack_dir / "skills" / "demo-skill" / "scripts"
scripts_dir.mkdir(parents=True)
os.symlink("../../../scripts/demo-group/run.py", scripts_dir / "run.py")

errors: list[str] = []
compass._check_skill_scripts_layout(
pack_dir.name, pack_dir / "skills" / "demo-skill", errors
)

self.assertTrue(errors)
self.assertIn("config.yaml", errors[0])

@r2dedios r2dedios Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be a case where the Markdown files are not being analyzed? I see test_missing_shared_script_symlink_flagged but I don't see a test like: test_forbidden_pack_scripts_path_in_skill_markdown_flagged

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also... Could we add a happy path test were everything is OK, so we confirm that the checks works correctly?

@r2dedios r2dedios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM :)

@r2dedios
r2dedios merged commit 1ff85af into RHEcosystemAppEng:main Sep 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants