docs: add Unleash feature flags documentation and deployment scripts#653
docs: add Unleash feature flags documentation and deployment scripts#653maskarb wants to merge 2 commits intoambient-code:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
Claude Code ReviewSummaryThis PR adds Unleash feature flag infrastructure: deployment manifests for Unleash + PostgreSQL, updated backend/frontend deployments to consume the new credentials secret, Makefile targets, and documentation. The overall approach is sound, but there is one blocker that must be fixed before merge and several other issues worth addressing. Issues by Severity🚫 Blocker Issues[B1] Production kustomization pointing to personal registry ( The PR accidentally replaces - newName: quay.io/ambient_code/vteam_backend
+ newName: quay.io/mskarbek/vteam_backend # ← personal registry, must revertFix: Revert all changes to 🔴 Critical Issues[C1] Frontend env vars missing The backend correctly marks all 6 Unleash env vars as # frontend-deployment.yaml — CURRENT (broken if secret absent)
- name: UNLEASH_URL
valueFrom:
secretKeyRef:
name: unleash-credentials
key: unleash-url
# optional: true ← MISSING
# REQUIRED fix
- name: UNLEASH_URL
valueFrom:
secretKeyRef:
name: unleash-credentials
key: unleash-url
optional: trueFix: Add [C2] Both Deployments missing Per CLAUDE.md security standards and the platform's own # Required on each container spec
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]Fix: Add 🟡 Major Issues[M1] PostgreSQL image from
Additionally, the RHEL PostgreSQL image uses Consider using [M2] Unleash added as mandatory base resource — all deployments now include it Adding Consider either:
[M3] The 🔵 Minor Issues[m1] Default credentials hardcoded in Makefile output @echo " Login: admin / unleash4all"While [m2] Hardcoded
[m3] Frontend docs don't align with project's React Query patterns
Positive Highlights
RecommendationsPrioritized action items before merge:
🔍 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. |
maskarb
force-pushed
the
rhoai-47012-docs-unleash-feature-flags
branch
from
b14e089 to
61bd219
Compare
Claude Code ReviewSummaryThis PR adds Unleash feature flag integration: deployment manifests, Makefile targets, and documentation. The documentation is well-structured and the deployment approach follows existing patterns. However, there are critical issues that will break existing deployments and the documentation references code that does not yet exist in the repository. Issues by Severity🚫 Blocker Issues1. Frontend deployment will break all existing installations
The comment says "optional" but the # Unleash Feature Flags configuration (optional) ← comment says optional
- name: UNLEASH_URL
valueFrom:
secretKeyRef:
name: unleash-credentials
key: unleash-url
# ← missing: optional: trueWithout 🔴 Critical Issues2. Documentation references code that does not exist The documentation describes and links to the following files, none of which are present in the repository:
The docs tell users how to call 3. Unleash added to base kustomization without prerequisite secret
Options:
🟡 Major Issues4. PostgreSQL image/path mismatch
The manifest mixes Red Hat image conventions with Debian PostgreSQL path conventions:
The RHEL PostgreSQL image does support 5. No SecurityContext on new pods
Neither the PostgreSQL nor the Unleash containers have a securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: false
capabilities:
drop: ["ALL"]OpenShift with restricted SCC will reject pods without these settings. 6. New docs not registered in mkdocs.yml
🔵 Minor Issues7. Third-party Docker Hub image
8. Default credentials exposed in Makefile
9. Example tokens are clearly placeholder but could be used as-is
These are non-functional placeholder strings, not actual token format. The warning exists but the placeholder values may confuse users about whether they need to generate real tokens vs. just substitute values. A comment clarifying that real tokens must be generated from the Unleash UI would help. Positive Highlights
RecommendationsPrioritized action items:
🔍 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. |
maskarb
force-pushed
the
rhoai-47012-docs-unleash-feature-flags
branch
from
61bd219 to
f9304de
Compare
Claude Code ReviewSummaryPR #653 adds Unleash feature flag infrastructure to the Ambient Code Platform: new PostgreSQL and Unleash Kubernetes deployments integrated into the base kustomize manifests, environment-specific overlays (kind, local-dev, production), documentation, a standalone e2e deployment script, and Makefile convenience targets. Despite the Issues by Severity🚫 Blocker Issues1. Documentation describes unimplemented code
Fix: Either add the implementation in this PR, or clearly label those sections as "Planned API (not yet implemented)". 2. Default admin password in Makefile echo @echo " Login: admin / unleash4all"
Fix: Remove the credential hint and point to the README instead, e.g. 🔴 Critical Issues3. Version mismatch between manifests and deploy script
Fix: Align on a single version across both files. 4. Two competing deployment paths The base kustomize manifests deploy Unleash via Fix: Choose one canonical deployment approach. If the script is for one-off or legacy scenarios, document that explicitly and mark it as non-authoritative. 5. Missing SecurityContext on all new pods Per the project's established security standards, every pod spec must include: securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]Neither Fix: Add explicit SecurityContext to both Deployment pod specs. 🟡 Major Issues6. Services added to base kustomization will silently break PostgreSQL and Unleash reference Fix: Add a pre-deploy check or clearly document the required manual steps at the top of the production overlay README. A 7. No CPU/memory resource requests or limits Neither the PostgreSQL nor Unleash Deployments show resource requests/limits. Without these, the Kubernetes scheduler cannot make good placement decisions, QoS class defaults to Fix: Add conservative 8. Commit type mismatch obscures scope The PR title and base commit are prefixed 🔵 Minor Issues9. The 10. Shell variable in SQL heredoc In the SELECT 'CREATE DATABASE $db_name'
WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = '$db_name')\gexecThe variable Fix: Use psql's 11. PVC sizing guidance absent for production The base uses Positive Highlights
Recommendations (prioritized)
Review generated by Claude Code (claude-sonnet-4-6) against project standards in CLAUDE.md and 🔍 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. |
Claude Code ReviewSummaryThis PR adds an Unleash feature flag server and shared PostgreSQL deployment to the platform manifests, along with documentation and a standalone deployment script. The Kubernetes overlay structure (kind/local-dev/production) is well thought-out and the RHEL image handling shows good platform awareness. However, there are critical issues: the documentation describes SDK integration code ( Issues by Severity🚫 Blocker Issues1. Documentation references non-existent code
None of these exist in this PR or the codebase. A developer following this documentation will hit dead ends immediately. The docs should either be scoped to infrastructure-only (what this PR actually delivers), or the implementation must accompany them. 🔴 Critical Issues2. This PR simultaneously adds Unleash to
The script reads like it was written before the decision to use kustomize manifests. It should either be removed (since 3. Missing SecurityContext on all workloads CLAUDE.md explicitly requires setting # Required for both postgresql and unleash deployments
spec:
template:
spec:
securityContext:
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault
containers:
- name: postgresql # or unleash
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]Note: PostgreSQL requires write access to its data directory so 🟡 Major Issues4. Default admin credentials exposed in Makefile
Hardcoding default credentials in the Makefile trains developers to rely on them and makes it easy to accidentally deploy with these defaults. Remove this line or replace with a pointer to the credentials secret. 5.
🔵 Minor Issues6. Duplicate overlay files
7. Unleash version inconsistency
If the script is kept, align the versions. 8. The new 9. PostgreSQL readiness probe uses hardcoded username readinessProbe:
exec:
command: [pg_isready, -U, postgres]This bypasses the secret-configured username. Should use Positive Highlights
RecommendationsPriority order:
Review generated by 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. |
- Add PostgreSQL deployment with credentials secret management - Add overlays for kind, local-dev, production, and e2e environments - Use JSON patches for RHEL PostgreSQL image in production/local-dev - Move init-scripts to kind overlay (production uses init containers) - Fix kustomize deprecation warnings (migrate to unified patches syntax) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Unleash deployment with credentials secret management - Add init container patches for RHEL environments (local-dev, production) - Add documentation for feature flags setup - Add Makefile targets for Unleash port-forwarding and status - Add deploy-unleash.sh script for standalone deployment Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
maskarb
force-pushed
the
rhoai-47012-docs-unleash-feature-flags
branch
from
cdc7964 to
9c16a20
Compare
Claude Code ReviewSummaryPR #653 adds Unleash feature flag deployment infrastructure (PostgreSQL + Unleash manifests, kustomize overlays for all environments, Makefile targets) and documentation. The manifest structure is well-organized with clear environment separation. However, as a DRAFT PR there are several issues—two of which will cause deployment failures—that should be resolved before merge. Issues by SeverityBlocker Issues1. Documentation references non-existent implementation files Both doc files reference backend/frontend code that does not exist in the repository:
Critical Issues2.
Fix: add a 3. Production/local-dev missing explicit pre-deploy documentation for required secrets Both production and local-dev are missing Major Issues4. Unleash image version mismatch
5. The new 6. Line 117 uses 7. Lines 354-356: echo "Credentials used:"
echo " PostgreSQL: $POSTGRES_PASSWORD"When Minor Issues8. Duplicate init script ConfigMaps
9. Default Unleash password hardcoded in Makefile
10. The script defaults 11. This is acceptable for dev, but for production setups with TLS-enabled PostgreSQL it silently disables SSL. Consider sourcing this from the Positive Highlights
RecommendationsBefore removing DRAFT status:
Before merge:
Reviewed by Claude Code using project memory system 🔍 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. |
1 participant
Adds documentation for feature flag management with Unleash and deployment script for Kubernetes/OpenShift clusters. Updates Makefile with unleash deployment targets.