-
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(application-system-api): add secret from aws parameter store for national registry client v3 #16782
Conversation
WalkthroughThe changes in this pull request involve updates to the configuration files for the application system API and associated services. Key modifications include the addition of a new secret, 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.
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 (1)
charts/islandis/values.dev.yaml (1)
Line range hint
1-1000
: Security implementation follows best practicesThe National Registry B2C client secret implementation follows security best practices:
- Centralized secret storage in AWS Parameter Store
- Consistent secret path across services
- Proper integration with Kubernetes secrets
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/application-system/api/infra/application-system-api.ts
(1 hunks)charts/islandis/values.dev.yaml
(1 hunks)charts/islandis/values.prod.yaml
(1 hunks)charts/islandis/values.staging.yaml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/application-system/api/infra/application-system-api.ts (1)
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 (6)
apps/application-system/api/infra/application-system-api.ts (1)
342-343
: LGTM! Verify secret configuration in AWS Parameter Store.
The secret addition follows the established naming conventions and is properly integrated with the NationalRegistryB2C xroad client.
Run the following script to verify the secret configuration:
✅ Verification successful
Secret configuration is properly integrated across services
The verification shows that:
- The secret path
/k8s/api/NATIONAL_REGISTRY_B2C_CLIENT_SECRET
is consistently configured across all environment values files (dev, staging, prod) - The secret is used by multiple services that integrate with NationalRegistryB2C client
- The NationalRegistryB2C client is properly defined in the xroad configuration and imported where needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the secret exists in AWS Parameter Store across environments
# Test: Check if the secret path exists in the codebase
rg -l '/k8s/api/NATIONAL_REGISTRY_B2C_CLIENT_SECRET'
# Test: Search for any references to the NationalRegistryB2C client to ensure proper integration
ast-grep --pattern 'NationalRegistryB2C'
Length of output: 1845
charts/islandis/values.staging.yaml (2)
796-796
: LGTM: Secret configuration added correctly
The addition of NATIONAL_REGISTRY_B2C_CLIENT_SECRET is properly configured to use AWS Parameter Store and follows the established pattern for sensitive values in the configuration.
793-796
: Verify B2C client configuration completeness
The B2C client configuration appears complete with all required parameters:
- CLIENT_ID
- ENDPOINT
- PATH
- SCOPE
- CLIENT_SECRET (newly added)
Let's verify no other services require this secret.
✅ Verification successful
Let me search for where this client secret is being used to ensure completeness of verification.
B2C client configuration is properly configured across services
Based on the verification results:
- The National Registry B2C client secret is consistently configured in all required services
- The secret is properly referenced in the national registry client configuration (libs/clients/national-registry/v3)
- All necessary services (application-system, user-profile, endorsements, regulations-admin, etc.) have the correct secret path configured
- The configuration is consistent across all environments (dev, staging, prod)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other services using the National Registry B2C client
rg "NATIONAL_REGISTRY_B2C_CLIENT_ID" -A 5
Length of output: 18993
Script:
#!/bin/bash
# Search for usage of the National Registry B2C client secret
rg "NATIONAL_REGISTRY_B2C_CLIENT_SECRET" -A 2
Length of output: 10326
charts/islandis/values.prod.yaml (1)
787-787
: LGTM: Secret configuration for National Registry B2C client looks correct.
The addition of NATIONAL_REGISTRY_B2C_CLIENT_SECRET
is properly configured and aligns with the other National Registry B2C environment variables already defined in the service.
✅ Verification successful
Let me generate one more verification script to understand which services are using this secret to ensure consistency.
Let me try one final verification to get the complete context of the services.
The National Registry B2C client secret configuration is consistent across environments
Based on the verification results:
- The secret is properly configured in all environments (dev, staging, prod)
- The secret path
/k8s/api/NATIONAL_REGISTRY_B2C_CLIENT_SECRET
is consistent across islandis services - Identity Server uses a different path
/k8s/services-auth/NATIONAL_REGISTRY_B2C_CLIENT_SECRET
which is expected as it's a separate application - The secret is referenced in the config file
nationalRegistryV3.config.ts
where it's used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of National Registry B2C secret across services
# that have the related environment variables configured.
# Test: Find all services that use National Registry B2C and verify they have the secret configured
rg -l "NATIONAL_REGISTRY_B2C_CLIENT_ID" | while read -r file; do
echo "Checking $file for B2C secret..."
rg "NATIONAL_REGISTRY_B2C_CLIENT_SECRET" "$file"
done
Length of output: 4174
Script:
#!/bin/bash
# Get the context of NATIONAL_REGISTRY_B2C_CLIENT_SECRET usage in values files
# to see which services are configuring it
rg -B 5 "NATIONAL_REGISTRY_B2C_CLIENT_SECRET" "charts/islandis/values.prod.yaml" | grep -E "name:|NATIONAL_REGISTRY_B2C"
Length of output: 734
Script:
#!/bin/bash
# Get more context around the secret usage to identify services
rg -B 10 "NATIONAL_REGISTRY_B2C_CLIENT_SECRET" "charts/islandis/values.prod.yaml" | grep -E "name:|NATIONAL_REGISTRY_B2C|application-system-api"
Length of output: 857
charts/islandis/values.dev.yaml (2)
799-799
: LGTM: National Registry B2C client secret addition
The secret addition follows the correct pattern for AWS Parameter Store integration and matches the PR objective.
Line range hint 1-1000
: Verify consistent secret configuration across services
Let's verify that all services using National Registry B2C have consistent configuration.
✅ Verification successful
Based on the search results, I can see that all services using the National Registry B2C client ID also have the corresponding client secret properly configured in their secrets section. The client secret is consistently referenced from /k8s/api/NATIONAL_REGISTRY_B2C_CLIENT_SECRET
for most services, with identity-server using its own path /k8s/services-auth/NATIONAL_REGISTRY_B2C_CLIENT_SECRET
.
Secret configuration is consistent across services
The configuration shows proper secret management practices:
- Each service that uses B2C client ID has corresponding secret configured
- Secret paths are consistently defined
- Environment-specific values are properly separated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all services with B2C client ID also have the secret configured
echo "Services with B2C client ID configuration:"
rg "NATIONAL_REGISTRY_B2C_CLIENT_ID" -B 2 -A 2
echo -e "\nServices with B2C client secret configuration:"
rg "NATIONAL_REGISTRY_B2C_CLIENT_SECRET" -B 2 -A 2
Length of output: 32665
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 98 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
…national registry client v3 (#16782) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Why
Checklist:
Summary by CodeRabbit
New Features
NATIONAL_REGISTRY_B2C_CLIENT_SECRET
, enhancing security across multiple services.Resource Management
Horizontal Pod Autoscaler (HPA)
Ingress Configuration