-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(auth-api): Zendesk credentials #16162
Conversation
WalkthroughThe changes introduce new configuration settings for a Zendesk contact form across multiple files. Specifically, a constant for the Zendesk subdomain is added, along with two new secrets for email and token, enhancing the service's configuration capabilities for different environments (development, staging, and production). Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
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
🧹 Outside diff range and nitpick comments (1)
apps/services/auth/admin-api/infra/auth-admin-api.ts (1)
75-76
: LGTM! Consider updating documentation for new Zendesk integration.The addition of Zendesk-related secrets (
ZENDESK_CONTACT_FORM_EMAIL
andZENDESK_CONTACT_FORM_TOKEN
) follows the existing pattern for secret management and adheres to best practices by using Kubernetes secrets.Consider updating the project documentation to reflect the new Zendesk integration, including any necessary setup steps for these new secrets in the Kubernetes environment.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/services/auth/admin-api/infra/auth-admin-api.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/auth/admin-api/infra/auth-admin-api.ts (2)
Pattern
apps/services/**/*
: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/services/auth/admin-api/infra/auth-admin-api.ts (2)
68-76
: Summary: Zendesk integration looks good, with minor suggestions.The changes introduce Zendesk-related configurations in line with the PR objectives. The implementation adheres to NestJS architecture, dependency injection patterns, and follows existing conventions for environment-specific configurations and secret management.
Key points:
- Zendesk subdomain configuration added (same for all environments).
- Zendesk-related secrets properly managed using Kubernetes.
Suggestions:
- Verify the intention of using the same Zendesk subdomain across all environments.
- Update project documentation to reflect the new Zendesk integration.
Overall, the changes are well-implemented and ready for merge, pending the minor verifications and documentation updates suggested above.
68-72
: Verify the intention of using the same Zendesk subdomain for all environments.The
ZENDESK_CONTACT_FORM_SUBDOMAIN
is set to 'digitaliceland' for all environments (prod, staging, and dev). While this follows the existing pattern for environment-specific configurations, it's worth confirming if this is intentional or if different subdomains should be used for different environments.✅ Verification successful
Zendesk subdomains are consistently set across all environments.
The
ZENDESK_CONTACT_FORM_SUBDOMAIN
is set to 'digitaliceland' for production, staging, and development environments. This uniform configuration suggests that using the same subdomain across all environments is intentional.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if other Zendesk-related configurations use different values per environment # Test: Search for other Zendesk-related configurations rg --type typescript 'ZENDESK_' apps/services/auth/admin-api/infra/Length of output: 102
Script:
#!/bin/bash # Description: Check if other Zendesk-related configurations use different values per environment # Test: Search for other Zendesk-related configurations rg --type ts 'ZENDESK_' apps/services/auth/admin-api/infra/Length of output: 404
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.
LGTM
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
🧹 Outside diff range and nitpick comments (4)
charts/identity-server/values.prod.yaml (2)
238-238
: LGTM! Zendesk integration looks good.The addition of Zendesk-related environment variables is well-implemented:
- Using a hardcoded subdomain for production is appropriate.
- Storing the email and token as Kubernetes secrets is a good security practice.
One minor suggestion:
Consider adding a comment explaining the purpose of these Zendesk variables for better maintainability. For example:
# Zendesk contact form integration ZENDESK_CONTACT_FORM_SUBDOMAIN: 'digitaliceland' ZENDESK_CONTACT_FORM_EMAIL: '/k8s/api/ZENDESK_CONTACT_FORM_EMAIL' ZENDESK_CONTACT_FORM_TOKEN: '/k8s/api/ZENDESK_CONTACT_FORM_TOKEN'Also applies to: 296-297
238-238
: Summary: Zendesk integration added successfullyThe changes in this file are focused on adding Zendesk contact form integration to the
services-auth-admin-api
service. The implementation looks secure and appropriate for a production environment. These additions will enable the authentication API to interact with Zendesk, likely for handling user inquiries or support requests.As this integration introduces a new external dependency, ensure that:
- The application code handles potential Zendesk API failures gracefully.
- Monitoring is in place to track the health and performance of this integration.
- Documentation is updated to reflect this new integration and its purpose in the system architecture.
Also applies to: 296-297
charts/identity-server/values.staging.yaml (1)
241-241
: LGTM! Consider parameterizing the Zendesk subdomain for flexibility.The addition of Zendesk-related environment variables is well-implemented:
- Using Kubernetes secrets for sensitive data (email and token) is a good security practice.
- The changes align with the PR objectives of introducing Zendesk credentials.
Consider parameterizing the
ZENDESK_CONTACT_FORM_SUBDOMAIN
value for better flexibility across different environments:- ZENDESK_CONTACT_FORM_SUBDOMAIN: 'digitaliceland' + ZENDESK_CONTACT_FORM_SUBDOMAIN: '{{ .Values.zendesk.subdomain }}'This change would allow easier configuration changes in the future if needed.
Also applies to: 299-300
charts/identity-server/values.dev.yaml (1)
241-241
: LGTM! Consider making the Zendesk subdomain configurable.The addition of Zendesk-related environment variables is well-structured and follows good security practices by using Kubernetes secrets for sensitive information (email and token).
Consider making the
ZENDESK_CONTACT_FORM_SUBDOMAIN
configurable through a Kubernetes secret or environment-specific value, rather than hardcoding it. This would allow for easier management across different environments (e.g., development, staging, production). You could modify it as follows:- ZENDESK_CONTACT_FORM_SUBDOMAIN: 'digitaliceland' + ZENDESK_CONTACT_FORM_SUBDOMAIN: '/k8s/zendesk/CONTACT_FORM_SUBDOMAIN'This change would make the configuration more flexible and consistent with how the email and token are handled.
Also applies to: 299-300
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- charts/identity-server/values.dev.yaml (2 hunks)
- charts/identity-server/values.prod.yaml (2 hunks)
- charts/identity-server/values.staging.yaml (2 hunks)
* fix zendesk credentials * chore: charts update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Zendesk credentials
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit