Snowflake-Labs / terraform-provider-snowflake

Terraform provider for managing Snowflake accounts
https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest
MIT License
549 stars 420 forks source link

[Bug]: 0.95.0 Data Snowflake_users returns a list of maps which are not easily navigable to access user data. #3084

Open kcarrillorhoads opened 1 month ago

kcarrillorhoads commented 1 month ago

Terraform CLI Version

1.5.3

Terraform Provider Version

0.95.0

Terraform Configuration

data "snowflake_users" "users" {
  like = "%"
}

output "airflow_users" {
  value = data.snowflake_users.users
}

Category

category:data_source

Object type(s)

data_source:users

Expected Behavior

The output of the data source has name based identifiers which allows the data object to be used as a parameter for another resource.

For example: resource "snowflake_network_policy_attachment" "example_policy_attachment" { network_policy_name = snowflake_network_policy.symedical_unmapped_terms_bot_network_policy.name set_for_account = false users = [data.snowflake_users.airflow_users.names ] }

The map has to be traversed down to the describe_output.name value to get an identifier which can be used to evaluate if the proper like was used. The input is a search pattern. A top level output or structure like JSON with named nodes would allow for easy accessing and parsing of the result.

Actual Behavior

Because of the structure of the output for data.snowflake_users.airflow_users the object has to be fully parsed and then evaluated in order for useful identifiers to be surfaced.

The map has to be traversed down to the describe_output.name value to get an identifier which can be used to evaluate if the proper like was used.

This creates a need to traverse down into one of the child maps and review its parameters. Previously a use of an simple index allowed users to create a list of users which could be provided to other snowflake_resources.

Steps to Reproduce

data "snowflake_users" "users" { like = "%" }

output "airflow_users" { value = data.snowflake_users.users }

copy the above terraform into a environment with a defined provider and run a plan.

It will output the shape of the users output.

How much impact is this issue causing?

High

Logs

No response

Additional Information

No response

Would you like to implement a fix?

sfc-gh-jcieslak commented 1 month ago

Hey @kcarrillorhoads 👋 Please, for now, use the following approach (mapping users to their names):

data "snowflake_users" "test" {
  with_describe = false
  with_parameters = false
}

output "users" {
  value = [for value in data.snowflake_users.test.users : value.show_output.0.name]
}

I'll discuss the change with the team, but we could include resulting IDs as another computed value that could help with filling fields that expect a list or set of object identifiers. Would that work for you?

kcarrillorhoads commented 1 month ago

The previous means of doing this was something like: data.snowflake_users.test.users[0].name which was possible because of the structure and a bit more compact.

and yes your pattern will work:

output "users" {
  value = [for value in data.snowflake_users.test.users : value.show_output.0.name ]
}

It just becomes increasingly verbose if we are concatenating a list of these data lookups at a time. We do this because we have users defined in multiple states and by using data lookups we can push any errors to plan time if an expected user isn't present.

sfc-gh-jcieslak commented 1 month ago

The previous means of doing this was something like: data.snowflake_users.test.users[0].name which was possible because of the structure and a bit more compact.

Yes, but it had fewer outputs available (only SHOW) and I'm not quite sure if data.snowflake_users.test.users.0.show_output.0.name is much more verbose than the previous version

It just becomes increasingly verbose if we are concatenating a list of these data lookups at a time.

Yes, but in the previous version you had to do the same to get the list of user names, but I get your point. Although, I think the separation into collections is pretty reasonable, it's strange to access them. The main reason why we keep them in one element list is because it's a Terraform SDK limitation. We cannot just define an object there; for some reason, it has to be an element list. In the future, we are planning to migrate to Terraform Plugin Framework, where we hope this can be solved and the usability of data sources (and resources) will be better. Nevertheless, we will try to improve the usability with the mentioned computed field that would store object IDs that could be used in cases such as those you provided with snowflake_network_policy_attachment.

kcarrillorhoads commented 1 month ago

@sfc-gh-jcieslak I hadn't realized that by nesting multiple collections in that way we could access it as: data.snowflake_users.test.users.0.show_output.0.name You are right its not as onerous as I thought, I was under the impression we would have to do the for comprehension even in scenarios where we expect a single username.

That works for now but a feature enhancement would be great to have an additional output that is just a list of strings for the user_names as a nonsensitive element. That would make usability so much cleaner and more intuitive.

Thank you for the prompt response!

sfc-gh-jcieslak commented 1 month ago

Hey 👋 Despite the misunderstanding, we would like to discuss and consider implementing this new computed field in data sources to hold all by SHOW IDs for easier use with resources that need such lists. I'll create an internal task for it, but If you would like to track it, I can create a feature request (as GH Issue). Would it be ok If we close this one as the problem was more or less resolved?