GoogleCloudPlatform/terraform-google-secured-data-warehouse

Trim the outputs list of the top level module

LuizSDCit opened this issue · 7 comments

LuizSDCit commented

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.
  • If the issue is assigned to a user, that user is claiming responsibility for the issue.

Description

Trim the outputs list of the top level module. Considering that the outputs can be incremented in the future.

References

erlanderlo commented

https://github.com/GoogleCloudPlatform/terraform-google-secured-data-warehouse/pull/129/files#r710493210

In addition to the above heuristic, consider the following:

  1. there seems to be multiple copyrights?
  2. I'd propose we eliminate the network creation in this initial release, then all these outputs won't be needed
  3. remove access level policies - these policies shouldn't be easily altered after deployed
  4. remove perimeter bridges - the structure shouldn't be altered after deployment
daniel-cit commented
  • Duplicate copyrights header still needs to be fixed
  • Network related outputs has been removed
  • Access policies may need to be altered to add additional members and it is hard to search for them in the Cloud Console, so they are useful outputs
  • the bridges do not need to be altered so they could be remove

also, the taxonomy related outputs

terraform-google-secured-data-warehouse/outputs.tf

Line 258 in d87315d

output "medium_policy_taxonomy_id" {

terraform-google-secured-data-warehouse/outputs.tf

Line 267 in d87315d

output "high_policy_taxonomy_id" {

terraform-google-secured-data-warehouse/outputs.tf

Line 276 in d87315d

output "member_policy_ssn_confidential" {

terraform-google-secured-data-warehouse/outputs.tf

Line 281 in d87315d

output "member_policy_name_confidential" {

terraform-google-secured-data-warehouse/outputs.tf

Line 290 in d87315d

output "member_policy_name_private" {

terraform-google-secured-data-warehouse/outputs.tf

Line 299 in d87315d

output "taxonomy_name" {

could be combined in a single taxonomy complex object with the taxonomy and the list of tags.

erlanderlo commented

need to have these trimmed before mvp

erlanderlo commented

@daniel-cit i think we already fixed this, so wondering what might be left?

daniel-cit commented

these outputs are only used in tests:

  • "cmek_keyring_full_name"
  • "cmek_keyring_name"
  • "cmek_ingestion_crypto_key_name"
  • "cmek_bigquery_crypto_key_name"
  • "cmek_reidentification_crypto_key_name"
  • "cmek_confidential_bigquery_crypto_key_name"

they could be removed if we use the full self link of the keys, like "cmek_ingestion_crypto_key" to do the tests.

We can do this change but we need to chose between fixing the current Ruby test or using the new GO way.

@bharathkkb @erlanderlo which test framework should we use for this?

erlanderlo commented

so long as we make the outputs useful for customer (i.e simplified list of what's needed) and can keep our schedule without increasing scope, I have no preference on framework. my understanding is that we'd convert to the newer framework incrementally to achieve eventual consistency with the newer go framework.

  • 👍1
bharathkkb commented

@daniel-cit we can fix the ruby tests as that seems lesser effort than rewrite. If its a new test we should use the new go framework.

  • 👍1