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 initial module passing all tests #3

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

Conversation

rswrz
Copy link
Member

@rswrz rswrz commented Dec 16, 2024

This PR adds a basic first module for Azure MSSQL on Virtual Machine. The aim is not to have a production-ready module, but a first v0 release.

neonwhiskers and others added 21 commits October 10, 2024 11:06
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Melody Sofia Eroshevich <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
Otherwise trusted launch is not possible

Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
@rswrz rswrz linked an issue Dec 16, 2024 that may be closed by this pull request
@rswrz rswrz added the feature label Dec 16, 2024
Signed-off-by: Roman Schwarz <[email protected]>
Signed-off-by: Roman Schwarz <[email protected]>
@rswrz rswrz added the weekly label Dec 17, 2024
Signed-off-by: Roman Schwarz <[email protected]>
@rswrz rswrz force-pushed the 2-add-initial-module-terraform-azurerm-mssql-vm branch from 3b13b8d to 559ef5d Compare December 17, 2024 10:30
Signed-off-by: Roman Schwarz <[email protected]>
@rswrz rswrz force-pushed the 2-add-initial-module-terraform-azurerm-mssql-vm branch from 559ef5d to cde6cfb Compare December 17, 2024 10:33
@rswrz rswrz self-assigned this Dec 17, 2024
@rswrz rswrz removed the weekly label Jan 7, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

file name is wrong

Comment on lines +175 to +202
run "should_fail_on_wrong_file_path" {
command = plan

variables {
storage_configuration = {
disk_type = "NEW"
storage_workload_type = "OLTP"
system_db_on_data_disk_enabled = false

data_settings = {
luns = [0]
disk_size_gb = 64
default_file_path = "D:\\data"
}
log_settings = {
luns = []
disk_size_gb = 64
default_file_path = "G:\\log"
}
temp_db_settings = {
luns = []
default_file_path = "H:\\tempDb"
}
}
}

expect_failures = [var.storage_configuration]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the testcase. The Paths seems to be valid from test parameter point of view. If the Test ist valid, please add a description to the method or the parameter what should lead to the failure.

Comment on lines +145 to +172
run "should_fail_" {
command = plan

variables {
storage_configuration = {
disk_type = "NEW"
storage_workload_type = "OLTP"
system_db_on_data_disk_enabled = false

data_settings = {
luns = []
disk_size_gb = 64
default_file_path = "D:\\data"
}
log_settings = {
luns = [1]
disk_size_gb = 64
default_file_path = "G:\\log"
}
temp_db_settings = {
luns = []
default_file_path = "H:\\tempDb"
}
}
}

expect_failures = [var.storage_configuration]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

"should_fail_" may be a oversight. Please choose a name to describe what is tested here.

Comment on lines +107 to +110
# validation {
# condition = (var.enable_backup_protected_vm && var.backup_policy_id != null) || !var.enable_backup_protected_vm
# error_message = "A backup policy ID is required when backup_protected_vm.enabled is true."
# }
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments in examples are OK, when referenced in the Readme. Otherwise remove commented code.

Comment on lines +137 to +140
# validation {
# condition = var.computer_name == null ? length(var.name) <= 15 : true
# error_message = "Windows computer name can be at most 15 characters. Variable \"computer_name\" is not set, falling back to \"name\" which is ${length(var.name)} chatacters long. Set \"computer_name\" explicitly."
# }
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments in examples are OK, when referenced in the Readme. Otherwise remove commented code.

Comment on lines +290 to +297
# validation {
# condition = var.key_vault_id == null ? (
# (var.authentication_type == "Password" && var.admin_password != null) || (var.authentication_type == "SSH" && var.admin_ssh_public_key != null)
# ) : (
# (var.authentication_type == "Password" && var.admin_password == null) || (var.authentication_type == "SSH" && var.admin_ssh_public_key == null)
# )
# error_message = "Invalid combination of key_vault_id, admin_password, and admin_ssh_public_key. If key_vault_id is null, admin_password or admin_ssh_public_key must be non-null. If key_vault_id is not null, admin_password and admin_ssh_public_key must be null."
# }
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments in examples are OK, when referenced in the Readme. Otherwise remove commented code.

Comment on lines +152 to +160
# dynamic "key_vault_credential" {
# for_each = var.enable_key_vault_credential ? [true] : []
# content {
# key_vault_url = var.key_vault_credential_key_vault_url
# name = var.key_vault_credential_name
# service_principal_name = var.key_vault_credential_service_principal_name
# service_principal_secret = var.key_vault_credential_service_principal_secret
# }
# }
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments in examples are OK, when referenced in the Readme. Otherwise remove commented code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add initial module terraform-azurerm-mssql-vm
3 participants