Skip to content
This repository was archived by the owner on Jan 17, 2025. It is now read-only.

Add redshift_database resource and data source.#12

Merged
winglot merged 3 commits intobrainly:masterfrom
sworisbreathing:database
Aug 6, 2021
Merged

Add redshift_database resource and data source.#12
winglot merged 3 commits intobrainly:masterfrom
sworisbreathing:database

Conversation

@sworisbreathing
Copy link
Copy Markdown
Contributor

@sworisbreathing sworisbreathing commented Jul 21, 2021

Currently the redshift database resource only supports local databases, while the data source supports both local and datashare databases. I'm still trying to figure out whether it's best to manage datashare databases in the same resource definition or a separate one. (edit: this is now working well with a nested configuration block for the datashare. if you go from non-datashare to datashare, or change anything in the datashare block, it will force-new)

The COLLATE CASE_SENSITIVE/CASE_INSENSITIVE option isn't yet supported as there doesn't seem to be a way to read it back that I can find (I've opened a support ticket with AWS about it to see if there's a way) Redshift only allows you to change it (i.e. ALTER DATABASE foo COLLATE CASE_INSENSITIVE) for the database you're currently connected to... you can't change it for other databases in the cluster.

(edit: what I thought was the inability to read the setting turned out to be my sql client's inability to render text[] columns properly)

@sworisbreathing sworisbreathing force-pushed the database branch 2 times, most recently from 7d238ff to a2815d1 Compare July 21, 2021 05:33
@sworisbreathing
Copy link
Copy Markdown
Contributor Author

Update: I've got some local changes working more or less for creating databases from datashares, however I still have some cleanup work to do on the changes so I haven't pushed the code up to my fork yet. Will try and close it off tomorrow.

One issue I can see is that acceptance testing the datashare stuff is much more complicated:

  • both the producer and consumer cluster must be in the RA3 family (DS2 and DC2 don't support data sharing)
  • you have to create a data share on the producer first and reference its namespace on the consumer, so either the acceptance test is going to leak a fair amount of info about my environment (which my security team wouldn't like) or it would need to spin up a producer cluster as part of the test and create a datashare on it (which is complicated, since I haven't started the work yet to add a redshift_datashare resource)

@sworisbreathing
Copy link
Copy Markdown
Contributor Author

Okay everything looks good now except for test coverage of a datashare database. For those I'm still not quite sure how to proceed, especially in light of the fact that terraform-provider-aws doesn't seem to expose the cluster namespace in the aws_redshift_cluster resource/data-source, so tests would either need the value hardcoded or resolved through environment variables.

Currently the redshift database resource only supports local databases,
while the data source supports both local and datashare databases.
I'm still trying to figure out whether it's best to manage datashare
databases in the same resource definition or a separate one.
@winglot winglot added the enhancement New feature or request label Jul 29, 2021
@winglot
Copy link
Copy Markdown
Member

winglot commented Aug 3, 2021

Okay everything looks good now except for test coverage of a datashare database. For those I'm still not quite sure how to proceed, especially in light of the fact that terraform-provider-aws doesn't seem to expose the cluster namespace in the aws_redshift_cluster resource/data-source, so tests would either need the value hardcoded or resolved through environment variables.

I know that would require some additional work, but how about a new data source (eg. redshift_current_namespace) that would return the current cluster namespace? It's possible with a CURRENT_NAMESPACE call https://docs.aws.amazon.com/redshift/latest/dg/r_CURRENT_NAMESPACE.html

edit: Just realized that this would only work if you manage multiple redshift clusters from single terraform repo. In case of multiple repos, you would have to export the value from the data source to output in terraform and do a cross-state reading of the output value.

Comment thread redshift/resource_redshift_database.go
@sworisbreathing
Copy link
Copy Markdown
Contributor Author

I know that would require some additional work, but how about a new data source (eg. redshift_current_namespace) that would return the current cluster namespace? It's possible with a CURRENT_NAMESPACE call https://docs.aws.amazon.com/redshift/latest/dg/r_CURRENT_NAMESPACE.html

Yeah I was planning to do that as part of my work on data shares (separate PR)

edit: Just realized that this would only work if you manage multiple redshift clusters from single terraform repo. In case of multiple repos, you would have to export the value from the data source to output in terraform and do a cross-state reading of the output value.

I prefer the first approach, that is using multiple provider configurations in the same repo (one for the producer and one for each consumer)

@winglot winglot merged commit e9d46ea into brainly:master Aug 6, 2021
@sworisbreathing sworisbreathing deleted the database branch August 17, 2021 05:01
StevenKGER referenced this pull request in dbsystel/terraform-provider-redshift Oct 25, 2024
Update module golang.org/x/net to v0.15.0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants