-
Notifications
You must be signed in to change notification settings - Fork 4
fix(portal, console-v2, console-v3): API_STACK_URL is required #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(portal, console-v2, console-v3): API_STACK_URL is required #121
Conversation
WalkthroughThis pull request updates the Helm chart versions for Cloudprem, Console, Console-V3, and Portal in the main README and their respective chart README files. Each of these charts now includes a new "Stargate configuration" section with two configuration keys: Changes
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 LanguageToolREADME.md[style] ~13-~13: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3026 characters long) (EN_EXCESSIVE_EXCLAMATION) ⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (9)
🪧 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 (
|
e9609dc to
df971a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
charts/console/README.md (1)
91-91: Deprecation notice for stargate_url is properly documented.The
config.stargate_urlparameter is correctly marked as deprecated. However, the default value "toto2" seems like a placeholder or test value. Consider using a more descriptive default value or leave it empty if it's being deprecated.-| config.stargate_url | string | `"toto2"` | Deprecated | +| config.stargate_url | string | `""` | Deprecated, use global.platform.stargate.stackApiUrl instead |charts/console-v3/templates/_helpers.tpl (1)
75-79: Good implementation of conditional API_URL based on Stargate feature flag.The conditional logic for
API_URLis implemented correctly. When Stargate is enabled, it uses the Stargate service URL, otherwise it falls back to the default gateway URL or a provided custom URL.One suggestion: add a comment explaining the placeholder formats like
#{organizationId}and#{stackId}to improve maintainability.- name: API_URL {{- if .Values.global.platform.stargate.enabled }} + # #{organizationId} and #{stackId} are placeholder variables that will be replaced at runtime value: {{ printf "http://%s-%s:8080/#{organizationId}/#{stackId}/api" .Release.Name "stargate" -}} {{- else }} + # #{organizationId} and #{stackId} are placeholder variables that will be replaced at runtime value: {{ default "http://gateway.#{organizationId}-#{stackId}.svc:8080/api" (default .Values.global.platform.stargate.stackApiUrl .Values.config.stargate_url) }} {{- end }}charts/console/templates/_helpers.tpl (1)
75-79: Good implementation of conditional API_URL based on Stargate feature flag.The conditional logic for
API_URLis implemented correctly, matching the implementation in console-v3. When Stargate is enabled, it uses the Stargate service URL, otherwise it falls back to the default gateway URL or a provided custom URL.Similar to console-v3, consider adding a comment explaining the placeholder formats.
- name: API_URL {{- if .Values.global.platform.stargate.enabled }} + # #{organizationId} and #{stackId} are placeholder variables that will be replaced at runtime value: {{ printf "http://%s-%s:8080/#{organizationId}/#{stackId}/api" .Release.Name "stargate" -}} {{- else }} + # #{organizationId} and #{stackId} are placeholder variables that will be replaced at runtime value: {{ default "http://gateway.#{organizationId}-#{stackId}.svc:8080/api" (default .Values.global.platform.stargate.stackApiUrl .Values.config.stargate_url) }} {{- end }}charts/cloudprem/README.md (1)
466-472: Stargate Configuration Section Added.
A new section for "Stargate configuration" has been introduced with the keysglobal.platform.stargate.enabledandglobal.platform.stargate.stackApiUrl. The functionality appears correct. For improved clarity, consider adding punctuation in the description (for example, insert a period after "not required") so that the templating detail is distinct from the condition description.charts/console-v3/README.md (2)
60-64: Stargate Configuration Section Added.
The addition of a "Stargate configuration" section is consistent with the updates across other chart READMEs. It addsglobal.platform.stargate.enabledandglobal.platform.stargate.stackApiUrlwith descriptive default values. As the description text is nearly identical to that in the Cloudprem file, consider adding proper punctuation (for example, a period after "not required") to enhance readability.
84-87: Cookie Configuration Keys (Other Values).
The cookie-related configuration keys (config.cookie.encryptionKey,config.cookie.existingSecret,config.cookie.name, andconfig.cookie.secretKeys) are now located in the "Other Values" section. Verify that this reorganization is clearly documented so users can easily find these settings if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
charts/cloudprem/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/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!**/*.yaml
📒 Files selected for processing (6)
README.md(1 hunks)charts/cloudprem/README.md(4 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)
🧰 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 3026 characters long)
Context: ...tions, users, roles, and permissions. | | | Portal | 2.1.4 | 5e7b404a3a208b1f38603719e02a8b1883c10acf | Formance Portal |
| | Regions | 2.15.1 | latest | Formance Private Regions Helm Chart |
| | Stargate | 0.7.3 | latest | Formance EE Stargate gRPC Gateway | [
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (7)
README.md (1)
8-10: Chart version updates look good.The version updates for Cloudprem (3.0.0 → 3.0.1), Console (2.1.3 → 2.1.4), Console-V3 (2.1.2 → 2.1.3), and Portal (2.1.3 → 2.1.4) are consistent with the changes to support the new Stargate configuration feature.
Also applies to: 14-14
charts/console/README.md (2)
3-3: Version badge update correctly reflects the version change.The version badge has been updated from 2.1.3 to 2.1.4, matching the version update in the main README.
62-67: Clear documentation for Stargate configuration.The new Stargate configuration section provides useful information about the new feature flags that control API URL generation.
charts/console/templates/_helpers.tpl (1)
2-35: API_STACK_URL environment variable is missing from documentation.According to the PR title "API_STACK_URL is required", but there's no documentation or implementation of this environment variable in the file. Consider adding it if it's required, or update the PR title if it's referring to a different change.
You may want to verify if the API_STACK_URL environment variable is intended to be added to this file, or if it's part of a different component not included in this PR. The PR title suggests it's a requirement.
charts/cloudprem/README.md (1)
4-4: Version Badge Updated.
The version badge now correctly displays "3.0.1" (along with the updated AppVersion), ensuring consistency with the release.charts/console-v3/README.md (2)
3-3: Version Badge Updated.
The version badge now correctly reflects Version 2.1.3, indicating that the chart release has been appropriately updated.
95-95: Deprecation Notice for config.stargate_url.
The entry forconfig.stargate_urlis marked as deprecated, which aligns with the new configuration approach.
No description provided.