feat(cloudprem): v3.0.0-rc.0 #94
Conversation
Dav-14
commented
Jan 24, 2025
Dav-14
commented
- feat(cloudprem): upgrade the chart to support membership breaking changes
- feat(cloudprem): v3.0.0-rc.0
WalkthroughThis pull request involves version updates across multiple Helm charts in the project. The changes primarily focus on updating chart and application versions for Cloudprem, Console-V3, Membership, and Portal components. The version increments range from Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/cloudprem/README.md.gotmpl (1)
278-278: Fix grammar in the membership change description.The phrase “Membership components content a behavior breaking changes” can be clarified:
- Membership components content a behavior breaking changes within the RBAC module. + Membership components contain behavior-breaking changes within the RBAC module.charts/cloudprem/README.md (1)
276-276: Correct the grammar in the membership change note.The sentence can be improved for clarity:
- Membership components content a behavior breaking changes within the RBAC module. + Membership components contain behavior-breaking changes within the RBAC module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
charts/cloudprem/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/cloudprem/Chart.yamlis excluded by!**/*.yamlcharts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/membership/Chart.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
README.md(1 hunks)charts/cloudprem/README.md(2 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/README.md(1 hunks)charts/membership/README.md(1 hunks)charts/portal/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- charts/portal/README.md
- charts/console-v3/README.md
- charts/membership/README.md
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3037 characters long)
Context: ...tions, users, roles, and permissions. | 
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (2)
README.md (1)
8-8: Version bump looks correct.These incremental version changes align with the new release and the membership updates. No issues found.
Also applies to: 10-10, 13-14
charts/cloudprem/README.md (1)
2-2: Major version bump confirmation.Transitioning from
2.2.0to3.0.0-rc.0correctly communicates the breaking changes introduced in this release.
Dav-14
force-pushed
the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
2 times, most recently
from
be7503a to
5a68713
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
test/helm/suite/console_test.go (1)
58-59: LGTM! Consider documenting the configuration path changes.The changes correctly update the configuration paths to align with the new structure under
global.platform.portal.oauth.cookie. The test logic remains robust, covering both encryption key scenarios.Consider adding a comment in the test file to document the configuration path changes from
global.platform.cookietoglobal.platform.portal.oauth.cookiefor future reference.Also applies to: 63-63, 76-76, 86-86, 88-88
.local/build.sh (1)
33-33: Consider adding validation flags to helm template command.The helm template command has been added, but it could benefit from additional validation flags.
Consider adding
--validateflag to enable schema validation:- helm template "$chart" --values "$chart/values.yaml" + helm template "$chart" --values "$chart/values.yaml" --validatecharts/portal/README.md (1)
67-67: Document the console feature flags.The console feature flags lack proper documentation about their purpose and impact.
Add descriptions for the console feature flags:
-| global.platform.console.enabled | bool | `true` | | +| global.platform.console.enabled | bool | `true` | Enable/disable the console component | -| global.platform.console-v3.enabled | bool | `false` | | +| global.platform.console-v3.enabled | bool | `false` | Enable/disable the experimental console-v3 component |Also applies to: 141-142
charts/cloudprem/README.md (2)
642-642: Update path configuration documentation.The ingress path and service port configurations could benefit from more detailed descriptions:
membership.ingress.hosts[0].paths[0]: Add information about the API path prefixmembership.service.ports.grpc.port: Add description for the gRPC portAlso applies to: 660-660
716-716: Add description for portal volume mounts.The
portal.volumeMountsconfiguration lacks a description. Consider adding one to maintain consistency with other configuration values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
charts/cloudprem/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/cloudprem/Chart.yamlis excluded by!**/*.yamlcharts/cloudprem/values.schema.jsonis excluded by!**/*.jsoncharts/cloudprem/values.yamlis excluded by!**/*.yamlcharts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/console-v3/values.schema.jsonis excluded by!**/*.jsoncharts/console-v3/values.yamlis excluded by!**/*.yamlcharts/console/Chart.yamlis excluded by!**/*.yamlcharts/console/values.schema.jsonis excluded by!**/*.jsoncharts/console/values.yamlis excluded by!**/*.yamlcharts/membership/Chart.yamlis excluded by!**/*.yamlcharts/membership/templates/configmap.yamlis excluded by!**/*.yamlcharts/membership/values.schema.jsonis excluded by!**/*.jsoncharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yamlcharts/portal/values.schema.jsonis excluded by!**/*.jsoncharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (14)
.local/build.sh(1 hunks)README.md(1 hunks)charts/cloudprem/README.md(13 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/README.md(3 hunks)charts/console-v3/templates/_helpers.tpl(1 hunks)charts/console/README.md(3 hunks)charts/console/templates/_helpers.tpl(1 hunks)charts/membership/README.md(7 hunks)charts/membership/templates/_helpers.tpl(1 hunks)charts/portal/README.md(4 hunks)charts/portal/templates/_helpers.tpl(1 hunks)test/helm/suite/cloudprem_test.go(1 hunks)test/helm/suite/console_test.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (15)
test/helm/suite/cloudprem_test.go (1)
110-110: LGTM! Configuration path standardization looks good.The change correctly moves application enable/disable flags under the
global.platformnamespace, maintaining consistency with the broader configuration restructuring.charts/console-v3/templates/_helpers.tpl (3)
38-44: LGTM! Cookie configuration moved to local scope.The change improves modularity by moving cookie configuration from global platform scope to local config scope.
Line range hint
71-84: LGTM! Environment variables well organized.The environment variables are now better organized with separate includes for cookie, OAuth client, sentry, and monitoring configurations.
52-68: Verify OAuth client configuration compatibility.The new OAuth client configuration looks good, but ensure that:
- The redirect URI construction matches the expected format for the OAuth provider
- The membership API URL format is compatible with the membership service
✅ Verification successful
OAuth client configuration is compatible with existing services
The implementation follows established patterns:
- Membership API URL uses the standard
scheme://host/apiformat consistent with other services- Redirect URI construction matches existing OAuth clients with proper override support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify membership API URL format in other templates rg -l 'membership.*api' charts/ | grep -v console-v3Length of output: 288
Script:
#!/bin/bash # Check membership API URL and OAuth redirect URI patterns echo "=== Membership API URL patterns ===" rg -A 2 -B 2 'membership.*api' charts/portal/templates/_helpers.tpl charts/membership/templates/_helpers.tpl charts/console/templates/_helpers.tpl echo -e "\n=== OAuth redirect URI patterns ===" rg -A 2 -B 2 'REDIRECT_URI|redirect.*url' charts/portal/templates/_helpers.tpl charts/membership/templates/_helpers.tpl charts/console/templates/_helpers.tplLength of output: 4046
charts/console/templates/_helpers.tpl (1)
59-66: LGTM! Encryption key configuration updated correctly.The encryption key configuration now properly uses portal OAuth cookie settings while maintaining backward compatibility with
config.encryption_key.charts/portal/templates/_helpers.tpl (3)
43-58: Review cookie domain configuration logic.The cookie domain configuration now depends on console enablement status. This change could affect session management and cross-domain functionality.
Consider documenting this behavior change in the migration guide to help users understand the implications of enabling/disabling the console.
79-83: LGTM: Environment variables configuration.The environment variables configuration is well-structured and includes essential settings.
33-39: Verify cookie secret configuration.The cookie secret configuration has been simplified, but we should ensure that existing secrets are properly migrated.
✅ Verification successful
Cookie secret configuration changes are properly implemented ✅
The changes maintain backward compatibility while supporting both direct values and external secrets. The implementation aligns with existing patterns across portal, console, and console-v3 components, with proper examples in the cloudprem profiles demonstrating secret mapping (e.g., "portal-cookie-secret").
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any existing cookie secrets that might need migration rg -l "cookie.*secret" charts/ -g "values*.yaml"Length of output: 50
Script:
#!/bin/bash # List all yaml/yml files in charts directory echo "=== Yaml files in charts ===" fd -e yaml -e yml . charts/ # Search for cookie-related configurations with broader pattern echo -e "\n=== Cookie configurations ===" rg -i "cookie" charts/ -g "*.{yaml,yml,tpl}" # Check for documentation echo -e "\n=== Documentation ===" fd "README" charts/Length of output: 8886
charts/membership/templates/_helpers.tpl (1)
77-89: Review dynamic OAuth client secret configuration.The new implementation dynamically generates OAuth client secrets for each enabled service. This is a significant architectural improvement but requires careful migration.
Ensure that:
- All existing services are properly configured with OAuth clients
- The migration guide includes steps for updating service configurations
✅ Verification successful
OAuth client configuration implementation verified successfully
The implementation correctly handles OAuth client secrets for all platform services:
- Supports both direct values and external secrets
- Includes proper configuration for console, console-v3, and portal services
- Provides migration path through external secret mapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for existing OAuth client configurations that need migration rg "oauth.*client.*secret" charts/ -g "values*.yaml"Length of output: 54
Script:
#!/bin/bash # List all yaml/yml files in charts directory echo "=== All YAML files in charts ===" fd -e yaml -e yml . charts/ echo -e "\n=== Searching for OAuth configurations ===" rg -i "oauth" charts/ -g "*.{yaml,yml}" -A 5 echo -e "\n=== Searching for platform services configuration ===" rg "platform:" charts/ -g "*.{yaml,yml}" -A 10Length of output: 12654
README.md (1)
8-14: Verify version compatibility across charts.Multiple charts have been upgraded to major versions (2.0.0 and 3.0.0-rc.0). This indicates breaking changes that need careful coordination.
✅ Verification successful
Version compatibility is properly maintained across charts
The version compatibility is well-coordinated:
- Charts use flexible version constraints (X notation) for internal dependencies
- Core chart (1.1.0) is consistently referenced as "1.X" by all dependent charts
- Cloudprem's dependencies correctly align with the major versions of upgraded charts (membership, portal, console, console-v3)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for version dependencies between charts for chart in charts/*/Chart.yaml; do echo "Checking dependencies in $chart" rg "version:" "$chart" -A 5 doneLength of output: 5733
charts/cloudprem/README.md (5)
2-2: LGTM: Version badge update.The version badge has been correctly updated to reflect the new version
3.0.0-rc.0.
272-283: LGTM: Clear migration documentation.The migration section effectively communicates the breaking changes in the RBAC module:
- No configuration changes required
- Removal of dynamic permission management
- Fallback mechanism now only applies to new users
- Updates to Console-v3 and Portal to support the new RBAC module
284-287: Add breaking changes documentation.The TODO comment indicates that breaking changes documentation needs to be added. This is important for users upgrading to v3.0.0.
Would you like me to help document the breaking changes based on the information provided in the migration section and configuration updates?
378-383: LGTM: New console-v3 configuration.The new configuration values for console-v3 are well-documented with clear descriptions:
- Host configuration
- OAuth client settings
- Scopes configuration
399-406: LGTM: Portal OAuth configuration.The portal OAuth configuration has been properly documented with:
- Client ID and secret management
- Scopes configuration
- Cookie encryption settings
Dav-14
force-pushed
the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
from
5a68713 to
d0e8578
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
charts/console-v3/templates/_helpers.tpl (1)
54-54:⚠️ Potential issueUpdate redirect URI format to match portal's expectations.
The redirect URI format should include
/auth/loginto match the portal's expected format.Apply this diff to fix the redirect URI format:
- value: {{ tpl (default (printf "%s://%s" .Values.global.platform.consoleV3.scheme .Values.global.platform.consoleV3.host) .Values.config.redirect_url) $ }} + value: {{ tpl (default (printf "%s://%s/auth/login" .Values.global.platform.consoleV3.scheme .Values.global.platform.consoleV3.host) .Values.config.redirect_url) $ }}charts/console/templates/_helpers.tpl (1)
40-40:⚠️ Potential issueUpdate redirect URI format to match portal's expectations.
The redirect URI format should include
/auth/loginto match the portal's expected format.Apply this diff to fix the redirect URI format:
- value: {{ tpl (default (printf "%s://%s" .Values.global.platform.console.scheme .Values.global.platform.console.host) .Values.config.redirect_url) $ }} + value: {{ tpl (default (printf "%s://%s/auth/login" .Values.global.platform.console.scheme .Values.global.platform.console.host) .Values.config.redirect_url) $ }}charts/console/README.md (1)
52-58:⚠️ Potential issueReplace hardcoded secrets with placeholders.
Avoid exposing sensitive values in documentation.
Apply this diff to use placeholder values:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId It is not shared with console-v3 and the domain is only limited to portal app | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId It is not shared with console-v3 and the domain is only limited to portal app |charts/portal/README.md (1)
53-59:⚠️ Potential issueReplace hardcoded secrets with placeholders.
Avoid exposing sensitive values in documentation.
Apply this diff to use placeholder values:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that store authentication between console-v2 and portal |charts/console-v3/README.md (1)
47-52:⚠️ Potential issueReplace hardcoded secrets with placeholders and improve documentation.
Avoid exposing sensitive values and enhance experimental status documentation.
Apply this diff to use placeholder values and improve documentation:
-| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"changeMe1","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL | +| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"<your-client-secret>","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL - This component is in beta and its configuration may change in future releases | -| config.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId | +| config.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId |Also applies to: 59-61
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
charts/cloudprem/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/cloudprem/Chart.yamlis excluded by!**/*.yamlcharts/cloudprem/values.schema.jsonis excluded by!**/*.jsoncharts/cloudprem/values.yamlis excluded by!**/*.yamlcharts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/console-v3/values.schema.jsonis excluded by!**/*.jsoncharts/console-v3/values.yamlis excluded by!**/*.yamlcharts/console/Chart.yamlis excluded by!**/*.yamlcharts/console/values.schema.jsonis excluded by!**/*.jsoncharts/console/values.yamlis excluded by!**/*.yamlcharts/membership/Chart.yamlis excluded by!**/*.yamlcharts/membership/templates/configmap.yamlis excluded by!**/*.yamlcharts/membership/values.schema.jsonis excluded by!**/*.jsoncharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yamlcharts/portal/values.schema.jsonis excluded by!**/*.jsoncharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (14)
.local/build.sh(1 hunks)README.md(1 hunks)charts/cloudprem/README.md(13 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/README.md(3 hunks)charts/console-v3/templates/_helpers.tpl(1 hunks)charts/console/README.md(3 hunks)charts/console/templates/_helpers.tpl(1 hunks)charts/membership/README.md(7 hunks)charts/membership/templates/_helpers.tpl(1 hunks)charts/portal/README.md(4 hunks)charts/portal/templates/_helpers.tpl(1 hunks)test/helm/suite/cloudprem_test.go(1 hunks)test/helm/suite/console_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- .local/build.sh
- test/helm/suite/cloudprem_test.go
- charts/membership/templates/_helpers.tpl
- test/helm/suite/console_test.go
- charts/portal/templates/_helpers.tpl
- charts/membership/README.md
- README.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (6)
charts/console-v3/templates/_helpers.tpl (1)
Line range hint
71-84: LGTM!The environment configuration is well-structured and follows best practices.
charts/cloudprem/README.md.gotmpl (2)
274-284: 🛠️ Refactor suggestionEnhance the migration guide with clearer explanations and examples.
The migration section needs the following improvements:
- Fix grammar: "Membership service contain" → "Membership service contains"
- Clarify the fallback mechanism's changes and their implications
- Add examples of how permissions worked before vs. after
- Document the impact on existing permissions
Apply this diff to improve the migration guide:
### From v2.X.X To v3.0.0 > No configuration changes are required for this upgrade. -Membership service contain a behavior breaking changes within the RBAC module. +Membership service contains breaking changes within the RBAC module. -Before, permissions were managed dynamically on the organization and stack with a *fallback* on the organization resource. (default organization accesses and default stack accesses). Which led to a lot of confusion and inconsistency regarding the users permissions +Before this change: +- Permissions were managed dynamically on both organization and stack levels +- A *fallback* mechanism automatically granted organization-level permissions to stack resources +- Example: A user with organization-level "read" permission automatically got "read" access to all stacks -Now, the fallback has been removed from the RBAC module and is only used when a new user joins the organization. +After this change: +- The fallback mechanism is only applied when a new user joins an organization +- Existing permissions remain unchanged +- New permissions must be explicitly granted at both organization and stack levels +- Example: To grant stack access, you must explicitly set both organization and stack permissions Note: `Console-v3` (experimental) and `Portal` have been updated to manage the new RBAC module.Likely invalid or redundant comment.
286-288:⚠️ Potential issueComplete the breaking changes section.
The breaking changes section should be completed to ensure users are aware of all breaking changes in v3.0.0.
Apply this diff to document the breaking changes:
## Breaking changes -TODO: Add breaking changes +### Version 3.0.0-rc.0 + +1. RBAC Module Changes: + - Removed dynamic permission fallback from organization to stack resources + - Impact: New permissions must be explicitly granted at both levels + - Migration: Existing permissions remain unchanged + +2. Component Updates: + - Console-v3 (experimental): Updated to support new RBAC module + - Portal: Updated to support new RBAC module + - Membership: Major version upgrade with RBAC changes + +3. Deployment Impact: + - No configuration changes required + - Existing permissions are preserved + - New user onboarding workflow changesLikely invalid or redundant comment.
charts/cloudprem/README.md (3)
2-2: Version updates align with breaking changes.The version bump to 3.0.0-rc.0 correctly follows semantic versioning for the breaking changes in the RBAC module.
380-386: Verify the experimental status of Console V3.The Console V3 configuration is marked as experimental. Please ensure that:
- Users are aware of the experimental status
- The OAuth client configuration is properly secured
- The default values are safe for production use
Run this script to check the experimental status documentation:
✅ Verification successful
Console V3's experimental status is properly documented and secured
The verification confirms that:
- The experimental status is clearly documented across multiple files
- Console V3 is disabled by default
- Security measures are in place with clear indicators for changing default credentials
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for experimental status documentation echo "=== Searching for Console V3 documentation ===" rg -i "experimental|beta|console.?v3" . --type md echo -e "\n=== Checking if Console V3 is disabled by default ===" rg "console.?v3.*enabled.*false" . --type yamlLength of output: 12361
478-482: Verify the platform configuration flags.The platform configuration includes several boolean flags that control component enablement. Please ensure that:
- The default values are appropriate
- The dependencies between components are documented
- The impact of disabling components is clear
Run this script to verify the configuration:
✅ Verification successful
Platform configuration flags are properly configured.
The default values and dependencies are correctly set:
- Console V2 (stable) enabled by default
- Console V3 (experimental) disabled by default
- Portal enabled by default
- Components can be enabled/disabled independently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check platform configuration echo "=== Checking platform configuration dependencies ===" rg "global\.platform\.(console|portal|membership|consoleV3).*enabled" . --type yaml echo -e "\n=== Checking for dependency documentation ===" rg -i "dependencies|requires|prerequisite" . --type mdLength of output: 1316
Script:
#!/bin/bash # Check default values and component relationships echo "=== Checking default values in values.yaml ===" rg "platform:" charts/cloudprem/values.yaml -A 20 echo -e "\n=== Examining Chart.yaml for component relationships ===" cat charts/cloudprem/Chart.yaml echo -e "\n=== Searching for component documentation ===" fd README.md --exec grep -l -i "platform" {} \; --exec cat {}Length of output: 86984
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
charts/cloudprem/README.md (1)
Line range hint
607-614: Secure migration job configuration.The migration job configuration should follow security best practices:
- Database credentials should be managed through secrets
- Job permissions should be minimal
Consider these improvements:
- Document the required database permissions for the migration job
- Add examples of using existing secrets for database credentials
- Add security context configuration examples for the job
Example configuration:
membership: config: migration: serviceAccount: create: true annotations: eks.amazonaws.com/role-arn: "arn:aws:iam::123456789012:role/migration-job-role" securityContext: runAsNonRoot: true runAsUser: 1000 readOnlyRootFilesystem: true
♻️ Duplicate comments (4)
charts/portal/README.md (1)
53-59:⚠️ Potential issueReplace hardcoded sensitive values with secure placeholders.
Similar to the console configuration, hardcoded values for OAuth client secrets and cookie encryption keys should be replaced.
Apply this diff to use secure placeholders:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that store authentication between console-v2 and portal |charts/console-v3/README.md (1)
60-62:⚠️ Potential issueReplace hardcoded cookie encryption key.
The cookie encryption key is hardcoded in the documentation.
Apply this diff:
-| config.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId | +| config.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId |charts/membership/README.md (2)
51-54:⚠️ Potential issueReplace hardcoded OAuth client secrets.
The OAuth client configuration contains hardcoded secrets for Console V3.
Apply this diff:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |
62-65:⚠️ Potential issueReplace hardcoded OAuth client secrets.
The OAuth client configuration contains hardcoded secrets for Portal.
Apply this diff:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
charts/console-v3/values.yamlis excluded by!**/*.yamlcharts/console/values.yamlis excluded by!**/*.yamlcharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
charts/cloudprem/README.md(12 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/README.md(3 hunks)charts/console/README.md(3 hunks)charts/membership/README.md(7 hunks)charts/portal/README.md(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (9)
charts/console/README.md (2)
91-92: LGTM: Ingress path configuration updated.The ingress path configuration has been properly restructured to use an object type with explicit path and pathType fields.
52-58:⚠️ Potential issueReplace hardcoded sensitive values with secure placeholders.
The configuration contains hardcoded values for OAuth client secrets and cookie encryption keys, which poses a security risk if committed to version control.
Apply this diff to use secure placeholders:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId It is not shared with console-v3 and the domain is only limited to portal app | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId It is not shared with console-v3 and the domain is only limited to portal app |Likely invalid or redundant comment.
charts/portal/README.md (1)
93-94: LGTM: Ingress path configuration updated.The ingress path configuration has been properly restructured to use an object type with explicit path and pathType fields.
charts/console-v3/README.md (1)
48-54:⚠️ Potential issueImprove security and documentation for experimental features.
- The configuration contains hardcoded secrets
- The experimental status needs better documentation
Apply this diff:
-| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"changeMe1","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL | +| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"<your-client-secret>","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL - This component is in beta and its configuration may change in future releases |Likely invalid or redundant comment.
charts/membership/README.md (1)
197-198: LGTM: Ingress path configuration updated.The ingress path configuration has been properly restructured to use an object type with explicit path and pathType fields.
charts/cloudprem/README.md (4)
2-2: LGTM! Version updates reflect a major release.The version updates from 2.2.1 to 3.0.0-rc.0 and AppVersion from v0.36.2 to v1.0.4 are properly documented.
272-292: LGTM! Breaking changes are well documented.The migration section clearly documents:
- RBAC module changes in Membership service
- Cookie and OAuth client separation between Portal and Console v3
- Configuration key relocations
405-412: Enhance security for Portal OAuth and cookie configuration.The Portal configuration contains sensitive values that need proper security measures.
391-397: 🛠️ Refactor suggestionEnhance security for Console V3 OAuth configuration.
The Console V3 OAuth client configuration contains sensitive values that should be properly secured:
- Client ID and secret are exposed in plain text
- Default values are used for sensitive fields
Consider these security improvements:
- Remove default values for sensitive fields
- Add validation for the secret format
- Document the minimum required length for secrets
global: platform: consoleV3: oauth: client: - secret: "changeMe1" + secret: "" # Required: Minimum 32 characters existingSecret: "" secretKeys: secret: ""Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/console-v3/templates/_helpers.tpl (1)
58-64: Consider usingemptyinstead of length check.The length check
gt (len .Values.global.platform.consoleV3.oauth.client.existingSecret) 0can be simplified using theemptyfunction for better readability.Apply this diff to simplify the condition:
- {{- if gt (len .Values.global.platform.consoleV3.oauth.client.existingSecret) 0 }} + {{- if not (empty .Values.global.platform.consoleV3.oauth.client.existingSecret) }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
charts/console/values.yamlis excluded by!**/*.yamlcharts/membership/templates/configmap.yamlis excluded by!**/*.yamlcharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
charts/console-v3/templates/_helpers.tpl(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (1)
charts/console-v3/templates/_helpers.tpl (1)
75-77: Verify the default API_URL template syntax.The default API_URL uses a template syntax with
#{organizationId}and#{stackId}placeholders. Ensure this syntax is compatible with the runtime environment and these placeholders are properly replaced.Run the following script to check for any documentation or related code that explains the placeholder replacement mechanism:
✅ Verification successful
The placeholder syntax is correct and follows the standard pattern
The
#{organizationId}-#{stackId}placeholder format in the default API_URL is intentionally designed to match Formance's stack naming convention, where stacks are identified as<organization ID>-<stack ID>. This is a valid template that gets replaced at runtime with the actual organization and stack IDs to form the correct Kubernetes service DNS name.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for documentation or code related to organizationId and stackId placeholder replacement echo "Searching for placeholder documentation..." rg -l "organizationId|stackId" --type md echo "Searching for code handling these placeholders..." rg "organizationId|stackId" --type go --type js --type tsLength of output: 386
Script:
#!/bin/bash # Check README contents echo "=== Checking README files ===" for f in charts/console/README.md charts/cloudprem/README.md charts/console-v3/README.md; do echo "=== Content of $f ===" cat "$f" done echo -e "\n=== Checking for similar URL patterns in templates ===" rg -A 2 -B 2 "#{.*}.*svc" charts/ echo -e "\n=== Looking for template processing logic ===" fd -e tpl -e yaml -e yml . charts/ --exec grep -l "organizationId\|stackId" {} \;Length of output: 60629
Script:
#!/bin/bash echo "=== Checking template implementations ===" echo "=== console-v3 helper template ===" cat charts/console-v3/templates/_helpers.tpl echo -e "\n=== console helper template ===" cat charts/console/templates/_helpers.tpl echo -e "\n=== Checking for similar gateway URL patterns ===" rg -A 2 -B 2 "gateway.*svc:8080/api" charts/Length of output: 8168
| {{- define "console.v3.cookie" }} | ||
| - name: COOKIE_SECRET | ||
| {{- if or .Values.global.platform.portal.cookie.existingSecret }} | ||
| {{- if or .Values.config.cookie.existingSecret }} |
There was a problem hiding this comment.
Fix incomplete condition in cookie secret configuration.
The condition {{- if or .Values.config.cookie.existingSecret }} is incomplete and could lead to unexpected behavior. The or operator requires at least two operands.
Apply this diff to fix the condition:
- {{- if or .Values.config.cookie.existingSecret }}
+ {{- if .Values.config.cookie.existingSecret }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if or .Values.config.cookie.existingSecret }} | |
| {{- if .Values.config.cookie.existingSecret }} |
Dav-14
force-pushed
the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
from
57bd338 to
8a2b706
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
charts/cloudprem/README.md (1)
405-412: 🛠️ Refactor suggestionEnhance security documentation for OAuth and cookie configurations.
The configuration documentation should emphasize security best practices.
🧹 Nitpick comments (2)
charts/console-v3/templates/_helpers.tpl (1)
52-68: Enhance security for OAuth client configuration.The OAuth client configuration is correctly structured but could benefit from security improvements:
- Consider using
existingSecretfor sensitive values- Ensure proper secret rotation mechanisms are in place
- Validate client ID/secret length and complexity
Example of using existing secrets:
global: platform: consoleV3: oauth: client: existingSecret: "console-v3-oauth-secret" secretKeys: secret: "oauth-client-secret"charts/cloudprem/README.md (1)
607-607: Fix documentation formatting.The migration annotations documentation contains a formatting issue.
Apply this fix:
-| membership.config.migration.annotations | object | `{}` | Membership job migration annotations Argo CD translate `pre-install,pre-upgrade` to: argocd.argoproj.io/hook: PreSync | +| membership.config.migration.annotations | object | `{}` | Membership job migration annotations. Argo CD translates `pre-install,pre-upgrade` to `argocd.argoproj.io/hook: PreSync` |🧰 Tools
🪛 LanguageTool
[typographical] ~607-~607: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translatepre-install,pre-upgradeto: argocd.argoproj.io/hook: PreSync | | me...(RP_COLON)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
charts/console/values.yamlis excluded by!**/*.yamlcharts/membership/templates/configmap.yamlis excluded by!**/*.yamlcharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
charts/cloudprem/README.md(12 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/templates/_helpers.tpl(1 hunks)charts/console/README.md(3 hunks)charts/portal/README.md(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/console/README.md
🧰 Additional context used
🪛 LanguageTool
charts/cloudprem/README.md
[grammar] ~401-~401: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[typographical] ~607-~607: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | me...
(RP_COLON)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (5)
charts/console-v3/templates/_helpers.tpl (3)
38-38: Fix incomplete condition in cookie secret configuration.The condition
{{- if or .Values.config.cookie.existingSecret }}is incomplete and could lead to unexpected behavior. Theoroperator requires at least two operands.Apply this diff to fix the condition:
- {{- if or .Values.config.cookie.existingSecret }} + {{- if .Values.config.cookie.existingSecret }}
49-49: LGTM: Cookie domain configuration aligns with breaking changes.The cookie domain is now correctly set on the app domain using the
tplfunction, which aligns with the documented breaking changes.
Line range hint
71-85: LGTM: Environment configuration is well-structured.The environment configuration correctly includes:
- Basic environment variables (NODE_ENV, API_URL, PORTAL_UI)
- Cookie configuration
- OAuth client configuration
- Monitoring configuration
- Additional environment variables support
charts/portal/README.md (1)
53-59: 🛠️ Refactor suggestionReplace sensitive default values with placeholders.
The documentation includes default values for sensitive configuration:
- OAuth client secret: "changeMe1"
- Cookie encryption key: "changeMe00"
Replace these with placeholders to avoid potential security risks.
Apply this diff:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that store authentication between console-v2 and portal |Likely invalid or redundant comment.
charts/cloudprem/README.md (1)
2-2: Version badge update looks good.The version badge has been correctly updated to reflect the new version 3.0.0-rc.0.
| ## OAuth clients and cookies | ||
|
|
||
| Portal and Console v3 are no longer sharing Oauth clients and cookies. The cookie domain is now set on the app domain. Enabling `console` will set the domain on the parent domain. See #breaking-changes for config changes. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify OAuth clients and cookies configuration.
The documentation should explain the impact of cookie domain changes and provide configuration examples.
Add configuration examples:
Portal and Console v3 are no longer sharing Oauth clients and cookies. The cookie domain is now set on the app domain. Enabling `console` will set the domain on the parent domain. See #breaking-changes for config changes.
+
+### Configuration Examples
+
+```yaml
+# Portal configuration
+global:
+ platform:
+ portal:
+ oauth:
+ cookie:
+ domain: "portal.example.com"
+
+# Console v3 configuration
+console-v3:
+ config:
+ cookie:
+ domain: "console.v3.example.com"
+```| ## Breaking changes | ||
|
|
||
| > The structure does not change | ||
|
|
||
| - `.global.platform.cookie` has been moved to `.global.platform.portal.oauth.cookie` | ||
| - `.global.platform.membership.oauthClient` has been moved to `.global.platform.portal.oauth.client` for console backward compatibility but can be different when using console-v3. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve breaking changes documentation.
The breaking changes section needs more context and examples.
Add migration examples:
## Breaking changes
> The structure does not change
- `.global.platform.cookie` has been moved to `.global.platform.portal.oauth.cookie`
- `.global.platform.membership.oauthClient` has been moved to `.global.platform.portal.oauth.client` for console backward compatibility but can be different when using console-v3.
+
+### Migration Examples
+
+Before:
+```yaml
+global:
+ platform:
+ cookie:
+ encryptionKey: "key123"
+ membership:
+ oauthClient:
+ id: "client123"
+```
+
+After:
+```yaml
+global:
+ platform:
+ portal:
+ oauth:
+ cookie:
+ encryptionKey: "key123"
+ client:
+ id: "client123"
+```
charts/cloudprem/README.md
Outdated
| | global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"changeMe1","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL | | ||
| | global.platform.consoleV3.host | string | `"console.v3.{{ .Values.global.serviceHost }}"` | is the host for the console | | ||
| | global.platform.consoleV3.oauth.client.existingSecret | string | `""` | is the name of the secret | | ||
| | global.platform.consoleV3.oauth.client.id | string | `"platform"` | is the id of the client | | ||
| | global.platform.consoleV3.oauth.client.scopes | list | `["supertoken","accesses","remember_me","keep_refresh_token"]` | is the name of the secret | | ||
| | global.platform.consoleV3.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | | ||
| | global.platform.consoleV3.oauth.client.secretKeys | object | `{"secret":""}` | is the key contained within the secret | | ||
| | global.platform.consoleV3.scheme | string | `"https"` | is the scheme for the console | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document experimental status of Console V3.
The Console V3 configuration is marked as experimental but lacks warning notes.
Add warning notes:
| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"platform","secret":"changeMe1","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL |
+| global.platform.consoleV3 | object | ... | Console V3: EXPERIMENTAL. Warning: This feature is not production-ready. Configuration and behavior may change without notice. |Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/membership/README.md (1)
62-65:⚠️ Potential issueReplace hardcoded OAuth client secrets.
Similar to other components, the OAuth client configuration contains hardcoded secrets.
Apply this diff to use placeholder values:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |
🧹 Nitpick comments (2)
charts/portal/README.md (1)
57-57: Improve cookie encryption key documentation.Add information about the required format and length of the encryption key.
Update the description to include:
- Required key length
- Supported formats
- Security implications
charts/console-v3/README.md (1)
Line range hint
391-391: Enhance experimental status warning.The experimental status of Console V3 should be more prominent in the documentation.
Update the description to:
-| global.platform.consoleV3 | object | ... | Console V3: EXPERIMENTAL | +| global.platform.consoleV3 | object | ... | Console V3: EXPERIMENTAL - Warning: This feature is not production-ready. Configuration and behavior may change without notice. |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
charts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
charts/cloudprem/README.md(12 hunks)charts/console-v3/README.md(3 hunks)charts/console-v3/templates/_helpers.tpl(1 hunks)charts/membership/README.md(7 hunks)charts/portal/README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
charts/cloudprem/README.md
[grammar] ~401-~401: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[typographical] ~607-~607: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | me...
(RP_COLON)
charts/membership/README.md
[grammar] ~57-~57: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ....serviceHost }}"` | is the host for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~58-~58: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~59-~59: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ... string | "https" | is the scheme the relying party | | global.platform.membership.sc...
(RELY_ON)
[typographical] ~158-~158: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | co...
(RP_COLON)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (8)
charts/console-v3/templates/_helpers.tpl (4)
47-49: LGTM: Cookie configuration changes look good.The changes properly isolate the console-v3 cookie configuration:
- Cookie name is now specific to console-v3
- Cookie domain is correctly set to the console-v3 host
Line range hint
71-85: LGTM: Environment configuration is comprehensive.The configuration properly includes all necessary components with appropriate templating.
38-38:⚠️ Potential issueFix incomplete condition in cookie configuration.
The condition
{{- if or .Values.config.cookie.existingSecret }}is incomplete and could lead to unexpected behavior. Theoroperator requires at least two operands.Apply this diff to fix the condition:
- {{- if or .Values.config.cookie.existingSecret }} + {{- if .Values.config.cookie.existingSecret }}Likely invalid or redundant comment.
52-68: LGTM: OAuth client configuration is well-structured.The implementation properly handles:
- Secure credential management with optional secret support
- Dynamic redirect URI construction
- Proper API URL configuration
Run this script to verify the OAuth configuration:
✅ Verification successful
✓ OAuth client configuration is correctly isolated and consistent
The console-v3 OAuth client implementation is properly separated from other components' configurations, maintaining backward compatibility while allowing independent OAuth client settings. The configuration structure and secret handling are consistent across all relevant files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify OAuth client configuration consistency across files # Test: Check if the client ID matches in templates and values rg -A 2 "oauth.*client.*id.*console-v3"Length of output: 2442
Script:
#!/bin/bash # Description: Check for any other OAuth-related configurations that might affect this implementation rg -g '!*.md' -A 2 'oauth.*client' charts/Length of output: 7117
charts/portal/README.md (1)
53-59:⚠️ Potential issueReplace hardcoded secrets with secure defaults.
The configuration contains hardcoded sensitive values that should not be exposed in documentation:
- OAuth client secrets
- Cookie encryption keys
Apply this diff to use placeholder values:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie that store authentication between console-v2 and portal |Likely invalid or redundant comment.
charts/console-v3/README.md (1)
Line range hint
391-397: Replace hardcoded secrets with secure defaults.Similar to the portal configuration, the console-v3 configuration contains hardcoded sensitive values.
Apply this diff to use placeholder values:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |charts/membership/README.md (1)
51-54:⚠️ Potential issueRemove hardcoded admin credentials from documentation.
The Dex configuration contains hardcoded admin credentials including the password hash. This poses a security risk.
Apply this diff to use placeholder values:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |Likely invalid or redundant comment.
charts/cloudprem/README.md (1)
272-297: LGTM: Breaking changes and migration guide are well documented.The documentation clearly explains:
- RBAC module behavior changes
- OAuth client and cookie management changes
- Configuration structure changes
- Migration steps
Dav-14
force-pushed
the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
from
64b83ef to
c791f2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
charts/membership/README.md (1)
62-65:⚠️ Potential issueRemove hardcoded OAuth client secrets.
The Portal OAuth client configuration contains hardcoded secrets.
Apply this diff to improve security:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
charts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
README.md(1 hunks)charts/cloudprem/README.md(12 hunks)charts/console-v3/README.md(3 hunks)charts/membership/README.md(7 hunks)charts/portal/README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
charts/cloudprem/README.md
[grammar] ~401-~401: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[typographical] ~607-~607: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | me...
(RP_COLON)
README.md
[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3014 characters long)
Context: ...tions, users, roles, and permissions. |
(EN_EXCESSIVE_EXCLAMATION)
charts/membership/README.md
[grammar] ~57-~57: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ....serviceHost }}"` | is the host for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~58-~58: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~59-~59: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ... string | "https" | is the scheme the relying party | | global.platform.membership.sc...
(RELY_ON)
[typographical] ~158-~158: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | co...
(RP_COLON)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (4)
README.md (1)
8-14: Version updates indicate breaking changes.Multiple components have received major version updates, signaling breaking changes:
- Cloudprem: 3.0.0-rc.0 (pre-release)
- Console, Console-V3, Membership, Portal: 2.0.0
Please ensure that:
- Release notes document all breaking changes
- Migration guides are available
- Users are notified about the pre-release status of Cloudprem
🧰 Tools
🪛 LanguageTool
[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3014 characters long)
Context: ...tions, users, roles, and permissions. || | Portal | 2.0.0 |9bc0913c2e5eecf5713a68312e43e7a80b4c61e5 | Formance Portal |
| | Regions | 2.8.1 |latest | Formance Private Regions Helm Chart |
| | Stargate | 0.6.0 |latest | Formance Stargate gRPC Gateway | [
charts/console-v3/README.md (2)
60-62:⚠️ Potential issueSecure cookie configuration.
The cookie configuration contains a hardcoded encryption key.
Apply this diff to improve security:
-| config.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId | +| config.cookie.encryptionKey | string | `"<your-encryption-key>"` | Cookie encryption key (min 32 chars, high entropy) used to encrypt authentication state between platform services |Likely invalid or redundant comment.
48-53:⚠️ Potential issueEnhance security and documentation for Console V3.
The configuration contains security and documentation issues:
- Hardcoded secrets
- Insufficient warning about experimental status
Apply this diff to improve security and documentation:
-| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"console-v3","secret":"changeMe2","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL | +| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"console-v3","secret":"<your-client-secret>","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL - Warning: This feature is not production-ready. Configuration and behavior may change without notice. | -| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) |Likely invalid or redundant comment.
charts/membership/README.md (1)
51-54:⚠️ Potential issueRemove hardcoded OAuth client secrets.
The OAuth client configuration contains hardcoded secrets.
Apply this diff to improve security:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) |Likely invalid or redundant comment.
| | global.platform.portal.oauth.client.existingSecret | string | `""` | is the name of the secret | | ||
| | global.platform.portal.oauth.client.id | string | `"portal"` | is the id of the client | | ||
| | global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | | ||
| | global.platform.portal.oauth.client.secretKeys | object | `{"secret":""}` | is the key contained within the secret | | ||
| | global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | | ||
| | global.platform.portal.oauth.cookie.existingSecret | string | `""` | is the name of the secret | | ||
| | global.platform.portal.oauth.cookie.secretKeys | object | `{"encryptionKey":""}` | is the key contained within the secret | |
There was a problem hiding this comment.
Replace hardcoded secrets with secure values.
The configuration contains hardcoded values for sensitive data:
- OAuth client secret
- Cookie encryption key
Apply this diff to use placeholder values and document security requirements:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client |
+| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) |
-| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal |
+| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | Cookie encryption key (min 32 chars, high entropy) |Consider using existingSecret instead of inline values:
global:
platform:
portal:
oauth:
client:
existingSecret: "portal-oauth-secret"
secretKeys:
secret: "oauth-client-secret"
cookie:
existingSecret: "portal-cookie-secret"
secretKeys:
encryptionKey: "cookie-encryption-key"| ### From v2.X.X To v3.0.0 | ||
|
|
||
| ## RBAC | ||
|
|
||
| Membership service contains a behavior-breaking change within the RBAC module. | ||
|
|
||
| Before, permissions were managed dynamically on the organization and stack with a *fallback* on the organization resource. (default organization accesses and default stack accesses). Which led to a lot of confusion and inconsistency regarding the user's permissions | ||
|
|
||
| The fallback has been removed from the RBAC module and is only used when a new user joins the organization. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance RBAC migration documentation.
The RBAC breaking changes are significant and need more detailed documentation.
Add the following details to the migration guide:
- Examples of how permissions worked before vs. after
- Steps to audit existing permissions
- Required actions for administrators
- Potential impact on existing integrations
Would you like me to generate a detailed migration guide with examples?
| ## OAuth clients and cookies | ||
|
|
||
| Portal and Console v3 are no longer sharing Oauth clients and cookies. The cookie domain is now set on the app domain. Enabling `console` will set the domain on the parent domain. See #breaking-changes for config changes. | ||
|
|
||
| ## Breaking changes | ||
|
|
||
| > The structure does not change | ||
| - `.global.platform.cookie` has been moved to `.global.platform.portal.oauth.cookie` | ||
| - `.global.platform.membership.oauthClient` has been moved to `.global.platform.portal.oauth.client` for console backward compatibility but can be different when using console-v3. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document OAuth and cookie changes.
The OAuth and cookie changes need more detailed documentation.
Add the following details:
- Steps to migrate existing OAuth clients
- Cookie domain changes and their impact
- Required actions for administrators
- Backward compatibility notes
Would you like me to generate detailed migration steps?
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
…cording to cookie and oauth spec
Bumped versions for Cloudprem, Console, Console-V3, Membership, and Portal to reflect the latest changes. This ensures version consistency and accurate documentation for these Helm charts.
flemzord
force-pushed
the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
from
cd9ebef to
2d2c7a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (13)
charts/console-v3/templates/_helpers.tpl (1)
38-38:⚠️ Potential issueFix incomplete
oroperator in condition.The condition is using an incomplete
oroperator which requires at least two operands.Apply this diff to fix the condition:
- {{- if or .Values.config.cookie.existingSecret }} + {{- if .Values.config.cookie.existingSecret }}charts/portal/templates/_helpers.tpl (1)
61-77:⚠️ Potential issueValidate OAuth client configuration.
The OAuth client configuration now uses portal-specific settings instead of membership API settings. This is a breaking change that requires careful migration.
Please ensure that:
- All existing OAuth client secrets are migrated to the new structure
- The migration guide includes steps for updating OAuth client configurations
- Add validation for required OAuth configuration values
charts/console/README.md (1)
52-58:⚠️ Potential issueReplace hardcoded secrets with secure values.
The configuration contains hardcoded values for sensitive data:
- OAuth client secret
- Cookie encryption key
Apply this diff to use placeholder values and document security requirements:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that share authentication between platform services (console, portal, ...),is used to store the current state organizationId-stackId It is not shared with console-v3 and the domain is only limited to portal app | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | Cookie encryption key (min 32 chars, high entropy) |charts/portal/README.md (1)
53-59:⚠️ Potential issueReplace hardcoded secrets with secure values.
Similar to the console configuration, default values for OAuth client secrets and cookie encryption keys should not be exposed in documentation.
Apply this diff to use placeholder values and document security requirements:
-| global.platform.portal.oauth.client.secret | string | `"changeMe1"` | is the secret of the client | +| global.platform.portal.oauth.client.secret | string | `"<your-client-secret>"` | Client secret (min 32 chars, high entropy) | -| global.platform.portal.oauth.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie that store authentication between console-v2 and portal | +| global.platform.portal.oauth.cookie.encryptionKey | string | `"<your-encryption-key>"` | Cookie encryption key (min 32 chars, high entropy) |charts/console-v3/README.md (2)
48-53:⚠️ Potential issueAvoid hardcoding sensitive values in OAuth configuration.
The OAuth client configuration contains hardcoded secrets which pose a security risk.
Apply this diff to use placeholder values:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |
60-62:⚠️ Potential issueAvoid hardcoding sensitive values in cookie configuration.
The cookie encryption key is hardcoded in the documentation.
Apply this diff to use placeholder values:
-| config.cookie.encryptionKey | string | `"changeMe00"` | is used to encrypt a cookie value | +| config.cookie.encryptionKey | string | `"<your-encryption-key>"` | is used to encrypt a cookie value |charts/cloudprem/README.md.gotmpl (2)
274-282: 🛠️ Refactor suggestionEnhance RBAC migration documentation.
The RBAC breaking changes need more detailed documentation to help users migrate successfully.
Would you like me to generate a detailed migration guide with examples that includes:
- Examples of how permissions worked before vs. after
- Steps to audit existing permissions
- Required actions for administrators
- Potential impact on existing integrations
282-285: 🛠️ Refactor suggestionEnhance OAuth and cookie changes documentation.
The OAuth and cookie changes need more detailed documentation to guide users through the migration.
Would you like me to generate detailed migration steps that include:
- Steps to migrate existing OAuth clients
- Cookie domain changes and their impact
- Required actions for administrators
- Backward compatibility notes
charts/membership/README.md (2)
51-54:⚠️ Potential issueAvoid hardcoding OAuth client secrets.
The OAuth client configuration contains hardcoded secrets for both Console V3 and Portal.
Apply this diff to use placeholder values:
-| global.platform.consoleV3.oauth.client.secret | string | `"changeMe2"` | is the secret of the client | +| global.platform.consoleV3.oauth.client.secret | string | `"<your-client-secret>"` | is the secret of the client |
102-107:⚠️ Potential issueRemove hardcoded admin credentials from documentation.
The Dex configuration contains hardcoded admin credentials including the password hash. This poses a security risk.
Apply this diff to use placeholder values:
-| dex.configOverrides.staticPasswords[0] | object | `{"email":"[email protected]","hash":"$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W","userID":"08a8684b-db88-4b73-90a9-3cd1661f5466","username":"admin"}` | static passwords email | +| dex.configOverrides.staticPasswords[0] | object | `{"email":"<admin-email>","hash":"<bcrypt-hash>","userID":"<uuid>","username":"<admin-username>"}` | static passwords email |charts/cloudprem/README.md (3)
274-281: 🛠️ Refactor suggestionEnhance RBAC migration documentation.
The RBAC breaking changes section needs more details to help users understand and migrate successfully:
- Examples showing how permissions worked before vs after
- Clear migration steps for existing permissions
- Impact assessment on existing organizations and stacks
- Recommendations for permission auditing
Would you like me to generate a detailed migration guide with examples?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~275-~275: You might be missing the article “the” here.
Context: ...on ### From v2.X.X To v3.0.0 ## RBAC Membership service contains a behavior-breaking ch...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
282-285: 🛠️ Refactor suggestionDocument OAuth and cookie changes.
The OAuth and cookie changes section needs more details:
- Steps to migrate existing OAuth clients
- Impact of cookie domain changes
- Backward compatibility notes
- Required actions for administrators
Would you like me to generate detailed migration steps?
394-394: 🛠️ Refactor suggestionDocument experimental status of Console V3.
The Console V3 configuration is marked as experimental but lacks warning notes. Add warning notes to help users understand the implications:
-| global.platform.consoleV3 | object | `{"host":"console.v3.{{ .Values.global.serviceHost }}","oauth":{"client":{"existingSecret":"","id":"console-v3","secret":"changeMe2","secretKeys":{"secret":""}}},"scheme":"https"}` | Console V3: EXPERIMENTAL | +| global.platform.consoleV3 | object | ... | Console V3: EXPERIMENTAL. Warning: This feature is not production-ready. Configuration and behavior may change without notice. |Also applies to: 396-400
🧹 Nitpick comments (1)
charts/cloudprem/README.md (1)
286-295: Enhance breaking changes documentation.Consider adding:
- Examples showing old vs new configuration for each change
- Step-by-step migration guide for each configuration move
- Validation steps to ensure successful migration
Example format:
# Before global: platform: cookie: ... # After global: platform: portal: oauth: cookie: ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
charts/cloudprem/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/cloudprem/Chart.yamlis excluded by!**/*.yamlcharts/cloudprem/values.schema.jsonis excluded by!**/*.jsoncharts/cloudprem/values.yamlis excluded by!**/*.yamlcharts/console-v3/Chart.yamlis excluded by!**/*.yamlcharts/console-v3/values.schema.jsonis excluded by!**/*.jsoncharts/console-v3/values.yamlis excluded by!**/*.yamlcharts/console/Chart.yamlis excluded by!**/*.yamlcharts/console/values.schema.jsonis excluded by!**/*.jsoncharts/console/values.yamlis excluded by!**/*.yamlcharts/membership/Chart.yamlis excluded by!**/*.yamlcharts/membership/templates/configmap.yamlis excluded by!**/*.yamlcharts/membership/values.schema.jsonis excluded by!**/*.jsoncharts/membership/values.yamlis excluded by!**/*.yamlcharts/portal/Chart.yamlis excluded by!**/*.yamlcharts/portal/values.schema.jsonis excluded by!**/*.jsoncharts/portal/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (14)
.local/build.sh(1 hunks)README.md(1 hunks)charts/cloudprem/README.md(12 hunks)charts/cloudprem/README.md.gotmpl(1 hunks)charts/console-v3/README.md(3 hunks)charts/console-v3/templates/_helpers.tpl(1 hunks)charts/console/README.md(3 hunks)charts/console/templates/_helpers.tpl(1 hunks)charts/membership/README.md(7 hunks)charts/membership/templates/_helpers.tpl(1 hunks)charts/portal/README.md(3 hunks)charts/portal/templates/_helpers.tpl(1 hunks)test/helm/suite/cloudprem_test.go(1 hunks)test/helm/suite/console_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- test/helm/suite/cloudprem_test.go
- .local/build.sh
- charts/console/templates/_helpers.tpl
- charts/membership/templates/_helpers.tpl
- test/helm/suite/console_test.go
🧰 Additional context used
🪛 LanguageTool
charts/cloudprem/README.md
[uncategorized] ~275-~275: You might be missing the article “the” here.
Context: ...on ### From v2.X.X To v3.0.0 ## RBAC Membership service contains a behavior-breaking ch...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~404-~404: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[typographical] ~610-~610: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | me...
(RP_COLON)
charts/membership/README.md
[grammar] ~57-~57: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ....serviceHost }}"` | is the host for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~58-~58: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...h | string | "" | is the path for the relying party issuer | | global.platform.member...
(RELY_ON)
[grammar] ~59-~59: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ... string | "https" | is the scheme the relying party | | global.platform.membership.sc...
(RELY_ON)
[typographical] ~158-~158: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... CD translate pre-install,pre-upgrade to: argocd.argoproj.io/hook: PreSync | | co...
(RP_COLON)
README.md
[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3014 characters long)
Context: ...tions, users, roles, and permissions. |
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (9)
README.md (1)
8-14: LGTM! Version updates align with PR objectives.The version updates across multiple charts (Cloudprem, Console, Console-V3, Membership, Portal) are consistent with the PR's objective to support membership breaking changes.
🧰 Tools
🪛 LanguageTool
[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3014 characters long)
Context: ...tions, users, roles, and permissions. |(EN_EXCESSIVE_EXCLAMATION)
charts/console-v3/templates/_helpers.tpl (2)
52-68: LGTM! OAuth client configuration is well-structured.The OAuth client configuration has been properly structured with:
- Clear separation of concerns for redirect URI and client credentials
- Proper secret management with existingSecret support
- Consistent naming conventions
Line range hint
71-85: LGTM! Environment variables are properly organized.The environment variables are well-organized with:
- Clear grouping of related variables
- Proper inclusion of core components
- Support for additional environment variables
charts/console-v3/README.md (2)
3-3: Version update aligns with breaking changes.The version bump to 2.0.0 correctly follows semantic versioning for breaking changes in the membership service.
95-97: LGTM! Improved ingress path configuration.The change to use object type for ingress paths improves type safety and validation.
charts/cloudprem/README.md.gotmpl (2)
Line range hint
2-2: Version update correctly reflects major breaking changes.The version bump to 3.0.0-rc.0 appropriately indicates major breaking changes in the membership service.
Line range hint
394-415: Enhance security for OAuth and cookie configuration.The configuration includes sensitive values that need proper security measures.
Consider these security improvements:
- Use
existingSecretinstead of inline values- Document the required cookie encryption key length
- Add validation for the encryption key format
Example of using existing secrets:
global: platform: consoleV3: oauth: client: existingSecret: "console-v3-oauth" secretKeys: secret: "oauth-client-secret" portal: oauth: cookie: existingSecret: "portal-cookie-secret" secretKeys: encryptionKey: "cookie-encryption-key"charts/membership/README.md (1)
3-3: Version update aligns with membership service changes.The version bump to 2.0.0 correctly reflects the breaking changes in the membership service.
charts/cloudprem/README.md (1)
2-2: LGTM! Version updates follow semantic versioning.The version updates correctly reflect a major release with breaking changes:
- Chart version: 3.0.0-rc.0 (release candidate)
- App version: v1.0.4
| {{- define "portal.cookie" }} | ||
| - name: COOKIE_SECRET | ||
| {{- if or .Values.config.cookie.existingSecret .Values.global.platform.portal.cookie.existingSecret }} | ||
| {{- if or .Values.config.cookie.existingSecret }} |
There was a problem hiding this comment.
Fix incomplete or operator in condition.
Similar to the console-v3 template, this condition has an incomplete or operator.
Apply this diff to fix the condition:
- {{- if or .Values.config.cookie.existingSecret }}
+ {{- if .Values.config.cookie.existingSecret }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if or .Values.config.cookie.existingSecret }} | |
| {{- if .Values.config.cookie.existingSecret }} |
flemzord
deleted the
feat/pla-199-featcloudprem-update-cloudprem-chart
branch
