Skip to content

Fix: Remove the need for client_id and client_secret (issue #59) #60

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.

Sign up for GitHub

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

Open
nalberti wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix: Remove the need for client_id and client_secret (issue #59) #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca09986
make credentials optional
nalberti Apr 8, 2026
3186385
Remove client credentials from provider settings
nalberti Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions modules/high-availability/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ variable "tenant_id" {
variable "client_id" {
description = "Application ID(Client ID)"
type = string
default = null
}

variable "client_secret" {
description = "A secret string that the application uses to prove its identity when requesting a token. Also can be referred to as application password."
type = string
sensitive = true
default = null
}
Comment on lines 12 to 23

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

client_id and client_secret are now optional (default null), but they are no longer referenced anywhere in the high-availability module after the provider changes. Keeping unused inputs is misleading (and currently conflicts with the module README, which describes these as required) and makes it unclear how Service Principal auth is intended to work.

Either remove/deprecate these variables, or reintroduce their usage so setting them has an effect.

Copilot uses AI. Check for mistakes.

variable "resource_group_name" {
Expand Down
4 changes: 0 additions & 4 deletions modules/high-availability/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@ terraform {
}
provider "azapi" {
subscription_id = var.subscription_id
client_id = var.client_id
client_secret = var.client_secret
tenant_id = var.tenant_id

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

By removing client_id/client_secret from the azapi provider config, the module no longer supports authenticating via these module inputs (even if callers still pass them). This is a breaking behavior change vs the previous module interface and can cause unexpected auth failures for Service Principal-based deployments.

Consider either (a) wiring the (now-optional) variables into the provider config so they work when set, or (b) removing/deprecating the variables and updating module docs to clearly state credentials must come from the ambient auth chain (Azure CLI/MSI/env vars).

Suggested change
tenant_id = var.tenant_id
tenant_id = var.tenant_id
client_id = var.client_id
client_secret = var.client_secret

Copilot uses AI. Check for mistakes.
}

provider "azurerm" {
subscription_id = var.subscription_id
client_id = var.client_id
client_secret = var.client_secret
tenant_id = var.tenant_id
features {}
}
Loading