-
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
fix(zendesk-service): Add missing Zendesk config values #16783
Conversation
WalkthroughThe pull request introduces modifications across several files to integrate Zendesk functionalities into various services. Key changes include the addition of a new environment variable, 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 (2)
apps/services/auth/personal-representative/infra/personal-representative.ts (1)
57-60
: Consider standardizing secret paths for better organization.The Zendesk secrets are stored in different K8s paths:
- Contact form credentials (
token
) are in/k8s/api/
- Webhook secret is in
/k8s/services-auth/
Consider moving all Zendesk-related secrets under a consistent path for better maintainability.
#!/bin/bash # Description: Check secret path patterns across services for consistency rg -A 3 "ZENDESK.*: '/k8s/" --type typescriptapps/services/auth/public-api/infra/auth-public-api.ts (1)
79-82
: Consider consolidating Zendesk secrets under a common path.The secrets configuration is correct, but the Zendesk secrets are split between two different paths:
/k8s/api/
for contact form credentials/k8s/services-auth/
for webhook secretConsider consolidating all Zendesk-related secrets under a common path (e.g.,
/k8s/services-auth/zendesk/
) for better organization and maintenance.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
apps/services/auth/delegation-api/infra/delegation-api.ts
(1 hunks)apps/services/auth/personal-representative/infra/personal-representative.ts
(1 hunks)apps/services/auth/public-api/infra/auth-public-api.ts
(1 hunks)charts/identity-server/values.dev.yaml
(6 hunks)charts/identity-server/values.prod.yaml
(6 hunks)charts/identity-server/values.staging.yaml
(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/services/auth/delegation-api/infra/delegation-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."
apps/services/auth/personal-representative/infra/personal-representative.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."
apps/services/auth/public-api/infra/auth-public-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 (15)
apps/services/auth/personal-representative/infra/personal-representative.ts (1)
50-54
: Verify the Zendesk subdomain configuration across environments.
While the configuration is structurally correct, using the same subdomain 'digitaliceland' across all environments (dev, staging, prod) might lead to cross-environment data mixing. Consider if separate subdomains should be used for dev and staging environments.
✅ Verification successful
Using the same Zendesk subdomain across environments is the standard configuration
Based on the codebase analysis, using 'digitaliceland' as the Zendesk subdomain across all environments (dev, staging, prod) is the established pattern across multiple services:
- auth-admin-api
- auth-public-api
- delegation-api
- personal-representative
- identity-server configurations
Only the apps/api
service uses a different subdomain ('stjanilofts') for dev environment. This appears to be an exception rather than the rule.
The current configuration in the personal-representative service follows the organization's standard practice and is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other services use different Zendesk subdomains for different environments
rg -A 3 "ZENDESK_CONTACT_FORM_SUBDOMAIN" --type typescript
Length of output: 93
Script:
#!/bin/bash
# Search for Zendesk subdomain configuration across all files
rg -A 3 "ZENDESK.*SUBDOMAIN"
# Also search with ripgrep for any Zendesk related configuration
rg -A 3 "digitaliceland"
# Use fd to find all infrastructure or configuration files
fd -e ts -e js "infra|config"
Length of output: 60872
apps/services/auth/delegation-api/infra/delegation-api.ts (2)
63-67
: Consider environment-specific Zendesk subdomains
Using the same hardcoded subdomain value 'digitaliceland' across all environments (dev, staging, prod) might limit testing capabilities and flexibility. Consider:
- Using different subdomains for non-production environments
- Externalizing this configuration to make it more maintainable
#!/bin/bash
# Verify if other services use different Zendesk subdomains
rg -g "*.ts" "ZENDESK_CONTACT_FORM_SUBDOMAIN" -A 5
70-73
: Review secret management strategy
The Zendesk secrets are stored in different Kubernetes paths:
/k8s/api/
for contact form credentials/k8s/services-auth/
for webhook secret
Consider standardizing the secret paths for better maintainability and security. All service-specific secrets should ideally be under the same path.
apps/services/auth/public-api/infra/auth-public-api.ts (1)
72-76
: Verify the Zendesk subdomain value.
The configuration looks good and follows the established patterns. However, please confirm that 'digitaliceland' is the correct subdomain for all environments (dev, staging, and prod).
✅ Verification successful
Zendesk subdomain configuration is consistent across all services
The verification shows that the 'digitaliceland' subdomain is consistently used across all auth-related services:
- auth/public-api
- auth/personal-representative
- auth/delegation-api
- auth/admin-api
All services use identical configuration for dev, staging, and prod environments, indicating this is the correct subdomain for the organization's Zendesk instance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the same Zendesk subdomain is used consistently across other services
# Expected: All services should use the same subdomain if they interact with the same Zendesk instance
echo "Checking Zendesk subdomain configuration across services..."
rg -A 3 "ZENDESK_CONTACT_FORM_SUBDOMAIN" apps/services/
Length of output: 1721
charts/identity-server/values.prod.yaml (3)
333-333
: LGTM: Zendesk configuration for delegation-api looks good!
The Zendesk configuration for the delegation API service is properly set up with the required environment variable and secrets.
Also applies to: 390-392
615-615
: LGTM: Zendesk configuration for personal-representative looks good!
The Zendesk configuration for the personal representative service is properly set up with the required environment variable and secrets.
Also applies to: 667-669
770-770
: LGTM: Zendesk configuration for public-api looks good!
The Zendesk configuration for the public API service is properly set up with the required environment variable and secrets.
Also applies to: 830-832
charts/identity-server/values.dev.yaml (5)
336-336
: LGTM: Zendesk configuration for delegation-api is properly configured
The Zendesk configuration for the delegation API service includes all required environment variables and secrets, properly sourced from AWS Parameter Store.
Also applies to: 393-395
618-618
: LGTM: Zendesk configuration for personal-representative service is properly configured
The Zendesk configuration for the personal representative service includes all required environment variables and secrets, properly sourced from AWS Parameter Store.
Also applies to: 670-672
773-773
: LGTM: Zendesk configuration for public-api is properly configured
The Zendesk configuration for the public API service includes all required environment variables and secrets, properly sourced from AWS Parameter Store.
Also applies to: 833-835
336-336
: Verify configuration across environments
The Zendesk configuration has been added to the dev environment. Please ensure that corresponding changes are also made in the staging and production environment files with appropriate values.
#!/bin/bash
# Description: Check for Zendesk configuration in other environment files
echo "Checking for Zendesk configuration in staging and prod environments:"
rg "ZENDESK_CONTACT_FORM" charts/identity-server/values.staging.yaml charts/identity-server/values.prod.yaml
Also applies to: 618-618, 773-773
Line range hint 824-828
: Verify memory limits for public-api service
The public-api service has lower memory limits (384Mi) compared to similar services (512Mi). While this might be intentional based on service requirements, please verify that these limits are sufficient for the additional Zendesk integration.
charts/identity-server/values.staging.yaml (3)
336-336
: LGTM: Zendesk configuration for delegation-api is properly configured.
The Zendesk configuration for the delegation API service follows the established pattern and references the correct secret paths.
Also applies to: 393-395
618-618
: LGTM: Zendesk configuration for personal-representative service is properly configured.
The Zendesk configuration for the personal representative service follows the established pattern and references the correct secret paths.
Also applies to: 670-672
773-773
: LGTM: Zendesk configuration for public-api is properly configured.
The Zendesk configuration for the public API service follows the established pattern and references the correct secret paths.
Also applies to: 833-835
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
* added missing zendesk config * chore: charts update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
services-auth-delegation-api
,services-auth-public-api
andservices-auth-personal-representative-api
where missing zendesk config valuesWhy
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
New Features
Configuration Updates
These changes collectively enhance the functionality and reliability of the services interacting with Zendesk.