feat(infrastructure): add availability test for apim #1327
Conversation
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve the enhancement of Bicep templates used for Azure infrastructure deployment. A new parameter, Changes
Possibly related PRs
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
|
arealmaas
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
.azure/infrastructure/main.bicep (2)
42-43: LGTM! Consider adding constraints to theapimUrlparameter.The new
apimUrlparameter is correctly defined and aligns with the PR objective. To enhance robustness, consider adding constraints such as@minLength(1)or a regex pattern to ensure a valid URL format.
123-133: LGTM! Consider adding a comment for clarity.The new
apimAvailabilityTestmodule is well-implemented and aligns perfectly with the PR objective. It correctly uses theapimUrlparameter and targets the deep health check endpoint.For improved readability, consider adding a brief comment above the module explaining its purpose and the significance of the '/health/deep' endpoint.
.azure/modules/applicationInsights/availabilityTest.bicep (1)
2-2: Consider renaming thenameparameter for clarity.Using a generic parameter name like
namecan lead to confusion. Renaming it to something more descriptive, such asavailabilityTestName, enhances readability and maintainability.Apply this diff to rename the parameter:
- param name string + param availabilityTestName stringAnd update its usage in the resource:
resource availabilityTest 'Microsoft.Insights/webtests@2022-06-15' = { - name: name + name: availabilityTestName ... - SyntheticMonitorId: name + SyntheticMonitorId: availabilityTestName - Name: name + Name: availabilityTestName - Description: 'Availability test for ${name}' + Description: 'Availability test for ${availabilityTestName}' }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- .azure/infrastructure/main.bicep (2 hunks)
- .azure/infrastructure/prod.bicepparam (1 hunks)
- .azure/infrastructure/staging.bicepparam (1 hunks)
- .azure/infrastructure/test.bicepparam (1 hunks)
- .azure/infrastructure/yt01.bicepparam (1 hunks)
- .azure/modules/applicationInsights/availabilityTest.bicep (1 hunks)
- .azure/modules/applicationInsights/create.bicep (1 hunks)
🧰 Additional context used
🔇 Additional comments (7)
.azure/modules/applicationInsights/create.bicep (1)
56-56: LGTM: New output for Application Insights resource ID.The addition of
appInsightsIdoutput is appropriate and consistent with the module's purpose. It exposes the unique identifier of the Application Insights resource, which is useful for referencing this resource in other parts of the infrastructure, such as the new APIM availability test mentioned in the PR description..azure/infrastructure/prod.bicepparam (1)
52-52: LGTM: NewapimUrlparameter added correctly.The new
apimUrlparameter has been added with the correct URL for the production environment. This aligns with the PR objectives to introduce an availability test for the Azure API Management (APIM).To ensure consistency across environments and verify the usage of this parameter, please run the following script:
✅ Verification successful
Verified:
apimUrlparameter is correctly defined and consistently used across all environments.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify apimUrl parameter across environments and its usage in main.bicep # Test 1: Check apimUrl in all environment files echo "Checking apimUrl in environment files:" rg -n 'param apimUrl =' .azure/infrastructure/*.bicepparam # Test 2: Verify usage in main.bicep echo "\nChecking usage in main.bicep:" rg -A 5 'apimUrl' .azure/infrastructure/main.bicepLength of output: 1146
.azure/infrastructure/yt01.bicepparam (1)
51-51: LGTM: NewapimUrlparameter added correctly.The addition of the
apimUrlparameter is consistent with the PR objectives and follows the pattern observed in other environment-specific files. This change supports the implementation of the APIM availability test without affecting existing parameters.To ensure the correctness of the URL for this environment, please run the following verification:
✅ Verification successful
Verified:
apimUrlfor yt01 is correctly set.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the apimUrl parameter across all environment files # Test: Check if the apimUrl is consistent across all environment files echo "Checking apimUrl consistency across environment files:" grep -H 'param apimUrl' .azure/infrastructure/*.bicepparam # Test: Verify that the yt01 environment uses the correct URL yt01_url=$(grep 'param apimUrl' .azure/infrastructure/yt01.bicepparam | awk -F"'" '{print $2}') expected_url="https://platform.yt01.altinn.cloud/dialogporten" if [ "$yt01_url" = "$expected_url" ]; then echo "yt01 apimUrl is correct: $yt01_url" else echo "Error: yt01 apimUrl mismatch. Expected: $expected_url, Found: $yt01_url" fiLength of output: 1185
.azure/infrastructure/staging.bicepparam (1)
51-51: LGTM: New parameterapimUrladded correctly.The new
apimUrlparameter has been added correctly with an appropriate URL for the staging environment (tt02). This aligns with the PR objective of introducing an availability test for the Azure API Management (APIM).To ensure consistency across environments, let's verify the
apimUrlparameter in other environment files:✅ Verification successful
Verification Successful:
apimUrlparameter is consistent across all environments.All environment files correctly include the
apimUrlparameter with URLs appropriate to each environment's configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify apimUrl parameter across environment files # Test: Check apimUrl in other environment files echo "Checking apimUrl in other environment files:" rg -n 'param apimUrl =' .azure/infrastructure/*.bicepparamLength of output: 701
.azure/infrastructure/test.bicepparam (1)
51-52: LGTM: New parameterapimUrladded correctly.The new parameter
apimUrlhas been added appropriately at the end of the file. The URL value 'https://altinn-dev-api.azure-api.net/dialogporten' seems consistent with a test environment setup.To ensure consistency across environments and proper usage, let's verify the following:
✅ Verification successful
LGTM:
apimUrlparameter is consistently added across all environments and correctly utilized inmain.bicep.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify apimUrl parameter across environments and its usage in main.bicep # Test 1: Check apimUrl in other environment files echo "Checking apimUrl in other environment files:" rg -n 'param apimUrl =' .azure/infrastructure/*.bicepparam # Test 2: Verify usage in main.bicep echo "\nChecking usage of apimUrl in main.bicep:" rg -A 5 'apimUrl' .azure/infrastructure/main.bicepLength of output: 1180
.azure/infrastructure/main.bicep (1)
Line range hint
1-324: Overall, the changes look good and achieve the PR objective.The additions of the
apimUrlparameter and theapimAvailabilityTestmodule successfully implement the availability test for APIM as intended. The changes are minimal, focused, and integrate well with the existing infrastructure setup without disrupting other resources.The use of a separate module for the availability test promotes modularity and reusability, which is a good practice. The implementation aligns with the PR objective of probing the deep version of health checks for the APIM instance.
.azure/modules/applicationInsights/availabilityTest.bicep (1)
20-22: Verify the syntax of the'hidden-link'tag key.Ensure that the string interpolation in the tag key is correctly formatted. In Bicep, you might need to adjust the syntax to properly reference
appInsightsId.Would you like assistance in correcting the tag key syntax?
|
🤖 I have created a release *beep* *boop* --- ## [1.27.0](v1.26.3...v1.27.0) (2024-10-29) ### Features * Add restrictions to Transmissions reference hierarchy ([#1310](#1310)) ([e3d53ca](e3d53ca)) * **graphql:** configure opentelemetry ([#1343](#1343)) ([e31c08b](e31c08b)) * **infrastructure:** add availability test for apim ([#1327](#1327)) ([1f9fa2b](1f9fa2b)) * **service:** configure opentelemetry ([#1342](#1342)) ([513d5e4](513d5e4)) * **utils:** configure open telemetry tracing for masstransit in aspnet package ([#1344](#1344)) ([5ec3b84](5ec3b84)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Are Almaas <[email protected]>
Description
An availability test for the backend. For now will send a health-check request to web-api-so so verify that the service is up and running with the all dependencies. The health-endpoint in APIM will send requests to web-api-so by default, the other services are not exposed yet.
The frontend availability test:
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
apimUrlfor capturing the APIM instance URL across various environments (production, staging, test, yt01).Enhancements