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

Support for id for data source mssql_database #23807

Closed
1 task done
tete17 opened this issue Nov 7, 2023 · 8 comments · Fixed by #23721
Closed
1 task done

Support for id for data source mssql_database #23807

tete17 opened this issue Nov 7, 2023 · 8 comments · Fixed by #23721
Assignees
Labels
documentation service/mssql Microsoft SQL Server
Milestone

Comments

@tete17
Copy link

tete17 commented Nov 7, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Description

Data Source: azurerm_mssql_database doesn't support the very basic id field

This is incredibly valuable to be able to reference databases in a different region to be able to recover from them in a disaster recovery scenario.

New or Affected Resource(s)/Data Source(s)

azurerm_mssql_database

Potential Terraform Configuration

data "azurerm_mssql_server" "backup" {
  name                = "production"
  resource_group_name = "production"
}

data "azurerm_mssql_database" "backup" {
  name      = "example-mssql-db"
  server_id = azurerm_mssql_server.backup.id
}


resource "azurerm_mssql_database" "recovery" {
  name      = "recovery"
  server_id = azurerm_mssql_server.recovery.id

  max_size_gb    = var.database_characteristic.max_size_gb
  zone_redundant = true

  sku_name                    = var.database_characteristic.sku_name
  min_capacity                = var.database_characteristic.min_capacity
  auto_pause_delay_in_minutes = 60

  // Used to recover from in another region
  create_mode         = "Recovery"
  recover_database_id = data.azurerm_mssql_database.backup.id
}

References

The example the documentation uses doesn't even work since it makes use of the inexistant id field https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/data-sources/mssql_database#example-usage

@github-actions github-actions bot added the service/mssql Microsoft SQL Server label Nov 7, 2023
@WodansSon WodansSon self-assigned this Nov 8, 2023
@WodansSon
Copy link
Collaborator

Hi @tete17, thank you for opening this issue. As luck would have it, I am already currently in this code upgrading it to the latest 2023-02-01-preview API version. So, since I am already in the code, I wouldn't see a problem with sneaking this fix into the data source for you as well. 🚀

@WodansSon
Copy link
Collaborator

Looking into this issue it seems very odd, since the id field should have been exposed during its initial release in March of 2020. That said, maybe the API back then didn't actually return the ID field? However, I know that my change in the linked PR will fix your issue, see the below repro output.

Outputs:

datasource_mssql_database_id = "/subscriptions/{subscription}/resourceGroups/repro-resources/providers/Microsoft.Sql/servers/repro-sqlserver/databases/repro-db"

Configuration code for the above output:

output "datasource_mssql_database_id" {
  value = data.azurerm_mssql_database.repro.id
}

@tombuildsstuff
Copy link
Contributor

@WodansSon this looks to be a documentation issue, every Data Source and Resource has to support the id field (else it won't be tracked in the state) - so it appears this is missing from the attribute reference for this Data Source? https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/data-sources/mssql_database#attributes-reference

@WodansSon
Copy link
Collaborator

@tombuildsstuff, totally agree, but maybe this is my bad, I had assumed they had already attempted to get the id field from the data source given the repro steps. 🙈

@tete17
Copy link
Author

tete17 commented Nov 8, 2023

Hi @WodansSon I did tried using the id field in my code and faces a terraform issue saying the parameter was not available.

I think it is more than just documentation given it the schema doesn't reference it but I may be missreading something.

Thanks for the quick turnaround and response. Now that you are revamping this resources can I abuse your trust and ask you to please look at the recovery_database_id field. I was thinking about opening a new ticket after this one and maybe I should.

I have tested and the raw ID of any database is not valid. Instead that field expects a parameter called recoverabledatabases rather than the databases the normal id returns. It seems to be a bug in here. I don't think we should allow a user to provide a raw id from a database in another region for example.

@WodansSon
Copy link
Collaborator

Hi @tete17, I can see why it would appear that way, however the id field is never defined in the schema of a resource it is kind of a meta-field that is always there. As @tombuildsstuff stated in his reply:

...every Data Source and Resource has to support the id field (else it won't be tracked in the state)...

For the resource you are referencing in your reply you can see that the id field is being set in the code here.

As for your recoverabledatabases issue, I believe the upgrade to the API will fix that as well since we have exposed the databases type as a commonid which basically means that the SDK will reflect on the field and automatically cast it to the required database type struct, so in theory, that issue should already be fixed as well. 🙂

@tete17
Copy link
Author

tete17 commented Nov 8, 2023

Hi @WodansSon indeed I see my mistake now. The id is present I must have made a mistake while writing the code and it complained about something else.

Will keep an eye on your MR to see when it gets merged to see if it indeed fixed my recoverabledatabases issue.

If I am being honest I didn't fully understood the explanation regarding the databases type and the reflection but I trust you.

@github-actions github-actions bot added this to the v3.82.0 milestone Nov 23, 2023
Copy link

github-actions bot commented May 1, 2024

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation service/mssql Microsoft SQL Server
Projects
None yet
4 participants