feat(RHOAIENG-50753): add LDAP user and group autocomplete for workspace sharing#805
feat(RHOAIENG-50753): add LDAP user and group autocomplete for workspace sharing#805
Conversation
This comment has been minimized.
This comment has been minimized.
mprpic
force-pushed
the
ldap-autocomplete
branch
from
a316335 to
18b453a
Compare
Ambient Code PlatformKubernetes-native AI automation platform that orchestrates agentic sessions through containerized microservices. Built with Go (backend, operator), NextJS + Shadcn (frontend), Python (runner), and Kubernetes CRDs.
Structure
Key Files
Session FlowCommandsmake build-all # Build all container images
make deploy # Deploy to cluster
make test # Run tests
make lint # Lint code
make kind-up # Start local Kind cluster
make test-e2e-local # Run E2E tests against KindPer-Component# Backend / Operator (Go)
cd components/backend && gofmt -l . && go vet ./... && golangci-lint run
cd components/operator && gofmt -l . && go vet ./... && golangci-lint run
# Frontend
cd components/frontend && npm run build # Must pass with 0 errors, 0 warnings
# Runner (Python)
cd components/runners/ambient-runner && uv venv && uv pip install -e .
# Docs
cd docs && npm run dev # http://localhost:4321Critical Context
Pre-commit HooksThe project uses the pre-commit framework to run linters locally before every commit. Configuration lives in Installmake setup-hooksWhat RunsOn every
On every
Run Manuallymake lint # All hooks, all files
pre-commit run gofmt-check --all-files # Single hook
pre-commit run --files path/to/file.go # Single fileSkip Hooksgit commit --no-verify # Skip pre-commit hooks
git push --no-verify # Skip pre-push hooksNotes
More InfoSee BOOKMARKS.md for architecture decisions, development context, code patterns, and component-specific guides. |
|
Claude Code Review SummaryThis PR adds LDAP-backed user and group autocomplete to the workspace sharing flow. The feature is well-scoped, optional-by-default (graceful degradation when unconfigured), and includes solid test coverage. The LDAP client uses proper filter escaping and input sanitization. A few issues need attention before merge: all three new handlers use the wrong variable for the auth nil check (deviating from the mandatory project pattern), and the frontend builds a custom combobox from scratch when a Shadcn equivalent exists. Issues by SeverityBlocker IssuesNone. Critical Issues1. Auth nil check uses wrong client variable in all 3 handlers Files: All three handlers discard _, k8sDyn := GetK8sClientsForRequest(c)
if k8sDyn == nil {The documented mandatory pattern across the entire codebase ( reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
c.Abort()
return
}While functionally equivalent today (both return nil together), this breaks the established convention, hides the unused Violates: Major Issues2. Custom combobox dropdown built from scratch instead of Shadcn Command File: The component renders a bare Per The Shadcn 3. New LDAP connection opened per search, no pooling File: Each search dials a fresh TCP+TLS connection and closes it immediately via 4. LDAP_GROUP_BASE_DN env var not mapped in deployment manifest File:
Minor Issues5. k8sDyn obtained but never used for K8s operations File: The dynamic client is obtained and only checked for nil, never used for actual K8s operations. Once Critical issue 1 is fixed, simplify to 6. Component exceeds 200-line guideline File:
7. Trivial JSDoc comments on self-evident functions Files: Comments like Positive Highlights
Recommendations
|
…ace sharing Integrate LDAP to provide autocomplete when sharing workspaces. The backend queries LDAP with substring matching on uid, givenName, and sn for users, and prefix matching on cn for groups. Results are cached in-memory with a 5-minute TTL. The frontend replaces the plain text input with an autocomplete dropdown that debounces queries and supports keyboard navigation. LDAP is optional and gracefully degrades to manual input when unconfigured or unreachable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Martin Prpič <mprpic@redhat.com>
mprpic
force-pushed
the
ldap-autocomplete
branch
from
18b453a to
134aa71
Compare
Claude Code ReviewSummaryPR #805 adds LDAP-backed user and group autocomplete for the workspace sharing flow. The implementation is well-structured: the Go backend adds a clean Issues by SeverityBlocker Issues1. TLS verification disabled in production manifest
# components/manifests/overlays/production/ldap-config.yaml
data:
LDAP_URL: "ldaps://ldap.corp.redhat.com"
LDAP_BASE_DN: "ou=users,dc=redhat,dc=com"
# LDAP_SKIP_TLS_VERIFY omitted — defaults to falseCritical IssuesNone. Major Issues1. LDAP query error state not surfaced in the autocomplete component
const { data: users, isLoading: isLoadingUsers, error: usersError } = useLDAPUserSearch(...)
const { data: groups, isLoading: isLoadingGroups, error: groupsError } = useLDAPGroupSearch(...)
const queryError = mode === 'user' ? usersError : groupsError;
// In the dropdown:
{queryError && (
<li className="px-3 py-2 text-sm text-destructive">
Search unavailable — enter name manually
</li>
)}2. Component exceeds 200-line limit
Minor Issues1. Unbounded cache growth in
2. Dead code:
3.
4. UTF-8 unsafe truncation in
runes := []rune(q)
if len(runes) > maxQueryLength {
q = string(runes[:maxQueryLength])
}5. Identical ConfigMaps duplicated across
Positive Highlights
Recommendations
🤖 Reviewed with Claude Code (claude-sonnet-4-6) 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
|
Disabling TLS verification is intentional for now until we switch to the authenticated LDAP server. |
1 participant
Integrate LDAP to provide autocomplete when sharing workspaces. The backend queries LDAP with substring matching on uid, givenName, and sn for users, and prefix matching on cn for groups. Results are cached in-memory with a 5-minute TTL. The frontend replaces the plain text input with an autocomplete dropdown that debounces queries and supports keyboard navigation. LDAP is optional and gracefully degrades to manual input when unconfigured or unreachable.