crossplane-contrib / provider-azure

Crossplane Azure Provider
Apache License 2.0
93 stars 64 forks source link

MSSQLServer azureadAuthenticationOnly asks for login username and password #375

Open Mikel-Landa opened 1 year ago

Mikel-Landa commented 1 year ago

What happened?

On the azureadAdministrator -> azureadAuthenticationOnly setting:

Specifies whether only AD Users and administrators (e.g. azuread_administrator.0.login_username) can be used to login, or also local database users (e.g. administrator_login). When true, the administrator_login and administrator_login_password properties can be omitted.

However, when I configure the resource with the setting set to true, the status is the following:

Warning  CannotObserveExternalResource  33s (x7 over 102s)  managed/sql.azure.upbound.io/v1beta1, kind=mssqlserver  cannot run refresh: refresh failed: Missing required argument: "administrator_login": all of `administrator_login,administrator_login_password` must be specified

Spec:

apiVersion: sql.azure.upbound.io/v1beta1
kind: MSSQLServer
spec:
  forProvider:
    location: "West Europe"
    resourceGroupNameSelector:
      matchControllerRef: true
    azureadAdministrator:
      - azureadAuthenticationOnly: true
        objectId: **redacted**
        loginUsername: **redacted**
    version: "12.0"
    minimumTlsVersion: "1.2"

The resource stays on a Ready and Synced status of false, although the server is actually created on azure. Also I can see it dynamically added the field administratorLogin to the resource with an autogenerated value.

How can we reproduce it?

Specify the azureadAdministrator[0].azureadAuthenticationOnly field but leave administratorLogin and administratorLoginPasswordSecretRef blank

What environment did it happen in?

Crossplane version: v1.13.2 Azure provider version: v0.36.0

Mikel-Landa commented 1 year ago

Managed to get it working after enabling management-policies in the sql provider and excluding LateInitialize. However, won't be closing the issue as I think this is just a workaround, especially taking into account the alpha status of management policies.

IMO it should have same default behavior as in terraform, e.g.

resource "azurerm_mssql_server" "this" {
  name                = "sql-${var.environment_name}-${var.unique_suffix}"
  resource_group_name = var.resource_group_name
  location            = var.location
  version             = "12.0"
  minimum_tls_version = "1.2"

  azuread_administrator {
    login_username              = var.ad_admin_group.display_name
    object_id                   = var.ad_admin_group.object_id
    azuread_authentication_only = true
  }

  tags = var.tags
} 

works out of the box in terraform, no need for ignore_changes and setting up initial values.