azurerm_data_factory_pipeline - support type for parameters and variables

[hqhqhqhqhqhqhqhqhqhqhq]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

Support type for parameters and variables for resource azurerm_data_factory_pipeline.
Example usage:

variable "parameters" {
  description = "List of parameters with their types and default values"
  type = list(object({
    name          = string
    type          = string
    default_value = string
  }))
  default = [
    {
      name          = "teststring"
      type          = "String"
      default_value = "test-string-value"
    },
    {
      name          = "testint"
      type          = "Int"
      default_value = "123"
    },
    {
      name          = "testfloat"
      type          = "Float"
      default_value = "123.45"
    },
    {
      name          = "testbool"
      type          = "Bool"
      default_value = "true"
    },
    {
      name          = "testarraystring"
      type          = "Array"
      default_value = "[\"value1\", \"value2\"]"
    },
    {
      name          = "testarrayint"
      type          = "Array"
      default_value = "[1, 2]"
    },
    {
      name          = "testobject"
      type          = "Object"
      default_value = "{\"key1\":\"value1\",\"key2\":\"value2\"}"
    },
    {
      name          = "testsecurestring"
      type          = "SecureString"
      default_value = "test-secure-string-value"
    }
  ]
}

variable "variables" {
  description = "List of variables with their types and default values"
  type = list(object({
    name          = string
    type          = string
    default_value = string
  }))
  default = [
    {
      name          = "foo"
      type          = "String"
      default_value = "test1"
    },
    {
      name          = "bar"
      type          = "String"
      default_value = "test2"
    },
    {
      name          = "qux"
      type          = "Array"
      default_value = "[\"a\", \"b\"]"
    },
    {
      name          = "quux"
      type          = "Array"
      default_value = "[1, 2, 3]"
    }
  ]
}

resource "azurerm_data_factory_pipeline" "example" {
  name            = "example-pipeline-p43"
  data_factory_id = azurerm_data_factory.example.id

  dynamic "parameters" {
    for_each = var.parameters
    content {
      name          = parameters.value.name
      type          = parameters.value.type
      default_value = parameters.value.default_value
    }
  }

  dynamic "variables" {
    for_each = var.variables
    content {
      name          = variables.value.name
      type          = variables.value.type
      default_value = variables.value.default_value
    }
  }
}

The API can handle the conversion from string default_value to the specified type. However it does not currently work for type being Bool for Variables (create new variable with type as Bool, it will still show up as a String value in Azure Data Factory), hence the Bool type is not currently added in validatefunc for Variables's Type property.

PR Checklist

• I have followed the guidelines in our Contributing Documentation.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
• I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source. -- updating in v4.0
• I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
  For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevent documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azurerm_data_factory_pipeline - support type for parameters and variables

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

Feature request: #13131

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

[ms-henglu]

Add a validation func here, the name should not be empty.

[hqhqhqhqhqhqhqhqhqhqhq]

added

[ms-henglu]

Add a validation func here, the name should not be empty.

[hqhqhqhqhqhqhqhqhqhqhq]

added

[ms-henglu]

Is this only necessary for variable? How about parameters?

[hqhqhqhqhqhqhqhqhqhqhq]

the api can do the conversion for parameters but not for variables

[ms-henglu]

Thanks for the PR, mostly LGTM, just some suggestions, please check.

[stephybun]

Thanks @hqhqhqhqhqhqhqhqhqhqhq. Since we're planning to ship 4.0 this week there isn't any need to use the feature flag. If you can make those changes quickly I think we can get this in just before we release this week. If you think you'll be unable to make the deadline then please follow the guidance I left in the comment in-line using the feature flag for the next major release features.FivePointOhBeta.

[stephybun]

For future reference, these types of schema changes should use the inverted feature flag e.g.

schema: map[string]*pluginsdk.Schema{
// We put the desired breaking schema changes in here
}

if !features.FourPointOhBeta {
// We patch over the breaking schema change with the old definition here
}

This makes cleaning up old/deprecated code post major release much easier since the only action is to delete the entire block wrapped in the !features.FourPointOhBeta flag instead of having to copy paste the new behaviour into the schema above.

However.. since we're planning to ship 4.0 this week there isn't any need for the flag at all, this is an exceptional week where you can go ahead and make the breaking change directly in the schema and it would be acceptable and fine to merge.

[hqhqhqhqhqhqhqhqhqhqhq]

@stephybun thanks for above.
Not relevant to the change required. Wanted to check what's the best practice to ensure the order is not checked when it comes to running terraform plan, e.g. I have made parameters a list of objects, when multiple paramters are declared, running plan will have diffs because of the order.
Looks like changing to TypeSet fix the problem, but not sure I should use TypeSet over TypeList.

[stephybun]

@hqhqhqhqhqhqhqhqhqhqhq it seems like the API doesn't preserve the order of the parameters, so we won't have any other choice but to use a Set here

[hqhqhqhqhqhqhqhqhqhqhq]

Just found that another resource azurerm_data_factory_trigger_schedule, it has a property called pipeline and this can receive parameters as well.
Common usage for this (as per the test cases) is:

resource "azurerm_data_factory_trigger_schedule" "test" {
  name            = "acctestdf%[1]d"
  data_factory_id = azurerm_data_factory.test.id

  pipeline {
    name       = azurerm_data_factory_pipeline.test1.name
    parameters = azurerm_data_factory_pipeline.test1.parameters
  }

  pipeline {
    name       = azurerm_data_factory_pipeline.test2.name
    parameters = azurerm_data_factory_pipeline.test2.parameters
  }

  annotations = ["test1", "test2", "test3"]
}

It is referencing parameters from azurerm_data_factory_pipeline directly, and it had no problem before because both of were type map, but now changing the parameters to a set of objects will break this behaviour.
Similarly for property pipeline_parameters in azurerm_data_factory_trigger_schedule.

I checked the api for azurerm_data_factory_trigger_schedule, this does not support type.

I wonder whether I should change the schema for azurerm_data_factory_trigger_schedule or leave it as it.
@ms-henglu @stephybun

[stephybun]

Changing the schema for the trigger schedule will be more complicated than it has been for the pipeline since whatever we define in the schema will need to be transformed to and from a generic map.

Given how close we are to the major release I'm leaning towards leaving this as is for now and to potentially do this as a soft deprecation post 4.0.

[hqhqhqhqhqhqhqhqhqhqhq]

Changing the schema for the trigger schedule will be more complicated than it has been for the pipeline since whatever we define in the schema will need to be transformed to and from a generic map.

Given how close we are to the major release I'm leaning towards leaving this as is for now and to potentially do this as a soft deprecation post 4.0.

Thanks, have updated the doc & tests, ready for review now.