Add RBACPolicy controller for Neutron RBAC policy support - #953
Open
zunken1337 wants to merge 4 commits into
Open
zunken1337 wants to merge 4 commits into
zunken1337 wants to merge 4 commits into
Conversation
Adds a new RBACPolicy resource managing Neutron RBAC policies, closing the gap described in k-orc#950: there was previously no way to share a Network with another project (access_as_shared) or expose it as an external gateway (access_as_external) without dropping to the raw OpenStack API. Scope is deliberately limited to object_type=network for this first pass - Neutron's RBAC API also covers qos-policy and security-group, left as a natural follow-up once there's a concrete use case (see the comment on RBACPolicyResourceSpec). targetProjectID is a raw OpenStack project ID string rather than a KubernetesNameRef to an ORC Project, since Project creation itself isn't usable on every cloud (some providers gate Keystone project/domain provisioning behind their own control plane). This deliberately fails the kube-api-linter noopenstackidref check, which doesn't support //nolint suppression - flagging this transparently for discussion rather than working around it silently. Includes KUTTL e2e tests for all required scenarios (create-minimal, create-full, dependency, import, import-error, update), two behaviors of which were verified against a live OpenStack deployment rather than assumed from documentation: Neutron does not validate target_tenant against Keystone (any string is accepted), and it rejects an exact duplicate policy (same object_id+action+target_tenant) but allows differing by any one field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
…tProjectID make generate had been run on the doc comment before its final wording (the "Deliberately fails the kube-api-linter noopenstackidref check..." paragraph) was added, so the committed CRD yaml and openapi.go were stale relative to the actual Go source - caught by CI's generate job. The go lint failure (noopenstackidref on RBACPolicy.TargetProjectID) is the same known, deliberate design tension already documented in the field's doc comment and the PR description. Standard //nolint suppression doesn't work for this custom kube-api-linter plugin - confirmed it's enabled/excluded via .golangci.yml, not inline comments, same as the project's two other project-wide sub-rule disables (arrayofstruct, optionalfields). Added a narrowly-scoped exclusion (path + text match) so only this specific, documented violation is suppressed, not the rule everywhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
make generate-bundle (a separate CI step from plain make generate, and one I hadn't run - it needs libgpgme-dev, not available in the admin3 build environment used for everything else in this PR) adds an owned-CRD entry per resource kind here. CI's own failure log already contained the exact diff this would produce, so applied it directly rather than fighting the gpgme dependency locally - verified byte-for-byte identical (same before/ after blob hashes in the diff) to what CI computed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
The controller was fully implemented and unit/envtest-tested, but never registered in cmd/manager/main.go's controller list - a manual step (import + an entry in the orcOpts.Controllers slice, following the exact pattern every other controller uses) that nothing automated catches: an unused exported New() function across packages doesn't fail go build, go vet, or unit tests, since those never go through the real manager wiring. Only the e2e CI job caught it: all 6 rbacpolicy-* KUTTL scenarios timed out waiting for status, and the operator's own pod logs confirmed why - zero mentions of "rbacpolicy" anywhere while every other controller (network, volume, region, etc.) was actively reconciling throughout the same window. Confirmed this is the only such gap by checking for other per-controller registration lists (RBAC/webhook setup, metrics) - none exist; main.go's controller slice is the sole place a new kind must be manually added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
zunken1337
pushed a commit
to zunken1337/openstack-resource-controller
that referenced
this pull request
Oct 1, 2026
- dnszone_types.go: rewrite the PRIMARY-requires-email CEL rule to use has(self.email) instead of comparing against an empty string literal. Go's gofmt doc-comment formatter collapses an empty '' pair into a single curly quote, corrupting the CEL expression; avoiding the empty-string comparison (matching every other CEL rule in this repo, which already prefers has()) sidesteps it entirely. Add MinLength:=1 to Email so an explicit empty string still can't satisfy "has". - dnszone_types.go, recordset_types.go: add the kube-api-linter-required +kubebuilder:validation:items:MaxLength marker to the Masters/Records status array fields (same pattern as Subnet's DNSNameservers). - .golangci.yml: scope an exclusion for DNSZoneShare.TargetProjectID's noopenstackidref finding, same rationale and precedent as RBACPolicy.TargetProjectID (PR k-orc#953) - it's a raw OpenStack project ID by design, not a reference to an ORC Project object. - config/manifests/bases/orc.clusterserviceversion.yaml: add the missing DNSZone/DNSZoneShare/RecordSet CSV entries that `make generate-bundle` needs committed (was causing the generate workflow's git diff check to fail). - Regenerate CRD YAMLs and crd-reference.md to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
4 of 5 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
RBACPolicyresource managing Neutron RBAC policies, closing #950:there was previously no way to share a Network with another project
(
access_as_shared) or expose it as an external gateway(
access_as_external) without dropping to the raw OpenStack API.Scope is deliberately limited to
object_type=networkfor this first pass.Neutron's RBAC API also covers
qos-policyandsecurity-groupas otherpossible object types; adding those as alternatives to
networkRef(adiscriminated union, the same pattern used by
RouterInterfaceSpec'stype/subnetRef) is a natural follow-up once there's a concrete use case.Design question for maintainers:
targetProjectIDtargetProjectIDis a raw OpenStack project ID string rather than aKubernetesNameRefto an ORCProjectobject. This is a deliberate choice,not an oversight: Project creation itself isn't usable on every cloud - some
providers gate Keystone project/domain provisioning behind their own control
plane outside Keystone, so no corresponding ORC
Projectobject may everexist to reference on those clouds. Mutability also matches Neutron's own
UpdateOpts, which only allows changing the target project of an existingpolicy (confirmed via gophercloud:
networkRef/actionhave no update pathand are immutable here,
targetProjectIDis the only mutable field).This deliberately fails the
kube-api-linternoopenstackidrefcheck.I confirmed this linter doesn't support
//nolintsuppression comments(
golangci-lintreports "unknown linters in //nolint directives" and therule still fires), so I'm raising it here transparently rather than working
around it silently. Happy to switch to a
KubernetesNameRef+ separate"raw ID" escape hatch, or whatever pattern maintainers prefer, if there's an
established convention for this I'm missing.
Testing
updateResource(the only mutable-field path) coveringno-op, update, and error-propagation cases.
test/apivalidationssuite extended with immutability and enumvalidation tests for
networkRef/action, and a mutability test fortargetProjectID.create-minimal,create-full,dependency,import,import-error,update).create-fullexercisesaccess_as_external(vs.access_as_sharedincreate-minimal) since RBACPolicy has no optional fields to add on top ofthe minimal case.
against a real OpenStack deployment rather than assumed from
documentation: Neutron does not validate
target_tenantagainstKeystone (any string is accepted), and it rejects an exact duplicate
policy (same
object_id+action+target_tenant) but allows differing byany single field.
Test plan
go build ./...,go vet ./...,gofmt -l .cleango test ./...passes (only unrelatedtest/e2e, which requires alive kind cluster, fails in this environment)
make generateoutput committed (CRD, deepcopy, adapter/controller,applyconfiguration/clientset/lister/informer, mocks, API reference docs)
🤖 Generated with Claude Code