Skip to content
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

Add Microsoft SQL Azure Database Server module #54

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

CDA0
Copy link
Contributor

@CDA0 CDA0 commented Jan 14, 2024

No description provided.

modules/mssql-server/README.md Show resolved Hide resolved
tags = merge(var.tags, local.tags)
}

resource "azurerm_monitor_diagnostic_setting" "main" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sure the server resource uses Diag Settings? I can't see it either on the resource view in portal or in https://portal.azure.com/#view/Microsoft_Azure_Monitoring/AzureMonitoringBrowseBlade/~/diagnosticsLogs

minimum_tls_version = var.minimum_tls_version
version = var.sql_server_version
administrator_login = var.administrator_login
administrator_login_password = var.administrator_password
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
administrator_login_password = var.administrator_password
administrator_login_password = var.administrator_login_password

Comment on lines +1 to +9
variable "administrator_password" {
type = string
default = null
}

variable "administrator_login" {
type = string
default = null
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
variable "administrator_password" {
type = string
default = null
}
variable "administrator_login" {
type = string
default = null
}
variable "administrator_login" {
type = string
default = null
}
variable "administrator_login_password" {
type = string
default = null
}

azuread_administrator = {
azuread_authentication_only = true
login_username = "admin"
object_id = "12345678"
Copy link
Contributor

@frasdav frasdav Jan 21, 2024

Choose a reason for hiding this comment

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

Suggested change
object_id = "12345678"
object_id = module.user_assigned_identity.principal_id

@@ -0,0 +1,10 @@
terraform {
Copy link
Contributor

Choose a reason for hiding this comment

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

This file in src too?

features {}
}

module "mssql-server" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
module "mssql-server" {
module "mssql_server" {

Comment on lines +14 to +19
log_analytics_workspace_id = "quux"
azuread_administrator = {
azuread_authentication_only = true
login_username = "corge"
object_id = "grault"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log_analytics_workspace_id = "quux"
azuread_administrator = {
azuread_authentication_only = true
login_username = "corge"
object_id = "grault"
}
azuread_administrator = {
azuread_authentication_only = true
login_username = "corge"
object_id = "grault"
}
log_analytics_workspace_id = "quux"


variable "minimum_tls_version" {
type = string
default = "1.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Docs say it defaults to 1.2, so think we can do this.

Suggested change
default = "1.2"
default = null

@frasdav frasdav changed the title Add mssql server Add Microsoft SQL Azure Database Server module Jan 21, 2024
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.

2 participants