-
Notifications
You must be signed in to change notification settings - Fork 3
[DO NOT MERGE] Working Branch for Cleanup #294
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
Conversation
| # Allow Azure Files SMB access for script volume mounts | ||
| security_rule { | ||
| name = "Allow-Storage-SMB-Outbound" | ||
| priority = 112 | ||
| direction = "Outbound" | ||
| access = "Allow" | ||
| protocol = "Tcp" | ||
| source_port_range = "*" | ||
| destination_port_range = "445" | ||
| source_address_prefixes = var.deployment_script_subnet_address_spaces | ||
| destination_address_prefix = "Storage" | ||
| } | ||
|
|
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.
I don't know if this is actually needed. Was an attempt before we found the issue with storage account. Will likely remove.
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.
Broke this out into its own file when i was trying to add diagnostic settings, not sure we need it separated.
| # checkov:skip=CKV_AZURE_59: Public network access required - Deployment Scripts service accesses storage over public endpoint with managed identity auth | ||
| # checkov:skip=CKV_AZURE_206: LRS sufficient for temporary deployment artifacts - data has 24-hour retention and is not business-critical | ||
| # checkov:skip=CKV_AZURE_35: Network default action 'Allow' required - Deployment Scripts service does not support storage firewall restrictions per Azure docs | ||
| # checkov:skip=CKV_AZURE_33: Queue service logging not applicable - deployment container only uses Blob and File storage services | ||
| # checkov:skip=CKV2_AZURE_41: SAS expiration policy not applicable - using managed identity RBAC authentication instead of SAS tokens | ||
| # checkov:skip=CKV2_AZURE_40: Shared key access required - Azure Container Instances can only mount file shares via storage account keys per platform limitation | ||
| # checkov:skip=CKV2_AZURE_33: Private endpoint not required - public network access with managed identity auth is deployment scripts architecture pattern | ||
| # checkov:skip=CKV2_AZURE_1: Customer-managed key encryption not required - temporary deployment artifacts with 24-hour retention, platform-managed keys sufficient |
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.
Real reasons we're exempting this resource. Yay!
| ] | ||
| } | ||
|
|
||
| # Enable diagnostic logging for deployment container storage account |
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.
might centralize all the diagnostic settings. think we should keep these but maybe in their own file. and enable for some other resources.
| "use_billing_policy": "${USE_BILLING_POLICY:-false}", | ||
| "azd_environment_name": "${AZURE_ENV_NAME}", | ||
| "resource_share_user": ${RESOURCE_SHARE_USER}, | ||
| "tags": ${RESOURCE_TAGS}, |
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.
This enables us to disable some governance controls that were the real issues resource creates were getting blocked. This will get its own PR with the updates to the workflow
| # Reduced default capacity to 50 (50 = 50k TPM) to avoid exceeding typical subscription quotas. | ||
| # If you need higher throughput, request a quota increase in the Azure Portal or set this value | ||
| # via tfvars for the environment after quota approval. | ||
| capacity = 50 | ||
| type = "Standard" |
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.
Ran into an issue where this was blocking us. Added something to troubleshooting guide and lowered the default. we should recommend raising this for production environments.
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>net8.0</TargetFramework> | ||
| <TargetFramework>net9.0</TargetFramework> |
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.
Switched to dotnet 9 because we were installing a tool that installed it anyway. and we were getting some conflicts.
.devcontainer/postCreate.sh
Outdated
| if [ -f "Directory.Build.props" ]; then | ||
| dotnet restore | ||
| echo ".NET packages restored successfully!" |
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.
this was causing postcreate to fail
.devcontainer/devcontainer.json
Outdated
| "ms-vscode.makefile-tools", | ||
| "DavidAnson.vscode-markdownlint", | ||
| "golang.go", | ||
| "azapi-vscode.azapi", |
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.
deprecated
.devcontainer/devcontainer.json
Outdated
| "azapi-vscode.azapi", | ||
| "ms-azuretools.vscode-azureterraform", | ||
| "terraform-linters.tflint-vscode", | ||
| "microsoft-IsvExpTools.powerplatform-vscode", |
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.
removed because it would slow down devcontainer and pop a modal dialog. and it adds no value. we already install pac cli
… directory for self-hosted runners
Found a bunch of stuff that we'll break out into discrete PRs but putting up here so we can see.
Description
This pull request introduces several significant infrastructure and developer environment improvements, primarily focused on enhanced monitoring, resource naming consistency, and updated dependencies. The changes add Log Analytics integration and diagnostic logging, improve resource group handling, update the .NET SDK and target framework to 9.0, and refine network and deployment script configurations. Below are the most important changes grouped by theme:
Monitoring and Diagnostics Integration
azurerm_log_analytics_workspaceresource and enabled diagnostic logging for deployment storage accounts and their blob/file services, conditional on theinclude_log_analyticsvariable. This provides improved observability for deployments. [1] [2]azapi_resourceresources, sending logs and metrics to Log Analytics.Resource Naming and Consistency
Resource Group Management
Developer Environment and Dependency Updates
global.json,Directory.Build.props, and.devcontainer/devcontainer.json, and updated the PowerApps CLI tool version. [1] [2] [3] [4].devcontainer/devcontainer.jsonfor a more streamlined dev environment.Infrastructure and Deployment Script Improvements
Other Notable Changes
RESOURCE_TAGSto the environment and configuration. [1] [2]These updates collectively enhance deployment reliability, observability, and developer experience.
Related Issue(s)
Link to the issue(s) this PR is related to. Prefix with "Fixes" or "Resolves". Every PR must be associated with an Issue
Example: Resolves #1234