Skip to content

Fix core API group detection in crdNameFromGVK#180

Open
nirdothan wants to merge 4 commits into
openshift-virtualization:mainfrom
nirdothan:fix-core-api-group-detection
Open

Fix core API group detection in crdNameFromGVK#180
nirdothan wants to merge 4 commits into
openshift-virtualization:mainfrom
nirdothan:fix-core-api-group-detection

Conversation

@nirdothan
Copy link
Copy Markdown
Contributor

@nirdothan nirdothan commented May 14, 2026

Summary

Fixes a bug where built-in Kubernetes API groups (apps, rbac.authorization.k8s.io, etc.) were incorrectly treated as CRDs, causing assets using these APIs to be skipped with "CRD not installed" errors.

Problem

The crdNameFromGVK function in pkg/assets/registry.go was treating ALL apiVersions containing / as CRDs. This caused core Kubernetes resources to be incorrectly flagged:

CRD clusterroles.rbac.authorization.k8s.io not installed, skipping asset
CRD deployments.apps not installed, skipping asset

Solution

Updated crdNameFromGVK to recognize built-in Kubernetes API groups and return empty RequiredCRD for them, allowing the reconciler to skip CRD checks for these resources.

Built-in API groups now recognized:

  • apps (Deployment, StatefulSet, DaemonSet, ReplicaSet)
  • batch (Job, CronJob)
  • rbac.authorization.k8s.io (ClusterRole, ClusterRoleBinding, Role, RoleBinding)
  • networking.k8s.io, policy, autoscaling, admissionregistration.k8s.io, etc.

Impact

This enables virt-platform-autopilot to deploy resources using built-in Kubernetes API groups, not just v1 resources and actual CRDs.

Test Plan

Tested with network-resources-injector deployment which uses:

  • apps/v1 (Deployment)
  • rbac.authorization.k8s.io/v1 (ClusterRole, ClusterRoleBinding)

Before fix: Resources skipped with "CRD not installed"
After fix: Resources deployed successfully

🤖 Generated with Claude Code

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 14, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dominikholler 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

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 14, 2026

Hi @nirdothan. Thanks for your PR.

I'm waiting for a openshift-virtualization member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan
Copy link
Copy Markdown
Contributor Author

nirdothan commented May 14, 2026

/cc @tiraboschi

@openshift-ci openshift-ci Bot requested a review from tiraboschi May 14, 2026 18:06
@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from 219a290 to b354934 Compare May 17, 2026 11:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from b354934 to 88fa1de Compare May 17, 2026 14:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from 88fa1de to e4c5a8a Compare May 17, 2026 14:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from fa133a3 to a6217ea Compare May 17, 2026 16:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan
assets: Fix core API group detection in crdNameFromGVK
2b94dd4 
The crdNameFromGVK function was incorrectly treating built-in
Kubernetes API groups (apps, rbac.authorization.k8s.io, etc.) as
CRDs, causing assets using these APIs to be skipped with
"CRD not installed" errors.

Now correctly identifies built-in API groups and returns empty
RequiredCRD for them, allowing Deployment, ClusterRole,
ClusterRoleBinding, and other core resources to be deployed.

This fixes deployment of network-resources-injector which uses:
- apps/v1 (Deployment)
- rbac.authorization.k8s.io/v1 (ClusterRole, ClusterRoleBinding)

Signed-off-by: Nir Dothan <ndothan@redhat.com>

Assisted by: Claude Sonnet 4.5 <noreply@anthropic.com>
@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from a6217ea to ab7da58 Compare May 17, 2026 16:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

❌ Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from ab7da58 to 7b9ad47 Compare May 17, 2026 19:44
nirdothan added 3 commits May 17, 2026 23:16
@nirdothan
rbac: Add transitive permissions for managed ClusterRoles
32c8093 
When virt-platform-autopilot creates ClusterRoles for managed
components, it must hold all permissions those ClusterRoles grant
(Kubernetes privilege escalation prevention).

Adds static RBAC rules for:
- Workload resources (pods, daemonsets, replicasets, statefulsets,
  replicationcontrollers, network-attachment-definitions)
- Webhook registration (mutatingwebhookconfigurations,
  validatingwebhookconfigurations)
- Configmap read access
- Secrets and services delete permission

These permissions allow the operator to create ClusterRoles that
grant these same permissions to managed components.

Updates ClusterRole templates to use explicit verbs (create, delete,
get, list, patch, update, watch) instead of wildcards, matching what
the operator can grant.

Signed-off-by: Nir Dothan <ndothan@redhat.com>

Assisted by: Claude Sonnet 4.5 <noreply@anthropic.com>

tests: Add comprehensive unit tests for RBAC generation

Adds tests for:
- StaticRules: Verify static infrastructure rules
- parseGVK: Test GVK parsing logic
- pluralize: Test resource name pluralization
- generateDynamicRules: Test dynamic rule generation with various scenarios
- AllRules: Integration test with test filesystem

Also fixes trailing whitespace in registry_test.go
@nirdothan
assets: Fix IsManagedCRD to reject empty string
6ace6ab 
IsManagedCRD is used by the CRD event handler to filter which CRD
install/removal events should trigger reconciliation. An empty string
is not a valid CRD name, so it should always return false.

The bug was exposed when assets with core API types (apiVersion: v1)
were added - these have empty RequiredCRD fields, causing IsManagedCRD("")
to incorrectly return true by matching the empty fields.

Signed-off-by: Nir Dothan <ndothan@redhat.com>

Assisted by: Claude Sonnet 4.5 <noreply@anthropic.com>
@nirdothan
chore: update CRDs and RBAC
e397d58 
make update-crds
make generate-rbac

Signed-off-by: Nir Dothan <ndothan@redhat.com>
@nirdothan nirdothan force-pushed the fix-core-api-group-detection branch from 7b9ad47 to e397d58 Compare May 17, 2026 20:17
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 20, 2026

PR needs rebase.

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.

@tiraboschi tiraboschi mentioned this pull request May 25, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiraboschi tiraboschi Awaiting requested review from tiraboschi

Assignees

No one assigned

Labels

needs-ok-to-test needs-rebase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nirdothan