Skip to content

Feature/fix wrong type 4ps module#456

Merged
kfaessen merged 13 commits into
developfrom
feature/fix-wrong-type-4ps-module
May 18, 2026
Merged

Feature/fix wrong type 4ps module#456
kfaessen merged 13 commits into
developfrom
feature/fix-wrong-type-4ps-module

Conversation

@kbakker-hanab
Copy link
Copy Markdown
Contributor

No description provided.

tom-reinders and others added 11 commits July 18, 2024 13:15
* Location of connection must be dynamic  (#432)

* make location dynamic

* Update main.tf

* Update variables.tf

* Update main.tf

* Added -Force param to Logic App Standard to bypass confict errors on local AB#21762

* Allow Access Policy name to be speficied explicitely AB#21752

* Add variable to configure logging extent  (#435)

* Add variable to configure logging extent

* Update variables.tf

* Update main.tf

* format

* Add dimension and frequency configuration (#437)

* Add dimention and frequncy config

* format

* fix null dimension access

* allow list of dimensions

* Added optional diagnostic settings for Standard Logic Apps

* Adding role validation into api policy

* Api management support multiple roles validation AB#22184 (#440)

* Api management support multiple roles validation

* fix fmt

* adding service principal role assignments

* resolving PR comments

* fix formating

* feat: Apply diagnostic settings changes once the deployment is finished (#441)

* feat: Apply diagnostic settings changes once the deployment is finished

* Added comment

* Upgraded default TLS on SB (since older are going to be deprecated)

* Update modules/azure/service_bus_public/variables.tf

Co-authored-by: tom-reinders <tom-reinders@users.noreply.github.com>

* Adding optional setting for diagnostic categories

* fix TF formating

* Adding property to enable/disable sftp on storage account

* feat: Added Standard Logic App with Managed Identity and IP restriction (for HTTP trigger)

* Merged Standard Logic App modules into a single one

* Omit MICROSOFT_PROVIDER_AUTHENTICATION_SECRET when not applicable

* Reverse condition

* Updated appsettings configuration

* fix: Removed validation for optional module AB#23089 (#446)

* added app_scale_limit into windows azure function module

* update of app_scale_limit to leverage null or not being set

* update the condition and remove comment

* app_scale_limit set default to 0

* feat: Enable option to configure runtime_scale_monitoring_enabled

* feat: Allow APIM API bytes_to_log to be specified

* Updated terraform providers to their latest minor versions (#450)

* Updated terraform providers to their latest minor versions

* Fixed azurerm version for service_plan

* reverted elastic/ec version

* Updated TF providers: Merging into develop (#451)

* Updated terraform providers to their latest minor versions

* Fixed azurerm version for service_plan

* reverted elastic/ec version

* Update main.tf

deprecated application_id -> client_id

* Update main.tf

* Added separate frontdoor firewall policy, extracted classic.

* Fixed managed_rule action and details link

* Added support for Front Door Standard

* Fixed origins parameter reference

* Added https_redirect_enabled=false

* Removed link_to_default_domain initialization

* FD: Updated optional variables

* Updated FD templates according to migrated FD example

* Fixed variable property name. Aligned spaces

* FD: Fixed variable interpolation. Initialized necessary property

* FD: Set link_to_default_domain to false

* Temporarily set cloudfare records as data source

* CF: added type filter

* CF: Temporarily commented outputs

* Update README.md

* Update README.md

* FD: introduced FD security policy

* Reverted main and outputs of cloudflare dns records

* Replaced some deprecated properties

* CF: fixed outputs

* Fixed setting application_id for app password

* Added temporary output of API policy XML content

* Reverted auth v2 logic for function app

* Creating an API will now always create an SP

* Fixed syntax error

* Fixed tenant auth settings URL

* Set use_existing to true for API SP

---------

Co-authored-by: ArtiomMatiom <89966532+ArtiomMatiom@users.noreply.github.com>
Co-authored-by: Michal Pipal <m.pipal@recognize.nl>
Co-authored-by: tom-reinders <tom-reinders@users.noreply.github.com>
Co-authored-by: ArtiomMatiom <a.matievschi@recognize.nl>
Co-authored-by: Patrik Kovacs <74901276+patrik-pa4k@users.noreply.github.com>
Co-authored-by: Michal Pipal <73311540+pipalmic@users.noreply.github.com>
Co-authored-by: zjanura <zdenek.janura@isonsoft.cz>
… apps

Replace dynamic enabled_log blocks (based on data.azurerm_monitor_diagnostic_categories)
with static category_group = "allLogs" to prevent "Provider produced inconsistent final plan"
errors. The issue occurs because Azure changes available diagnostic categories after a
function app is modified during apply, causing a mismatch between plan and actual state.

Affected modules:
- function_app_linux
- function_app_linux_managed_identity
- function_app_windows

Note: 26 other modules use the same dynamic pattern and may be affected.
Run terraform fmt -recursive to fix formatting in:
- api_management_api_simple/main.tf
- frontdoor_standard/main.tf
- frontdoor_standard/variables.tf
- logic_app_standard/main.tf
- cloudflare/dns_records/main.tf
…nconsistent-plan

fix: resolve diagnostic settings inconsistent plan error for function apps
Replace dynamic enabled_log/metric blocks with static category_group = "allLogs"
in all 29 remaining modules. Removes all data.azurerm_monitor_diagnostic_categories
data sources that caused "Provider produced inconsistent final plan" errors.
…ogic-apps

fix: resolve diagnostic settings inconsistent plan error in all modules
Copy link
Copy Markdown
Contributor

@kfaessen kfaessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Een paar punten waar een beslissing over genomen moet worden - zie inline comments. Belangrijkste: filter-variabelen (categories/metrics/log_analytics_diagnostic_categories) worden in meerdere modules stilletjes genegeerd, maar staan nog in variables.tf. Daarnaast PR-titel + body dekken de scope niet en er is geen terraform plan output bijgevoegd.

Comment thread modules/azure/api_management_api/variables.tf
Comment thread modules/azure/event_grid_topic/main.tf
Comment thread modules/azure/event_hub/main.tf
Comment thread modules/azure/log_analytics_workspace/main.tf
Comment thread modules/azure/network_security_group/main.tf
Comment thread modules/azure/stream_analytics/main.tf
Comment thread modules/azure/virtual_network/main.tf
Comment thread modules/azure/logic_app_standard/main.tf
Comment thread modules/azure/storage_account_public/main.tf
# Endpoint
resource "azurerm_cdn_frontdoor_endpoint" "fd_endpoint" {
name = var.name
name = var.name
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Veel formatting-only wijzigingen in dit bestand. Liever in een aparte terraform fmt PR zodat de echte diff hier (diagnostic settings) leesbaar blijft.

…ic_setting

The diagnostic settings fix (#454/#455) replaced dynamic per-category
filtering with static category_group = "allLogs" / category = "AllMetrics".
The categories and metrics attributes on loganalytics_diagnostic_setting
are no longer read in any module, so they are removed.

Affected: event_grid_topic, event_hub, log_analytics_workspace,
network_security_group, public_ip, service_plan, storage_account_public
(blob/queue/table/file), stream_analytics, virtual_network.

Breaking for external callers still passing categories/metrics.
@kfaessen
Copy link
Copy Markdown
Contributor

Heads-up on the base branch

This PR targets develop, but the branch was cut from main (merge-base with main is b621f12, the parent of the only intended commit dd804ae). main is currently ahead of develop by the diagnostic-settings fixes (#454, #455) plus the recursive terraform fmt.

As a result the diff against develop is 45 files / +227 / -877, while the actually intended changes are small:

  • subscription_required type string -> bool (api_management_api)
  • removal of the now-unused categories/metrics from loganalytics_diagnostic_setting (9 modules)

Merging as-is also pulls all of main's diagnostic-settings changes into develop. That may be intended (a back-merge), but it is currently invisible to reviewers: the title "Feature/fix wrong type 4ps module" is misleading (the change is in the generic api_management_api module, nothing 4PS-specific) and the description is empty.

Decision needed:

  1. Retarget the PR base to main (keeps the diff to just the intended changes), or
  2. Keep develop as base but make the main -> develop back-merge explicit in the title + description.

Both intended changes are also breaking for external module consumers (string vs bool; removed categories/metrics), so they should be called out in the changelog / release-please.

…c_app_standard

Same cleanup as the loganalytics_diagnostic_setting change: the diagnostic
settings now use a static category_group = "allLogs", so this override
variable is no longer read anywhere and is removed.

Breaking for external callers still passing log_analytics_diagnostic_categories.
@kfaessen kfaessen merged commit 9817e27 into develop May 18, 2026
2 checks passed
@kfaessen kfaessen deleted the feature/fix-wrong-type-4ps-module branch May 18, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants