Skip to content

Conversation

@filippolmt
Copy link
Contributor

No description provided.

@filippolmt filippolmt force-pushed the task/3957_fix_triggers_sql_proxy branch from b88c362 to 229ae86 Compare November 11, 2025 16:39
@filippolmt filippolmt requested a review from Copilot November 11, 2025 16:40
Copilot finished reviewing on behalf of filippolmt November 11, 2025 16:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds explicit triggers blocks to the execute_cloud_sql_proxy and kill_cloud_sql_proxy null resources to ensure they respond to changes in permissions_refresh_id, improving the lifecycle management of Cloud SQL proxy operations when permissions need to be reapplied.

  • Added triggers block with refresh_id = var.permissions_refresh_id to both proxy resources
  • Updated CHANGELOG.md to document version 0.5.1 with these lifecycle improvements

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
main.tf Added triggers blocks to execute_cloud_sql_proxy and kill_cloud_sql_proxy resources to track permissions_refresh_id changes
CHANGELOG.md Added version 0.5.1 entry documenting the lifecycle management improvements
Comments suppressed due to low confidence (2)

main.tf:14

  • The triggers block and replace_triggered_by create redundant tracking of the same condition. The null_resource.force_permissions_refresh already has triggers = { refresh_id = var.permissions_refresh_id } (line 74-76), so replace_triggered_by will trigger replacement when permissions_refresh_id changes. Adding an identical triggers block here creates double-tracking of the same variable. Consider removing the triggers block from execute_cloud_sql_proxy and relying solely on the replace_triggered_by mechanism for cleaner lifecycle management.
  triggers = {
    refresh_id = var.permissions_refresh_id
  }

  lifecycle {
    replace_triggered_by = [
      null_resource.force_permissions_refresh.id
    ]
  }

main.tf:134

  • The triggers block and replace_triggered_by create redundant tracking of the same condition. The null_resource.force_permissions_refresh already has triggers = { refresh_id = var.permissions_refresh_id } (line 74-76), so replace_triggered_by will trigger replacement when permissions_refresh_id changes. Adding an identical triggers block here creates double-tracking of the same variable. Consider removing the triggers block from kill_cloud_sql_proxy and relying solely on the replace_triggered_by mechanism for cleaner lifecycle management.
  triggers = {
    refresh_id = var.permissions_refresh_id
  }

  lifecycle {
    replace_triggered_by = [
      null_resource.force_permissions_refresh.id
    ]
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@filippolmt filippolmt self-assigned this Nov 11, 2025
Copy link
Contributor

@Monska85 Monska85 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@filippolmt filippolmt merged commit cd9003a into main Nov 11, 2025
7 checks passed
@filippolmt filippolmt deleted the task/3957_fix_triggers_sql_proxy branch November 11, 2025 16: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.

3 participants