Implement Admin-operator for auth-admin team - #265
Conversation
|
Warning Review limit reached
More reviews will be available in 40 minutes and 15 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughIntroduces a complete Kubernetes operator ( ChangesAdmin Operator
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Makefile
participant Docker
participant Kubernetes
participant AdminAppProfileReconciler
Developer->>Makefile: make run-admin-operator
Makefile->>Docker: docker build admin-operator:latest
Makefile->>Kubernetes: kubectl apply crd.yaml, rbac.yaml, operator-deployment.yaml
Kubernetes->>AdminAppProfileReconciler: Start manager / watch AdminAppProfile
Developer->>Makefile: make apply-cr
Makefile->>Kubernetes: kubectl apply example-cr.yaml
Kubernetes->>AdminAppProfileReconciler: Reconcile(AdminAppProfile)
AdminAppProfileReconciler->>Kubernetes: Get Deployment (admin-auth-api)
AdminAppProfileReconciler->>Kubernetes: Patch spec.replicas
AdminAppProfileReconciler->>Kubernetes: Update AdminAppProfile status conditions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
Makefile (1)
14-14: ⚡ Quick winUnused
OPERATOR_IMAGEvariable.The
OPERATOR_IMAGEvariable is defined on line 14 but not used in therun-admin-operatortarget; instead, the target hardcodesadmin-operator:latest. This suggests either the variable was intended but not connected, or it's a remnant from earlier design.If the intention is to build locally for development, clarify this assumption. If the operator should eventually use a registry-qualified image, align the variable usage across all operator-related targets.
Also applies to: 223-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` at line 14, The OPERATOR_IMAGE variable is defined but not used in the run-admin-operator target, which hardcodes the image reference as admin-operator:latest instead. Update the run-admin-operator target to use the OPERATOR_IMAGE variable reference. Also check line 223 and any other operator-related targets to ensure consistency in using the OPERATOR_IMAGE variable throughout the Makefile rather than hardcoding image references. If OPERATOR_IMAGE is not needed elsewhere, document the purpose of this variable or consider removing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/Dockerfile.admin-operator`:
- Line 10: The RUN command in the Dockerfile is hardcoding GOARCH=arm64, which
restricts the binary to only run on arm64 architecture nodes and breaks
deployments on other architectures like amd64. Remove the hardcoded GOARCH=arm64
parameter from the go build command in the RUN instruction that builds the
manager binary, and instead allow the build system to use the appropriate target
architecture automatically. This will enable the Dockerfile to support
multi-platform builds and work correctly on different node architectures.
In `@deploy/k8s/operators/admin-auth/operator-deployment.yaml`:
- Line 24: Replace the image reference in the admin-auth operator deployment
from the `:latest` tag to use an immutable image reference. Either update the
image field to use a commit-based tag (e.g., `admin-operator:sha-abc1234`) that
aligns with the project's CI pipeline tagging strategy, or alternatively update
the imagePullPolicy field to `IfNotPresent` to rely on locally built images
without requiring a specific tag. If using the commit hash approach, ensure the
Makefile target that builds and pushes this operator image is updated to
generate and apply the appropriate commit-based tag during the build process.
In `@deploy/k8s/operators/admin-auth/rbac.yaml`:
- Around line 15-17: The RBAC rule for adminappprofiles resource is granting
excessive permissions that the reconciler does not actually use. The reconciler
only performs r.Get() on adminappprofiles and r.Status().Update() on the status
sub-resource. Remove the unnecessary verbs "create", "delete", "update", and
"patch" from the verbs array in the rule with apiGroups ["admin.sharebite.dev"]
and resources ["adminappprofiles", "adminappprofiles/status"], keeping only
"watch", "get", and "list" to follow the principle of least privilege.
In `@docs/k8s/admin-operator.md`:
- Line 151: The documentation at line 151 in the developer guide references an
incorrect Makefile target name `make run-auth-service`. Replace this with the
correct target name `make run-admin-service` to match the actual Makefile
definition and ensure the documented instructions will execute successfully.
- Line 170: The documentation in step 4 of the developer guide references an
incorrect Makefile target name. Replace `make stop-auth-service` with `make
stop-admin-service` on line 170 of the admin-operator.md file to match the
actual Makefile target that is defined in the Makefile.
- Line 110: The RBAC permissions table contains an incorrect resource name in
the row under the admin.sharebite.dev API group. Replace the resource reference
from `businessappprofiles/status` to `adminappprofiles/status` to align with the
actual custom resource definition and RBAC rules documented elsewhere. This
ensures consistency and prevents user confusion about the correct API resource
to reference.
- Line 3: The overview statement on line 3 incorrectly identifies the Deployment
being scaled as "business-api" when it should be "admin-auth-api" according to
the CRD manifest and controller implementation. Replace the incorrect Deployment
name "business-api" with the correct name "admin-auth-api" in the description
that states what Deployment the operator scales from the AdminAppProfile
resource.
In `@operators/admin-operator/controller/controller_test.go`:
- Around line 42-43: The cl.Get() calls in the test are ignoring errors by using
the blank identifier, which allows failures to go undetected and cause
misleading test failures. Replace the `_ = cl.Get(...)` statements with proper
error handling that captures the error and immediately fails the test if the Get
operation fails, such as using t.Fatalf or requiring that the error is nil.
Apply this fix to all occurrences of ignored cl.Get() errors in the test file,
including the calls around line 42-43 and 98-99.
In `@operators/admin-operator/controller/controller.go`:
- Around line 102-105: The SetupWithManager method in the
AdminAppProfileReconciler currently only watches for AdminAppProfile changes,
but does not track changes to the Deployment resource it manages. This allows
manual scaling of the Deployment to persist without reconciliation until the
next AdminAppProfile event. Add `.Owns(&appsv1.Deployment{})` to the controller
setup chain between the `.For(&adminv1alpha1.AdminAppProfile{})` call and the
`.Complete(r)` call to ensure the controller reconciles whenever the managed
Deployment is modified, maintaining the desired replica count promptly.
---
Nitpick comments:
In `@Makefile`:
- Line 14: The OPERATOR_IMAGE variable is defined but not used in the
run-admin-operator target, which hardcodes the image reference as
admin-operator:latest instead. Update the run-admin-operator target to use the
OPERATOR_IMAGE variable reference. Also check line 223 and any other
operator-related targets to ensure consistency in using the OPERATOR_IMAGE
variable throughout the Makefile rather than hardcoding image references. If
OPERATOR_IMAGE is not needed elsewhere, document the purpose of this variable or
consider removing it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ae4566e-23dc-44bf-a4dd-e79cc46a8921
📒 Files selected for processing (14)
.gitignoreMakefilebuild/Dockerfile.admin-operatorcmd/admin-operator/main.godeploy/k8s/operators/admin-auth/crd.yamldeploy/k8s/operators/admin-auth/example-cr.yamldeploy/k8s/operators/admin-auth/operator-deployment.yamldeploy/k8s/operators/admin-auth/rbac.yamldocs/k8s/admin-operator.mdoperators/admin-operator/api/v1alpha1/adminappprofile_types.gooperators/admin-operator/api/v1alpha1/deep_copy.gooperators/admin-operator/api/v1alpha1/groupversion_info.gooperators/admin-operator/controller/controller.gooperators/admin-operator/controller/controller_test.go
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests